Merge branch 'feat/dsi_lcd_iram_safe' into 'master'

MIPI DSI IRAM Safe

See merge request espressif/esp-idf!30416

components/esp_hw_support/dma/Kconfig.dma

@@ -19,7 +19,7 @@ menu "GDMA Configurations"
         bool "Enable debug log"
         default n
         help
-            Wether to enable the debug log message for GDMA driver.
+            Whether to enable the debug log message for GDMA driver.
             Note that, this option only controls the GDMA driver log, won't affect other drivers.
 endmenu # GDMA Configurations
 
@@ -40,6 +40,13 @@ menu "DW_GDMA Configurations"
             Place DW_GDMA setter functions (e.g. dw_gdma_channel_set_block_markers) into IRAM,
             so that these functions can be IRAM-safe and able to be called in the other IRAM interrupt context.
 
+    config DW_GDMA_GETTER_FUNC_IN_IRAM
+        bool
+        default n
+        help
+            Place DW_GDMA getter functions (e.g. dw_gdma_link_list_get_item) into IRAM,
+            so that these functions can be IRAM-safe and able to be called in the other IRAM interrupt context.
+
     config DW_GDMA_ISR_IRAM_SAFE
         bool
         default n
@@ -52,7 +59,7 @@ menu "DW_GDMA Configurations"
         bool "Enable debug log"
         default n
         help
-            Wether to enable the debug log message for DW_GDMA driver.
+            Whether to enable the debug log message for DW_GDMA driver.
             Note that, this option only controls the DW_GDMA driver log, won't affect other drivers.
 endmenu # DW_GDMA Configurations
 

components/esp_hw_support/dma/linker.lf

@@ -78,10 +78,15 @@ entries:
     # put DW_GDMA control functions in IRAM
     if DW_GDMA_CTRL_FUNC_IN_IRAM = y:
         dw_gdma: dw_gdma_channel_continue (noflash)
+        dw_gdma: dw_gdma_channel_enable_ctrl (noflash)
 
     if DW_GDMA_SETTER_FUNC_IN_IRAM = y:
         dw_gdma: dw_gdma_channel_set_block_markers (noflash)
         dw_gdma: dw_gdma_lli_set_block_markers (noflash)
+        dw_gdma: dw_gdma_channel_use_link_list (noflash)
+
+    if DW_GDMA_GETTER_FUNC_IN_IRAM = y:
+        dw_gdma: dw_gdma_link_list_get_item (noflash)
 
 [mapping:dma2d_driver]
 archive: libesp_hw_support.a

components/esp_lcd/Kconfig

@@ -36,5 +36,20 @@ menu "LCD and Touch Panel"
                     Only need to enable it when in your application, the DMA can't deliver data
                     as fast as the LCD consumes it.
         endif # SOC_LCD_RGB_SUPPORTED
+
+        if SOC_MIPI_DSI_SUPPORTED
+            config LCD_DSI_ISR_IRAM_SAFE
+                bool "DSI LCD ISR IRAM-Safe"
+                default n
+                select DW_GDMA_ISR_IRAM_SAFE
+                select DW_GDMA_CTRL_FUNC_IN_IRAM
+                select DW_GDMA_SETTER_FUNC_IN_IRAM
+                select DW_GDMA_GETTER_FUNC_IN_IRAM
+                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_MIPI_DSI_SUPPORTED
     endmenu
 endmenu

components/esp_lcd/dsi/esp_lcd_mipi_dsi_bus.c

@@ -55,6 +55,15 @@ esp_err_t esp_lcd_new_dsi_bus(const esp_lcd_dsi_bus_config_t *bus_config, esp_lc
         mipi_dsi_ll_enable_phy_reference_clock(bus_id, true);
     }
 
