-
Notifications
You must be signed in to change notification settings - Fork 178
fix(lcd): use new mipi dma2d api (BSP-755) #694
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.
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
base: master
Are you sure you want to change the base?
Conversation
|
@suda-morris @espzav PTAL,thanks |
4d186b9 to
8c798f9
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This PR is being reviewed by Cursor Bugbot
Details
Your team is on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle for each member of your team.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
| // Enable DMA2D for ESP-IDF 6.0+ | ||
| ESP_GOTO_ON_ERROR(esp_lcd_dpi_panel_enable_dma2d(*ret_panel), err, TAG, | ||
| "enable DMA2D failed"); | ||
| #endif |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: DPI panel not deleted on DMA2D enable failure
When esp_lcd_dpi_panel_enable_dma2d() fails after the DPI panel has already been successfully created by esp_lcd_new_panel_dpi(), the error handler only frees the driver struct and resets the GPIO, but does not delete the created DPI panel. This causes a resource leak since *ret_panel (or panel_handle) contains a valid panel pointer that is never cleaned up on this failure path.
Additional Locations (2)
|
|
||
| #if ESP_IDF_VERSION >= ESP_IDF_VERSION_VAL(6, 0, 0) | ||
| // Enable DMA2D for ESP-IDF 6.0+ | ||
| ESP_GOTO_ON_ERROR(esp_lcd_dpi_panel_enable_dma2d(*ret_panel), err, TAG, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since DMA2D is not a very stable feature, it will have restriction when we enable ext mem encryption. Can we give the choice to the user instead of always enabling the dma2d mode?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It can be moved into BSP.
ESP-BSP Pull Request checklist
Change description
Please describe your change here
Note
Enable DMA2D on IDF ≥6.0 using
esp_lcd_dpi_panel_enable_dma2dand conditionally keep.flags.use_dma2dfor older IDF across ILI9881C, LT8912B, and ST7796; bump component versions.esp_lcd_dpi_panel_enable_dma2d()inesp_lcd_ili9881c.c,esp_lcd_lt8912b.c,esp_lcd_st7796_mipi.c.#include "esp_idf_version.h"and gate.flags.use_dma2d = truewith#if ESP_IDF_VERSION < ESP_IDF_VERSION_VAL(6, 0, 0)inesp_lcd_ili9881c.handesp_lcd_st7796.h.ESP_LCD_DPI_PANEL_DMA2D_FLAGS()inesp_lcd_lt8912b.hand apply it across DPI timing macros.components/lcd/esp_lcd_ili9881c/idf_component.yml:1.0.2components/lcd/esp_lcd_lt8912b/idf_component.yml:0.1.2components/lcd/esp_lcd_st7796/idf_component.yml:1.3.42024-2025in touched files.Written by Cursor Bugbot for commit 8c798f9. This will update automatically on new commits. Configure here.