search

modm-io / modm

modm: a C++23 library generator for AVR and ARM Cortex-M devices
https://modm.io
Mozilla Public License 2.0
748 stars 132 forks source link

SAMD51/SAME5x cache not enabled? #1033

Closed ghost closed 1 year ago

ghost commented 1 year ago

While debugging a program I wrote for a Feather M4 example, I determined that the cache controller was not enabled. I could find nothing in the SAMD51/SAME5x startup code relating to it.

Is this just an oversight, or is there a reason for it?

(Edited earlier post for brevity.)

Thanks, Tom Rush

chris-durand commented 1 year ago

Hi Tom, the only reason is that the SAME5x support was never really finished. I had planned to use the chips in a project at work to replace some unobtainable STM32 but they also went out of stock back then. Feel free to open a PR to enable the data cache, if you like.

ghost commented 1 year ago

Hi Chris, thanks for the reply. Actually I was thinking more ot the instruction cache, but before I would attempt a PR I have some questions (and bear in mind that I am no guru).

  1. The cache controller handles data-only cache (instruction disabled), instruction-only cache (data disabled), or both. There is also the option for tightly coupled memory (TCM), where you disable some or all of the cache and, I presume, load that area at startup from the linker script, like modm_fastcode or modm_fastdata.
  2. I've been under the impression that just enabling the cache for both instructions and data and letting the controller do its own thing is generally a good idea. Do you think I should try this method and just see how it goes? (I did run across some PRs on the adafruit/circuitpython repo that were trying to solve a problem with the QSPI apparently caused by the CMCC being enabled; so I guess it's not always good.)
  3. The program I've been working on involves modm::delay_ns(), which is set for modm_fastcode loading into RAM. Since doing it that way puts it on the S-Bus, the instruction fetch takes two cycles. If I enable the cache and delay_ns() gets automatically loaded into it, how does that affect the accuracy of the delay since it is based on a predetermined constant?
  4. Where would be the best place to enable the cache controller? I found a substitution macro in src/modm/platform/core/cortex/startup.c.in (__modm_startup()) for enabling the instruction cache on an m7 core; but if I were to enable both I and D cache, I would think it would be better to move it past all the memory initialization there and do it just before calling main(). (Or since this cache controller is probably SAM specific, it could just be placed in Board::initialize() for any applicable board).

Sorry to hammer you with so many questions. If it were just me I would enable it and be done with it; but if I submit a PR I want it to fit the quality of the project.

BTW, thanks for the work you did on the SAM D51/E5x. I made a lame attempt at it myself, but I am not in the same league!

On Fri, Jun 2, 2023 at 10:43 AM Christopher Durand @.***> wrote:

Hi Tom, the only reason is that the SAME5x support was never really finished. I had planned to use the chips in a project at work to replace some unobtainable STM32 but they also went out of stock back then. Feel free to open a PR to enable the data cache, if you like.

— Reply to this email directly, view it on GitHub https://github.com/modm-io/modm/issues/1033#issuecomment-1573852782, or unsubscribe https://github.com/notifications/unsubscribe-auth/ALBESCU7FP4GJTCAH6QMLETXJH3ZBANCNFSM6AAAAAAYXVT2AI . You are receiving this because you authored the thread.Message ID: @.***>

ghost commented 1 year ago

I'm going to answer my own questions :-)

2 and 4. When (if) my Feather M4 BSP PR gets approved and merged, I believe I will submit a PR for 'board.hpp' that turns on the cache in the 'initialization()' function. I don't think it should go in core:cortex-m because the cache was not an ARM option in the M4 boards; it was an add-on done by some vendors and thus vendor specific. Any boards with the SAMD51/SAME5x could also add something to their initialization if desired; but if not, it should probably be mentioned in the docs that the cache is not enabled by default. At some point it may be useful to have a platform/cache/(vendor) module to handle enabling/disabling, invalidating entries, locking, and other assorted functions of a cache controller.

  1. Having the cache enabled does NOT affect the delay_ns() function. I did a test with interrupts disabled, and reading the SysTick value immediately before and after the delay_ns() wrapper call (four 16-bit thumb instructions). Delaying for 50000ns, the result should have been ~6000 clock ticks: with cache off, it was 6067; cache on, 6064; and those numbers were consistent, loop after loop. I interpret that to mean that delay_ns() was running about 1% long with the tuning parameters in the sam platform:core module.lb, and that cache is not faster than modm_fastcode in RAM.

  2. A corollary to the previous item is that using the cache memory as TCM with a linker script entry would not be worthwhile.

chris-durand commented 1 year ago

To 4: If you want to enable the cache we could put that code into the platform-specific core module, e.g here.

Enabling the cache should be unproblematic for D51/E5x because we don't support any feature yet that could interfere with caching, like DMA.

ghost commented 1 year ago

If you think that wouldn't be too early, then I'll try it there. I just wasn't sure what effect that might have on initializing the memory. (The opening comment in that file sounds so ominous! :-)