+#if CONFIG_PM_ENABLE
+    // When MIPI DSI is working, we don't expect the clock source would be turned off
+    esp_pm_lock_type_t pm_lock_type = ESP_PM_NO_LIGHT_SLEEP;
+    ret  = esp_pm_lock_create(pm_lock_type, 0, "dsi_phy", &dsi_bus->pm_lock);
+    ESP_GOTO_ON_ERROR(ret, err, TAG, "create PM lock failed");
+    // before we configure the PLL, we want the clock source to be stable
+    esp_pm_lock_acquire(dsi_bus->pm_lock);
+#endif
+
     // initialize HAL context
     mipi_dsi_hal_config_t hal_config = {
         .bus_id = bus_id,
@@ -125,6 +134,10 @@ esp_err_t esp_lcd_del_dsi_bus(esp_lcd_dsi_bus_handle_t bus)
     DSI_RCC_ATOMIC() {
         mipi_dsi_ll_enable_bus_clock(bus_id, false);
     }
+    if (bus->pm_lock) {
+        esp_pm_lock_release(bus->pm_lock);
+        esp_pm_lock_delete(bus->pm_lock);
+    }
     free(bus);
     return ESP_OK;
 }

components/esp_lcd/dsi/esp_lcd_panel_dpi.c

@@ -14,6 +14,7 @@
 #include "esp_cache.h"
 #include "mipi_dsi_priv.h"
 #include "esp_async_fbcpy.h"
+#include "esp_memory_utils.h"
 #include "esp_private/dw_gdma.h"
 #include "hal/cache_hal.h"
 #include "hal/cache_ll.h"
@@ -42,6 +43,7 @@ struct esp_lcd_dpi_panel_t {
     dw_gdma_link_list_handle_t link_lists[DPI_PANEL_MAX_FB_NUM]; // DMA link list
     esp_async_fbcpy_handle_t fbcpy_handle; // Use DMA2D to do frame buffer copy
     SemaphoreHandle_t draw_sem;            // A semaphore used to synchronize the draw operations when DMA2D is used
+    esp_pm_lock_handle_t pm_lock;          // Power management lock
     esp_lcd_dpi_panel_color_trans_done_cb_t on_color_trans_done; // Callback invoked when color data transfer has finished
     esp_lcd_dpi_panel_refresh_done_cb_t on_refresh_done; // Callback invoked when one refresh operation finished (kinda like a vsync end)
     void *user_ctx; // User context for the callback
@@ -232,6 +234,14 @@ esp_err_t esp_lcd_new_panel_dpi(esp_lcd_dsi_bus_handle_t bus, const esp_lcd_dpi_
         mipi_dsi_ll_enable_dpi_clock(bus_id, true);
     }
 
+#if CONFIG_PM_ENABLE
+    // When MIPI DSI is working, we don't expect the clock source would be turned off
+    esp_pm_lock_type_t pm_lock_type = ESP_PM_NO_LIGHT_SLEEP;
+    ret  = esp_pm_lock_create(pm_lock_type, 0, "dsi_dpi", &dpi_panel->pm_lock);
+    ESP_GOTO_ON_ERROR(ret, err, TAG, "create PM lock failed");
+    esp_pm_lock_acquire(dpi_panel->pm_lock);
+#endif
+
     // create DMA resources
     ESP_GOTO_ON_ERROR(dpi_panel_create_dma_link(dpi_panel), err, TAG, "initialize DMA link failed");
 
@@ -318,6 +328,10 @@ static esp_err_t dpi_panel_del(esp_lcd_panel_t *panel)
     if (dpi_panel->draw_sem) {
         vSemaphoreDelete(dpi_panel->draw_sem);
     }
+    if (dpi_panel->pm_lock) {
+        esp_pm_lock_release(dpi_panel->pm_lock);
+        esp_pm_lock_delete(dpi_panel->pm_lock);
+    }
     free(dpi_panel);
     return ESP_OK;
 }
