lcd: workaround auto next frame hardware bug

Closes https://github.com/espressif/esp-idf/issues/8381
2022-03-03 15:39:24 +08:00 · 2022-03-03 15:39:24 +08:00 · f06a13ad82
commit f06a13ad82
--- 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;
--- 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) {
--- 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.
--- 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.
--- 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));

--- 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
--- 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