Skip to content

fix(esp_lvgl_port): Put RGB vsync callback in IRAM when LCD_RGB_ISR_IRAM_SAFE=y#755

Closed
vmsh0 wants to merge 1 commit into
espressif:masterfrom
vmsh0:master
Closed

fix(esp_lvgl_port): Put RGB vsync callback in IRAM when LCD_RGB_ISR_IRAM_SAFE=y#755
vmsh0 wants to merge 1 commit into
espressif:masterfrom
vmsh0:master

Conversation

@vmsh0
Copy link
Copy Markdown

@vmsh0 vmsh0 commented Apr 3, 2026

ESP-BSP Pull Request checklist

  • Version of modified component bumped -- NO, do I need to do this for a small fix?
  • CI passing

Change description

When LCD_RGB_ISR_IRAM_SAFE=y, RGB panel callbacks must be placed in IRAM, otherwise lvgl_port_add_disp_rgb fails due to esp_lcd_rgb_panel_register_event_callbacks refusing to accept callbacks outside of IRAM.

This patch places the lvgl_port_flush_rgb_vsync_ready_callback routine in IRAM when LCD_RGB_ISR_IRAM_SAFE=y.

Addresses e.g. #515


Note

Medium Risk
Touches ESP32S3 RGB panel ISR callback placement; incorrect attribute/macro handling could break builds or cause runtime issues depending on CONFIG_LCD_RGB_ISR_IRAM_SAFE and IDF version.

Overview
Ensures the ESP32S3 RGB panel on_vsync/bounce-buffer completion callback (lvgl_port_flush_rgb_vsync_ready_callback) is annotated to run from IRAM when CONFIG_LCD_RGB_ISR_IRAM_SAFE is enabled, satisfying IDF requirements for ISR-safe RGB callbacks.

This introduces a compile-time macro to conditionally apply IRAM_ATTR to the callback so esp_lcd_rgb_panel_register_event_callbacks accepts the function under IRAM-safe configurations.

Written by Cursor Bugbot for commit 70be509. This will update automatically on new commits. Configure here.

@CLAassistant
Copy link
Copy Markdown

CLAassistant commented Apr 3, 2026

CLA assistant check
All committers have signed the CLA.

@espzav
Copy link
Copy Markdown
Collaborator

espzav commented Apr 7, 2026

@vmsh0 thank you for this PR. Did you see this #732 ?

@vmsh0
Copy link
Copy Markdown
Author

vmsh0 commented Apr 7, 2026

@vmsh0 thank you for this PR. Did you see this #732 ?

Aaaah, oops, I did not. Point 2 of that PR was also something I wanted to raise, as moving the callback into IRAM is probably not enough if you then need to call something else.

Anyway, I think this can be abandoned as the other one looks much better already. Sorry for the noise!

@espzav espzav mentioned this pull request May 7, 2026
2 tasks
@tome9111991
Copy link
Copy Markdown

Tested this PR on an ESP32-S3-WROOM-1 N16R8 with a 800×480 RGB panel
(ESP32-8048S043C board), ESP-IDF v6.0.1.

Panel config:

  • fb_in_psram = true, num_fbs = 2
  • bounce_buffer_size_px = 800 * 10
  • CONFIG_LCD_RGB_ISR_IRAM_SAFE=y
  • CONFIG_LCD_RGB_RESTART_IN_VSYNC=y

With the patch applied, lvgl_port_add_disp_rgb no longer fails — the callback
registration goes through fine, panel inits, LVGL starts and renders one frame.
The device then crashes on the very first NVS read during boot (before any OTA
or user-triggered flash op), so I never got far enough to evaluate the original
goal of the patch.

Crash:
Guru Meditation Error: Core 0 panic'ed (Cache error).
Cache disabled but cached memory region accessed
PC: 0x40056f64 (memcpy in ROM)
EXCVADDR: 0x00000000
A3 = 0x3c2b5cc0 (PSRAM framebuffer address)

Backtrace:
memcpy (ROM)
lcd_rgb_panel_fill_bounce_buffer (esp_lcd_panel_rgb.c:926)
lcd_rgb_panel_eof_handler (esp_lcd_panel_rgb.c:973)
gdma_default_tx_isr (gdma.c:963)
_xt_lowint1
spi_flash_hal_read (during nvs_get_str → wifi_credentials_load)

Root cause looks architectural, not specific to this patch:
lcd_rgb_panel_fill_bounce_buffer copies from the PSRAM-resident framebuffer
into the SRAM bounce buffer. On ESP32-S3 the cache controller gates both flash
and PSRAM, so any flash read (NVS, OTA, etc.) that disables the cache also
makes the PSRAM framebuffer inaccessible. The IRAM-safe ISR then runs as
intended but immediately faults inside memcpy.

So LCD_RGB_ISR_IRAM_SAFE=y appears to be fundamentally incompatible with
fb_in_psram=true regardless of the LVGL port wrapper — which is the most
common configuration for boards with displays large enough to need PSRAM in
the first place. Is this documented somewhere as a known limitation? If so,
maybe esp_lvgl_port could log a warning (or refuse registration) when both
options are combined, since the failure mode is non-obvious.

Minor nit on the patch itself: the #else branch at line 49 has a typo
(LVGL_PORT_RBG_IRAM vs LVGL_PORT_RGB_IRAM). Harmless today because the
macro isn't used when LCD_RGB_ISR_IRAM_SAFE=n, but worth fixing.

@espzav
Copy link
Copy Markdown
Collaborator

espzav commented May 15, 2026

@tome9111991 I reworked this in #778 You can test it. It should work.

@espzav espzav closed this in #778 May 15, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants