Skip to content

soc:esp32: Add config allow MbedTLS heap and LVGL Frambuffer use SPI RAM #86049

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

soc:esp32: Add config allow MbedTLS heap and LVGL Frambuffer use SPI RAM #86049

wants to merge 2 commits into from
+42 −3

Conversation

phb98
Copy link
Contributor

@phb98 phb98 commented Feb 20, 2025 •
edited
Loading

Add 2 follow Config Symbol: ESP_MBEDTLS_HEAP_ALLOC_SPIRAM and ESP_LVGL_FRAMEBUFFER_ALLOC_SPIRAM to allow user place heavy-ram usage mbedtls heap and lvgl frame buffer to SPI ram.
Tested with HTTP_get sample, with TLS enable.

Have not tested with LVGL yet, i have no LCD for now. Only sure it can compile with config enable

This is a replacement PR of #85979 because somehow i messed up my git repo :(

@zephyrbot zephyrbot added the platform: ESP32 Espressif ESP32 label Feb 20, 2025
@phb98 phb98 force-pushed the esp_spiram_lvgl_mbed branch 6 times, most recently from 2ff3af8 to bb383f7 Compare February 20, 2025 17:55
@sylvioalves
Copy link
Collaborator

@phb98:
author email (phb98 <phanhaibac98@gmail.com>) needs to match one of the signed-off-by entries.

@phb98
Copy link
Contributor Author

phb98 commented Feb 21, 2025

Hi @sylvioalves, Do you know how can i change the author, i've edited my commit message but looklike it doesn't fix it.

@kartben
Copy link
Collaborator

kartben commented Feb 21, 2025 •
edited
Loading

@phb98

git commit --amend --author="John Doe <john.doe@example.com>" --no-edit

@kartben kartben closed this Feb 21, 2025
@kartben kartben reopened this Feb 21, 2025
@phb98 phb98 force-pushed the esp_spiram_lvgl_mbed branch from bb383f7 to aa1ac98 Compare February 21, 2025 10:40
uLipe
uLipe previously approved these changes Feb 21, 2025
View reviewed changes
sylvioalves
sylvioalves previously approved these changes Feb 21, 2025
View reviewed changes
wmrsouza
wmrsouza previously approved these changes Feb 21, 2025
View reviewed changes
@kartben kartben added this to the v4.2.0 milestone Feb 24, 2025
rodrigopex
rodrigopex reviewed Feb 24, 2025
View reviewed changes
soc/espressif/common/Kconfig.spiram Outdated
@@ -140,6 +140,24 @@ config SPIRAM_ECC_ENABLE
Enable MSPI Error-Correcting Code function when accessing SPIRAM.
If enabled, 1/16 of the SPI RAM total size will be reserved for error-correcting code.

config ESP_SPIRAM_ALLOC_MBED_HEAP
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please keep in mind the naming here. Another configuration about SPIRAM allocation uses different naming conventions: CONFIG_ESP32_WIFI_NET_ALLOC_SPIRAM.

Maybe using CONFIG_ESP_MBED_HEAP_ALLOC_SPIRAM and CONFIG_ESP_LVGL_FRAMEBUFFER_ALLOC_SPIRAM would be closer to the current configuration. The other option is to change the current one. The worst is having different naming conventions for the same thing in the same company devices (Espressif).

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree. In fact, please use MBEDTLS complete name: CONFIG_ESP_MBEDTLS_HEAP_ALLOC_SPIRAM

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@phb98

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay i've renamed 2 Kconfig symbols.

@phb98 phb98 dismissed stale reviews from wmrsouza and sylvioalves via 1ef1801 February 24, 2025 18:18
@phb98 phb98 force-pushed the esp_spiram_lvgl_mbed branch 2 times, most recently from 1ef1801 to 02da51b Compare February 24, 2025 18:25
phb98 added 2 commits February 26, 2025 22:02
@phb98
soc: esp32: Add Config symbol allow MbedTLS heap use SPI RAM
e611195 
- Add KConfig ESP_MBEDTLS_HEAP_ALLOC_SPIRAM
  to allow MbedTLS heap use SPI RAM

Signed-off-by: bac phan <phanhaibac98@gmail.com>
@phb98
soc: esp32: Add Config symbol allow LVGL framebuffer use SPI RAM
b1b0985 
- Add KConfig ESP_LVGL_FRAMEBUFFER_ALLOC_SPIRAM
  to allow LVGL framebuffer use SPI RAM

Signed-off-by: bac phan <phanhaibac98@gmail.com>
@phb98 phb98 force-pushed the esp_spiram_lvgl_mbed branch from 02da51b to b1b0985 Compare February 26, 2025 15:03
kartben
kartben requested changes Feb 28, 2025
View reviewed changes
Copy link
Collaborator

@kartben kartben left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems to be backwards: not sure hardcoding things at ESP32 level really scales when new memory hungry subsystems appear, nor that it benefits other platforms facing the exact same issue.

Please look at shared multiheap and how video subsystem can be configured to use a preferred "type" of memory (e.g. SMH_REG_ATTR_EXTERNAL). You'll probably want to look at doing something similar to extend the available LV_Z_MEMORY_POOL options, same for mbedtls

@kartben
Copy link
Collaborator

kartben commented Feb 28, 2025

This seems to be backwards: not sure hardcoding things at ESP32 level really scales when new memory hungry subsystems appear, nor that it benefits other platforms facing the exact same issue.

Please look at shared multiheap and how video subsystem can be configured to use a preferred "type" of memory (e.g. SMH_REG_ATTR_EXTERNAL). You'll probably want to look at doing something similar to extend the available LV_Z_MEMORY_POOL options, same for mbedtls

I will let @LucasTambor confirm I'm not suggesting something that's too stupid though :)

@phb98
Copy link
Contributor Author

phb98 commented Feb 28, 2025 •
edited
Loading

Hi @kartben
Thanks for your suggestion. Here is some of my thoughts.

1: We have config CONFIG_LV_Z_VDB_CUSTOM_SECTION that i assume its purpose is to allow platform to place that chunk of memory where they want. In fact, i saw some boards from STM32 and Renesas already did the same thing.

2: Currently on espressif platform, we are very close to running out of ram (At least that's what I see from Discord). Enable some basic stuff like Mbedtls and wifi, with reasonable heap and stack config, already use around 80%-85% the internal ram, making using LVGL or other subsystem like Bluetooth is quite impossible. Imo relieving the internal ram is quite needed right now.

3: At this moment this does not scale well on other subsytem. But I saw that share multi heap is treating every external ram on different platforms similarly (only 1 enum SMH_REG_ATTR_EXTERNAL), which i don't think it's always true. On espressif platform, we cannot use ext ram when writting flash, so Filesystem should not use ext ram. Not sure if this is the same case on other platform. We may need a way for subsystem to know what kind of capability of ext ram we have so they can decide to use or not. This is out of scope of this PR imo.

@phb98
Copy link
Contributor Author

phb98 commented Mar 11, 2025

Hi @kartben , do you find anything else need to be addressed ?

@rftafas rftafas mentioned this pull request Mar 11, 2025
Open
@LucasTambor
Copy link
Collaborator

@phb98 Were you able to test this change properly?

At first look I see no problem in placing the MbedTLS and LVGL framebuffer into .ext_ram section since they are being allocated statically. If we need to allocate them dynamically, then using SMH_REG_ATTR_EXTERNAL would be the way to go.

LucasTambor
LucasTambor requested changes Apr 29, 2025
View reviewed changes
soc/espressif/common/Kconfig.spiram
depends on ESP_SPIRAM
depends on MBEDTLS_ENABLE_HEAP
select MBEDTLS_HEAP_CUSTOM_SECTION
default n
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need for default n. N is already the default value ;)

soc/espressif/common/Kconfig.spiram
depends on ESP_SPIRAM
depends on LV_Z_BUFFER_ALLOC_STATIC
select LV_Z_VDB_CUSTOM_SECTION
default n
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same

@marekmatej
Copy link
Collaborator

hey @phb98, MbedTLS and LVGL are good examples of subsystems with extensive memory usage, but there could be more such subsystems. With that said, wouldn't be using EXT_RAM_NOINIT_ATTR and/or EXT_RAM_BSS_ATTR a better solution than introducing new Kconfig options?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rodrigopex rodrigopex rodrigopex left review comments

@kartben kartben kartben requested changes

@LucasTambor LucasTambor LucasTambor requested changes

@uLipe uLipe uLipe approved these changes

@marekmatej marekmatej Awaiting requested review from marekmatej

@raffarost raffarost Awaiting requested review from raffarost

@wmrsouza wmrsouza Awaiting requested review from wmrsouza

@sylvioalves sylvioalves Awaiting requested review from sylvioalves

Requested changes must be addressed to merge this pull request.

Assignees

@sylvioalves sylvioalves

Labels
platform: ESP32 Espressif ESP32
Projects
None yet
Milestone
v4.2.0
Development

Successfully merging this pull request may close these issues.
9 participants
@phb98 @sylvioalves @kartben @LucasTambor @marekmatej @rodrigopex @uLipe @wmrsouza @zephyrbot