@@ -515,6 +529,17 @@ esp_err_t esp_lcd_dpi_panel_register_event_callbacks(esp_lcd_panel_handle_t pane
 {
     ESP_RETURN_ON_FALSE(panel && cbs, ESP_ERR_INVALID_ARG, TAG, "invalid argument");
     esp_lcd_dpi_panel_t *dpi_panel = __containerof(panel, esp_lcd_dpi_panel_t, base);
+#if CONFIG_LCD_DSI_ISR_IRAM_SAFE
+    if (cbs->on_color_trans_done) {
+        ESP_RETURN_ON_FALSE(esp_ptr_in_iram(cbs->on_color_trans_done), ESP_ERR_INVALID_ARG, TAG, "on_color_trans_done callback not in IRAM");
+    }
+    if (cbs->on_refresh_done) {
+        ESP_RETURN_ON_FALSE(esp_ptr_in_iram(cbs->on_refresh_done), ESP_ERR_INVALID_ARG, TAG, "on_refresh_done callback not in IRAM");
+    }
+    if (user_ctx) {
+        ESP_RETURN_ON_FALSE(esp_ptr_internal(user_ctx), ESP_ERR_INVALID_ARG, TAG, "user context not in internal RAM");
+    }
+#endif // CONFIG_LCD_RGB_ISR_IRAM_SAFE
     dpi_panel->on_color_trans_done = cbs->on_color_trans_done;
     dpi_panel->on_refresh_done = cbs->on_refresh_done;
     dpi_panel->user_ctx = user_ctx;

components/esp_lcd/dsi/mipi_dsi_priv.h

@@ -9,6 +9,7 @@
 #include "hal/mipi_dsi_ll.h"
 #include "esp_heap_caps.h"
 #include "esp_private/periph_ctrl.h"
+#include "esp_pm.h"
 
 #if SOC_PERIPH_CLK_CTRL_SHARED
 #define DSI_CLOCK_SRC_ATOMIC() PERIPH_RCC_ATOMIC()
@@ -22,7 +23,11 @@
 #define DSI_RCC_ATOMIC()
 #endif
 
-#define DSI_MEM_ALLOC_CAPS MALLOC_CAP_DEFAULT
+#if CONFIG_LCD_DSI_ISR_IRAM_SAFE
+#define DSI_MEM_ALLOC_CAPS      (MALLOC_CAP_INTERNAL | MALLOC_CAP_8BIT)
+#else
+#define DSI_MEM_ALLOC_CAPS      MALLOC_CAP_DEFAULT
+#endif
 
 #define DPI_PANEL_MAX_FB_NUM 3 // maximum number of supported frame buffers for DPI panel
 
@@ -35,6 +40,7 @@ extern "C" {
 typedef struct esp_lcd_dsi_bus_t {
     int bus_id;
     mipi_dsi_hal_context_t hal;
+    esp_pm_lock_handle_t pm_lock;
 } esp_lcd_dsi_bus_t;
 
 #ifdef __cplusplus

components/esp_lcd/test_apps/.build-test-rules.yml

@@ -38,7 +38,7 @@ components/esp_lcd/test_apps/mipi_dsi_lcd:
   disable_test:
     - if: IDF_TARGET == "esp32p4"
       temporary: true
-      reason: lack of runners
+      reason: lack of runners, DSI can't work without an LCD connected
 
 components/esp_lcd/test_apps/rgb_lcd:
   depends_components:

components/esp_lcd/test_apps/mipi_dsi_lcd/CMakeLists.txt

@@ -6,3 +6,16 @@ set(COMPONENTS main)
 
 include($ENV{IDF_PATH}/tools/cmake/project.cmake)
 project(mipi_dsi_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-dirs ${CMAKE_BINARY_DIR}/esp-idf/esp_lcd/,${CMAKE_BINARY_DIR}/esp-idf/hal/
+                      --elf-file ${CMAKE_BINARY_DIR}/mipi_dsi_lcd_panel_test.elf
+                      find-refs
+                      --from-sections=.iram0.text
+                      --to-sections=.flash.text,.flash.rodata
+                      --exit-code
+                      DEPENDS ${elf}
+                      )
+endif()

components/esp_lcd/test_apps/mipi_dsi_lcd/main/CMakeLists.txt

@@ -2,6 +2,10 @@ set(srcs "test_app_main.c"
          "test_mipi_dsi_board.c"
          "test_mipi_dsi_panel.c")
 
+if(CONFIG_LCD_DSI_ISR_IRAM_SAFE)
+    list(APPEND srcs "test_mipi_dsi_iram.c")
+endif()
+
 # In order for the cases defined by `TEST_CASE` to be linked into the final elf,
 # the component can be registered as WHOLE_ARCHIVE
 idf_component_register(SRCS ${srcs}

components/esp_lcd/test_apps/mipi_dsi_lcd/main/test_mipi_dsi_iram.c

@@ -0,0 +1,119 @@
+/*
+ * SPDX-FileCopyrightText: 2023-2024 Espressif Systems (Shanghai) CO LTD
+ *
+ * SPDX-License-Identifier: Apache-2.0
+ */
+#include <stdio.h>
+#include <string.h>
+#include <inttypes.h>
+#include "freertos/FreeRTOS.h"
+#include "freertos/task.h"
+#include "unity.h"
+#include "unity_test_utils.h"
+#include "esp_lcd_mipi_dsi.h"
+#include "esp_lcd_panel_ops.h"
+#include "esp_lcd_panel_io.h"
+#include "esp_random.h"
+#include "esp_attr.h"
+#include "test_mipi_dsi_board.h"
+#include "esp_lcd_ili9881c.h"
+
+IRAM_ATTR static bool test_rgb_panel_count_in_callback(esp_lcd_panel_handle_t panel, esp_lcd_dpi_panel_event_data_t *edata, void *user_ctx)
+{
+    uint32_t *count = (uint32_t *)user_ctx;
+    *count = *count + 1;
+    return false;
+}
+
+static void IRAM_ATTR test_delay_post_cache_disable(void *args)
+{
+    esp_rom_delay_us(200000);
+}
+
+#define TEST_IMG_SIZE (100 * 100 * sizeof(uint16_t))
+
+TEST_CASE("MIPI DSI draw bitmap (ILI9881C) IRAM Safe", "[mipi_dsi]")
+{
+    esp_lcd_dsi_bus_handle_t mipi_dsi_bus;
+    esp_lcd_panel_io_handle_t mipi_dbi_io;
+    esp_lcd_panel_handle_t mipi_dpi_panel;
+    esp_lcd_panel_handle_t ili9881c_ctrl_panel;
+
+    test_bsp_enable_dsi_phy_power();
+
+    uint8_t *img = malloc(TEST_IMG_SIZE);
+    TEST_ASSERT_NOT_NULL(img);
+
+    esp_lcd_dsi_bus_config_t bus_config = {
+        .bus_id = 0,
+        .num_data_lanes = 2,
+        .phy_clk_src = MIPI_DSI_PHY_CLK_SRC_DEFAULT,
+        .lane_bit_rate_mbps = 1000, // 1000 Mbps
+    };
+    TEST_ESP_OK(esp_lcd_new_dsi_bus(&bus_config, &mipi_dsi_bus));
+
+    esp_lcd_dbi_io_config_t dbi_config = {
+        .virtual_channel = 0,
+        .lcd_cmd_bits = 8,
+        .lcd_param_bits = 8,
+    };
+    TEST_ESP_OK(esp_lcd_new_panel_io_dbi(mipi_dsi_bus, &dbi_config, &mipi_dbi_io));
+
+    esp_lcd_panel_dev_config_t lcd_dev_config = {
+        .bits_per_pixel = 16,
+        .rgb_ele_order = LCD_RGB_ELEMENT_ORDER_RGB,
+        .reset_gpio_num = -1,
+    };
+    TEST_ESP_OK(esp_lcd_new_panel_ili9881c(mipi_dbi_io, &lcd_dev_config, &ili9881c_ctrl_panel));
+    TEST_ESP_OK(esp_lcd_panel_reset(ili9881c_ctrl_panel));
+    TEST_ESP_OK(esp_lcd_panel_init(ili9881c_ctrl_panel));
+    // turn on display
+    TEST_ESP_OK(esp_lcd_panel_disp_on_off(ili9881c_ctrl_panel, true));
+
+    esp_lcd_dpi_panel_config_t dpi_config = {
+        .dpi_clk_src = MIPI_DSI_DPI_CLK_SRC_DEFAULT,
+        .dpi_clock_freq_mhz = MIPI_DSI_DPI_CLK_MHZ,
+        .virtual_channel = 0,
+        .pixel_format = LCD_COLOR_PIXEL_FORMAT_RGB565,
+        .video_timing = {
+            .h_size = MIPI_DSI_LCD_H_RES,
+            .v_size = MIPI_DSI_LCD_V_RES,
+            .hsync_back_porch = MIPI_DSI_LCD_HBP,
+            .hsync_pulse_width = MIPI_DSI_LCD_HSYNC,
+            .hsync_front_porch = MIPI_DSI_LCD_HFP,
+            .vsync_back_porch = MIPI_DSI_LCD_VBP,
+            .vsync_pulse_width = MIPI_DSI_LCD_VSYNC,
+            .vsync_front_porch = MIPI_DSI_LCD_VFP,
+        },
+    };
+    TEST_ESP_OK(esp_lcd_new_panel_dpi(mipi_dsi_bus, &dpi_config, &mipi_dpi_panel));
+    TEST_ESP_OK(esp_lcd_panel_init(mipi_dpi_panel));
+    uint32_t callback_calls = 0;
+    esp_lcd_dpi_panel_event_callbacks_t cbs = {
+        .on_refresh_done = test_rgb_panel_count_in_callback,
+    };
+    TEST_ESP_OK(esp_lcd_dpi_panel_register_event_callbacks(mipi_dpi_panel, &cbs, &callback_calls));
+
+    uint8_t color_byte = rand() & 0xFF;
+    int x_start = rand() % (MIPI_DSI_LCD_H_RES - 100);
+    int y_start = rand() % (MIPI_DSI_LCD_V_RES - 100);
+    memset(img, color_byte, TEST_IMG_SIZE);
+    esp_lcd_panel_draw_bitmap(mipi_dpi_panel, x_start, y_start, x_start + 100, y_start + 100, img);
+    vTaskDelay(pdMS_TO_TICKS(100));
+
+    printf("The LCD driver should keep flushing the color block in the background\r\n");
+
+    // disable the cache for a while, the LCD driver should stay working
+    printf("disable the cache for a while\r\n");
+    unity_utils_run_cache_disable_stub(test_delay_post_cache_disable, NULL);
+    printf("callback calls: %"PRIu32"\r\n", callback_calls);
+    TEST_ASSERT(callback_calls > 2);
+
+    TEST_ESP_OK(esp_lcd_panel_del(mipi_dpi_panel));
+    TEST_ESP_OK(esp_lcd_panel_del(ili9881c_ctrl_panel));
+    TEST_ESP_OK(esp_lcd_panel_io_del(mipi_dbi_io));
+    TEST_ESP_OK(esp_lcd_del_dsi_bus(mipi_dsi_bus));
+    free(img);
+
+    test_bsp_disable_dsi_phy_power();
+}

components/esp_lcd/test_apps/mipi_dsi_lcd/pytest_mipi_dsi_lcd.py

@@ -1,11 +1,18 @@
 # SPDX-FileCopyrightText: 2023-2024 Espressif Systems (Shanghai) CO LTD
 # SPDX-License-Identifier: CC0-1.0
-
 import pytest
 from pytest_embedded import Dut
 
 
 @pytest.mark.esp32p4
 @pytest.mark.generic
-def test_rgb_lcd(dut: Dut) -> None:
+@pytest.mark.parametrize(
+    'config',
+    [
+        'iram_safe',
+        'release',
+    ],
+    indirect=True,
+)
+def test_dsi_lcd(dut: Dut) -> None:
     dut.run_all_single_board_cases()

components/esp_lcd/test_apps/mipi_dsi_lcd/sdkconfig.ci.iram_safe

@@ -0,0 +1,8 @@
+CONFIG_COMPILER_DUMP_RTL_FILES=y
+CONFIG_COMPILER_OPTIMIZATION_NONE=y
+# silent the error check, as the error string are stored in rodata, causing RTL check failure
+CONFIG_COMPILER_OPTIMIZATION_CHECKS_SILENT=y
+CONFIG_COMPILER_OPTIMIZATION_ASSERTIONS_SILENT=y
+# place non-ISR FreeRTOS functions in Flash
+CONFIG_FREERTOS_PLACE_FUNCTIONS_INTO_FLASH=y
+CONFIG_LCD_DSI_ISR_IRAM_SAFE=y

components/esp_lcd/test_apps/mipi_dsi_lcd/sdkconfig.ci.release

@@ -0,0 +1,5 @@
+CONFIG_PM_ENABLE=y
+CONFIG_FREERTOS_USE_TICKLESS_IDLE=y
+CONFIG_COMPILER_OPTIMIZATION_SIZE=y
+CONFIG_BOOTLOADER_COMPILER_OPTIMIZATION_SIZE=y
+CONFIG_COMPILER_OPTIMIZATION_ASSERTIONS_SILENT=y