Merge branch 'bugfix/spi_lcd_max_trans_size_v4.4' into 'release/v4.4'

spi_lcd: maximum transfer size should respect bus configuration (v4.4)

See merge request espressif/esp-idf!23231

.gitlab/ci/target-test.yml

@@ -545,7 +545,7 @@ UT_006:
 
 UT_007:
   extends: .unit_test_esp32_template
-  parallel: 7
+  parallel: 8
   tags:
     - ESP32_IDF
     - UT_T1_1

components/esp_lcd/src/esp_lcd_panel_io_spi.c

@@ -13,13 +13,12 @@
 #include "esp_lcd_panel_io.h"
 #include "hal/spi_ll.h"
 #include "driver/spi_master.h"
+#include "driver/spi_common_internal.h"
 #include "driver/gpio.h"
 #include "esp_log.h"
 #include "esp_check.h"
 #include "esp_lcd_common.h"
 
-#define LCD_SPI_MAX_DATA_SIZE (SPI_LL_DATA_MAX_BIT_LEN / 8)
-
 static const char *TAG = "lcd_panel.io.spi";
 
 static esp_err_t panel_io_spi_rx_param(esp_lcd_panel_io_t *io, int lcd_cmd, void *param, size_t param_size);
@@ -41,6 +40,7 @@ typedef struct {
 typedef struct {
     esp_lcd_panel_io_t base;     // Base class of generic lcd panel io
     spi_device_handle_t spi_dev; // SPI device handle
+    size_t spi_trans_max_bytes;  // Maximum bytes that can be transmitted in one spi transaction
     int dc_gpio_num;             // D/C line GPIO number
     esp_lcd_panel_io_color_trans_done_cb_t on_color_trans_done; // User register's callback, invoked when color data trans done
     void *user_ctx;           // User's private data, passed directly to callback on_color_trans_done
@@ -102,6 +102,7 @@ esp_err_t esp_lcd_new_panel_io_spi(esp_lcd_spi_bus_handle_t bus, const esp_lcd_p
     spi_panel_io->base.tx_color = panel_io_spi_tx_color;
     spi_panel_io->base.del = panel_io_spi_del;
     spi_panel_io->base.register_event_callbacks = panel_io_spi_register_event_callbacks;
+    spi_panel_io->spi_trans_max_bytes = spi_bus_get_attr((spi_host_device_t)bus)->max_transfer_sz;
     *ret_io = &(spi_panel_io->base);
     ESP_LOGD(TAG, "new spi lcd panel io @%p", spi_panel_io);
 
@@ -308,18 +309,21 @@ static esp_err_t panel_io_spi_tx_color(esp_lcd_panel_io_t *io, int lcd_cmd, cons
     spi_transaction_t *spi_trans = NULL;
     lcd_spi_trans_descriptor_t *lcd_trans = NULL;
     esp_lcd_panel_io_spi_t *spi_panel_io = __containerof(io, esp_lcd_panel_io_spi_t, base);
-    bool send_cmd = (lcd_cmd >= 0);
 
-    // before issue a polling transaction, need to wait queued transactions finished
-    for (size_t i = 0; i < spi_panel_io->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 = 0;
-    lcd_trans = &spi_panel_io->trans_pool[0];
-    memset(lcd_trans, 0, sizeof(lcd_spi_trans_descriptor_t));
+    ESP_RETURN_ON_ERROR(spi_device_acquire_bus(spi_panel_io->spi_dev, portMAX_DELAY), TAG, "acquire spi bus failed");
 
+    bool send_cmd = (lcd_cmd >= 0);
     if (send_cmd) {
+        // before issue a polling transaction, need to wait queued transactions finished
+        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--;
+        }
+        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);
         lcd_trans->base.user = spi_panel_io;
         lcd_trans->flags.dc_gpio_level = !spi_panel_io->flags.dc_data_level; // set D/C line to command mode
@@ -337,8 +341,7 @@ static esp_err_t panel_io_spi_tx_color(esp_lcd_panel_io_t *io, int lcd_cmd, cons
         ESP_GOTO_ON_ERROR(ret, err, TAG, "spi transmit (polling) command failed");
     }
 
-    // split to chunks if required:
-    // the SPI bus has a maximum transaction size determined by SPI_USR_MOSI_DBITLEN's bit width
+    // if the color buffer is big, we want to split it into chunks, and queue the chunks one by one
     do {
         size_t chunk_size = color_size;
 
@@ -354,13 +357,15 @@ static esp_err_t panel_io_spi_tx_color(esp_lcd_panel_io_t *io, int lcd_cmd, cons
         }
         memset(lcd_trans, 0, sizeof(lcd_spi_trans_descriptor_t));
 
-        // SPI per-transfer size has its limitation, if the color buffer is too big, we need to split it into multiple trunks
-        if (chunk_size > LCD_SPI_MAX_DATA_SIZE) {
+        // SPI per-transfer size has its limitation, if the color buffer is too big, we need to split it into multiple chunks
+        if (chunk_size > spi_panel_io->spi_trans_max_bytes) {
             // cap the transfer size to the maximum supported by the bus
-            chunk_size = LCD_SPI_MAX_DATA_SIZE;
+            chunk_size = spi_panel_io->spi_trans_max_bytes;
+            lcd_trans->base.flags |= SPI_TRANS_CS_KEEP_ACTIVE;
         } else {
             // mark en_trans_done_cb only at the last round to avoid premature completion callback
             lcd_trans->flags.en_trans_done_cb = 1;
+            lcd_trans->base.flags &= ~SPI_TRANS_CS_KEEP_ACTIVE;
         }
 
         lcd_trans->base.user = spi_panel_io;
@@ -386,6 +391,7 @@ static esp_err_t panel_io_spi_tx_color(esp_lcd_panel_io_t *io, int lcd_cmd, cons
     } while (color_size > 0); // continue while we have remaining data to transmit
 
 err:
+    spi_device_release_bus(spi_panel_io->spi_dev);
     return ret;
 }
 

components/esp_lcd/test/test_spi_lcd_panel.c

@@ -1,6 +1,8 @@
 #include <stdio.h>
 #include <string.h>
 #include "sdkconfig.h"
+#include "freertos/FreeRTOS.h"
+#include "freertos/task.h"
 #include "unity.h"
 #include "test_utils.h"
 #include "driver/spi_master.h"
@@ -9,6 +11,7 @@
 #include "esp_lcd_panel_vendor.h"
 #include "esp_lcd_panel_ops.h"
 #include "esp_system.h"
+#include "esp_lcd_panel_commands.h"
 #include "soc/soc_caps.h"
 #include "test_spi_board.h"
 
@@ -29,7 +32,7 @@ static void lcd_initialize_spi(esp_lcd_panel_io_handle_t *io_handle, esp_lcd_pan
         .miso_io_num = -1,
         .quadwp_io_num = -1,
         .quadhd_io_num = -1,
-        .max_transfer_sz = TEST_LCD_H_RES * TEST_LCD_V_RES * sizeof(uint16_t)
+        .max_transfer_sz = 100 * 100 * sizeof(uint16_t),
     };
     if (oct_mode) {
         buscfg.data1_io_num = TEST_LCD_DATA1_GPIO;
@@ -179,3 +182,71 @@ TEST_CASE("lcd panel with 1-line spi interface (st7789)", "[lcd]")
     TEST_ESP_OK(esp_lcd_new_panel_st7789(io_handle, &panel_config, &panel_handle));
     lcd_panel_test(io_handle, panel_handle);
 }
+
+TEST_CASE("spi_lcd_send_colors_to_fixed_region", "[lcd]")
+{
+    int x_start = 100;
+    int y_start = 100;
+    int x_end = 200;
+    int y_end = 200;
+    size_t color_size = (x_end - x_start) * (y_end - y_start) * 2;
+    void *color_data = malloc(color_size);
+    TEST_ASSERT_NOT_NULL(color_data);
+    uint8_t color_byte = esp_random() & 0xFF;
+    memset(color_data, color_byte, color_size);
+
+    esp_lcd_panel_io_handle_t io_handle = NULL;
+    esp_lcd_panel_handle_t panel_handle = NULL;
+    lcd_initialize_spi(&io_handle, NULL, NULL, 8, 8, false);
+
+    // we don't use the panel handle in this test, creating the panel just for a quick initialization
+    esp_lcd_panel_dev_config_t panel_config = {
+        .reset_gpio_num = TEST_LCD_RST_GPIO,
+        .color_space = ESP_LCD_COLOR_SPACE_RGB,
+        .bits_per_pixel = 16,
+    };
+    TEST_ESP_OK(esp_lcd_new_panel_st7789(io_handle, &panel_config, &panel_handle));
+    esp_lcd_panel_reset(panel_handle);
+    esp_lcd_panel_init(panel_handle);
+    esp_lcd_panel_invert_color(panel_handle, true);
+    // the gap is LCD panel specific, even panels with the same driver IC, can have different gap value
+    esp_lcd_panel_set_gap(panel_handle, 0, 20);
+    // turn on display
+    esp_lcd_panel_disp_on_off(panel_handle, true);
+    // turn on backlight
+    gpio_set_level(TEST_LCD_BK_LIGHT_GPIO, 1);
+
+    printf("set the flush window for only once\r\n");
+    esp_lcd_panel_io_tx_param(io_handle, LCD_CMD_CASET, (uint8_t[]) {
+        (x_start >> 8) & 0xFF,
+        x_start & 0xFF,
+        ((x_end - 1) >> 8) & 0xFF,
+        (x_end - 1) & 0xFF,
+    }, 4);
+    esp_lcd_panel_io_tx_param(io_handle, LCD_CMD_RASET, (uint8_t[]) {
+        (y_start >> 8) & 0xFF,
+        y_start & 0xFF,
+        ((y_end - 1) >> 8) & 0xFF,
+        (y_end - 1) & 0xFF,
+    }, 4);
+    esp_lcd_panel_io_tx_param(io_handle, LCD_CMD_RAMWR, NULL, 0);
+
+    printf("send colors to the fixed region in multiple steps\r\n");
+    const int steps = 10;
+    int color_size_per_step = color_size / steps;
+    for (int i = 0; i < steps; i++) {
+        TEST_ESP_OK(esp_lcd_panel_io_tx_color(io_handle, -1, color_data + i * color_size_per_step, color_size_per_step));
+    }
+    vTaskDelay(pdMS_TO_TICKS(1000));
+    // change to another color
+    color_byte = esp_random() & 0xFF;
+    memset(color_data, color_byte, color_size);
+    for (int i = 0; i < steps; i++) {
+        TEST_ESP_OK(esp_lcd_panel_io_tx_color(io_handle, -1, color_data + i * color_size_per_step, color_size_per_step));
+    }
+
+    TEST_ESP_OK(esp_lcd_panel_del(panel_handle));
+    TEST_ESP_OK(esp_lcd_panel_io_del(io_handle));
+    TEST_ESP_OK(spi_bus_free(TEST_SPI_HOST_ID));
+    free(color_data);
+}