ARMmbed / mbed-os

Arm Mbed OS is a platform operating system designed for the internet of things
https://mbed.com
Other
4.67k stars 2.98k forks source link

SystemInit_ExtMemCtl(), DATA_IN_ExtSDRAM and SDRAM support #12770

Closed tdjastrzebski closed 4 years ago

tdjastrzebski commented 4 years ago

Looking for the right way of using external SDRAM memory on my DISCO_F769NI board I found this official document on STM32F7 Series system architecture and performance - AN4667 Application note. According to this document:

DATA_IN_ExtSDRAM: if defined in the configuration project, the SDRAM is configured to be used either for data storage or for code execution.

But I figured that mbed-os 5 does not support DATA_IN_ExtSDRAM macro, at least not for DISCO_F769NI target. What is more, it seems that the new way is to define own HAL_SDRAM_MspInit() methof or some MspInitCallback() which gets called from HAL_SDRAM_Init(). However, unlike SystemInit_ExtMemCtl() HAL_SDRAM_Init() is not executed upon system start from SystemInit(). Also, I found SystemInit_ExtMemCtl() is mentioned in startup_stm32f769xx.s file - some publications say that it should be invoked from there but I am not sure it really is.

I am not asking just how to make SDRAM working - there are some examples available. I am asking how to do it the right, mbed-os 5/6 way so I can submit PR and make sure next release will not overwrite necessary OS changes - if any are required.

Also, probably LTDC buffer and DMA, should be set up at the same, early stage. What is the recommended pattern?

Could some architect point me to the right direction, please?

ciarmcom commented 4 years ago

Internal Jira reference: https://jira.arm.com/browse/MBOTRIAGE-2621

jeromecoutant commented 4 years ago

Hi

To be honest, I don't know a lot about SDRAM feature... Maybe you can start with this example: https://os.mbed.com/teams/ST/code/DISCO-F769NI_SDRAM_demo/

@kjbracey-arm

Regards,

tdjastrzebski commented 4 years ago

@jeromecoutant This is a nice demo but I am afraid it is not a 'production grade' solution.
To be safely used for heap, stack and LTDC buffer, SDRAM should be initialized before control is passed to main().

This demo uses stm32f769i_discovery_lcd and stm32f769i_discovery_sdram files no longer included with mbed-os 5. Probably those files were excluded for a reason.

Like I said, there are demos available, but I am asking what is the correct mbed-os 5/6 pattern. A pattern which would allow the OS to set SDRAM for heap, stack and LTDC buffer. It is a typical requirement of any larger production applications using MIPI-DSI display.

0xc0170 commented 4 years ago

https://os.mbed.com/docs/mbed-os/latest/reference/bootstrap.html - for bootstrap. This should clarify where you can execute functionality prior main.

There is an example for gcc to set up heap: https://github.com/sabmeua/heap_ext_DISCO_F746NG. as sbrk is weak, you can overwrite it like this.

To set stack, you should modify your linker script.

tdjastrzebski commented 4 years ago

@0xc0170 Thank you for the example.
My question is of architectural nature. What is the correct mbed-os 5 usage pattern? Should SystemInit_ExtMemCtl() function and DATA_IN_ExtSDRAM macro be used?
If so, how exactly they should be used?
How about HAL_SDRAM_MspInit()?

tdjastrzebski commented 4 years ago

@0xc0170 One comment. I assumed mbed-os has ambitions to address proper memory initialization, including heap/stack in SDRAM as well as LTDC buffer. Some code I found in current version suggest that.
However, maybe this is an area planned to be addressed in future (7+?) releases. To be honest, I have not seen any road map.

0xc0170 commented 4 years ago

The functions are target specific, so in this case someone from @ARMmbed/team-st-mcd could know more.

My question is of architectural nature. What is the correct mbed-os 5 usage pattern?

There is no generic support for this currently. You have to set it up manually as it's target specific (what is there is to use hooks in the bootstrap). What would you like to have there, provide more details.

@0xc0170 One comment. I assumed mbed-os has ambitions to address proper memory initialization, including heap/stack in SDRAM as well as LTDC buffer. Some code I found in current version suggest that.

Give an example how would you imagine have this working - external memory initialization with available hooks Mbed OS provides. What is missing ? What code have you found, can you share?

kjbracey commented 4 years ago

A bit more info: Mbed OS does not have any current generic notion of different classes of memory. All memory layout decisions are fundamentally down to the target, and to some extent the toolchain (each target has one linker map per toolchain).

It would be the target initialisation code's responsibility to initialise the SDRAM. The files in the higher directory levels of each TARGET_xxx for STM are generally the Mbed OS glue where that should happen.

