From f35edeb5a32e0386f37ddaf77a0e443cf50b5f98 Mon Sep 17 00:00:00 2001 From: morris Date: Thu, 3 Mar 2022 15:34:32 +0800 Subject: [PATCH 1/6] lcd: add debug log on/off Kconfig --- components/driver/gptimer.c | 6 +++--- components/driver/pulse_cnt.c | 6 +++--- components/esp_lcd/Kconfig | 6 ++++++ components/esp_lcd/src/esp_lcd_panel_io_i2c.c | 13 ++++++++++--- components/esp_lcd/src/esp_lcd_panel_io_i2s.c | 16 ++++++++++------ components/esp_lcd/src/esp_lcd_panel_io_i80.c | 14 +++++++++----- components/esp_lcd/src/esp_lcd_panel_io_spi.c | 13 ++++++++++--- components/esp_lcd/src/esp_lcd_panel_nt35510.c | 13 ++++++++++--- components/esp_lcd/src/esp_lcd_panel_ssd1306.c | 13 ++++++++++--- components/esp_lcd/src/esp_lcd_panel_st7789.c | 13 ++++++++++--- components/esp_lcd/src/esp_lcd_rgb_panel.c | 13 ++++++++----- 11 files changed, 89 insertions(+), 37 deletions(-) diff --git a/components/driver/gptimer.c b/components/driver/gptimer.c index 381b32025c..5ddb46a7de 100644 --- a/components/driver/gptimer.c +++ b/components/driver/gptimer.c @@ -341,9 +341,6 @@ esp_err_t gptimer_stop(gptimer_handle_t timer) static gptimer_group_t *gptimer_acquire_group_handle(int group_id) { -#if CONFIG_GPTIMER_ENABLE_DEBUG_LOG - esp_log_level_set(TAG, ESP_LOG_DEBUG); -#endif bool new_group = false; gptimer_group_t *group = NULL; @@ -494,6 +491,9 @@ esp_err_t gptimer_get_pm_lock(gptimer_handle_t timer, esp_pm_lock_handle_t *ret_ __attribute__((constructor)) static void check_gptimer_driver_conflict(void) { +#if CONFIG_GPTIMER_ENABLE_DEBUG_LOG + esp_log_level_set(TAG, ESP_LOG_DEBUG); +#endif extern int timer_group_driver_init_count; timer_group_driver_init_count++; if (timer_group_driver_init_count > 1) { diff --git a/components/driver/pulse_cnt.c b/components/driver/pulse_cnt.c index a2d0d9794b..8c72896da1 100644 --- a/components/driver/pulse_cnt.c +++ b/components/driver/pulse_cnt.c @@ -605,9 +605,6 @@ esp_err_t pcnt_channel_set_level_action(pcnt_channel_handle_t chan, pcnt_channel static pcnt_group_t *pcnt_acquire_group_handle(int group_id) { -#if CONFIG_PCNT_ENABLE_DEBUG_LOG - esp_log_level_set(TAG, ESP_LOG_DEBUG); -#endif bool new_group = false; pcnt_group_t *group = NULL; @@ -705,6 +702,9 @@ IRAM_ATTR static void pcnt_default_isr(void *args) __attribute__((constructor)) static void check_pulse_cnt_driver_conflict(void) { +#if CONFIG_PCNT_ENABLE_DEBUG_LOG + esp_log_level_set(TAG, ESP_LOG_DEBUG); +#endif extern int pcnt_driver_init_count; pcnt_driver_init_count++; if (pcnt_driver_init_count > 1) { diff --git a/components/esp_lcd/Kconfig b/components/esp_lcd/Kconfig index 02a47a09c4..95136ff705 100644 --- a/components/esp_lcd/Kconfig +++ b/components/esp_lcd/Kconfig @@ -6,5 +6,11 @@ menu "LCD and Touch Panel" help LCD driver allocates an internal buffer to transform the data into a proper format, because of the endian order mismatch. This option is to set the size of the buffer, in bytes. + config LCD_ENABLE_DEBUG_LOG + bool "Enable debug log" + default n + help + Wether to enable the debug log message for LCD driver. + Note that, this option only controls the LCD driver log, won't affect other drivers. endmenu endmenu diff --git a/components/esp_lcd/src/esp_lcd_panel_io_i2c.c b/components/esp_lcd/src/esp_lcd_panel_io_i2c.c index 30900ed105..6457bf6426 100644 --- a/components/esp_lcd/src/esp_lcd_panel_io_i2c.c +++ b/components/esp_lcd/src/esp_lcd_panel_io_i2c.c @@ -1,14 +1,18 @@ /* - * SPDX-FileCopyrightText: 2021 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2021-2022 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ -// #define LOG_LOCAL_LEVEL ESP_LOG_DEBUG - #include #include #include +#include "sdkconfig.h" +#if CONFIG_LCD_ENABLE_DEBUG_LOG +// The local log level must be defined before including esp_log.h +// Set the maximum log level for this source file +#define LOG_LOCAL_LEVEL ESP_LOG_DEBUG +#endif #include "esp_lcd_panel_io_interface.h" #include "esp_lcd_panel_io.h" #include "driver/i2c.h" @@ -40,6 +44,9 @@ typedef struct { esp_err_t esp_lcd_new_panel_io_i2c(esp_lcd_i2c_bus_handle_t bus, const esp_lcd_panel_io_i2c_config_t *io_config, esp_lcd_panel_io_handle_t *ret_io) { +#if CONFIG_LCD_ENABLE_DEBUG_LOG + esp_log_level_set(TAG, ESP_LOG_DEBUG); +#endif esp_err_t ret = ESP_OK; lcd_panel_io_i2c_t *i2c_panel_io = NULL; ESP_GOTO_ON_FALSE(io_config && ret_io, ESP_ERR_INVALID_ARG, err, TAG, "invalid argument"); diff --git a/components/esp_lcd/src/esp_lcd_panel_io_i2s.c b/components/esp_lcd/src/esp_lcd_panel_io_i2s.c index 8313945877..d1fac6fd01 100644 --- a/components/esp_lcd/src/esp_lcd_panel_io_i2s.c +++ b/components/esp_lcd/src/esp_lcd_panel_io_i2s.c @@ -1,11 +1,9 @@ /* - * SPDX-FileCopyrightText: 2021 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2021-2022 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ -// #define LOG_LOCAL_LEVEL ESP_LOG_DEBUG - ///////////////////////////////////////////////////////////////////////////////////////////////////////////// // Although we're manipulating I2S peripheral (on esp32/s2 target), it has nothing to do with the AUDIO BUS. // In fact, we're simulating the Intel 8080 bus with I2S peripheral, in a special parallel mode. @@ -16,6 +14,12 @@ #include #include #include +#include "sdkconfig.h" +#if CONFIG_LCD_ENABLE_DEBUG_LOG +// The local log level must be defined before including esp_log.h +// Set the maximum log level for this source file +#define LOG_LOCAL_LEVEL ESP_LOG_DEBUG +#endif #include "freertos/FreeRTOS.h" #include "freertos/task.h" #include "freertos/queue.h" @@ -33,7 +37,6 @@ #include "hal/gpio_hal.h" #include "driver/gpio.h" #include "esp_private/periph_ctrl.h" -#if SOC_I2S_LCD_I80_VARIANT #include "esp_private/i2s_platform.h" #include "soc/lcd_periph.h" #include "hal/i2s_hal.h" @@ -117,6 +120,9 @@ struct lcd_panel_io_i80_t { esp_err_t esp_lcd_new_i80_bus(const esp_lcd_i80_bus_config_t *bus_config, esp_lcd_i80_bus_handle_t *ret_bus) { +#if CONFIG_LCD_ENABLE_DEBUG_LOG + esp_log_level_set(TAG, ESP_LOG_DEBUG); +#endif esp_err_t ret = ESP_OK; esp_lcd_i80_bus_t *bus = NULL; ESP_GOTO_ON_FALSE(bus_config && ret_bus, ESP_ERR_INVALID_ARG, err, TAG, "invalid argument"); @@ -748,5 +754,3 @@ static IRAM_ATTR void lcd_default_isr_handler(void *args) portYIELD_FROM_ISR(); } } - -#endif // SOC_I2S_LCD_I80_VARIANT diff --git a/components/esp_lcd/src/esp_lcd_panel_io_i80.c b/components/esp_lcd/src/esp_lcd_panel_io_i80.c index f745b96a28..da834bce08 100644 --- a/components/esp_lcd/src/esp_lcd_panel_io_i80.c +++ b/components/esp_lcd/src/esp_lcd_panel_io_i80.c @@ -4,13 +4,17 @@ * SPDX-License-Identifier: Apache-2.0 */ -// #define LOG_LOCAL_LEVEL ESP_LOG_DEBUG - #include #include #include #include #include +#include "sdkconfig.h" +#if CONFIG_LCD_ENABLE_DEBUG_LOG +// The local log level must be defined before including esp_log.h +// Set the maximum log level for this source file +#define LOG_LOCAL_LEVEL ESP_LOG_DEBUG +#endif #include "freertos/FreeRTOS.h" #include "freertos/task.h" #include "freertos/queue.h" @@ -28,7 +32,6 @@ #include "esp_private/gdma.h" #include "driver/gpio.h" #include "esp_private/periph_ctrl.h" -#if SOC_LCDCAM_SUPPORTED #include "esp_lcd_common.h" #include "soc/lcd_periph.h" #include "hal/lcd_ll.h" @@ -119,6 +122,9 @@ struct lcd_panel_io_i80_t { esp_err_t esp_lcd_new_i80_bus(const esp_lcd_i80_bus_config_t *bus_config, esp_lcd_i80_bus_handle_t *ret_bus) { +#if CONFIG_LCD_ENABLE_DEBUG_LOG + esp_log_level_set(TAG, ESP_LOG_DEBUG); +#endif esp_err_t ret = ESP_OK; esp_lcd_i80_bus_t *bus = NULL; ESP_GOTO_ON_FALSE(bus_config && ret_bus, ESP_ERR_INVALID_ARG, err, TAG, "invalid argument"); @@ -659,5 +665,3 @@ IRAM_ATTR static void lcd_default_isr_handler(void *args) portYIELD_FROM_ISR(); } } - -#endif // SOC_LCDCAM_SUPPORTED diff --git a/components/esp_lcd/src/esp_lcd_panel_io_spi.c b/components/esp_lcd/src/esp_lcd_panel_io_spi.c index 95791d8654..b5395de43a 100644 --- a/components/esp_lcd/src/esp_lcd_panel_io_spi.c +++ b/components/esp_lcd/src/esp_lcd_panel_io_spi.c @@ -1,14 +1,18 @@ /* - * SPDX-FileCopyrightText: 2021 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2021-2022 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ -// #define LOG_LOCAL_LEVEL ESP_LOG_DEBUG - #include #include #include +#include "sdkconfig.h" +#if CONFIG_LCD_ENABLE_DEBUG_LOG +// The local log level must be defined before including esp_log.h +// Set the maximum log level for this source file +#define LOG_LOCAL_LEVEL ESP_LOG_DEBUG +#endif #include "esp_lcd_panel_io_interface.h" #include "esp_lcd_panel_io.h" #include "driver/spi_master.h" @@ -53,6 +57,9 @@ typedef struct { esp_err_t esp_lcd_new_panel_io_spi(esp_lcd_spi_bus_handle_t bus, const esp_lcd_panel_io_spi_config_t *io_config, esp_lcd_panel_io_handle_t *ret_io) { +#if CONFIG_LCD_ENABLE_DEBUG_LOG + esp_log_level_set(TAG, ESP_LOG_DEBUG); +#endif esp_err_t ret = ESP_OK; esp_lcd_panel_io_spi_t *spi_panel_io = NULL; ESP_GOTO_ON_FALSE(bus && io_config && ret_io, ESP_ERR_INVALID_ARG, err, TAG, "invalid argument"); diff --git a/components/esp_lcd/src/esp_lcd_panel_nt35510.c b/components/esp_lcd/src/esp_lcd_panel_nt35510.c index 3bae8dae1b..15a33a0a72 100644 --- a/components/esp_lcd/src/esp_lcd_panel_nt35510.c +++ b/components/esp_lcd/src/esp_lcd_panel_nt35510.c @@ -1,13 +1,17 @@ /* - * SPDX-FileCopyrightText: 2021 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2021-2022 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ -// #define LOG_LOCAL_LEVEL ESP_LOG_DEBUG - #include #include +#include "sdkconfig.h" +#if CONFIG_LCD_ENABLE_DEBUG_LOG +// The local log level must be defined before including esp_log.h +// Set the maximum log level for this source file +#define LOG_LOCAL_LEVEL ESP_LOG_DEBUG +#endif #include "freertos/FreeRTOS.h" #include "freertos/task.h" #include "esp_lcd_panel_interface.h" @@ -45,6 +49,9 @@ typedef struct { esp_err_t esp_lcd_new_panel_nt35510(const esp_lcd_panel_io_handle_t io, const esp_lcd_panel_dev_config_t *panel_dev_config, esp_lcd_panel_handle_t *ret_panel) { +#if CONFIG_LCD_ENABLE_DEBUG_LOG + esp_log_level_set(TAG, ESP_LOG_DEBUG); +#endif esp_err_t ret = ESP_OK; nt35510_panel_t *nt35510 = NULL; ESP_GOTO_ON_FALSE(io && panel_dev_config && ret_panel, ESP_ERR_INVALID_ARG, err, TAG, "invalid argument"); diff --git a/components/esp_lcd/src/esp_lcd_panel_ssd1306.c b/components/esp_lcd/src/esp_lcd_panel_ssd1306.c index 9e56a1548b..aa0129591f 100644 --- a/components/esp_lcd/src/esp_lcd_panel_ssd1306.c +++ b/components/esp_lcd/src/esp_lcd_panel_ssd1306.c @@ -1,13 +1,17 @@ /* - * SPDX-FileCopyrightText: 2021 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2021-2022 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ -// #define LOG_LOCAL_LEVEL ESP_LOG_DEBUG - #include #include +#include "sdkconfig.h" +#if CONFIG_LCD_ENABLE_DEBUG_LOG +// The local log level must be defined before including esp_log.h +// Set the maximum log level for this source file +#define LOG_LOCAL_LEVEL ESP_LOG_DEBUG +#endif #include "freertos/FreeRTOS.h" #include "freertos/task.h" #include "esp_lcd_panel_interface.h" @@ -56,6 +60,9 @@ typedef struct { esp_err_t esp_lcd_new_panel_ssd1306(const esp_lcd_panel_io_handle_t io, const esp_lcd_panel_dev_config_t *panel_dev_config, esp_lcd_panel_handle_t *ret_panel) { +#if CONFIG_LCD_ENABLE_DEBUG_LOG + esp_log_level_set(TAG, ESP_LOG_DEBUG); +#endif esp_err_t ret = ESP_OK; ssd1306_panel_t *ssd1306 = NULL; ESP_GOTO_ON_FALSE(io && panel_dev_config && ret_panel, ESP_ERR_INVALID_ARG, err, TAG, "invalid argument"); diff --git a/components/esp_lcd/src/esp_lcd_panel_st7789.c b/components/esp_lcd/src/esp_lcd_panel_st7789.c index e182f969c8..6e22faddff 100644 --- a/components/esp_lcd/src/esp_lcd_panel_st7789.c +++ b/components/esp_lcd/src/esp_lcd_panel_st7789.c @@ -1,13 +1,17 @@ /* - * SPDX-FileCopyrightText: 2021 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2021-2022 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ -// #define LOG_LOCAL_LEVEL ESP_LOG_DEBUG - #include #include +#include "sdkconfig.h" +#if CONFIG_LCD_ENABLE_DEBUG_LOG +// The local log level must be defined before including esp_log.h +// Set the maximum log level for this source file +#define LOG_LOCAL_LEVEL ESP_LOG_DEBUG +#endif #include "freertos/FreeRTOS.h" #include "freertos/task.h" #include "esp_lcd_panel_interface.h" @@ -45,6 +49,9 @@ typedef struct { esp_err_t esp_lcd_new_panel_st7789(const esp_lcd_panel_io_handle_t io, const esp_lcd_panel_dev_config_t *panel_dev_config, esp_lcd_panel_handle_t *ret_panel) { +#if CONFIG_LCD_ENABLE_DEBUG_LOG + esp_log_level_set(TAG, ESP_LOG_DEBUG); +#endif esp_err_t ret = ESP_OK; st7789_panel_t *st7789 = NULL; ESP_GOTO_ON_FALSE(io && panel_dev_config && ret_panel, ESP_ERR_INVALID_ARG, err, TAG, "invalid argument"); diff --git a/components/esp_lcd/src/esp_lcd_rgb_panel.c b/components/esp_lcd/src/esp_lcd_rgb_panel.c index 969f603867..fd7695ef03 100644 --- a/components/esp_lcd/src/esp_lcd_rgb_panel.c +++ b/components/esp_lcd/src/esp_lcd_rgb_panel.c @@ -4,13 +4,16 @@ * SPDX-License-Identifier: Apache-2.0 */ -// #define LOG_LOCAL_LEVEL ESP_LOG_DEBUG - #include #include #include #include #include "sdkconfig.h" +#if CONFIG_LCD_ENABLE_DEBUG_LOG +// The local log level must be defined before including esp_log.h +// Set the maximum log level for this source file +#define LOG_LOCAL_LEVEL ESP_LOG_DEBUG +#endif #include "freertos/FreeRTOS.h" #include "freertos/task.h" #include "freertos/semphr.h" @@ -31,7 +34,6 @@ #if CONFIG_SPIRAM #include "spiram.h" #endif -#if SOC_LCDCAM_SUPPORTED #include "esp_lcd_common.h" #include "soc/lcd_periph.h" #include "hal/lcd_hal.h" @@ -93,6 +95,9 @@ struct esp_rgb_panel_t { esp_err_t esp_lcd_new_rgb_panel(const esp_lcd_rgb_panel_config_t *rgb_panel_config, esp_lcd_panel_handle_t *ret_panel) { +#if CONFIG_LCD_ENABLE_DEBUG_LOG + esp_log_level_set(TAG, ESP_LOG_DEBUG); +#endif esp_err_t ret = ESP_OK; esp_rgb_panel_t *rgb_panel = NULL; ESP_GOTO_ON_FALSE(rgb_panel_config && ret_panel, ESP_ERR_INVALID_ARG, err, TAG, "invalid parameter"); @@ -529,5 +534,3 @@ IRAM_ATTR static void lcd_default_isr_handler(void *args) portYIELD_FROM_ISR(); } } - -#endif // SOC_LCDCAM_SUPPORTED From 9422fe077a765a91b9bd0c7f7c0ddd4a518eb259 Mon Sep 17 00:00:00 2001 From: morris Date: Thu, 3 Mar 2022 15:35:43 +0800 Subject: [PATCH 2/6] lcd: support I2S1 LCD mode on esp32 --- components/esp_lcd/src/esp_lcd_common.h | 4 +-- components/esp_lcd/src/esp_lcd_panel_io_i2s.c | 19 +++++++---- components/esp_lcd/src/esp_lcd_panel_io_i80.c | 16 ++++----- .../test_apps/i2c_lcd/main/CMakeLists.txt | 3 +- .../test_apps/i80_lcd/main/CMakeLists.txt | 3 +- .../test_apps/spi_lcd/main/CMakeLists.txt | 3 +- components/hal/esp32s3/include/hal/lcd_ll.h | 32 +++++++++--------- .../soc/esp32/include/soc/Kconfig.soc_caps.in | 4 +-- components/soc/esp32/include/soc/soc_caps.h | 2 +- components/soc/esp32/lcd_periph.c | 33 ++++++++++++++++++- .../esp32s2/include/soc/Kconfig.soc_caps.in | 4 +-- components/soc/esp32s2/include/soc/soc_caps.h | 2 +- .../esp32s3/include/soc/Kconfig.soc_caps.in | 8 ++--- components/soc/esp32s3/include/soc/soc_caps.h | 4 +-- 14 files changed, 85 insertions(+), 52 deletions(-) diff --git a/components/esp_lcd/src/esp_lcd_common.h b/components/esp_lcd/src/esp_lcd_common.h index 6c534bc9f9..f036bc5fa8 100644 --- a/components/esp_lcd/src/esp_lcd_common.h +++ b/components/esp_lcd/src/esp_lcd_common.h @@ -19,8 +19,8 @@ extern "C" { #endif -#define LCD_INTR_ALLOC_FLAGS ESP_INTR_FLAG_INTRDISABLED -#define LCD_MEM_ALLOC_CAPS MALLOC_CAP_DEFAULT +#define LCD_I80_INTR_ALLOC_FLAGS ESP_INTR_FLAG_INTRDISABLED +#define LCD_I80_MEM_ALLOC_CAPS MALLOC_CAP_DEFAULT #define LCD_PERIPH_CLOCK_PRE_SCALE (2) // This is the minimum divider that can be applied to LCD peripheral diff --git a/components/esp_lcd/src/esp_lcd_panel_io_i2s.c b/components/esp_lcd/src/esp_lcd_panel_io_i2s.c index d1fac6fd01..c8ccdf308f 100644 --- a/components/esp_lcd/src/esp_lcd_panel_io_i2s.c +++ b/components/esp_lcd/src/esp_lcd_panel_io_i2s.c @@ -57,7 +57,7 @@ static esp_err_t i2s_lcd_init_dma_link(esp_lcd_i80_bus_handle_t bus); static esp_err_t i2s_lcd_configure_gpio(esp_lcd_i80_bus_handle_t bus, const esp_lcd_i80_bus_config_t *bus_config); static void i2s_lcd_trigger_quick_trans_done_event(esp_lcd_i80_bus_handle_t bus); static void lcd_i80_switch_devices(lcd_panel_io_i80_t *cur_device, lcd_panel_io_i80_t *next_device); -static IRAM_ATTR void lcd_default_isr_handler(void *args); +static void lcd_default_isr_handler(void *args); struct esp_lcd_i80_bus_t { int bus_id; // Bus ID, index from 0 @@ -149,9 +149,16 @@ esp_err_t esp_lcd_new_i80_bus(const esp_lcd_i80_bus_config_t *bus_config, esp_lc bus->format_buffer = heap_caps_calloc(1, CONFIG_LCD_PANEL_IO_FORMAT_BUF_SIZE, MALLOC_CAP_INTERNAL | MALLOC_CAP_DMA); #endif // SOC_I2S_TRANS_SIZE_ALIGN_WORD ESP_GOTO_ON_FALSE(bus->format_buffer, ESP_ERR_NO_MEM, err, TAG, "no mem for format buffer"); - // I2S0 has the LCD mode, but the LCD mode can't work with other modes at the same time, we need to register the driver object to the I2S platform - ESP_GOTO_ON_ERROR(i2s_priv_register_object(bus, 0), err, TAG, "register to I2S platform failed"); - bus->bus_id = 0; + // LCD mode can't work with other modes at the same time, we need to register the driver object to the I2S platform + int bus_id = -1; + for (int i = 0; i < SOC_LCD_I80_BUSES; i++) { + if (i2s_priv_register_object(bus, i) == ESP_OK) { + bus_id = i; + break; + } + } + ESP_GOTO_ON_FALSE(bus_id != -1, ESP_ERR_NOT_FOUND, err, TAG, "no free i80 bus slot"); + bus->bus_id = bus_id; // initialize HAL layer i2s_hal_init(&bus->hal, bus->bus_id); // set peripheral clock resolution @@ -163,7 +170,7 @@ esp_err_t esp_lcd_new_i80_bus(const esp_lcd_i80_bus_config_t *bus_config, esp_lc i2s_ll_tx_reset_fifo(bus->hal.dev); // install interrupt service, (I2S LCD mode only uses the "TX Unit", which leaves "RX Unit" for other purpose) // So the interrupt should also be able to share with other functionality - int isr_flags = ESP_INTR_FLAG_INTRDISABLED | ESP_INTR_FLAG_SHARED; + int isr_flags = LCD_I80_INTR_ALLOC_FLAGS | ESP_INTR_FLAG_SHARED; ret = esp_intr_alloc_intrstatus(lcd_periph_signals.buses[bus->bus_id].irq_id, isr_flags, (uint32_t)i2s_ll_get_intr_status_reg(bus->hal.dev), I2S_LL_EVENT_TX_EOF, lcd_default_isr_handler, bus, &bus->intr); @@ -257,7 +264,7 @@ esp_err_t esp_lcd_new_panel_io_i80(esp_lcd_i80_bus_handle_t bus, const esp_lcd_p uint32_t pclk_prescale = bus->resolution_hz / 2 / io_config->pclk_hz; ESP_GOTO_ON_FALSE(pclk_prescale > 0 && pclk_prescale <= I2S_LL_BCK_MAX_PRESCALE, ESP_ERR_NOT_SUPPORTED, err, TAG, "prescaler can't satisfy PCLK clock %u", io_config->pclk_hz); - i80_device = calloc(1, sizeof(lcd_panel_io_i80_t) + io_config->trans_queue_depth * sizeof(lcd_i80_trans_descriptor_t)); + i80_device = heap_caps_calloc(1, sizeof(lcd_panel_io_i80_t) + io_config->trans_queue_depth * sizeof(lcd_i80_trans_descriptor_t), LCD_I80_MEM_ALLOC_CAPS); ESP_GOTO_ON_FALSE(i80_device, ESP_ERR_NO_MEM, err, TAG, "no mem for i80 panel io"); // create two queues for i80 device i80_device->trans_queue = xQueueCreate(io_config->trans_queue_depth, sizeof(lcd_i80_trans_descriptor_t *)); diff --git a/components/esp_lcd/src/esp_lcd_panel_io_i80.c b/components/esp_lcd/src/esp_lcd_panel_io_i80.c index da834bce08..64920d5cb0 100644 --- a/components/esp_lcd/src/esp_lcd_panel_io_i80.c +++ b/components/esp_lcd/src/esp_lcd_panel_io_i80.c @@ -55,7 +55,7 @@ static esp_err_t lcd_i80_select_periph_clock(esp_lcd_i80_bus_handle_t bus, lcd_c static esp_err_t lcd_i80_bus_configure_gpio(esp_lcd_i80_bus_handle_t bus, const esp_lcd_i80_bus_config_t *bus_config); static void lcd_i80_switch_devices(lcd_panel_io_i80_t *cur_device, lcd_panel_io_i80_t *next_device); static void lcd_start_transaction(esp_lcd_i80_bus_t *bus, lcd_i80_trans_descriptor_t *trans_desc); -static IRAM_ATTR void lcd_default_isr_handler(void *args); +static void lcd_default_isr_handler(void *args); struct esp_lcd_i80_bus_t { int bus_id; // Bus ID, index from 0 @@ -153,7 +153,7 @@ esp_err_t esp_lcd_new_i80_bus(const esp_lcd_i80_bus_config_t *bus_config, esp_lc ESP_GOTO_ON_ERROR(ret, err, TAG, "select periph clock %d failed", bus_config->clk_src); // install interrupt service, (LCD peripheral shares the same interrupt source with Camera peripheral with different mask) // interrupt is disabled by default - int isr_flags = LCD_INTR_ALLOC_FLAGS | ESP_INTR_FLAG_SHARED; + int isr_flags = LCD_I80_INTR_ALLOC_FLAGS | ESP_INTR_FLAG_SHARED; ret = esp_intr_alloc_intrstatus(lcd_periph_signals.buses[bus_id].irq_id, isr_flags, (uint32_t)lcd_ll_get_interrupt_status_reg(bus->hal.dev), LCD_LL_EVENT_TRANS_DONE, lcd_default_isr_handler, bus, &bus->intr); @@ -250,7 +250,7 @@ esp_err_t esp_lcd_new_panel_io_i80(esp_lcd_i80_bus_handle_t bus, const esp_lcd_p uint32_t pclk_prescale = bus->resolution_hz / io_config->pclk_hz; ESP_GOTO_ON_FALSE(pclk_prescale > 0 && pclk_prescale <= LCD_LL_CLOCK_PRESCALE_MAX, ESP_ERR_NOT_SUPPORTED, err, TAG, "prescaler can't satisfy PCLK clock %u", io_config->pclk_hz); - i80_device = heap_caps_calloc(1, sizeof(lcd_panel_io_i80_t) + io_config->trans_queue_depth * sizeof(lcd_i80_trans_descriptor_t), LCD_MEM_ALLOC_CAPS); + i80_device = heap_caps_calloc(1, sizeof(lcd_panel_io_i80_t) + io_config->trans_queue_depth * sizeof(lcd_i80_trans_descriptor_t), LCD_I80_MEM_ALLOC_CAPS); ESP_GOTO_ON_FALSE(i80_device, ESP_ERR_NO_MEM, err, TAG, "no mem for i80 panel io"); // create two queues for i80 device i80_device->trans_queue = xQueueCreate(io_config->trans_queue_depth, sizeof(lcd_i80_trans_descriptor_t *)); @@ -393,9 +393,8 @@ static esp_err_t panel_io_i80_tx_param(esp_lcd_panel_io_t *io, int lcd_cmd, cons // switch devices if necessary lcd_i80_switch_devices(cur_device, next_device); // set data format - lcd_ll_swap_data_byte_order(bus->hal.dev, false); - lcd_ll_reverse_data_bit_order(bus->hal.dev, false); - lcd_ll_reverse_data_8bits_order(bus->hal.dev, next_device->lcd_param_bits > bus->bus_width); + lcd_ll_reverse_bit_order(bus->hal.dev, false); + lcd_ll_swap_byte_order(bus->hal.dev, bus->bus_width, next_device->lcd_param_bits > bus->bus_width); bus->cur_trans = NULL; bus->cur_device = next_device; // package a transaction @@ -643,9 +642,8 @@ IRAM_ATTR static void lcd_default_isr_handler(void *args) // switch devices if necessary lcd_i80_switch_devices(cur_device, next_device); // only reverse data bit/bytes for color data - lcd_ll_reverse_data_bit_order(bus->hal.dev, next_device->flags.reverse_color_bits); - lcd_ll_swap_data_byte_order(bus->hal.dev, next_device->flags.swap_color_bytes); - lcd_ll_reverse_data_8bits_order(bus->hal.dev, false); + lcd_ll_reverse_bit_order(bus->hal.dev, next_device->flags.reverse_color_bits); + lcd_ll_swap_byte_order(bus->hal.dev, bus->bus_width, next_device->flags.swap_color_bytes); bus->cur_trans = trans_desc; bus->cur_device = next_device; // mount data to DMA links diff --git a/components/esp_lcd/test_apps/i2c_lcd/main/CMakeLists.txt b/components/esp_lcd/test_apps/i2c_lcd/main/CMakeLists.txt index 687518660d..1e685eb807 100644 --- a/components/esp_lcd/test_apps/i2c_lcd/main/CMakeLists.txt +++ b/components/esp_lcd/test_apps/i2c_lcd/main/CMakeLists.txt @@ -1,7 +1,6 @@ set(srcs "test_app_main.c" "test_i2c_lcd_panel.c") -idf_component_register(SRCS ${srcs} - PRIV_REQUIRES esp_lcd unity driver) +idf_component_register(SRCS ${srcs}) target_link_libraries(${COMPONENT_LIB} INTERFACE "-u test_app_include_i2c_lcd") diff --git a/components/esp_lcd/test_apps/i80_lcd/main/CMakeLists.txt b/components/esp_lcd/test_apps/i80_lcd/main/CMakeLists.txt index ef0741a205..eef50b7e7c 100644 --- a/components/esp_lcd/test_apps/i80_lcd/main/CMakeLists.txt +++ b/components/esp_lcd/test_apps/i80_lcd/main/CMakeLists.txt @@ -1,7 +1,6 @@ set(srcs "test_app_main.c" "test_i80_lcd_panel.c") -idf_component_register(SRCS ${srcs} - PRIV_REQUIRES esp_lcd unity driver) +idf_component_register(SRCS ${srcs}) target_link_libraries(${COMPONENT_LIB} INTERFACE "-u test_app_include_i80_lcd") diff --git a/components/esp_lcd/test_apps/spi_lcd/main/CMakeLists.txt b/components/esp_lcd/test_apps/spi_lcd/main/CMakeLists.txt index 8fce5f1489..aacecc1821 100644 --- a/components/esp_lcd/test_apps/spi_lcd/main/CMakeLists.txt +++ b/components/esp_lcd/test_apps/spi_lcd/main/CMakeLists.txt @@ -1,7 +1,6 @@ set(srcs "test_app_main.c" "test_spi_lcd_panel.c") -idf_component_register(SRCS ${srcs} - PRIV_REQUIRES esp_lcd unity driver) +idf_component_register(SRCS ${srcs}) target_link_libraries(${COMPONENT_LIB} INTERFACE "-u test_app_include_spi_lcd") diff --git a/components/hal/esp32s3/include/hal/lcd_ll.h b/components/hal/esp32s3/include/hal/lcd_ll.h index 548c897c8e..89b7674f7e 100644 --- a/components/hal/esp32s3/include/hal/lcd_ll.h +++ b/components/hal/esp32s3/include/hal/lcd_ll.h @@ -207,6 +207,7 @@ static inline void lcd_ll_start(lcd_cam_dev_t *dev) * * @param dev LCD register base address */ +__attribute__((always_inline)) static inline void lcd_ll_stop(lcd_cam_dev_t *dev) { dev->lcd_user.lcd_start = 0; @@ -230,34 +231,32 @@ static inline void lcd_ll_reset(lcd_cam_dev_t *dev) * @param en True to reverse, False to not reverse */ __attribute__((always_inline)) -static inline void lcd_ll_reverse_data_bit_order(lcd_cam_dev_t *dev, bool en) +static inline void lcd_ll_reverse_bit_order(lcd_cam_dev_t *dev, bool en) { // whether to change LCD_DATA_out[N:0] to LCD_DATA_out[0:N] dev->lcd_user.lcd_bit_order = en; } /** - * @brief Whether to swap data byte order, i.e. data[15:0] -> data[7:0][15:8] + * @brief Whether to swap adjacent two bytes * * @param dev LCD register base address + * @param width Bus width * @param en True to swap the byte order, False to not swap */ __attribute__((always_inline)) -static inline void lcd_ll_swap_data_byte_order(lcd_cam_dev_t *dev, bool en) +static inline void lcd_ll_swap_byte_order(lcd_cam_dev_t *dev, uint32_t width, bool en) { - dev->lcd_user.lcd_byte_order = en; -} - -/** - * @brief Whether to reverse the 8bits order - * - * @param dev LCD register base address - * @param en True to reverse, False to not reverse - */ -__attribute__((always_inline)) -static inline void lcd_ll_reverse_data_8bits_order(lcd_cam_dev_t *dev, bool en) -{ - dev->lcd_user.lcd_8bits_order = en; + HAL_ASSERT(width == 8 || width == 16); + if (width == 8) { + // {B0}{B1}{B2}{B3} => {B1}{B0}{B3}{B2} + dev->lcd_user.lcd_8bits_order = en; + dev->lcd_user.lcd_byte_order = 0; + } else if (width == 16) { + // {B1,B0},{B3,B2} => {B0,B1}{B2,B3} + dev->lcd_user.lcd_byte_order = en; + dev->lcd_user.lcd_8bits_order = 0; + } } /** @@ -265,6 +264,7 @@ static inline void lcd_ll_reverse_data_8bits_order(lcd_cam_dev_t *dev, bool en) * * @param dev LCD register base address */ +__attribute__((always_inline)) static inline void lcd_ll_fifo_reset(lcd_cam_dev_t *dev) { dev->lcd_misc.lcd_afifo_reset = 1; // self clear diff --git a/components/soc/esp32/include/soc/Kconfig.soc_caps.in b/components/soc/esp32/include/soc/Kconfig.soc_caps.in index 489a4cb2ba..e13b9238ac 100644 --- a/components/soc/esp32/include/soc/Kconfig.soc_caps.in +++ b/components/soc/esp32/include/soc/Kconfig.soc_caps.in @@ -260,8 +260,8 @@ config SOC_LCD_I80_SUPPORTED default y config SOC_LCD_I80_BUSES - bool - default y + int + default 2 config SOC_LCD_I80_BUS_WIDTH int diff --git a/components/soc/esp32/include/soc/soc_caps.h b/components/soc/esp32/include/soc/soc_caps.h index 79127e4911..8a9c32ec3a 100644 --- a/components/soc/esp32/include/soc/soc_caps.h +++ b/components/soc/esp32/include/soc/soc_caps.h @@ -185,7 +185,7 @@ /*-------------------------- LCD CAPS ----------------------------------------*/ /* Notes: On esp32, LCD intel 8080 timing is generated by I2S peripheral */ #define SOC_LCD_I80_SUPPORTED (1) /*!< Intel 8080 LCD is supported */ -#define SOC_LCD_I80_BUSES (1) /*!< Only I2S0 has LCD mode */ +#define SOC_LCD_I80_BUSES (2) /*!< Both I2S0/1 have LCD mode */ #define SOC_LCD_I80_BUS_WIDTH (24) /*!< Intel 8080 bus width */ /*-------------------------- LEDC CAPS ---------------------------------------*/ diff --git a/components/soc/esp32/lcd_periph.c b/components/soc/esp32/lcd_periph.c index a3bc25a335..148a1527aa 100644 --- a/components/soc/esp32/lcd_periph.c +++ b/components/soc/esp32/lcd_periph.c @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2021 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2021-2022 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -40,6 +40,37 @@ const lcd_signal_conn_t lcd_periph_signals = { I2S0O_DATA_OUT23_IDX, }, .wr_sig = I2S0O_WS_OUT_IDX, + }, + [1] = { + .module = PERIPH_I2S1_MODULE, + .irq_id = ETS_I2S1_INTR_SOURCE, + .data_sigs = { + I2S1O_DATA_OUT0_IDX, + I2S1O_DATA_OUT1_IDX, + I2S1O_DATA_OUT2_IDX, + I2S1O_DATA_OUT3_IDX, + I2S1O_DATA_OUT4_IDX, + I2S1O_DATA_OUT5_IDX, + I2S1O_DATA_OUT6_IDX, + I2S1O_DATA_OUT7_IDX, + I2S1O_DATA_OUT8_IDX, + I2S1O_DATA_OUT9_IDX, + I2S1O_DATA_OUT10_IDX, + I2S1O_DATA_OUT11_IDX, + I2S1O_DATA_OUT12_IDX, + I2S1O_DATA_OUT13_IDX, + I2S1O_DATA_OUT14_IDX, + I2S1O_DATA_OUT15_IDX, + I2S1O_DATA_OUT16_IDX, + I2S1O_DATA_OUT17_IDX, + I2S1O_DATA_OUT18_IDX, + I2S1O_DATA_OUT19_IDX, + I2S1O_DATA_OUT20_IDX, + I2S1O_DATA_OUT21_IDX, + I2S1O_DATA_OUT22_IDX, + I2S1O_DATA_OUT23_IDX, + }, + .wr_sig = I2S1O_WS_OUT_IDX, } } }; diff --git a/components/soc/esp32s2/include/soc/Kconfig.soc_caps.in b/components/soc/esp32s2/include/soc/Kconfig.soc_caps.in index d18fecaccc..95c4e03538 100644 --- a/components/soc/esp32s2/include/soc/Kconfig.soc_caps.in +++ b/components/soc/esp32s2/include/soc/Kconfig.soc_caps.in @@ -328,8 +328,8 @@ config SOC_LCD_I80_SUPPORTED default y config SOC_LCD_I80_BUSES - bool - default y + int + default 1 config SOC_LCD_I80_BUS_WIDTH int diff --git a/components/soc/esp32s2/include/soc/soc_caps.h b/components/soc/esp32s2/include/soc/soc_caps.h index 8e6fd779a3..e62053c214 100644 --- a/components/soc/esp32s2/include/soc/soc_caps.h +++ b/components/soc/esp32s2/include/soc/soc_caps.h @@ -173,7 +173,7 @@ /*-------------------------- LCD CAPS ----------------------------------------*/ /* Notes: On esp32-s2, LCD intel 8080 timing is generated by I2S peripheral */ #define SOC_LCD_I80_SUPPORTED (1) /*!< Intel 8080 LCD is supported */ -#define SOC_LCD_I80_BUSES (1) /*!< Only I2S0 has LCD mode */ +#define SOC_LCD_I80_BUSES (1U) /*!< Only I2S0 has LCD mode */ #define SOC_LCD_I80_BUS_WIDTH (24) /*!< Intel 8080 bus width */ /*-------------------------- LEDC CAPS ---------------------------------------*/ diff --git a/components/soc/esp32s3/include/soc/Kconfig.soc_caps.in b/components/soc/esp32s3/include/soc/Kconfig.soc_caps.in index be6eb1a2ed..8b8a667e33 100644 --- a/components/soc/esp32s3/include/soc/Kconfig.soc_caps.in +++ b/components/soc/esp32s3/include/soc/Kconfig.soc_caps.in @@ -528,12 +528,12 @@ config SOC_LCD_RGB_SUPPORTED default y config SOC_LCD_I80_BUSES - bool - default y + int + default 1 config SOC_LCD_RGB_PANELS - bool - default y + int + default 1 config SOC_LCD_I80_BUS_WIDTH int diff --git a/components/soc/esp32s3/include/soc/soc_caps.h b/components/soc/esp32s3/include/soc/soc_caps.h index 88d6e7cfc7..6502f72046 100644 --- a/components/soc/esp32s3/include/soc/soc_caps.h +++ b/components/soc/esp32s3/include/soc/soc_caps.h @@ -194,8 +194,8 @@ /* Notes: On esp32-s3, I80 bus and RGB timing generator can't work at the same time */ #define SOC_LCD_I80_SUPPORTED (1) /*!< Intel 8080 LCD is supported */ #define SOC_LCD_RGB_SUPPORTED (1) /*!< RGB LCD is supported */ -#define SOC_LCD_I80_BUSES (1) /*!< Has one LCD Intel 8080 bus */ -#define SOC_LCD_RGB_PANELS (1) /*!< Support one RGB LCD panel */ +#define SOC_LCD_I80_BUSES (1U) /*!< Has one LCD Intel 8080 bus */ +#define SOC_LCD_RGB_PANELS (1U) /*!< Support one RGB LCD panel */ #define SOC_LCD_I80_BUS_WIDTH (16) /*!< Intel 8080 bus width */ #define SOC_LCD_RGB_DATA_WIDTH (16) /*!< Number of LCD data lines */ From f06a13ad82efd52a66ab8632fe7318a790121899 Mon Sep 17 00:00:00 2001 From: morris Date: Thu, 3 Mar 2022 15:39:24 +0800 Subject: [PATCH 3/6] lcd: workaround auto next frame hardware bug Closes https://github.com/espressif/esp-idf/issues/8381 --- .../esp_lcd/include/esp_lcd_panel_rgb.h | 4 +- components/esp_lcd/src/esp_lcd_rgb_panel.c | 96 ++++++++----------- docs/en/migration-guides/peripherals.rst | 7 ++ examples/peripherals/lcd/rgb_panel/README.md | 2 +- .../lcd/rgb_panel/main/rgb_lcd_example_main.c | 17 +--- .../lcd/rgb_panel/sdkconfig.defaults | 1 + .../lcd/rgb_panel/sdkconfig.defaults.esp32s3 | 2 - 7 files changed, 56 insertions(+), 73 deletions(-) diff --git a/components/esp_lcd/include/esp_lcd_panel_rgb.h b/components/esp_lcd/include/esp_lcd_panel_rgb.h index 6f06deb816..836fc343de 100644 --- a/components/esp_lcd/include/esp_lcd_panel_rgb.h +++ b/components/esp_lcd/include/esp_lcd_panel_rgb.h @@ -59,12 +59,12 @@ typedef struct { unsigned int hsync_front_porch; /*!< Horizontal front porch, number of PCLK between the end of active data and the next hsync */ unsigned int vsync_pulse_width; /*!< Vertical sync width, unit: number of lines */ unsigned int vsync_back_porch; /*!< Vertical back porch, number of invalid lines between vsync and start of frame */ - unsigned int vsync_front_porch; /*!< Vertical front porch, number of invalid lines between then end of frame and the next vsync */ + unsigned int vsync_front_porch; /*!< Vertical front porch, number of invalid lines between the end of frame and the next vsync */ struct { unsigned int hsync_idle_low: 1; /*!< The hsync signal is low in IDLE state */ unsigned int vsync_idle_low: 1; /*!< The vsync signal is low in IDLE state */ unsigned int de_idle_high: 1; /*!< The de signal is high in IDLE state */ - unsigned int pclk_active_neg: 1; /*!< Whether the display data is clocked out at the falling edge of PCLK */ + unsigned int pclk_active_pos: 1; /*!< Whether the display data is clocked out on the rising edge of PCLK */ unsigned int pclk_idle_high: 1; /*!< The PCLK stays at high level in IDLE phase */ } flags; } esp_lcd_rgb_timing_t; diff --git a/components/esp_lcd/src/esp_lcd_rgb_panel.c b/components/esp_lcd/src/esp_lcd_rgb_panel.c index fd7695ef03..d86e48ddd7 100644 --- a/components/esp_lcd/src/esp_lcd_rgb_panel.c +++ b/components/esp_lcd/src/esp_lcd_rgb_panel.c @@ -58,7 +58,8 @@ static esp_err_t rgb_panel_disp_off(esp_lcd_panel_t *panel, bool off); static esp_err_t lcd_rgb_panel_select_periph_clock(esp_rgb_panel_t *panel, lcd_clock_source_t clk_src); static esp_err_t lcd_rgb_panel_create_trans_link(esp_rgb_panel_t *panel); static esp_err_t lcd_rgb_panel_configure_gpio(esp_rgb_panel_t *panel, const esp_lcd_rgb_panel_config_t *panel_config); -static IRAM_ATTR void lcd_default_isr_handler(void *args); +static void lcd_rgb_panel_start_transmission(esp_rgb_panel_t *rgb_panel); +static void lcd_default_isr_handler(void *args); struct esp_rgb_panel_t { esp_lcd_panel_t base; // Base class of generic lcd panel @@ -77,9 +78,6 @@ struct esp_rgb_panel_t { size_t resolution_hz; // Peripheral clock resolution esp_lcd_rgb_timing_t timings; // RGB timing parameters (e.g. pclk, sync pulse, porch width) gdma_channel_handle_t dma_chan; // DMA channel handle - int new_frame_id; // ID for new frame, we use ID to identify whether the frame content has been updated - int cur_frame_id; // ID for current transferring frame - SemaphoreHandle_t done_sem; // Binary semaphore, indicating if the new frame has been flushed to LCD esp_lcd_rgb_panel_frame_trans_done_cb_t on_frame_trans_done; // Callback, invoked after frame trans done void *user_ctx; // Reserved user's data of callback functions int x_gap; // Extra gap in x coordinate, it's used when calculate the flush window @@ -87,7 +85,6 @@ struct esp_rgb_panel_t { struct { unsigned int disp_en_level: 1; // The level which can turn on the screen by `disp_gpio_num` unsigned int stream_mode: 1; // If set, the LCD transfers data continuously, otherwise, it stops refreshing the LCD when transaction done - unsigned int new_frame: 1; // Whether the frame we're going to flush is a new one unsigned int fb_in_psram: 1; // Whether the frame buffer is in PSRAM } flags; dma_descriptor_t dma_nodes[]; // DMA descriptor pool of size `num_dma_nodes` @@ -120,6 +117,7 @@ esp_err_t esp_lcd_new_rgb_panel(const esp_lcd_rgb_panel_config_t *rgb_panel_conf rgb_panel->panel_id = panel_id; // enable APB to access LCD registers periph_module_enable(lcd_periph_signals.panels[panel_id].module); + periph_module_reset(lcd_periph_signals.panels[panel_id].module); // alloc frame buffer bool alloc_from_psram = false; // fb_in_psram is only an option, if there's no PSRAM on board, we still alloc from SRAM @@ -143,10 +141,6 @@ esp_err_t esp_lcd_new_rgb_panel(const esp_lcd_rgb_panel_config_t *rgb_panel_conf rgb_panel->sram_trans_align = sram_trans_align; rgb_panel->fb_size = fb_size; rgb_panel->flags.fb_in_psram = alloc_from_psram; - // semaphore indicates new frame trans done - rgb_panel->done_sem = xSemaphoreCreateBinary(); - ESP_GOTO_ON_FALSE(rgb_panel->done_sem, ESP_ERR_NO_MEM, err, TAG, "create done sem failed"); - xSemaphoreGive(rgb_panel->done_sem); // initialize the semaphore count to 1 // initialize HAL layer, so we can call LL APIs later lcd_hal_init(&rgb_panel->hal, panel_id); // set peripheral clock resolution @@ -187,7 +181,7 @@ esp_err_t esp_lcd_new_rgb_panel(const esp_lcd_rgb_panel_config_t *rgb_panel_conf rgb_panel->base.set_gap = rgb_panel_set_gap; // return base class *ret_panel = &(rgb_panel->base); - ESP_LOGD(TAG, "new rgb panel(%d) @%p, fb_size=%zu", rgb_panel->panel_id, rgb_panel, rgb_panel->fb_size); + ESP_LOGD(TAG, "new rgb panel(%d) @%p, fb @%p, size=%zu", rgb_panel->panel_id, rgb_panel, rgb_panel->fb, rgb_panel->fb_size); return ESP_OK; err: @@ -199,9 +193,6 @@ err: if (rgb_panel->fb) { free(rgb_panel->fb); } - if (rgb_panel->done_sem) { - vSemaphoreDelete(rgb_panel->done_sem); - } if (rgb_panel->dma_chan) { gdma_disconnect(rgb_panel->dma_chan); gdma_del_channel(rgb_panel->dma_chan); @@ -221,14 +212,12 @@ err: static esp_err_t rgb_panel_del(esp_lcd_panel_t *panel) { esp_rgb_panel_t *rgb_panel = __containerof(panel, esp_rgb_panel_t, base); - xSemaphoreTake(rgb_panel->done_sem, portMAX_DELAY); // wait for last flush done int panel_id = rgb_panel->panel_id; gdma_disconnect(rgb_panel->dma_chan); gdma_del_channel(rgb_panel->dma_chan); esp_intr_free(rgb_panel->intr); periph_module_disable(lcd_periph_signals.panels[panel_id].module); lcd_com_remove_device(LCD_COM_DEVICE_TYPE_RGB, rgb_panel->panel_id); - vSemaphoreDelete(rgb_panel->done_sem); free(rgb_panel->fb); if (rgb_panel->pm_lock) { esp_pm_lock_release(rgb_panel->pm_lock); @@ -261,7 +250,7 @@ static esp_err_t rgb_panel_init(esp_lcd_panel_t *panel) rgb_panel->timings.pclk_hz = rgb_panel->resolution_hz / pclk_prescale; // pixel clock phase and polarity lcd_ll_set_clock_idle_level(rgb_panel->hal.dev, rgb_panel->timings.flags.pclk_idle_high); - lcd_ll_set_pixel_clock_edge(rgb_panel->hal.dev, rgb_panel->timings.flags.pclk_active_neg); + lcd_ll_set_pixel_clock_edge(rgb_panel->hal.dev, !rgb_panel->timings.flags.pclk_active_pos); // enable RGB mode and set data width lcd_ll_enable_rgb_mode(rgb_panel->hal.dev, true); lcd_ll_set_data_width(rgb_panel->hal.dev, rgb_panel->data_width); @@ -283,12 +272,16 @@ static esp_err_t rgb_panel_init(esp_lcd_panel_t *panel) lcd_ll_enable_output_hsync_in_porch_region(rgb_panel->hal.dev, true); // generate the hsync at the very begining of line lcd_ll_set_hsync_position(rgb_panel->hal.dev, 0); - // starting sending next frame automatically - lcd_ll_enable_auto_next_frame(rgb_panel->hal.dev, rgb_panel->flags.stream_mode); + // restart flush by hardware has some limitation, instead, the driver will restart the flush in the VSYNC end interrupt by software + lcd_ll_enable_auto_next_frame(rgb_panel->hal.dev, false); // trigger interrupt on the end of frame lcd_ll_enable_interrupt(rgb_panel->hal.dev, LCD_LL_EVENT_VSYNC_END, true); // enable intr esp_intr_enable(rgb_panel->intr); + // start transmission + if (rgb_panel->flags.stream_mode) { + lcd_rgb_panel_start_transmission(rgb_panel); + } ESP_LOGD(TAG, "rgb panel(%d) start, pclk=%uHz", rgb_panel->panel_id, rgb_panel->timings.pclk_hz); err: return ret; @@ -308,7 +301,7 @@ static esp_err_t rgb_panel_draw_bitmap(esp_lcd_panel_t *panel, int x_start, int x_end = MIN(x_end, rgb_panel->timings.h_res); y_start = MIN(y_start, rgb_panel->timings.v_res); y_end = MIN(y_end, rgb_panel->timings.v_res); - xSemaphoreTake(rgb_panel->done_sem, portMAX_DELAY); // wait for last transaction done + // convert the frame buffer to 3D array int bytes_per_pixel = rgb_panel->data_width / 8; int pixels_per_line = rgb_panel->timings.h_res; @@ -326,20 +319,12 @@ static esp_err_t rgb_panel_draw_bitmap(esp_lcd_panel_t *panel, int x_start, int // CPU writes data to PSRAM through DCache, data in PSRAM might not get updated, so write back Cache_WriteBack_Addr((uint32_t)&to[y_start][0][0], (y_end - y_start) * rgb_panel->timings.h_res * bytes_per_pixel); } - // we don't care the exact frame ID, as long as it's different from the previous one - rgb_panel->new_frame_id++; + + // restart the new transmission if (!rgb_panel->flags.stream_mode) { - // in one-off mode, the "new frame" flag is controlled by this API - rgb_panel->cur_frame_id = rgb_panel->new_frame_id; - rgb_panel->flags.new_frame = 1; - // reset FIFO of DMA and LCD, incase there remains old frame data - gdma_reset(rgb_panel->dma_chan); - lcd_ll_stop(rgb_panel->hal.dev); - lcd_ll_fifo_reset(rgb_panel->hal.dev); - gdma_start(rgb_panel->dma_chan, (intptr_t)rgb_panel->dma_nodes); + lcd_rgb_panel_start_transmission(rgb_panel); } - // start LCD engine - lcd_ll_start(rgb_panel->hal.dev); + return ESP_OK; } @@ -475,12 +460,8 @@ static esp_err_t lcd_rgb_panel_create_trans_link(esp_rgb_panel_t *panel) panel->dma_nodes[i].dw0.owner = DMA_DESCRIPTOR_BUFFER_OWNER_CPU; panel->dma_nodes[i].next = &panel->dma_nodes[i + 1]; } - // fix the last DMA descriptor according to whether the LCD works in stream mode - if (panel->flags.stream_mode) { - panel->dma_nodes[panel->num_dma_nodes - 1].next = &panel->dma_nodes[0]; // chain into a circle - } else { - panel->dma_nodes[panel->num_dma_nodes - 1].next = NULL; // one-off DMA chain - } + // one-off DMA chain + panel->dma_nodes[panel->num_dma_nodes - 1].next = NULL; // mount the frame buffer to the DMA descriptors lcd_com_mount_dma_data(panel->dma_nodes, panel->fb, panel->fb_size); // alloc DMA channel and connect to LCD peripheral @@ -502,32 +483,37 @@ err: return ret; } +static void lcd_rgb_panel_start_transmission(esp_rgb_panel_t *rgb_panel) +{ + // reset FIFO of DMA and LCD, incase there remains old frame data + gdma_reset(rgb_panel->dma_chan); + lcd_ll_stop(rgb_panel->hal.dev); + lcd_ll_fifo_reset(rgb_panel->hal.dev); + gdma_start(rgb_panel->dma_chan, (intptr_t)rgb_panel->dma_nodes); + // delay 1us is sufficient for DMA to pass data to LCD FIFO + // in fact, this is only needed when LCD pixel clock is set too high + esp_rom_delay_us(1); + // start LCD engine + lcd_ll_start(rgb_panel->hal.dev); +} + IRAM_ATTR static void lcd_default_isr_handler(void *args) { - esp_rgb_panel_t *panel = (esp_rgb_panel_t *)args; + esp_rgb_panel_t *rgb_panel = (esp_rgb_panel_t *)args; bool need_yield = false; - BaseType_t high_task_woken = pdFALSE; - uint32_t intr_status = lcd_ll_get_interrupt_status(panel->hal.dev); - lcd_ll_clear_interrupt_status(panel->hal.dev, intr_status); + uint32_t intr_status = lcd_ll_get_interrupt_status(rgb_panel->hal.dev); + lcd_ll_clear_interrupt_status(rgb_panel->hal.dev, intr_status); if (intr_status & LCD_LL_EVENT_VSYNC_END) { - if (panel->flags.new_frame) { // the finished one is a new frame - if (panel->on_frame_trans_done) { - if (panel->on_frame_trans_done(&panel->base, NULL, panel->user_ctx)) { - need_yield = true; - } - } - xSemaphoreGiveFromISR(panel->done_sem, &high_task_woken); - if (high_task_woken == pdTRUE) { + // call user registered callback + if (rgb_panel->on_frame_trans_done) { + if (rgb_panel->on_frame_trans_done(&rgb_panel->base, NULL, rgb_panel->user_ctx)) { need_yield = true; } } - // in stream mode, the "new frame" flag is controlled by comparing "new frame id" and "cur frame id" - if (panel->flags.stream_mode) { - // new_frame_id is only modified in `rgb_panel_draw_bitmap()`, fetch first and use below to avoid inconsistent - int new_frame_id = panel->new_frame_id; - panel->flags.new_frame = (panel->cur_frame_id != new_frame_id); - panel->cur_frame_id = new_frame_id; + // to restart the transmission + if (rgb_panel->flags.stream_mode) { + lcd_rgb_panel_start_transmission(rgb_panel); } } if (need_yield) { diff --git a/docs/en/migration-guides/peripherals.rst b/docs/en/migration-guides/peripherals.rst index 58cb0a342b..4b71dcb82a 100644 --- a/docs/en/migration-guides/peripherals.rst +++ b/docs/en/migration-guides/peripherals.rst @@ -115,3 +115,10 @@ I2C - ``rmt_set_intr_enable_mask`` and ``rmt_clr_intr_enable_mask`` are removed, as the interrupt is handled by the driver, user doesn't need to take care of it. - ``rmt_set_pin`` is removed, as ``rmt_set_gpio`` can do the same thing. - ``rmt_memory_rw_rst`` is removed, user can use ``rmt_tx_memory_reset`` and ``rmt_rx_memory_reset`` for TX and RX channel respectively. + +.. only:: SOC_LCD_RGB_SUPPORTED + + RGB LCD Driver + -------------- + + - The `pclk_active_neg` in the RGB timing configuration structure :cpp:type:`esp_lcd_rgb_timing_t` has been changed into `pclk_active_pos`. This was made to change the default PCLK sample moment to **falling** edge. From user side, you don't need to explicitly assign `pclk_active_neg = true` anymore. diff --git a/examples/peripherals/lcd/rgb_panel/README.md b/examples/peripherals/lcd/rgb_panel/README.md index 6169166eef..8998e56ec1 100644 --- a/examples/peripherals/lcd/rgb_panel/README.md +++ b/examples/peripherals/lcd/rgb_panel/README.md @@ -83,6 +83,6 @@ I (741) example: Display LVGL Scatter Chart * The frame buffer of RGB panel is located in ESP side (unlike other controller based LCDs, where the frame buffer is located in external chip). As the frame buffer usually consumes much RAM (depends on the LCD resolution and color depth), we recommend to put the frame buffer into PSRAM (like what we do in this example). However, putting frame buffer in PSRAM will limit the PCLK to around 12MHz (due to the bandwidth of PSRAM). * LCD screen drift * Slow down the PCLK frequency - * Adjust other timing parameters like PCLK clock edge (by `pclk_active_neg`), sync porches like HBP (by `hsync_back_porch`) according to your LCD spec + * Adjust other timing parameters like PCLK clock edge (by `pclk_active_pos`), sync porches like HBP (by `hsync_back_porch`) according to your LCD spec For any technical queries, please open an [issue] (https://github.com/espressif/esp-idf/issues) on GitHub. We will get back to you soon. \ No newline at end of file diff --git a/examples/peripherals/lcd/rgb_panel/main/rgb_lcd_example_main.c b/examples/peripherals/lcd/rgb_panel/main/rgb_lcd_example_main.c index 25b8930e23..e6915a3091 100644 --- a/examples/peripherals/lcd/rgb_panel/main/rgb_lcd_example_main.c +++ b/examples/peripherals/lcd/rgb_panel/main/rgb_lcd_example_main.c @@ -54,13 +54,6 @@ static const char *TAG = "example"; extern void example_lvgl_demo_ui(lv_obj_t *scr); -static bool example_notify_lvgl_flush_ready(esp_lcd_panel_handle_t panel, esp_lcd_rgb_panel_event_data_t *event_data, void *user_data) -{ - lv_disp_drv_t *disp_driver = (lv_disp_drv_t *)user_data; - lv_disp_flush_ready(disp_driver); - return false; -} - static void example_lvgl_flush_cb(lv_disp_drv_t *drv, const lv_area_t *area, lv_color_t *color_map) { esp_lcd_panel_handle_t panel_handle = (esp_lcd_panel_handle_t) drv->user_data; @@ -70,6 +63,7 @@ static void example_lvgl_flush_cb(lv_disp_drv_t *drv, const lv_area_t *area, lv_ int offsety2 = area->y2; // copy a buffer's content to a specific area of the display esp_lcd_panel_draw_bitmap(panel_handle, offsetx1, offsety1, offsetx2 + 1, offsety2 + 1, color_map); + lv_disp_flush_ready(drv); } static void example_increase_lvgl_tick(void *arg) @@ -124,17 +118,14 @@ void app_main(void) .h_res = EXAMPLE_LCD_H_RES, .v_res = EXAMPLE_LCD_V_RES, // The following parameters should refer to LCD spec - .hsync_back_porch = 68, + .hsync_back_porch = 40, .hsync_front_porch = 20, - .hsync_pulse_width = 5, - .vsync_back_porch = 18, + .hsync_pulse_width = 1, + .vsync_back_porch = 8, .vsync_front_porch = 4, .vsync_pulse_width = 1, - .flags.pclk_active_neg = 1, // RGB data is clocked out on falling edge }, .flags.fb_in_psram = 1, // allocate frame buffer in PSRAM - .on_frame_trans_done = example_notify_lvgl_flush_ready, - .user_ctx = &disp_drv, }; ESP_ERROR_CHECK(esp_lcd_new_rgb_panel(&panel_config, &panel_handle)); diff --git a/examples/peripherals/lcd/rgb_panel/sdkconfig.defaults b/examples/peripherals/lcd/rgb_panel/sdkconfig.defaults index 6bcdadb68e..d21ca32513 100644 --- a/examples/peripherals/lcd/rgb_panel/sdkconfig.defaults +++ b/examples/peripherals/lcd/rgb_panel/sdkconfig.defaults @@ -2,3 +2,4 @@ CONFIG_LV_MEM_CUSTOM=y CONFIG_LV_MEMCPY_MEMSET_STD=y CONFIG_LV_USE_USER_DATA=y CONFIG_LV_USE_CHART=y +CONFIG_LV_USE_PERF_MONITOR=y diff --git a/examples/peripherals/lcd/rgb_panel/sdkconfig.defaults.esp32s3 b/examples/peripherals/lcd/rgb_panel/sdkconfig.defaults.esp32s3 index 238f484b06..2877855408 100644 --- a/examples/peripherals/lcd/rgb_panel/sdkconfig.defaults.esp32s3 +++ b/examples/peripherals/lcd/rgb_panel/sdkconfig.defaults.esp32s3 @@ -1,5 +1,3 @@ CONFIG_ESP32S3_SPIRAM_SUPPORT=y CONFIG_SPIRAM_MODE_OCT=y CONFIG_SPIRAM_SPEED_80M=y -# Can't set the FPS too high due to the limitation of PSRAM bandwidth -CONFIG_LV_DISP_DEF_REFR_PERIOD=100 From 7112009473e37f048241d0aa7ee1f7693caadfde Mon Sep 17 00:00:00 2001 From: morris Date: Mon, 7 Mar 2022 18:04:02 +0800 Subject: [PATCH 4/6] lcd: support rgb lcd interupt iram safe --- components/driver/gdma.c | 20 +-- components/driver/gptimer.c | 16 +- components/driver/pulse_cnt.c | 17 ++- components/esp_lcd/CMakeLists.txt | 16 +- components/esp_lcd/Kconfig | 13 ++ components/esp_lcd/linker.lf | 6 + components/esp_lcd/src/esp_lcd_rgb_panel.c | 18 ++- .../esp_lcd/test_apps/rgb_lcd/CMakeLists.txt | 13 ++ .../test_apps/rgb_lcd/main/CMakeLists.txt | 3 +- .../test_apps/rgb_lcd/main/test_app_main.c | 2 +- .../test_apps/rgb_lcd/main/test_rgb_panel.c | 144 +++++++++++++++--- .../test_apps/rgb_lcd/pytest_rgb_lcd.py | 1 + .../test_apps/rgb_lcd/sdkconfig.ci.iram_safe | 5 + .../rgb_lcd/sdkconfig.defaults.esp32s3 | 1 + 14 files changed, 218 insertions(+), 57 deletions(-) create mode 100644 components/esp_lcd/linker.lf create mode 100644 components/esp_lcd/test_apps/rgb_lcd/sdkconfig.ci.iram_safe diff --git a/components/driver/gdma.c b/components/driver/gdma.c index 1d0c49a0d5..5250d49eac 100644 --- a/components/driver/gdma.c +++ b/components/driver/gdma.c @@ -26,13 +26,17 @@ static const char *TAG = "gdma"; -#if CONFIG_GDMA_ISR_IRAM_SAFE -#define GDMA_INTR_ALLOC_FLAGS (ESP_INTR_FLAG_IRAM | ESP_INTR_FLAG_INTRDISABLED) +#if CONFIG_GDMA_ISR_IRAM_SAFE || CONFIG_GDMA_CTRL_FUNC_IN_IRAM #define GDMA_MEM_ALLOC_CAPS (MALLOC_CAP_INTERNAL | MALLOC_CAP_8BIT) #else -#define GDMA_INTR_ALLOC_FLAGS ESP_INTR_FLAG_INTRDISABLED #define GDMA_MEM_ALLOC_CAPS MALLOC_CAP_DEFAULT -#endif // CONFIG_GDMA_ISR_IRAM_SAFE +#endif + +#if CONFIG_GDMA_ISR_IRAM_SAFE +#define GDMA_INTR_ALLOC_FLAGS (ESP_INTR_FLAG_IRAM | ESP_INTR_FLAG_INTRDISABLED) +#else +#define GDMA_INTR_ALLOC_FLAGS ESP_INTR_FLAG_INTRDISABLED +#endif #define GDMA_INVALID_PERIPH_TRIG (0x3F) #define SEARCH_REQUEST_RX_CHANNEL (1 << 0) @@ -385,9 +389,7 @@ esp_err_t gdma_register_tx_event_callbacks(gdma_channel_handle_t dma_chan, gdma_ ESP_GOTO_ON_FALSE(esp_ptr_in_iram(cbs->on_trans_eof), ESP_ERR_INVALID_ARG, err, TAG, "on_trans_eof not in IRAM"); } if (user_data) { - ESP_GOTO_ON_FALSE(esp_ptr_in_dram(user_data) || - esp_ptr_in_diram_dram(user_data) || - esp_ptr_in_rtc_dram_fast(user_data), ESP_ERR_INVALID_ARG, err, TAG, "user context not in DRAM"); + ESP_GOTO_ON_FALSE(esp_ptr_internal(user_data), ESP_ERR_INVALID_ARG, err, TAG, "user context not in internal RAM"); } #endif // CONFIG_GDMA_ISR_IRAM_SAFE @@ -423,9 +425,7 @@ esp_err_t gdma_register_rx_event_callbacks(gdma_channel_handle_t dma_chan, gdma_ ESP_GOTO_ON_FALSE(esp_ptr_in_iram(cbs->on_recv_eof), ESP_ERR_INVALID_ARG, err, TAG, "on_recv_eof not in IRAM"); } if (user_data) { - ESP_GOTO_ON_FALSE(esp_ptr_in_dram(user_data) || - esp_ptr_in_diram_dram(user_data) || - esp_ptr_in_rtc_dram_fast(user_data), ESP_ERR_INVALID_ARG, err, TAG, "user context not in DRAM"); + ESP_GOTO_ON_FALSE(esp_ptr_internal(user_data), ESP_ERR_INVALID_ARG, err, TAG, "user context not in internal RAM"); } #endif // CONFIG_GDMA_ISR_IRAM_SAFE diff --git a/components/driver/gptimer.c b/components/driver/gptimer.c index 5ddb46a7de..0c51207eb2 100644 --- a/components/driver/gptimer.c +++ b/components/driver/gptimer.c @@ -31,13 +31,17 @@ // If ISR handler is allowed to run whilst cache is disabled, // Make sure all the code and related variables used by the handler are in the SRAM -#if CONFIG_GPTIMER_ISR_IRAM_SAFE -#define GPTIMER_INTR_ALLOC_FLAGS (ESP_INTR_FLAG_IRAM | ESP_INTR_FLAG_INTRDISABLED) +#if CONFIG_GPTIMER_ISR_IRAM_SAFE || CONFIG_GPTIMER_CTRL_FUNC_IN_IRAM #define GPTIMER_MEM_ALLOC_CAPS (MALLOC_CAP_INTERNAL | MALLOC_CAP_8BIT) #else -#define GPTIMER_INTR_ALLOC_FLAGS ESP_INTR_FLAG_INTRDISABLED #define GPTIMER_MEM_ALLOC_CAPS MALLOC_CAP_DEFAULT -#endif //CONFIG_GPTIMER_ISR_IRAM_SAFE +#endif + +#if CONFIG_GPTIMER_ISR_IRAM_SAFE +#define GPTIMER_INTR_ALLOC_FLAGS (ESP_INTR_FLAG_IRAM | ESP_INTR_FLAG_INTRDISABLED) +#else +#define GPTIMER_INTR_ALLOC_FLAGS ESP_INTR_FLAG_INTRDISABLED +#endif #define GPTIMER_PM_LOCK_NAME_LEN_MAX 16 @@ -240,9 +244,7 @@ esp_err_t gptimer_register_event_callbacks(gptimer_handle_t timer, const gptimer ESP_RETURN_ON_FALSE(esp_ptr_in_iram(cbs->on_alarm), ESP_ERR_INVALID_ARG, TAG, "on_alarm callback not in IRAM"); } if (user_data) { - ESP_RETURN_ON_FALSE(esp_ptr_in_dram(user_data) || - esp_ptr_in_diram_dram(user_data) || - esp_ptr_in_rtc_dram_fast(user_data), ESP_ERR_INVALID_ARG, TAG, "user context not in DRAM"); + ESP_RETURN_ON_FALSE(esp_ptr_internal(user_data), ESP_ERR_INVALID_ARG, TAG, "user context not in internal RAM"); } #endif diff --git a/components/driver/pulse_cnt.c b/components/driver/pulse_cnt.c index 8c72896da1..15a8ebdb16 100644 --- a/components/driver/pulse_cnt.c +++ b/components/driver/pulse_cnt.c @@ -32,13 +32,17 @@ // If ISR handler is allowed to run whilst cache is disabled, // Make sure all the code and related variables used by the handler are in the SRAM -#if CONFIG_PCNT_ISR_IRAM_SAFE -#define PCNT_INTR_ALLOC_FLAGS (ESP_INTR_FLAG_IRAM | ESP_INTR_FLAG_INTRDISABLED | ESP_INTR_FLAG_SHARED) +#if CONFIG_PCNT_ISR_IRAM_SAFE || CONFIG_PCNT_CTRL_FUNC_IN_IRAM #define PCNT_MEM_ALLOC_CAPS (MALLOC_CAP_INTERNAL | MALLOC_CAP_8BIT) #else +#define PCNT_MEM_ALLOC_CAPS MALLOC_CAP_DEFAULT +#endif + +#if CONFIG_PCNT_ISR_IRAM_SAFE +#define PCNT_INTR_ALLOC_FLAGS (ESP_INTR_FLAG_IRAM | ESP_INTR_FLAG_INTRDISABLED | ESP_INTR_FLAG_SHARED) +#else #define PCNT_INTR_ALLOC_FLAGS (ESP_INTR_FLAG_INTRDISABLED | ESP_INTR_FLAG_SHARED) -#define PCNT_MEM_ALLOC_CAPS MALLOC_CAP_DEFAULT -#endif //CONFIG_PCNT_ISR_IRAM_SAFE +#endif #define PCNT_PM_LOCK_NAME_LEN_MAX 16 @@ -340,10 +344,7 @@ esp_err_t pcnt_unit_register_event_callbacks(pcnt_unit_handle_t unit, const pcnt ESP_RETURN_ON_FALSE(esp_ptr_in_iram(cbs->on_reach), ESP_ERR_INVALID_ARG, TAG, "on_reach callback not in IRAM"); } if (user_data) { - ESP_RETURN_ON_FALSE(esp_ptr_in_dram(user_data) || - esp_ptr_in_diram_dram(user_data) || - esp_ptr_in_rtc_dram_fast(user_data), - ESP_ERR_INVALID_ARG, TAG, "user context not in DRAM"); + ESP_RETURN_ON_FALSE(esp_ptr_internal(user_data), ESP_ERR_INVALID_ARG, TAG, "user context not in internal RAM"); } #endif diff --git a/components/esp_lcd/CMakeLists.txt b/components/esp_lcd/CMakeLists.txt index ffc0cd5119..81ca044e4e 100644 --- a/components/esp_lcd/CMakeLists.txt +++ b/components/esp_lcd/CMakeLists.txt @@ -1,17 +1,23 @@ set(srcs "src/esp_lcd_common.c" "src/esp_lcd_panel_io.c" "src/esp_lcd_panel_io_i2c.c" - "src/esp_lcd_panel_io_i2s.c" "src/esp_lcd_panel_io_spi.c" - "src/esp_lcd_panel_io_i80.c" "src/esp_lcd_panel_nt35510.c" "src/esp_lcd_panel_ssd1306.c" "src/esp_lcd_panel_st7789.c" - "src/esp_lcd_panel_ops.c" - "src/esp_lcd_rgb_panel.c") + "src/esp_lcd_panel_ops.c") set(includes "include" "interface") set(priv_requires "driver") +if(CONFIG_SOC_I2S_LCD_I80_VARIANT) + list(APPEND srcs "src/esp_lcd_panel_io_i2s.c") +endif() + +if(CONFIG_SOC_LCDCAM_SUPPORTED) + list(APPEND srcs "src/esp_lcd_panel_io_i80.c" "src/esp_lcd_rgb_panel.c") +endif() + idf_component_register(SRCS ${srcs} INCLUDE_DIRS ${includes} - PRIV_REQUIRES ${priv_requires}) + PRIV_REQUIRES ${priv_requires} + LDFRAGMENTS linker.lf) diff --git a/components/esp_lcd/Kconfig b/components/esp_lcd/Kconfig index 95136ff705..5e62153575 100644 --- a/components/esp_lcd/Kconfig +++ b/components/esp_lcd/Kconfig @@ -6,11 +6,24 @@ menu "LCD and Touch Panel" help LCD driver allocates an internal buffer to transform the data into a proper format, because of the endian order mismatch. This option is to set the size of the buffer, in bytes. + config LCD_ENABLE_DEBUG_LOG bool "Enable debug log" default n help Wether to enable the debug log message for LCD driver. Note that, this option only controls the LCD driver log, won't affect other drivers. + + if SOC_LCD_RGB_SUPPORTED + config LCD_RGB_ISR_IRAM_SAFE + bool "RGB LCD ISR IRAM-Safe" + default n + select GDMA_CTRL_FUNC_IN_IRAM # need to restart GDMA in the LCD ISR + help + Ensure the LCD interrupt is IRAM-Safe by allowing the interrupt handler to be + executable when the cache is disabled (e.g. SPI Flash write). + If you want the LCD driver to keep flushing the screen even when cache ops disabled, + you can enable this option. Note, this will also increase the IRAM usage. + endif # SOC_LCD_RGB_SUPPORTED endmenu endmenu diff --git a/components/esp_lcd/linker.lf b/components/esp_lcd/linker.lf new file mode 100644 index 0000000000..51f703659e --- /dev/null +++ b/components/esp_lcd/linker.lf @@ -0,0 +1,6 @@ +[mapping:esp_lcd] +archive: libesp_lcd.a +entries: + if LCD_RGB_ISR_IRAM_SAFE = y: + esp_lcd_common: lcd_com_mount_dma_data (noflash) + esp_lcd_rgb_panel: lcd_rgb_panel_start_transmission (noflash) diff --git a/components/esp_lcd/src/esp_lcd_rgb_panel.c b/components/esp_lcd/src/esp_lcd_rgb_panel.c index d86e48ddd7..b93218bfdf 100644 --- a/components/esp_lcd/src/esp_lcd_rgb_panel.c +++ b/components/esp_lcd/src/esp_lcd_rgb_panel.c @@ -39,6 +39,12 @@ #include "hal/lcd_hal.h" #include "hal/lcd_ll.h" +#if CONFIG_LCD_RGB_ISR_IRAM_SAFE +#define LCD_RGB_INTR_ALLOC_FLAGS (ESP_INTR_FLAG_IRAM | ESP_INTR_FLAG_INTRDISABLED) +#else +#define LCD_RGB_INTR_ALLOC_FLAGS ESP_INTR_FLAG_INTRDISABLED +#endif + static const char *TAG = "lcd_panel.rgb"; typedef struct esp_rgb_panel_t esp_rgb_panel_t; @@ -100,6 +106,16 @@ esp_err_t esp_lcd_new_rgb_panel(const esp_lcd_rgb_panel_config_t *rgb_panel_conf ESP_GOTO_ON_FALSE(rgb_panel_config && ret_panel, ESP_ERR_INVALID_ARG, err, TAG, "invalid parameter"); ESP_GOTO_ON_FALSE(rgb_panel_config->data_width == 16, ESP_ERR_NOT_SUPPORTED, err, TAG, "unsupported data width %d", rgb_panel_config->data_width); + +#if CONFIG_LCD_RGB_ISR_IRAM_SAFE + if (rgb_panel_config->on_frame_trans_done) { + ESP_RETURN_ON_FALSE(esp_ptr_in_iram(rgb_panel_config->on_frame_trans_done), ESP_ERR_INVALID_ARG, TAG, "on_frame_trans_done callback not in IRAM"); + } + if (rgb_panel_config->user_ctx) { + ESP_RETURN_ON_FALSE(esp_ptr_internal(rgb_panel_config->user_ctx), ESP_ERR_INVALID_ARG, TAG, "user context not in internal RAM"); + } +#endif + // calculate the number of DMA descriptors size_t fb_size = rgb_panel_config->timings.h_res * rgb_panel_config->timings.v_res * rgb_panel_config->data_width / 8; size_t num_dma_nodes = fb_size / DMA_DESCRIPTOR_BUFFER_MAX_SIZE; @@ -147,7 +163,7 @@ esp_err_t esp_lcd_new_rgb_panel(const esp_lcd_rgb_panel_config_t *rgb_panel_conf ret = lcd_rgb_panel_select_periph_clock(rgb_panel, rgb_panel_config->clk_src); ESP_GOTO_ON_ERROR(ret, err, TAG, "select periph clock failed"); // install interrupt service, (LCD peripheral shares the interrupt source with Camera by different mask) - int isr_flags = LCD_INTR_ALLOC_FLAGS | ESP_INTR_FLAG_SHARED; + int isr_flags = LCD_RGB_INTR_ALLOC_FLAGS | ESP_INTR_FLAG_SHARED; ret = esp_intr_alloc_intrstatus(lcd_periph_signals.panels[panel_id].irq_id, isr_flags, (uint32_t)lcd_ll_get_interrupt_status_reg(rgb_panel->hal.dev), LCD_LL_EVENT_VSYNC_END, lcd_default_isr_handler, rgb_panel, &rgb_panel->intr); diff --git a/components/esp_lcd/test_apps/rgb_lcd/CMakeLists.txt b/components/esp_lcd/test_apps/rgb_lcd/CMakeLists.txt index e74a9f5358..4852f4edea 100644 --- a/components/esp_lcd/test_apps/rgb_lcd/CMakeLists.txt +++ b/components/esp_lcd/test_apps/rgb_lcd/CMakeLists.txt @@ -3,3 +3,16 @@ cmake_minimum_required(VERSION 3.5) include($ENV{IDF_PATH}/tools/cmake/project.cmake) project(rgb_lcd_panel_test) + +if(CONFIG_COMPILER_DUMP_RTL_FILES) + add_custom_target(check_test_app_sections ALL + COMMAND ${PYTHON} $ENV{IDF_PATH}/tools/ci/check_callgraph.py + --rtl-dir ${CMAKE_BINARY_DIR}/esp-idf/esp_lcd/ + --elf-file ${CMAKE_BINARY_DIR}/rgb_lcd_panel_test.elf + find-refs + --from-sections=.iram0.text + --to-sections=.flash.text,.flash.rodata + --exit-code + DEPENDS ${elf} + ) +endif() diff --git a/components/esp_lcd/test_apps/rgb_lcd/main/CMakeLists.txt b/components/esp_lcd/test_apps/rgb_lcd/main/CMakeLists.txt index 34d2e684ea..ecad51a17c 100644 --- a/components/esp_lcd/test_apps/rgb_lcd/main/CMakeLists.txt +++ b/components/esp_lcd/test_apps/rgb_lcd/main/CMakeLists.txt @@ -1,7 +1,6 @@ set(srcs "test_app_main.c" "test_rgb_panel.c") -idf_component_register(SRCS ${srcs} - PRIV_REQUIRES esp_lcd unity) +idf_component_register(SRCS ${srcs}) target_link_libraries(${COMPONENT_LIB} INTERFACE "-u test_app_include_rgb_lcd") diff --git a/components/esp_lcd/test_apps/rgb_lcd/main/test_app_main.c b/components/esp_lcd/test_apps/rgb_lcd/main/test_app_main.c index 434a50e362..7b7ff5c868 100644 --- a/components/esp_lcd/test_apps/rgb_lcd/main/test_app_main.c +++ b/components/esp_lcd/test_apps/rgb_lcd/main/test_app_main.c @@ -9,7 +9,7 @@ #include "esp_heap_caps.h" // Some resources are lazy allocated in LCD driver, the threadhold is left for that case -#define TEST_MEMORY_LEAK_THRESHOLD (-300) +#define TEST_MEMORY_LEAK_THRESHOLD (-500) static size_t before_free_8bit; static size_t before_free_32bit; diff --git a/components/esp_lcd/test_apps/rgb_lcd/main/test_rgb_panel.c b/components/esp_lcd/test_apps/rgb_lcd/main/test_rgb_panel.c index 12ae23cec5..272897f4cc 100644 --- a/components/esp_lcd/test_apps/rgb_lcd/main/test_rgb_panel.c +++ b/components/esp_lcd/test_apps/rgb_lcd/main/test_rgb_panel.c @@ -5,22 +5,30 @@ */ #include #include +#include "freertos/FreeRTOS.h" +#include "freertos/task.h" #include "unity.h" #include "esp_lcd_panel_rgb.h" #include "esp_lcd_panel_ops.h" #include "esp_random.h" +#include "esp_attr.h" +#include "nvs_flash.h" #include "test_rgb_board.h" +#if CONFIG_LCD_RGB_ISR_IRAM_SAFE +#define TEST_LCD_CALLBACK_ATTR IRAM_ATTR +#else +#define TEST_LCD_CALLBACK_ATTR +#endif // CONFIG_LCD_RGB_ISR_IRAM_SAFE + +#define TEST_IMG_SIZE (100 * 100 * sizeof(uint16_t)) + void test_app_include_rgb_lcd(void) { } -TEST_CASE("lcd_rgb_lcd_panel", "[lcd]") +static esp_lcd_panel_handle_t test_rgb_panel_initialization(bool stream_mode, esp_lcd_rgb_panel_frame_trans_done_cb_t cb, void *user_data) { -#define TEST_IMG_SIZE (100 * 100 * sizeof(uint16_t)) - uint8_t *img = malloc(TEST_IMG_SIZE); - TEST_ASSERT_NOT_NULL(img); - esp_lcd_panel_handle_t panel_handle = NULL; esp_lcd_rgb_panel_config_t panel_config = { .data_width = 16, @@ -59,27 +67,117 @@ TEST_CASE("lcd_rgb_lcd_panel", "[lcd]") .vsync_back_porch = 18, .vsync_front_porch = 4, .vsync_pulse_width = 1, - .flags.pclk_active_neg = 1, // RGB data is clocked out on falling edge }, + .on_frame_trans_done = cb, + .user_ctx = user_data, .flags.fb_in_psram = 1, // allocate frame buffer in PSRAM + .flags.relax_on_idle = !stream_mode, }; - // Test stream mode and one-off mode - for (int i = 0; i < 2; i++) { - panel_config.flags.relax_on_idle = i; - TEST_ESP_OK(esp_lcd_new_rgb_panel(&panel_config, &panel_handle)); - TEST_ESP_OK(esp_lcd_panel_reset(panel_handle)); - TEST_ESP_OK(esp_lcd_panel_init(panel_handle)); - for (int i = 0; i < 200; i++) { - uint8_t color_byte = esp_random() & 0xFF; - int x_start = esp_random() % (TEST_LCD_H_RES - 100); - int y_start = esp_random() % (TEST_LCD_V_RES - 100); - memset(img, color_byte, TEST_IMG_SIZE); - esp_lcd_panel_draw_bitmap(panel_handle, x_start, y_start, x_start + 100, y_start + 100, img); - } + TEST_ESP_OK(esp_lcd_new_rgb_panel(&panel_config, &panel_handle)); + TEST_ESP_OK(esp_lcd_panel_reset(panel_handle)); + TEST_ESP_OK(esp_lcd_panel_init(panel_handle)); - TEST_ESP_OK(esp_lcd_panel_del(panel_handle)); - } - free(img); -#undef TEST_IMG_SIZE + return panel_handle; } + +TEST_CASE("lcd_rgb_panel_stream_mode", "[lcd]") +{ + uint8_t *img = malloc(TEST_IMG_SIZE); + TEST_ASSERT_NOT_NULL(img); + + printf("initialize RGB panel with stream mode\r\n"); + esp_lcd_panel_handle_t panel_handle = test_rgb_panel_initialization(true, NULL, NULL); + printf("flush random color block\r\n"); + for (int i = 0; i < 200; i++) { + uint8_t color_byte = esp_random() & 0xFF; + int x_start = esp_random() % (TEST_LCD_H_RES - 100); + int y_start = esp_random() % (TEST_LCD_V_RES - 100); + memset(img, color_byte, TEST_IMG_SIZE); + esp_lcd_panel_draw_bitmap(panel_handle, x_start, y_start, x_start + 100, y_start + 100, img); + } + printf("delete RGB panel\r\n"); + TEST_ESP_OK(esp_lcd_panel_del(panel_handle)); + free(img); +} + +TEST_LCD_CALLBACK_ATTR static bool test_rgb_panel_trans_done(esp_lcd_panel_handle_t panel, esp_lcd_rgb_panel_event_data_t *edata, void *user_ctx) +{ + TaskHandle_t task_to_notify = (TaskHandle_t)user_ctx; + BaseType_t high_task_wakeup; + vTaskNotifyGiveFromISR(task_to_notify, &high_task_wakeup); + return high_task_wakeup == pdTRUE; +} + +TEST_CASE("lcd_rgb_panel_one_shot_mode", "[lcd]") +{ + uint8_t *img = malloc(TEST_IMG_SIZE); + TEST_ASSERT_NOT_NULL(img); + TaskHandle_t cur_task = xTaskGetCurrentTaskHandle(); + + printf("initialize RGB panel with ont-shot mode\r\n"); + esp_lcd_panel_handle_t panel_handle = test_rgb_panel_initialization(false, test_rgb_panel_trans_done, cur_task); + printf("flush random color block\r\n"); + for (int i = 0; i < 200; i++) { + uint8_t color_byte = esp_random() & 0xFF; + int x_start = esp_random() % (TEST_LCD_H_RES - 100); + int y_start = esp_random() % (TEST_LCD_V_RES - 100); + memset(img, color_byte, TEST_IMG_SIZE); + esp_lcd_panel_draw_bitmap(panel_handle, x_start, y_start, x_start + 100, y_start + 100, img); + // wait for flush done + TEST_ASSERT_NOT_EQUAL(0, ulTaskNotifyTake(pdFALSE, pdMS_TO_TICKS(1000))); + } + + printf("delete RGB panel\r\n"); + TEST_ESP_OK(esp_lcd_panel_del(panel_handle)); + free(img); +} + +#if CONFIG_LCD_RGB_ISR_IRAM_SAFE +TEST_CASE("lcd_rgb_panel_with_nvs_read_write", "[lcd]") +{ + uint8_t *img = malloc(TEST_IMG_SIZE); + TEST_ASSERT_NOT_NULL(img); + + printf("initialize RGB panel with stream mode\r\n"); + esp_lcd_panel_handle_t panel_handle = test_rgb_panel_initialization(true, NULL, NULL); + printf("flush one clock block to the LCD\r\n"); + uint8_t color_byte = esp_random() & 0xFF; + int x_start = esp_random() % (TEST_LCD_H_RES - 100); + int y_start = esp_random() % (TEST_LCD_V_RES - 100); + memset(img, color_byte, TEST_IMG_SIZE); + esp_lcd_panel_draw_bitmap(panel_handle, x_start, y_start, x_start + 100, y_start + 100, img); + printf("The LCD driver should keep flushing the color block in the background (as it's in stream mode)\r\n"); + + // read/write the SPI Flash by NVS APIs, the LCD driver should stay work + printf("initialize NVS flash\r\n"); + esp_err_t err = nvs_flash_init(); + if (err == ESP_ERR_NVS_NO_FREE_PAGES || err == ESP_ERR_NVS_NEW_VERSION_FOUND) { + // NVS partition was truncated and needs to be erased + TEST_ESP_OK(nvs_flash_erase()); + // Retry nvs_flash_init + err = nvs_flash_init(); + } + TEST_ESP_OK(err); + printf("open NVS storage\r\n"); + nvs_handle_t my_handle; + TEST_ESP_OK(nvs_open("storage", NVS_READWRITE, &my_handle)); + TEST_ESP_OK(nvs_erase_all(my_handle)); + int test_count; + for (int i = 0; i < 50; i++) { + printf("write %d to NVS partition\r\n", i); + TEST_ESP_OK(nvs_set_i32(my_handle, "test_count", i)); + TEST_ESP_OK(nvs_commit(my_handle)); + TEST_ESP_OK(nvs_get_i32(my_handle, "test_count", &test_count)); + TEST_ASSERT_EQUAL(i, test_count); + vTaskDelay(pdMS_TO_TICKS(50)); + } + printf("close NVS storage\r\n"); + nvs_close(my_handle); + TEST_ESP_OK(nvs_flash_deinit()); + + printf("delete RGB panel\r\n"); + TEST_ESP_OK(esp_lcd_panel_del(panel_handle)); + free(img); +} +#endif // CONFIG_LCD_RGB_ISR_IRAM_SAFE diff --git a/components/esp_lcd/test_apps/rgb_lcd/pytest_rgb_lcd.py b/components/esp_lcd/test_apps/rgb_lcd/pytest_rgb_lcd.py index 52c04a369f..99d1b1df70 100644 --- a/components/esp_lcd/test_apps/rgb_lcd/pytest_rgb_lcd.py +++ b/components/esp_lcd/test_apps/rgb_lcd/pytest_rgb_lcd.py @@ -10,6 +10,7 @@ from pytest_embedded import Dut @pytest.mark.parametrize( 'config', [ + 'iram_safe', 'release', ], indirect=True, diff --git a/components/esp_lcd/test_apps/rgb_lcd/sdkconfig.ci.iram_safe b/components/esp_lcd/test_apps/rgb_lcd/sdkconfig.ci.iram_safe new file mode 100644 index 0000000000..8043d59c96 --- /dev/null +++ b/components/esp_lcd/test_apps/rgb_lcd/sdkconfig.ci.iram_safe @@ -0,0 +1,5 @@ +CONFIG_COMPILER_DUMP_RTL_FILES=y +CONFIG_LCD_RGB_ISR_IRAM_SAFE=y + +# silent the error check, as the error string are stored in rodata, causing RTL check failure +CONFIG_COMPILER_OPTIMIZATION_CHECKS_SILENT=y diff --git a/components/esp_lcd/test_apps/rgb_lcd/sdkconfig.defaults.esp32s3 b/components/esp_lcd/test_apps/rgb_lcd/sdkconfig.defaults.esp32s3 index 2877855408..aac45bf22e 100644 --- a/components/esp_lcd/test_apps/rgb_lcd/sdkconfig.defaults.esp32s3 +++ b/components/esp_lcd/test_apps/rgb_lcd/sdkconfig.defaults.esp32s3 @@ -1,3 +1,4 @@ CONFIG_ESP32S3_SPIRAM_SUPPORT=y +CONFIG_ESPTOOLPY_OCT_FLASH=y CONFIG_SPIRAM_MODE_OCT=y CONFIG_SPIRAM_SPEED_80M=y From a019db6880cf0519f310c0aa1b45159f82310931 Mon Sep 17 00:00:00 2001 From: morris Date: Thu, 10 Mar 2022 18:48:09 +0800 Subject: [PATCH 5/6] lcd: delay 1us between DMA start and LCD start Closes https://github.com/espressif/esp-idf/issues/8212 --- components/esp_lcd/src/esp_lcd_panel_io_i80.c | 3 +++ components/hal/esp32s3/include/hal/gdma_ll.h | 4 ++++ 2 files changed, 7 insertions(+) diff --git a/components/esp_lcd/src/esp_lcd_panel_io_i80.c b/components/esp_lcd/src/esp_lcd_panel_io_i80.c index 64920d5cb0..b5e53055a0 100644 --- a/components/esp_lcd/src/esp_lcd_panel_io_i80.c +++ b/components/esp_lcd/src/esp_lcd_panel_io_i80.c @@ -565,6 +565,9 @@ static void lcd_start_transaction(esp_lcd_i80_bus_t *bus, lcd_i80_trans_descript lcd_ll_set_command(bus->hal.dev, bus->bus_width, trans_desc->cmd_value); if (trans_desc->data) { // some specific LCD commands can have no parameters gdma_start(bus->dma_chan, (intptr_t)(bus->dma_nodes)); + // delay 1us is sufficient for DMA to pass data to LCD FIFO + // in fact, this is only needed when LCD pixel clock is set too high + esp_rom_delay_us(1); } lcd_ll_start(bus->hal.dev); } diff --git a/components/hal/esp32s3/include/hal/gdma_ll.h b/components/hal/esp32s3/include/hal/gdma_ll.h index 4035c76934..873cab32d3 100644 --- a/components/hal/esp32s3/include/hal/gdma_ll.h +++ b/components/hal/esp32s3/include/hal/gdma_ll.h @@ -190,6 +190,8 @@ static inline uint32_t gdma_ll_rx_get_fifo_bytes(gdma_dev_t *dev, uint32_t chann return dev->channel[channel].in.infifo_status.infifo_cnt_l2; case 3: return dev->channel[channel].in.infifo_status.infifo_cnt_l3; + default: + return 0; } } @@ -434,6 +436,8 @@ static inline uint32_t gdma_ll_tx_get_fifo_bytes(gdma_dev_t *dev, uint32_t chann return dev->channel[channel].out.outfifo_status.outfifo_cnt_l2; case 3: return dev->channel[channel].out.outfifo_status.outfifo_cnt_l3; + default: + return 0; } } From 3517ae6387e10ebb01c34134afcae6873d3500d9 Mon Sep 17 00:00:00 2001 From: morris Date: Mon, 14 Mar 2022 10:47:33 +0800 Subject: [PATCH 6/6] lcd: check return value of xQueueReceive Fix warnnings reported by Coverity Scan Test --- components/esp_lcd/src/esp_lcd_panel_io_i2s.c | 27 ++++++++++++------- components/esp_lcd/src/esp_lcd_panel_io_i80.c | 20 +++++++++----- components/esp_lcd/src/esp_lcd_panel_io_spi.c | 14 ++++++---- 3 files changed, 40 insertions(+), 21 deletions(-) diff --git a/components/esp_lcd/src/esp_lcd_panel_io_i2s.c b/components/esp_lcd/src/esp_lcd_panel_io_i2s.c index c8ccdf308f..e1f805500f 100644 --- a/components/esp_lcd/src/esp_lcd_panel_io_i2s.c +++ b/components/esp_lcd/src/esp_lcd_panel_io_i2s.c @@ -323,9 +323,12 @@ static esp_err_t panel_io_i80_del(esp_lcd_panel_io_t *io) lcd_panel_io_i80_t *i80_device = __containerof(io, lcd_panel_io_i80_t, base); esp_lcd_i80_bus_t *bus = i80_device->bus; lcd_i80_trans_descriptor_t *trans_desc = NULL; + size_t num_trans_inflight = i80_device->num_trans_inflight; // wait all pending transaction to finish - for (size_t i = 0; i < i80_device->num_trans_inflight; i++) { - xQueueReceive(i80_device->done_queue, &trans_desc, portMAX_DELAY); + for (size_t i = 0; i < num_trans_inflight; i++) { + ESP_RETURN_ON_FALSE(xQueueReceive(i80_device->done_queue, &trans_desc, portMAX_DELAY) == pdTRUE, + ESP_FAIL, TAG, "recycle inflight transactions failed"); + i80_device->num_trans_inflight--; } // remove from device list portENTER_CRITICAL(&bus->spinlock); @@ -466,12 +469,13 @@ static esp_err_t panel_io_i80_tx_param(esp_lcd_panel_io_t *io, int lcd_cmd, cons lcd_i80_trans_descriptor_t *trans_desc = NULL; assert(param_size <= (bus->num_dma_nodes * DMA_DESCRIPTOR_BUFFER_MAX_SIZE) && "parameter bytes too long, enlarge max_transfer_bytes"); assert(param_size <= CONFIG_LCD_PANEL_IO_FORMAT_BUF_SIZE && "format buffer too small, increase CONFIG_LCD_PANEL_IO_FORMAT_BUF_SIZE"); - + size_t num_trans_inflight = next_device->num_trans_inflight; // before issue a polling transaction, need to wait queued transactions finished - for (size_t i = 0; i < next_device->num_trans_inflight; i++) { - xQueueReceive(next_device->done_queue, &trans_desc, portMAX_DELAY); + for (size_t i = 0; i < num_trans_inflight; i++) { + ESP_RETURN_ON_FALSE(xQueueReceive(next_device->done_queue, &trans_desc, portMAX_DELAY) == pdTRUE, + ESP_FAIL, TAG, "recycle inflight transactions failed"); + next_device->num_trans_inflight--; } - next_device->num_trans_inflight = 0; i2s_ll_clear_intr_status(bus->hal.dev, I2S_LL_EVENT_TX_EOF); // switch devices if necessary @@ -530,12 +534,13 @@ static esp_err_t panel_io_i80_tx_color(esp_lcd_panel_io_t *io, int lcd_cmd, cons lcd_panel_io_i80_t *cur_device = bus->cur_device; lcd_i80_trans_descriptor_t *trans_desc = NULL; assert(color_size <= (bus->num_dma_nodes * DMA_DESCRIPTOR_BUFFER_MAX_SIZE) && "color bytes too long, enlarge max_transfer_bytes"); - + size_t num_trans_inflight = next_device->num_trans_inflight; // before issue a polling transaction, need to wait queued transactions finished - for (size_t i = 0; i < next_device->num_trans_inflight; i++) { - xQueueReceive(next_device->done_queue, &trans_desc, portMAX_DELAY); + for (size_t i = 0; i < num_trans_inflight; i++) { + ESP_RETURN_ON_FALSE(xQueueReceive(next_device->done_queue, &trans_desc, portMAX_DELAY) == pdTRUE, + ESP_FAIL, TAG, "recycle inflight transactions failed"); + next_device->num_trans_inflight--; } - next_device->num_trans_inflight = 0; i2s_ll_clear_intr_status(bus->hal.dev, I2S_LL_EVENT_TX_EOF); // switch devices if necessary @@ -730,6 +735,8 @@ static IRAM_ATTR void lcd_default_isr_handler(void *args) if (high_task_woken == pdTRUE) { need_yield = true; } + // sanity check + assert(trans_desc); // only clear the interrupt status when we're sure there still remains transaction to handle i2s_ll_clear_intr_status(bus->hal.dev, I2S_LL_EVENT_TX_EOF); // switch devices if necessary diff --git a/components/esp_lcd/src/esp_lcd_panel_io_i80.c b/components/esp_lcd/src/esp_lcd_panel_io_i80.c index b5e53055a0..351c0a2ee5 100644 --- a/components/esp_lcd/src/esp_lcd_panel_io_i80.c +++ b/components/esp_lcd/src/esp_lcd_panel_io_i80.c @@ -315,8 +315,11 @@ static esp_err_t panel_io_i80_del(esp_lcd_panel_io_t *io) esp_lcd_i80_bus_t *bus = i80_device->bus; lcd_i80_trans_descriptor_t *trans_desc = NULL; // wait all pending transaction to finish - for (size_t i = 0; i < i80_device->num_trans_inflight; i++) { - xQueueReceive(i80_device->done_queue, &trans_desc, portMAX_DELAY); + size_t num_trans_inflight = i80_device->num_trans_inflight; + for (size_t i = 0; i < num_trans_inflight; i++) { + ESP_RETURN_ON_FALSE(xQueueReceive(i80_device->done_queue, &trans_desc, portMAX_DELAY) == pdTRUE, + ESP_FAIL, TAG, "recycle inflight transactions failed"); + i80_device->num_trans_inflight--; } // remove from device list portENTER_CRITICAL(&bus->spinlock); @@ -383,10 +386,12 @@ static esp_err_t panel_io_i80_tx_param(esp_lcd_panel_io_t *io, int lcd_cmd, cons i80_lcd_prepare_cmd_buffer(bus, next_device, &lcd_cmd); uint32_t param_len = i80_lcd_prepare_param_buffer(bus, next_device, param, param_size); // wait all pending transaction in the queue to finish - for (size_t i = 0; i < next_device->num_trans_inflight; i++) { - xQueueReceive(next_device->done_queue, &trans_desc, portMAX_DELAY); + size_t num_trans_inflight = next_device->num_trans_inflight; + for (size_t i = 0; i < num_trans_inflight; i++) { + ESP_RETURN_ON_FALSE( xQueueReceive(next_device->done_queue, &trans_desc, portMAX_DELAY) == pdTRUE, + ESP_FAIL, TAG, "recycle inflight transactions failed"); + next_device->num_trans_inflight--; } - next_device->num_trans_inflight = 0; uint32_t intr_status = lcd_ll_get_interrupt_status(bus->hal.dev); lcd_ll_clear_interrupt_status(bus->hal.dev, intr_status); @@ -437,7 +442,8 @@ static esp_err_t panel_io_i80_tx_color(esp_lcd_panel_io_t *io, int lcd_cmd, cons trans_desc = &i80_device->trans_pool[i80_device->num_trans_inflight]; } else { // transaction pool has used up, recycle one from done_queue - xQueueReceive(i80_device->done_queue, &trans_desc, portMAX_DELAY); + ESP_RETURN_ON_FALSE(xQueueReceive(i80_device->done_queue, &trans_desc, portMAX_DELAY) == pdTRUE, + ESP_FAIL, TAG, "recycle inflight transactions failed"); i80_device->num_trans_inflight--; } trans_desc->i80_device = i80_device; @@ -640,6 +646,8 @@ IRAM_ATTR static void lcd_default_isr_handler(void *args) if (high_task_woken == pdTRUE) { need_yield = true; } + // sanity check + assert(trans_desc); // only clear the interrupt status when we're sure there still remains transaction to handle lcd_ll_clear_interrupt_status(bus->hal.dev, intr_status); // switch devices if necessary diff --git a/components/esp_lcd/src/esp_lcd_panel_io_spi.c b/components/esp_lcd/src/esp_lcd_panel_io_spi.c index b5395de43a..6a355e08e1 100644 --- a/components/esp_lcd/src/esp_lcd_panel_io_spi.c +++ b/components/esp_lcd/src/esp_lcd_panel_io_spi.c @@ -124,9 +124,11 @@ static esp_err_t panel_io_spi_del(esp_lcd_panel_io_t *io) esp_lcd_panel_io_spi_t *spi_panel_io = __containerof(io, esp_lcd_panel_io_spi_t, base); // wait all pending transaction to finish - for (size_t i = 0; i < spi_panel_io->num_trans_inflight; i++) { + size_t num_trans_inflight = spi_panel_io->num_trans_inflight; + for (size_t i = 0; i < num_trans_inflight; i++) { ret = spi_device_get_trans_result(spi_panel_io->spi_dev, &spi_trans, portMAX_DELAY); ESP_GOTO_ON_ERROR(ret, err, TAG, "recycle spi transactions failed"); + spi_panel_io->num_trans_inflight--; } spi_bus_remove_device(spi_panel_io->spi_dev); if (spi_panel_io->dc_gpio_num >= 0) { @@ -175,11 +177,12 @@ static esp_err_t panel_io_spi_tx_param(esp_lcd_panel_io_t *io, int lcd_cmd, cons esp_lcd_panel_io_spi_t *spi_panel_io = __containerof(io, esp_lcd_panel_io_spi_t, base); // before issue a polling transaction, need to wait queued transactions finished - for (size_t i = 0; i < spi_panel_io->num_trans_inflight; i++) { + size_t num_trans_inflight = spi_panel_io->num_trans_inflight; + for (size_t i = 0; i < num_trans_inflight; i++) { ret = spi_device_get_trans_result(spi_panel_io->spi_dev, &spi_trans, portMAX_DELAY); ESP_GOTO_ON_ERROR(ret, err, TAG, "recycle spi transactions failed"); + spi_panel_io->num_trans_inflight--; } - spi_panel_io->num_trans_inflight = 0; lcd_trans = &spi_panel_io->trans_pool[0]; memset(lcd_trans, 0, sizeof(lcd_spi_trans_descriptor_t)); spi_lcd_prepare_cmd_buffer(spi_panel_io, &lcd_cmd); @@ -223,11 +226,12 @@ static esp_err_t panel_io_spi_tx_color(esp_lcd_panel_io_t *io, int lcd_cmd, cons esp_lcd_panel_io_spi_t *spi_panel_io = __containerof(io, esp_lcd_panel_io_spi_t, base); // before issue a polling transaction, need to wait queued transactions finished - for (size_t i = 0; i < spi_panel_io->num_trans_inflight; i++) { + size_t num_trans_inflight = spi_panel_io->num_trans_inflight; + for (size_t i = 0; i < num_trans_inflight; i++) { ret = spi_device_get_trans_result(spi_panel_io->spi_dev, &spi_trans, portMAX_DELAY); ESP_GOTO_ON_ERROR(ret, err, TAG, "recycle spi transactions failed"); + spi_panel_io->num_trans_inflight--; } - spi_panel_io->num_trans_inflight = 0; lcd_trans = &spi_panel_io->trans_pool[0]; memset(lcd_trans, 0, sizeof(lcd_spi_trans_descriptor_t)); spi_lcd_prepare_cmd_buffer(spi_panel_io, &lcd_cmd);