Add support for using XIP cache as SRAM#2932
Conversation
Allows for much simpler custom linker scripts
…ript_var variables Means that CMake doesn't need to know the default memory addresses for different platforms
…l files easier Restructured so that it includes the platform-specific files before common ones, so common ones can be overridden
Use new include_linker_script_dir and use_linker_script_file functions to add the linker arguments
Breaking change for Bazel builds using different binary types, instead of setting PICO_DEFAULT_LINKER_SCRIPT to eg `//src/rp2_common/pico_crt0:no_flash_linker_script` it is now `//src/rp2_common/pico_standard_link:no_flash_linker_script`
Treat rp2040 layout (boot2 instead of embedded blocks) as the outlier
Allows overriding what to exclude from .text/.rodata and put in .data
These were being placed in Flash due to a missing space, and also are under libc on some compilers
Add PICO_LINKER_SCRIPT_INCLUDE_DIRS and PICO_LINKER_DEFAULT_LOCATIONS_PATH, instead of hardcoded paths under PICO_LINKER_SCRIPT_PATH Also improve pico_set_linker_script_var and pico_add_linker_script_override_path to better utilise generator expressions
Required changes to the way bazel views these files
…d sections_... which contain multiple Only exceptions are section_..._data and section_bss, as these contain data/bss and tdata/tbss - these are kept together as they are treated as a single section
…n in the future by CMake/bazel functions
…ink_libraries, as it uses generator expressions Also add more checks to kitchen_sink
Intended for platform specific overrides, whereas post_end is for cross-platform overrides, similar to section_end vs section_platform_end
Use spinlock IDs that are unaffected by E2
The new The stuff we discussed before is now replaced with the
Placing all of |
Special treatment is no longer in the future
Place RAM_SECTION_NAME in .time_critical.text Typo in __time_critical_func description
| // In a NO_FLASH binary, don't perform .data etc copy, since it's loaded | ||
| // in-place by the SRAM load. Still need to clear .bss | ||
| #if !PICO_NO_FLASH | ||
| #if !PICO_CRT0_NO_DATA_COPY |
There was a problem hiding this comment.
i guess this #ifdef is still useful to avoid wasting space, but for my own edification, does the "src == dest" take care of everything (i.e. in NO_FLASH binaries, was it the case that src == dest in all case)
There was a problem hiding this comment.
Currently yes, in NO_FLASH binaries src == dest for everything, as there is no copying performed - although new PSRAM variables will end up with src != dest, but that's handled in runtime_init not here.
These changes aren't actually needed anymore to make this PR work, but I think they're still useful to allow creating no_flash binaries that do copy their data around (eg if you need to load a sparse binary over the UART bootloader, squashing it together makes the transfer faster).
The code is still loaded before boot on rp2040, so doesn't need copying
kilograham
left a comment
There was a problem hiding this comment.
Generally looks good
- I'm a bit confused as to what the RP2040/RP2350 options are - is this strictly xip_cache as ram (separate from using PSRAM as per the other PR)
- given the above, perhaps
xip_ramis a poor name (not we also use xip_sram elsewhere), but: - All the existing names are bad, so maybe this would be a good time to rationilze and maybe even add aliases for the old ones (or rather make the old ones aliases)
no_flash => not sure (ram, ram_stored?) - note i wonder if PICO_NO_FLASH isn't strictly the binary type define; it means there is nothing in flash, right, but that could be set for a binary which uses sram & xip_ram - maybe no_flash IS the base binary type, and you use the other controls to turn on XIP_RAM additionally?
blocked_ram => not sure i care
copy_to_ram => i guess this is ok
xip_ram -> not sure
Just think it is worth enumerating all the possibilites (i.e. maybe there are subtypes of no_flash); how does PSRAM play with the above etc?
|
Just to clarify regarding 1, this is two separate ideas combined into one PR, because they both use XIP SRAM
This does not add the ability to place arbitrary data in XIP SRAM - I can add that if it would be useful |
|
yeah, i mean historically i guess we needed different binary types for different linker scripts, but that is no longer strictly the case. it probably makes sense to continue to have binary types for "where stuff gets put by the linker script", but up til now that is mostly "where all the stuff goes". If we have main ram and xip ram, it is unlikely the main code/data will be split between, so you're likely to just want to access the xip-cache; which can(/may already be) something you can specify via cmake. By this i guess I'm talking about the enabling of XIP RAM (pinning) vs the storing of code/data there (though the latter requires the former not vice versa) Maybe it is just me, but i find xip-ram confusing since PSRAM is also via XIP. I don't know if xip-cache is better, since it isn't the cache when being used as ram! |
|
Hi @will-v-pi I took a swing at adding that support for placing a whole file's functions into XIP SRAM. I think it turned out better than I anticipated, and it should be compatible with CMake 3.13. Please consider adding my changes into your PR, and let me know what you think of it. (the tests section probably needs some work) you can find it here Thanks, |
Skip low power unpinning when PICO_USE_XIP_CACHE_AS_RAM is set Take xip_ram as argument to pico_set_persistent_data_loc, to allow placing normally instead of specifying address Add pico_critical_xip_sram_test_low_power test to check usin xip_ram for both works
I think it's still useful to have "standard" binary types which have their own memmap (and it in necessary for
The first point of this PR (see "For RP2040 & RP2350 ..." in my comment above) is to split code between main ram and xip ram - it puts time critical function code in xip ram, and all other code/data in main ram. Pinning can already be enabled using the
It's called "XIP_SRAM" in the |
|
Here's a full description of binary types, as I think they will be after this PR merged, if you want to propose new names for any of them:
For binaries that can use main SRAM, they can place other code/data there using For binaries that can use Flash, they can place other data there using the For binaries that can use PSRAM, they can place other data there using For binaries that can use XIP_SRAM (does not include Any other placements (eg code in PSRAM, data in XIP_SRAM) currently require custom linker script sections and custom section attributes. |
This adds a new
xip_rambinary type (RP2350 only) which puts all code & data in XIP SRAM, and apico_use_xip_sram_for_time_criticalCMake function to put functions marked as time_critical in XIP SRAM instead of regular SRAM. Thepico_use_xip_sram_for_time_criticalis only useful forcopy_to_ramandno_flashbinary types - it will still work fordefaultbinary types, but makes everything slower instead of faster.Adds tests of both - currently the
pico_xip_sram_testdoesn't fit on Clang or Bazel so is skipped on both, andpico_use_xip_sram_for_time_criticalis not implemented in Bazel, sopico_critical_xip_sram_testuses a custom linker script instead, for the same effect.Supercedes #2660, and fixes #2653
Includes #2645, which links with raspberrypi/pico-extras#99