SystemInit is in system_clock.c for Mbed OS targets, and it seems like that's where SDRAM init should happen, as the next step is static memory initialisation which will need it ready. A target with SDRAM would definitely be a distinct Mbed OS target, as it would have a clearly distinct memory map. It could either be implemented by cloning an existing board to make a new board, or you could add a bunch of #ifdefs into the code, including the map files.

If STM documentation suggests having a separate SystemInit_ExtMemCtrl, that would work too, but as you note you would need to update the relevant startup.s files.

As long as some form of system init gets the RAM ready, then you should be free to reorganise the linker maps to put whatever you want there rather than in SRAM. If the linker map says the static data and/or heap and/or main stack go there, then they will go there. Initialisation is automatic - you just need to specify the addresses in the linker map.

Different toolchains have different capabilities about how to direct stuff to multiple regions, which can be annoying. But the simplest case of "put basically everything in SDRAM except maybe a few named items in SRAM" should be doable. SDRAM doesn't offer any new challenges there - we already have devices with multiple SRAM regions you can copy patterns from.

tdjastrzebski commented 4 years ago

@0xc0170 DATA_IN_ExtSDRAM mentioned in AN4667 can be found in:

system_clock.c

#if defined (DATA_IN_ExtSRAM) || defined (DATA_IN_ExtSDRAM)
    SystemInit_ExtMemCtl();
#endif /* DATA_IN_ExtSRAM || DATA_IN_ExtSDRAM */

startup_stm32f769xx.S

/* stack used for SystemInit_ExtMemCtl; always internal RAM used */

I do not know how this should work. That is why I ask hoping there was some mbed-os architect who could share his/her vision.

kjbracey commented 4 years ago

Okay, it looks like at least a subset of targets do have this functionality present. So the simplest pattern to me would seem to be

The boot/interrupt stack should remain in SRAM. It has to be there for bootstrap, and it makes sense to keep it there for interrupt response speed. The other stacks can live in SDRAM (thread stacks for the RTOS, and the stack used for "main".)

adbridge commented 4 years ago

@tdjastrzebski please note in the future these type of questions should be raised on our forums rather than here. GitHub is for actual defects only. Thanks.

tdjastrzebski commented 4 years ago

@kjbracey-arm Thanks for your response. But wait, what about SystemInit_ExtMemCtl() content? Is it actually defined in startup_stm32xxxx.S file? Perhaps this is a stupid question but I do not know assembler enough to tell what is going there.

Or SystemInit_ExtMemCtl() should simply invoke HAL_SDRAM_Init()? If so, why do we need SystemInit_ExtMemCtl() at all? Not to mention no header file declares SystemInit_ExtMemCtl(). Am I missing something obvious? Perhaps.

Is there any example available? Does any target already implement what you suggest? Is so, which one should I use as a template?

tdjastrzebski commented 4 years ago

@adbridge Thanks, I will keep that in mind. I thought this was a bug - incomplete DATA_IN_ExtSDRAM implementation.

kjbracey commented 4 years ago

This is all STM-specific code, rather than Mbed OS, but reading it, my understanding is

It's currently not quite all hanging together because the original STM code has been modified for Mbed OS, and in Mbed OS it has never actually used the external memory. The SystemInit function has been moved out from the HAL core (eg system_stm32f4xx.c) to the Mbed OS-specific per-target system_clock.c. SystemInit_ExtMemCtl() was a static function in the original file. That's why you're not seeing it in a header. You would need to copy it over too.

It looks like the SDRAM SystemInit_ExtMemCtl in system_stm32f4xx isn't actually using the HAL_InitSDRAM for setup at all - it's just poking the registers itself. I don't know whether that or calling HAL_InitSDRAM would be the correct model for STM32F7.

But it appears all the code for external RAM is not present for STM32F7 in Mbed OS - the hooks I see in STM32F4 are not there. You would probably need to import some from other (newer?) versions of the STM32F7 code.

Maybe @ARMmbed/team-st-mcd can advise.

tdjastrzebski commented 4 years ago

@kjbracey-arm Thank you again for your insight. Probably in order to place heap and stack in SDRAM some linker customization is required as well, at least for GCC.
It would be a nice mbed cli feature. Probably we agree that memory setup is a critical area which is somehow overlooked. What I envision is simple settings in mbed_app.json which would get the job done, rather than a separate approach for every toolchain (icf, ld, sct file modification).
Otherwise, the amount of time newbie like myself needs to spend to rediscover the basics is huge.

0xc0170 commented 4 years ago

@tdjastrzebski please provide this feature request on our forum. I'll close this for now.