Merge branch 'feature/rgb_lcd_restart' into 'master'

rgb_lcd: support restart dma transmission manually

Closes IDF-5892

See merge request espressif/esp-idf!20295

components/esp_lcd/include/esp_lcd_panel_rgb.h

@@ -79,7 +79,7 @@ typedef struct {
 /**
  * @brief RGB LCD VSYNC event callback prototype
  *
- * @param[in] panel LCD panel handle, returned from `esp_lcd_new_rgb_panel()`
+ * @param[in] panel LCD panel handle, returned from `esp_lcd_new_rgb_panel`
  * @param[in] edata Panel event data, fed by driver
  * @param[in] user_ctx User data, passed from `esp_lcd_rgb_panel_register_event_callbacks()`
  * @return Whether a high priority task has been waken up by this function
@@ -89,7 +89,7 @@ typedef bool (*esp_lcd_rgb_panel_vsync_cb_t)(esp_lcd_panel_handle_t panel, const
 /**
  * @brief Prototype for function to re-fill a bounce buffer, rather than copying from the frame buffer
  *
- * @param[in] panel LCD panel handle, returned from `esp_lcd_new_rgb_panel()`
+ * @param[in] panel LCD panel handle, returned from `esp_lcd_new_rgb_panel`
  * @param[in] bounce_buf Bounce buffer to write data into
  * @param[in] pos_px How many pixels already were sent to the display in this frame, in other words,
  *                   at what pixel the routine should start putting data into bounce_buf
@@ -157,7 +157,7 @@ esp_err_t esp_lcd_new_rgb_panel(const esp_lcd_rgb_panel_config_t *rgb_panel_conf
 /**
  * @brief Register LCD RGB panel event callbacks
  *
- * @param[in] panel LCD panel handle, returned from `esp_lcd_new_rgb_panel()`
+ * @param[in] panel LCD panel handle, returned from `esp_lcd_new_rgb_panel`
  * @param[in] callbacks Group of callback functions
  * @param[in] user_ctx User data, which will be passed to the callback functions directly
  * @return
@@ -176,18 +176,36 @@ esp_err_t esp_lcd_rgb_panel_register_event_callbacks(esp_lcd_panel_handle_t pane
  * @note This function doesn't cause the hardware to update the PCLK immediately but to record the new frequency and set a flag internally.
  *       Only in the next VSYNC event handler, will the driver attempt to update the PCLK frequency.
  *
- * @param[in] panel LCD panel handle, returned from `esp_lcd_new_rgb_panel()`
+ * @param[in] panel LCD panel handle, returned from `esp_lcd_new_rgb_panel`
  * @param[in] freq_hz Frequency of pixel clock, in Hz
  * @return
  *      - ESP_ERR_INVALID_ARG: Set PCLK frequency failed because of invalid argument
  *      - ESP_OK: Set PCLK frequency successfully
  */
 esp_err_t esp_lcd_rgb_panel_set_pclk(esp_lcd_panel_handle_t panel, uint32_t freq_hz);
 
+/**
+ * @brief Restart the LCD transmission
+ *
+ * @note This function can be useful when the LCD controller is out of sync with the DMA because of insufficient bandwidth.
+ *       To save the screen from a permanent shift, you can call this function to restart the LCD DMA.
+ * @note This function doesn't restart the DMA immediately but to set a flag internally.
+ *       Only in the next VSYNC event handler, will the driver attempt to do the restart job.
+ * @note If CONFIG_LCD_RGB_RESTART_IN_VSYNC is enabled, you don't need to call this function manually,
+ *       because the restart job will be done automatically in the VSYNC event handler.
+ *
+ * @param[in] panel panel LCD panel handle, returned from `esp_lcd_new_rgb_panel`
+ * @return
+ *      - ESP_ERR_INVALID_ARG: Restart the LCD failed because of invalid argument
+ *      - ESP_ERR_INVALID_STATE: Restart the LCD failed because the LCD diver is working in refresh-on-demand mode
+ *      - ESP_OK: Restart the LCD successfully
+ */
+esp_err_t esp_lcd_rgb_panel_restart(esp_lcd_panel_handle_t panel);
+
 /**
  * @brief Get the address of the frame buffer(s) that allocated by the driver
  *
- * @param[in] panel LCD panel handle, returned from `esp_lcd_new_rgb_panel()`
+ * @param[in] panel LCD panel handle, returned from `esp_lcd_new_rgb_panel`
  * @param[in] fb_num Number of frame buffer(s) to get. This value must be the same as the number of the following parameters.
  * @param[out] fb0 Returned address of the frame buffer 0
  * @param[out] ... List of other frame buffer addresses
@@ -202,7 +220,7 @@ esp_err_t esp_lcd_rgb_panel_get_frame_buffer(esp_lcd_panel_handle_t panel, uint3
  *
  * @note This function should only be called when the RGB panel is working under the `refresh_on_demand` mode.
  *
- * @param[in] panel LCD panel handle, returned from `esp_lcd_new_rgb_panel()`
+ * @param[in] panel LCD panel handle, returned from `esp_lcd_new_rgb_panel`
  * @return
  *      - ESP_ERR_INVALID_ARG: Start a refresh failed because of invalid argument
  *      - ESP_ERR_INVALID_STATE: Start a refresh failed because the LCD panel is not created with the `refresh_on_demand` flag enabled.

components/esp_lcd/src/esp_lcd_rgb_panel.c

@@ -115,6 +115,7 @@ struct esp_rgb_panel_t {
         uint32_t no_fb: 1;               // No frame buffer allocated in the driver
         uint32_t fb_in_psram: 1;         // Whether the frame buffer is in PSRAM
         uint32_t need_update_pclk: 1;    // Whether to update the PCLK before start a new transaction
+        uint32_t need_restart: 1;        // Whether to restart the LCD controller and the DMA
         uint32_t bb_invalidate_cache: 1; // Whether to do cache invalidation in bounce buffer mode
     } flags;
     dma_descriptor_t *dma_links[2];    // fbs[0] <-> dma_links[0], fbs[1] <-> dma_links[1]
@@ -362,6 +363,19 @@ esp_err_t esp_lcd_rgb_panel_set_pclk(esp_lcd_panel_handle_t panel, uint32_t freq
     return ESP_OK;
 }
 
+esp_err_t esp_lcd_rgb_panel_restart(esp_lcd_panel_handle_t panel)
+{
+    ESP_RETURN_ON_FALSE(panel, ESP_ERR_INVALID_ARG, TAG, "invalid argument");
+    esp_rgb_panel_t *rgb_panel = __containerof(panel, esp_rgb_panel_t, base);
+    ESP_RETURN_ON_FALSE(rgb_panel->flags.stream_mode, ESP_ERR_INVALID_STATE, TAG, "not in stream mode");
+
+    // the underlying restart job will be done in the `LCD_LL_EVENT_VSYNC_END` event handler
+    portENTER_CRITICAL(&rgb_panel->spinlock);
+    rgb_panel->flags.need_restart = true;
+    portEXIT_CRITICAL(&rgb_panel->spinlock);
+    return ESP_OK;
+}
+
 esp_err_t esp_lcd_rgb_panel_get_frame_buffer(esp_lcd_panel_handle_t panel, uint32_t fb_num, void **fb0, ...)
 {
     ESP_RETURN_ON_FALSE(panel, ESP_ERR_INVALID_ARG, TAG, "invalid argument");
@@ -996,16 +1010,15 @@ static esp_err_t lcd_rgb_panel_create_trans_link(esp_rgb_panel_t *panel)
         }
     }
 
-#if CONFIG_LCD_RGB_RESTART_IN_VSYNC
     // On restart, the data sent to the LCD peripheral needs to start LCD_FIFO_PRESERVE_SIZE_PX pixels after the FB start
     // so we use a dedicated DMA node to restart the DMA transaction
+    // see also `lcd_rgb_panel_try_restart_transmission`
     memcpy(&panel->dma_restart_node, &panel->dma_nodes[0], sizeof(panel->dma_restart_node));
     int restart_skip_bytes = LCD_FIFO_PRESERVE_SIZE_PX * sizeof(uint16_t);
     uint8_t *p = (uint8_t *)panel->dma_restart_node.buffer;
     panel->dma_restart_node.buffer = &p[restart_skip_bytes];
     panel->dma_restart_node.dw0.length -= restart_skip_bytes;
     panel->dma_restart_node.dw0.size -= restart_skip_bytes;
-#endif
 
     // alloc DMA channel and connect to LCD peripheral
     gdma_channel_alloc_config_t dma_chan_config = {
@@ -1030,9 +1043,31 @@ static esp_err_t lcd_rgb_panel_create_trans_link(esp_rgb_panel_t *panel)
     return ESP_OK;
 }
 
-#if CONFIG_LCD_RGB_RESTART_IN_VSYNC
-static IRAM_ATTR void lcd_rgb_panel_restart_transmission_in_isr(esp_rgb_panel_t *panel)
+// reset the GDMA channel every VBlank to stop permanent desyncs from happening.
+// Note that this fix can lead to single-frame desyncs itself, as in: if this interrupt
+// is late enough, the display will shift as the LCD controller already read out the
+// first data bytes, and resetting DMA will re-send those. However, the single-frame
+// desync this leads to is preferable to the permanent desync that could otherwise
+// happen. It's also not super-likely as this interrupt has the entirety of the VBlank
+// time to reset DMA.
+static IRAM_ATTR void lcd_rgb_panel_try_restart_transmission(esp_rgb_panel_t *panel)
 {
+    bool do_restart = false;
+#if CONFIG_LCD_RGB_RESTART_IN_VSYNC
+    do_restart = true;
+#else
+    portENTER_CRITICAL_ISR(&panel->spinlock);
+    if (panel->flags.need_restart) {
+        panel->flags.need_restart = false;
+        do_restart = true;
+    }
+    portEXIT_CRITICAL_ISR(&panel->spinlock);
+#endif // CONFIG_LCD_RGB_RESTART_IN_VSYNC
+
+    if (!do_restart) {
+        return;
+    }
+
     if (panel->bb_size) {
         // Catch de-synced frame buffer and reset if needed.
         if (panel->bounce_pos_px > panel->bb_size) {
@@ -1055,7 +1090,6 @@ static IRAM_ATTR void lcd_rgb_panel_restart_transmission_in_isr(esp_rgb_panel_t
         }
     }
 }
-#endif
 
 static void lcd_rgb_panel_start_transmission(esp_rgb_panel_t *rgb_panel)
 {
@@ -1109,16 +1143,8 @@ IRAM_ATTR static void lcd_default_isr_handler(void *args)
         lcd_rgb_panel_try_update_pclk(rgb_panel);
 
         if (rgb_panel->flags.stream_mode) {
-#if CONFIG_LCD_RGB_RESTART_IN_VSYNC
-            // reset the GDMA channel every VBlank to stop permanent desyncs from happening.
-            // Note that this fix can lead to single-frame desyncs itself, as in: if this interrupt
-            // is late enough, the display will shift as the LCD controller already read out the
-            // first data bytes, and resetting DMA will re-send those. However, the single-frame
-            // desync this leads to is preferable to the permanent desync that could otherwise
-            // happen. It's also not super-likely as this interrupt has the entirety of the VBlank
-            // time to reset DMA.
-            lcd_rgb_panel_restart_transmission_in_isr(rgb_panel);
-#endif
+            // check whether to restart the transmission
+            lcd_rgb_panel_try_restart_transmission(rgb_panel);
         }
 
     }

components/esp_lcd/test_apps/rgb_lcd/main/test_rgb_panel.c

@@ -214,6 +214,31 @@ TEST_CASE("lcd_rgb_panel_update_pclk", "[lcd]")
     free(img);
 }
 
+TEST_CASE("lcd_rgb_panel_restart", "[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(16, 16, 0, false, 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");
+    vTaskDelay(pdMS_TO_TICKS(1000));
+
+    printf("Restart the DMA transmission in the background\r\n");
+    TEST_ESP_OK(esp_lcd_rgb_panel_restart(panel_handle));
+    vTaskDelay(pdMS_TO_TICKS(1000));
+
+    printf("delete RGB panel\r\n");
+    TEST_ESP_OK(esp_lcd_panel_del(panel_handle));
+    free(img);
+}
+
 TEST_CASE("lcd_rgb_panel_rotate", "[lcd]")
 {
     const int w = 200;

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

@@ -1,5 +1,6 @@
 CONFIG_COMPILER_DUMP_RTL_FILES=y
 CONFIG_LCD_RGB_ISR_IRAM_SAFE=y
+CONFIG_GDMA_CTRL_FUNC_IN_IRAM=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

docs/en/api-reference/peripherals/lcd.rst

@@ -218,7 +218,7 @@ After we get the LCD handle, the remaining LCD operations are the same for diffe
     .. note::
 
         It should never happen in a well-designed embedded application, but it can in theory be possible that the DMA cannot deliver data as fast as the LCD consumes it. In the {IDF_TARGET_NAME} hardware, this leads to the LCD simply outputting dummy bytes while DMA waits for data. If we were to run DMA in a stream fashion, this would mean a de-sync between the LCD address the DMA reads the data for and the LCD address the LCD peripheral thinks it outputs data for, leading to a **permanently** shifted image.
-        In order to stop this from happening, you can enable the :ref:`CONFIG_LCD_RGB_RESTART_IN_VSYNC` option, so the driver will restart the DMA in the VBlank interrupt; this way we always know where it starts.
+        In order to stop this from happening, you can either enable the :ref:`CONFIG_LCD_RGB_RESTART_IN_VSYNC` option, so the driver can restart the DMA in the VBlank interrupt automatically or call :cpp:func:`esp_lcd_rgb_panel_restart` to restart the DMA manually. Note :cpp:func:`esp_lcd_rgb_panel_restart` doesn't restart the DMA immediately, the DMA will still be restarted in the next VSYNC event.
 
 Application Example
 -------------------