Conversation
Supports either enabling PSRAM via boot2, or by injecting a function that runs when restoring QMI CS1 settings
If not detected, it sets address translation to 0, so all PSRAM access Busfaults
It barely fit in boot2 anyway, and would have required embedding boot2 on RP2350
Also remove some bazel TODOs - CMake gates RP2350 only stuff in src/cmake/rp2_common.cmake
Attempts size detection with every possible CS pin
Also fix auto-detection GPIO setup
Also add kitchen sink test that it works
This will always run runtime_init_setup_psram function when hardware_psram is linked If PICO_PSRAM_SIZE_BYTES and PICO_AUTO_DETECT_PSRAM_SIZE are both 0 or undefined, it will only initialise when flash_devinfo is already setup (eg with OTP) Also setup to bus fault with too small PSRAM, add kitchen sink test for those bus faults, and don't trigger those bus faults in runtime_init_setup_psram
Also fix no_flash binaries, and the tiny_psram test
These will either statically allocate in psram, or malloc if the static allocation is not in available psram. Sections are sorted, so the group can be used to set priority. Not sure how useful these are, so could be removed if not wanted.
This removes the need for default_psram_region in bazel linker scripts, and prevents PSRAM appearing in RP2040 elf.map files
| * \ingroup hardware_psram | ||
| * | ||
| * This runs \ref psram_detect_size() for each CS GPIO in the array in turn, | ||
| * and returns the size as soon as a PSRAM chip is detected. |
There was a problem hiding this comment.
Perhaps this description needs updating now that the implementation has changed 😉
| * and returns the size as soon as a PSRAM chip is detected. | |
| * stopping as soon as a PSRAM chip is detected. |
Also, maybe this function-description should warn that it will "wiggle" the CS GPIOs?
| * This runs \ref psram_detect_size() for each CS GPIO in the array in turn, | ||
| * and returns the size as soon as a PSRAM chip is detected. | ||
| * | ||
| * This will setup the CS GPIO using flash_devinfo if PSRAM is found. |
There was a problem hiding this comment.
I think this might be clearer like:
| * This will setup the CS GPIO using flash_devinfo if PSRAM is found. | |
| * If PSRAM is found, the CS GPIO will be configured in flash_devinfo. |
| if (psram_words > (flash_devinfo_size_to_bytes(flash_devinfo_get_cs_size(1)) >> 2)) { // >>2 is /4, for words | ||
| // Setup to bus fault for variables that don't fit in available PSRAM | ||
| flash_devinfo_size_t psram_size = flash_devinfo_get_cs_size(1); | ||
| int clear_start = 8; // Clear no regions by default |
There was a problem hiding this comment.
I found this code pretty hard to read. Instead of setting a clear_start value and then iterating from clear_start to 7, would it make more sense to have a clear_regions value and then iterate from (8 - clear_regions) to 7? (or similar, if I've got the maths slightly wrong)
Fixup function setting in psram_detect_cs_and_size has_psram -> psram_initialized Move PICO_AVAILABLE_CS1_GPIOS into psram.c Reduce calls to flash_devinfo_get_cs_size in runtime_init_setup_psram
Improve PICO_AUTO_DETECT_PSRAM_CS_SKIP_DEFAULTS description Add docs from PR description to psram.h Fix kitchen sink compile for small/tiny psram Rearrange runtime_init_setup_psram to make it clearer that stuff gets skipped if setup in OTP
Inconsistent naming, whitespace, still use count_of for uint8_t, psram is rwx
Clang was trying to place persistent_data in PSRAM at the relevant address (eg 0x200xxxxx), and then complaining that it overflowed Fix is to add `> XXX` to the end, to explicitly put it in RAM/XIP_RAM
| uint8_t cs_gpios[] = PICO_AVAILABLE_CS1_GPIOS; | ||
| size_t num_cs_gpios = count_of(cs_gpios); | ||
| #if PICO_AUTO_DETECT_PSRAM_CS_SKIP_DEFAULTS | ||
| num_cs_gpios = remove_defaults_from_cs_gpios(cs_gpios, count_of(cs_gpios)); |
There was a problem hiding this comment.
Seems like this could just be:
| num_cs_gpios = remove_defaults_from_cs_gpios(cs_gpios, count_of(cs_gpios)); | |
| num_cs_gpios = remove_defaults_from_cs_gpios(cs_gpios, num_cs_gpios); |
?
| uint32_t psram_words = (uint32_t)(&__psram_end__ - &__psram_start__); | ||
| if (psram_words > psram_word_size) { | ||
| // Setup to bus fault for variables that don't fit in available PSRAM | ||
| int clear_regions = 0; // Clear no regions by default |
There was a problem hiding this comment.
This is more readable now thanks, and I can now see that this "Clear no regions by default" applies when the PSRAM size is 16MB 🙂
| * When using the runtime_init initialisation, this can initialise PSRAM in 3 ways, | ||
| * listed from highest to lowest priority: | ||
| * 1. If flash_devinfo is setup (e.g. configured in OTP), it will initialise PSRAM with | ||
| * the flash_devinfo CS1 size and GPIO |
There was a problem hiding this comment.
I was confused by this line at first, and thought it was missing a comma after "CS1" 😉 But now that I see what it's saying, would this be clearer?
| * the flash_devinfo CS1 size and GPIO | |
| * the flash_devinfo size and GPIO for CS1 |
| * 1. If flash_devinfo is setup (e.g. configured in OTP), it will initialise PSRAM with | ||
| * the flash_devinfo CS1 size and GPIO | ||
| * 2. If `PICO_AUTO_DETECT_PSRAM` is set it will attempt to detect PSRAM size and CS GPIO | ||
| * on CS1. This will attempt to use all available CS GPIOs as chip selects, so they |
There was a problem hiding this comment.
| * on CS1. This will attempt to use all available CS GPIOs as chip selects, so they | |
| * on CS1. This will attempt to use all available QMI CS1n GPIOs as chip selects, so they |
might be slightly clearer?
| * will be wiggled. By default, it will skip over some which are defined in the board | ||
| * header (see `PICO_AUTO_DETECT_PSRAM_CS_SKIP_DEFAULTS`). |
There was a problem hiding this comment.
Hmm, I wonder if there's a use-case for being able to say "never attempt to probe this list-of-GPIOs for PSRAM CS" if there are certain pins which the user doesn't want to get wiggled, but which aren't covered by any of the board-header _DEFAULT_ defines? 🤔
| * on CS1. This will attempt to use all available CS GPIOs as chip selects, so they | ||
| * will be wiggled. By default, it will skip over some which are defined in the board | ||
| * header (see `PICO_AUTO_DETECT_PSRAM_CS_SKIP_DEFAULTS`). | ||
| * - If the CS GPIO is know and set in `PICO_PSRAM_CS_PIN`, you can just enable |
There was a problem hiding this comment.
| * - If the CS GPIO is know and set in `PICO_PSRAM_CS_PIN`, you can just enable | |
| * - If the CS GPIO is known and set in `PICO_PSRAM_CS_PIN`, you can just enable |
| * board header, or with \ref pico_override_psram_size) it will initialise PSRAM with | ||
| * that size and CS GPIO | ||
| * | ||
| * Only the `PICO_AUTO_DETECT_PSRAM` method will verify that PSRAM is present before |
There was a problem hiding this comment.
Should this be
| * Only the `PICO_AUTO_DETECT_PSRAM` method will verify that PSRAM is present before | |
| * Only the `PICO_AUTO_DETECT_PSRAM` and `PICO_AUTO_DETECT_PSRAM_SIZE` methods will verify that PSRAM is present before |
?
| * access to PSRAM addresses greater than the size available. The \ref psram_check_address | ||
| * function should be used before accessing variables in PSRAM when auto-detection is on, | ||
| * to prevent these bus faults. |
There was a problem hiding this comment.
Should / could the kitchen_sink example demonstrate doing this?
| * using it. | ||
| * | ||
| * Variables can be placed in PSRAM using __psram or __psram_uninitialised macros, and | ||
| * you can also write directly to the memory addresses. |
There was a problem hiding this comment.
| * you can also write directly to the memory addresses. | |
| * you can also read/write the memory addresses directly. |
? (I assume that PSRAM isn't write-only 😉 )
| * Variables can be placed in PSRAM using __psram or __psram_uninitialised macros, and | ||
| * you can also write directly to the memory addresses. |
There was a problem hiding this comment.
This might be one of my stupid questions, but if you wanted to have variables placed in PSRAM and also read/write PSRAM directly, is there a way of knowing which addresses in PSRAM aren't used by the variables placed in PSRAM?
(or maybe that's something you'd never need to do in practice?)
This adds a
hardware_psramlibrary to the SDK to add PSRAM support, as requested in #2372Adds PSRAM defines to board headers for boards that have PSRAM - I may not have captured them all
Has three ways it will detect PSRAM when you link
hardware_psram, in order of priority:PICO_AUTO_DETECT_PSRAMis set it will attempt to detect PSRAM size and GPIO on CS1PICO_PSRAM_SIZE_BYTESis set (eg configured in board header, or withpico_override_psram_size) it will initialise PSRAM with that size and GPIOVariables can be placed in PSRAM using
__psramor__psram_uninitialisedmacros, and you can also write directly to the memory addresses.If there are variables placed in PSRAM, it will setup ATRANS to prevent any access to PSRAM addresses greater than the size available (from flash_devinfo, auto-detection, or
PICO_PSRAM_SIZE_BYTES), which will cause bus faults. Thepsram_is_availablefunction should be used before accessing variables in PSRAM when auto-detection is on, to prevent these bus faults.This support uses flash_devinfo to enable the bootrom support for PSRAM, then adds a callback with
flash_set_qmi_cs1_setup_functionwhich runs as part offlash_rp2350_restore_qmi_cs1to enable quad mode and setup faster timings. The RP2350-E14 workaround is included.The CS pin auto-detection iterates through all possible CS pins, skipping any that are defined as defaults in board header files with
remove_defaults_from_cs_gpios- this is an ugly function, but without it you get some garbled characters on the UART as pin 0 is a possible CS pin. Maybe some of the defaults could be removed from this, if they are unlikely to be affected by this auto-detection?This has
psram_or_mallocandpsram_or_freemacros, which statically allocate in PSRAM and use that if available, but usemallocif not available - not sure how useful these are, could be removed if we don't want them. They were to demostratepsram_check_addresswhich checks if an address is in available PSRAM, so you can avoid busfaults when using variables in PSRAM.In passing this also: