From bd3194a24b69db9c3dc164967206aceb9aca3f1a Mon Sep 17 00:00:00 2001 From: Valentin Roland Date: Thu, 16 Nov 2023 17:22:15 +0100 Subject: [PATCH] fix CKV drift and frame restart in LCD driver (#263) fixes an issue where the CKV clock would drift for 8 bit screen due to the use of dummy cycles which work around an issue in the ESP32 LCD FIFO. Additionally, dummy cycles are now also used for 16-bit parallel displays, fixing a pixel offset issue. #256 --- src/output_lcd/lcd_driver.c | 42 ++++++++++++++++++++++--------------- 1 file changed, 25 insertions(+), 17 deletions(-) diff --git a/src/output_lcd/lcd_driver.c b/src/output_lcd/lcd_driver.c index f8be5108..6523a204 100644 --- a/src/output_lcd/lcd_driver.c +++ b/src/output_lcd/lcd_driver.c @@ -73,7 +73,8 @@ typedef rmt_mem_t rmt_block_mem_t ; extern rmt_block_mem_t RMTMEM; #endif -#define DEBUG_PIN GPIO_NUM_46 +// spinlock for protecting the critical section at frame start +static portMUX_TYPE frame_start_spinlock = portMUX_INITIALIZER_UNLOCKED; typedef struct { lcd_hal_context_t hal; @@ -128,7 +129,9 @@ static IRAM_ATTR bool fill_bounce_buffer(uint8_t* buffer) { for (int i=0; i < BOUNCE_BUF_LINES; i++) { if (lcd.line_source_cb != NULL) { - task_awoken |= lcd.line_source_cb(lcd.line_cb_payload, &buffer[i * (line_bytes + dummy_bytes) + dummy_bytes]); + // this is strange, with 16 bit need a dummy cycle. But still, the first byte in the FIFO is correct. + // So we only need a true dummy byte in the FIFO in the 8 bit configuration. + task_awoken |= lcd.line_source_cb(lcd.line_cb_payload, &buffer[i * (line_bytes + dummy_bytes) + (dummy_bytes % 2)]); } else { memset(&buffer[i * line_bytes], 0x00, line_bytes); } @@ -153,7 +156,7 @@ void IRAM_ATTR epd_lcd_start_frame() { // hsync: pulse with, back porch, active width, front porch int end_line = lcd.line_cycles - lcd.lcd_res_h - lcd.config.le_high_time - lcd.config.line_front_porch; lcd_ll_set_horizontal_timing(lcd.hal.dev, - lcd.config.le_high_time, + lcd.config.le_high_time - (dummy_bytes > 0), lcd.config.line_front_porch, // a dummy byte is neeed in 8 bit mode to work around LCD peculiarities lcd.lcd_res_h + (dummy_bytes > 0), @@ -176,20 +179,29 @@ void IRAM_ATTR epd_lcd_start_frame() { fill_bounce_buffer(lcd.bounce_buffer[0]); fill_bounce_buffer(lcd.bounce_buffer[1]); + // the start of DMA should be prior to the start of LCD engine gdma_start(lcd.dma_chan, (intptr_t)&lcd.dma_nodes[0]); + + // enter a critical section to ensure the frame start timing is correct + taskENTER_CRITICAL(&frame_start_spinlock); + // 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 gpio_set_level(lcd.config.bus.stv, 0); - esp_rom_delay_us(1); - // start LCD engine - start_ckv_cycles(initial_lines + 6); - esp_rom_delay_us(1); - gpio_set_level(lcd.config.bus.stv, 1); + //esp_rom_delay_us(1); // for picture clarity, it seems to be important to start CKV at a "good" // time, seemingly start or towards end of line. - esp_rom_delay_us(lcd.line_length_us - lcd.config.ckv_high_time / 10 - 1); + start_ckv_cycles(initial_lines + 5); + esp_rom_delay_us(lcd.line_length_us); + gpio_set_level(lcd.config.bus.stv, 1); + esp_rom_delay_us(lcd.line_length_us); + esp_rom_delay_us(lcd.config.ckv_high_time / 10); + + // start LCD engine lcd_ll_start(lcd.hal.dev); + + taskEXIT_CRITICAL(&frame_start_spinlock); } /** @@ -266,8 +278,10 @@ IRAM_ATTR static void lcd_isr_vsync(void *args) } // apparently, this is needed for the new timing to take effect. lcd_ll_start(lcd.hal.dev); - // ensure we skip "vsync" with CKV, so we don't skip a line. + + // skip the LCD front porch line, which is not actual data esp_rom_delay_us(lcd.line_length_us); + start_ckv_cycles(ckv_cycles); } @@ -417,13 +431,7 @@ void IRAM_ATTR epd_lcd_init(const LcdEpdConfig_t* config, int display_width, int .pin_bit_mask = 1ull << lcd.config.bus.stv, }; - gpio_config_t debug_gpio_conf = { - .mode = GPIO_MODE_OUTPUT, - .pin_bit_mask = 1ull << DEBUG_PIN, - }; - gpio_config(&vsync_gpio_conf); - gpio_config(&debug_gpio_conf); gpio_set_level(lcd.config.bus.stv, 1); //gpio_set_level(S3_LCD_PIN_NUM_MODE, 0); @@ -440,7 +448,7 @@ void IRAM_ATTR epd_lcd_init(const LcdEpdConfig_t* config, int display_width, int // because the LCD peripheral behaves weirdly. // Also see: // https://blog.adafruit.com/2022/06/14/esp32uesday-hacking-the-esp32-s3-lcd-peripheral/ - int dummy_bytes = (lcd.config.bus_width == 8); + int dummy_bytes = lcd.config.bus_width / 8; lcd.bb_size = BOUNCE_BUF_LINES * (line_bytes + dummy_bytes); //assert(lcd.bb_size % (line_bytes) == 1);