Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ESP32 PSRAM behaves erratically if flashed using 80Mhz flash mode #1566

Closed
daagaak opened this issue May 19, 2024 · 9 comments
Closed

ESP32 PSRAM behaves erratically if flashed using 80Mhz flash mode #1566

daagaak opened this issue May 19, 2024 · 9 comments
Assignees
Labels
chip:esp32 Issue related to ESP32 chip
Milestone

Comments

@daagaak
Copy link

daagaak commented May 19, 2024

I've been experiencing some erratic behaviour in my ESP32 trying to enable PSRAM on my WROOM-E module (confirmed to have an 8MB PSRAM module).

Attempting to synthesize this down to a broken test case, I generated a new esp-template project, and replaced the heap section with:

fn init_heap(p: PSRAM) {
    esp_hal::psram::init_psram(p);
    unsafe {
        ALLOCATOR.init(
            esp_hal::psram::psram_vaddr_start() as *mut u8,
            esp_hal::psram::PSRAM_BYTES,
        );
    }
}

And changed the init_heap call to be:

    init_heap(peripherals.PSRAM);

This actually works as expected when you espflash with the default settings. However, if you bump up the flash speed to 80mhz with -f 80mhz then I've seen it either:

  • Hang during PSRAM init
  • Throw a LoadProhibited during PSRAM init
  • Throw a IllegalInstruction during PSRAM init.

I think this is because the esp32 HAL PSRAM code assumes that you're running in 40mhz Flash/40mhz PSRAM mode:

 345⋮    │        let mode = PsramCacheSpeed::PsramCacheF40mS40m; // How to make this configurable
@MabezDev
Copy link
Member

I think this is because the esp32 HAL PSRAM code assumes that you're running in 40mhz Flash/40mhz PSRAM mode:

Could you try changing this locally in esp-hal and seeing if with the correct mode it works?

@daagaak
Copy link
Author

daagaak commented May 20, 2024

Yes is does. I've been running with it set (unconditionally) to PsramCacheF80S80 since yesterday and it seems to work reliably.

@bjoernQ
Copy link
Contributor

bjoernQ commented May 21, 2024

I think the comment // How to make this configurable is the key thing - we could use (another) cargo-feature for it but ideally, we should have #1111

@MabezDev MabezDev added the status:blocked Unable to progress - dependent on another task label Jun 12, 2024
@tom-borcin tom-borcin added the chip:esp32 Issue related to ESP32 chip label Aug 13, 2024
@MabezDev MabezDev removed the status:blocked Unable to progress - dependent on another task label Sep 16, 2024
@MabezDev
Copy link
Member

I poked around at this a little bit. I think that the features can be removed completely, and instead we can pass this to psram::init as a configuration.

@MabezDev MabezDev added this to the 0.21.0 milestone Sep 16, 2024
@bjoernQ
Copy link
Contributor

bjoernQ commented Sep 17, 2024

I poked around at this a little bit. I think that the features can be removed completely, and instead we can pass this to psram::init as a configuration.

Oh!

esp-hal/esp-hal/build.rs

Lines 215 to 235 in 45806df

#[cfg(feature = "esp32s2")]
fn generate_memory_extras() -> Vec<u8> {
let reserved_cache = if cfg!(any(
feature = "psram-2m",
feature = "psram-4m",
feature = "psram-8m"
)) {
"0x4000"
} else {
"0x2000"
};
format!(
"
/* reserved at the start of DRAM/IRAM */
RESERVE_CACHES = {reserved_cache};
"
)
.as_bytes()
.to_vec()
}
makes completely removing the features not a great option - we could always reserve the full amount of cache like we do on S3 but especially on S2 8k less RAM (when not having access to PSRAM) is not ideal

We can of course make the RAM size and the flash-frequency a run-time config - but I think we should keep a psram and a octal-psram feature

@MabezDev
Copy link
Member

I think that should be fine, we will probably need at least one feature because at some point we'll need to resolve #2027 by adding or changing the way we compile some things. Funnily enough that issue doesn't actually affect the s2 :D.

I think we just need a single psram feature though, right? S2 doesn't support octal, and we already use all the available cache for s3 - I did a quick look and I don't think the opsram features are used anywhere else (other than where we will replace them).

@bjoernQ
Copy link
Contributor

bjoernQ commented Sep 17, 2024

I guess we need to keep both features - also ESP-IDF does this

    if(CONFIG_SPIRAM_MODE_QUAD)
        list(APPEND srcs "${target}/esp_psram_impl_quad.c")
    elseif(CONFIG_SPIRAM_MODE_OCT)
        list(APPEND srcs "${target}/esp_psram_impl_octal.c")
    endif()

We could just include both implementations and decide at runtime but a lot of that code needs to live in IRAM so I'm not sure that is the best option

@MabezDev
Copy link
Member

Closed via #2178

@github-project-automation github-project-automation bot moved this from Todo to Done in esp-rs Sep 23, 2024
@williams-one
Copy link

williams-one commented Sep 26, 2024

Hello, I've been trying out the example psram_octal in the latest master on a ESP32-S3.
The example works perfectly, but if I change the speed of the PSRAM to 80 mhz in the following way, the execution gets past the PSRAM initialization but gets stuck inside init_psram_heap

let peripherals = esp_hal::init(esp_hal::Config::default());

let (start, size) = psram::init_psram(
    peripherals.PSRAM,
    psram::PsramConfig {
        ram_frequency: esp_hal::psram::SpiRamFreq::Freq80m,
        ..Default::default()
    },
);
// gets here
init_psram_heap(start, size);
// but not here

Should I change anything else and/or is this feature supported on all S3 or it also requires some support in the chip/board?
I'm using a ESP32-S3-WROOM-1-N16R8 if that matters

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chip:esp32 Issue related to ESP32 chip
Projects
Archived in project
Development

No branches or pull requests

5 participants