Merge #19816

19816: drivers/lcd: code deduplication for st7735 and ili9341 r=aabadie a=gschorcht

### Contribution description

In preparation for the parallel interface support the following changes were made:

1. The code for basic communication (acquire/release SPI device, SPI transfers), which were always duplicated by copy & paste in each display driver again and again, has been moved to the LCD driver as low-level functions that are now used by the display drivers. These low level function allow 
- code deduplication and
- to define a more abstract communication interface that can then be extended later by parallel communication interface
2. Identical GPIO initializations has also been moved from display drivers to the LCD driver.
3. Using a default implementation of `lcd_set_area` function allows further code deduplication.

Finally, the `ili9341` and `st7735` drivers only implement the inialization sequence for the LCD driver IC.

### Testing procedure

`tests/drivers/ili9341` should still work for a board with an ILI9341. Tested with `esp32-wrover-kit`.
`tests/drivers/st7735` should still work for a board with a ST77xx. Tested with `esp32s3-usb-otg`.

### Issues/PRs references


Co-authored-by: Gunar Schorcht <[email protected]>

Author: bors[bot]

drivers/ili9341/ili9341.c

@@ -1,5 +1,6 @@
 /*
  * Copyright (C) 2018 Koen Zandberg
+ *               2023 Gunar Schorcht
  *
  * This file is subject to the terms and conditions of the GNU Lesser
  * General Public License v2.1. See the file LICENSE in the top level
@@ -14,6 +15,7 @@
  * @brief       Device driver implementation for the ili9341 display controller
  *
  * @author      Koen Zandberg <[email protected]>
+ * @author      Gunar Schorcht <[email protected]>
  *
  * @}
  */
@@ -22,8 +24,8 @@
 #include <string.h>
 #include "byteorder.h"
 #include "periph/spi.h"
-#include "ztimer.h"
 #include "kernel_defines.h"
+#include "ztimer.h"
 
 #include "ili9341.h"
 #include "ili9341_internal.h"
@@ -33,18 +35,6 @@
 #define ENABLE_DEBUG 0
 #include "debug.h"
 
-static void _write_cmd(const lcd_t *dev, uint8_t cmd, const uint8_t *data,
-                       size_t len)
-{
-    gpio_clear(dev->params->dcx_pin);
-    spi_transfer_byte(dev->params->spi, dev->params->cs_pin, len ? true : false, cmd);
-    gpio_set(dev->params->dcx_pin);
-    if (len) {
-        spi_transfer_bytes(dev->params->spi, dev->params->cs_pin, false, data,
-                           NULL, len);
-    }
-}
-
 /* datasheet page 178, table converted to equation.
  * gvdd in 1mv increments: 4850 = 4.85V */
 static uint8_t _ili9341_calc_pwrctl1(uint16_t gvdd)
@@ -66,125 +56,92 @@ static uint8_t _ili9341_calc_vml(int16_t vcoml)
 static int _init(lcd_t *dev, const lcd_params_t *params)
 {
     assert(params->lines >= 16 && params->lines <= 320 && !(params->lines & 0x7));
-    dev->params = params;
-    uint8_t command_params[4] = { 0 };
-    gpio_init(dev->params->dcx_pin, GPIO_OUT);
-    int res = spi_init_cs(dev->params->spi, dev->params->cs_pin);
-    if (res != SPI_OK) {
-        DEBUG("[ili9341] init: error initializing the CS pin [%i]\n", res);
-        return -1;
-    }
 
-    if (gpio_is_valid(dev->params->rst_pin)) {
-        gpio_init(dev->params->rst_pin, GPIO_OUT);
-        gpio_clear(dev->params->rst_pin);
-        ztimer_sleep(ZTIMER_MSEC, 120);
-        gpio_set(dev->params->rst_pin);
-    }
-    ztimer_sleep(ZTIMER_MSEC, 120);
+    uint8_t command_params[4] = { 0 };
 
     /* Acquire once at release at the end */
-    spi_acquire(dev->params->spi, dev->params->cs_pin, dev->params->spi_mode,
-                dev->params->spi_clk);
+    lcd_ll_acquire(dev);
 
     /* Soft Reset */
-    _write_cmd(dev, LCD_CMD_SWRESET, NULL, 0);
+    lcd_ll_write_cmd(dev, LCD_CMD_SWRESET, NULL, 0);
     ztimer_sleep(ZTIMER_MSEC, 120);
 
     /* Display off */
-    _write_cmd(dev, LCD_CMD_DISPOFF, NULL, 0);
+    lcd_ll_write_cmd(dev, LCD_CMD_DISPOFF, NULL, 0);
 
     /* PWRCTL1/2 */
     command_params[0] = _ili9341_calc_pwrctl1(CONFIG_ILI9341_GVDD);
-    _write_cmd(dev, LCD_CMD_PWCTRL1, command_params, 1);
+    lcd_ll_write_cmd(dev, LCD_CMD_PWCTRL1, command_params, 1);
 
     command_params[0] = 0x10; /* PWRCTL 0 0 0 */
-    _write_cmd(dev, LCD_CMD_PWCTRL2, command_params, 1);
+    lcd_ll_write_cmd(dev, LCD_CMD_PWCTRL2, command_params, 1);
 
     /* VCOMCTL */
     command_params[0] = _ili9341_calc_vmh(CONFIG_ILI9341_VCOMH);
     command_params[1] = _ili9341_calc_vml(CONFIG_ILI9341_VCOML);
-    _write_cmd(dev, LCD_CMD_VMCTRL1, command_params, 2);
+    lcd_ll_write_cmd(dev, LCD_CMD_VMCTRL1, command_params, 2);
 
     command_params[0] = 0x86;
-    _write_cmd(dev, LCD_CMD_VMCTRL2, command_params, 1);
+    lcd_ll_write_cmd(dev, LCD_CMD_VMCTRL2, command_params, 1);
 
     /* Memory access CTL */
     command_params[0] = dev->params->rotation;
     command_params[0] |= dev->params->rgb ? 0 : LCD_MADCTL_BGR;
-    _write_cmd(dev, LCD_CMD_MADCTL, command_params, 1);
+    lcd_ll_write_cmd(dev, LCD_CMD_MADCTL, command_params, 1);
 
     /* Frame control */
     command_params[0] = 0x00;
     command_params[1] = 0x18;
-    _write_cmd(dev, LCD_CMD_FRAMECTL1, command_params, 2);
+    lcd_ll_write_cmd(dev, LCD_CMD_FRAMECTL1, command_params, 2);
 
     /* Display function control */
     command_params[0] = 0x08;
     command_params[1] = 0x82;
     /* number of lines, see datasheet p. 166 (DISCTRL::NL) */
     command_params[2] = (params->lines >> 3) - 1;
-    _write_cmd(dev, LCD_CMD_DFUNC, command_params, 3);
+    lcd_ll_write_cmd(dev, LCD_CMD_DFUNC, command_params, 3);
 
     /* Pixel format */
     command_params[0] = 0x55; /* 16 bit mode */
-    _write_cmd(dev, LCD_CMD_PIXSET, command_params, 1);
+    lcd_ll_write_cmd(dev, LCD_CMD_PIXSET, command_params, 1);
 
     command_params[0] = 0x01;
-    _write_cmd(dev, LCD_CMD_GAMSET, command_params, 1);
+    lcd_ll_write_cmd(dev, LCD_CMD_GAMSET, command_params, 1);
 
     /* Gamma correction */
     {
         static const uint8_t gamma_pos[] = {
             0x0F, 0x31, 0x2B, 0x0C, 0x0E, 0x08, 0x4E, 0xF1,
             0x37, 0x07, 0x10, 0x03, 0x0E, 0x09, 0x00
         };
-        _write_cmd(dev, LCD_CMD_PGAMCTRL, gamma_pos,
-                   sizeof(gamma_pos));
+        lcd_ll_write_cmd(dev, LCD_CMD_PGAMCTRL, gamma_pos,
+                         sizeof(gamma_pos));
     }
     {
         static const uint8_t gamma_neg[] = {
             0x00, 0x0E, 0x14, 0x03, 0x11, 0x07, 0x31, 0xC1,
             0x48, 0x08, 0x0F, 0x0C, 0x31, 0x36, 0x0F
         };
-        _write_cmd(dev, LCD_CMD_NGAMCTRL, gamma_neg,
-                   sizeof(gamma_neg));
+        lcd_ll_write_cmd(dev, LCD_CMD_NGAMCTRL, gamma_neg,
+                         sizeof(gamma_neg));
 
     }
 
     if (dev->params->inverted) {
-        _write_cmd(dev, LCD_CMD_DINVON, NULL, 0);
+        lcd_ll_write_cmd(dev, LCD_CMD_DINVON, NULL, 0);
     }
     /* Sleep out (turn off sleep mode) */
-    _write_cmd(dev, LCD_CMD_SLPOUT, NULL, 0);
+    lcd_ll_write_cmd(dev, LCD_CMD_SLPOUT, NULL, 0);
     /* Display on */
-    _write_cmd(dev, LCD_CMD_DISPON, NULL, 0);
-    spi_release(dev->params->spi);
-    return 0;
-}
+    lcd_ll_write_cmd(dev, LCD_CMD_DISPON, NULL, 0);
 
-static void _set_area(const lcd_t *dev, uint16_t x1, uint16_t x2,
-                      uint16_t y1, uint16_t y2)
-{
-    be_uint16_t params[2];
-
-    x1 += dev->params->offset_x;
-    x2 += dev->params->offset_x;
-    y1 += dev->params->offset_y;
-    y2 += dev->params->offset_y;
-
-    params[0] = byteorder_htons(x1);
-    params[1] = byteorder_htons(x2);
-
-    _write_cmd(dev, LCD_CMD_CASET, (uint8_t *)params,
-               sizeof(params));
-    params[0] = byteorder_htons(y1);
-    params[1] = byteorder_htons(y2);
-    _write_cmd(dev, LCD_CMD_PASET, (uint8_t *)params,
-               sizeof(params));
+    /* Finally release the device */
+    lcd_ll_release(dev);
+
+    return 0;
 }
 
 const lcd_driver_t lcd_ili9341_driver = {
     .init = _init,
-    .set_area = _set_area,
+    .set_area = NULL, /* default implementation is used */
 };

drivers/include/lcd.h

@@ -126,6 +126,11 @@ struct lcd_driver {
     /**
      * @brief   Set area LCD work area
      *
+     * This function pointer can be NULL if the controller specific driver
+     * does not require anything special. In this case the default
+     * implementation is used which sets the column addresses and the row
+     * addresses of the area including the coordinates of the opposite corner.
+     *
      * @param[in] dev  Pointer to the selected driver
      * @param[in] x1   x coordinate of the first corner
      * @param[in] x2   x coordinate of the opposite corner
@@ -137,6 +142,52 @@ struct lcd_driver {
                      uint16_t y2);
 };
 
+/**
+ * @brief   Low Level to acquire the device
+ *
+ * @param[out]  dev     device descriptor
+ */
+void lcd_ll_acquire(const lcd_t *dev);
+
+/**
+ * @brief   Low Level function to release the device
+ *
+ * @param[out]  dev     device descriptor
+ */
+void lcd_ll_release(const lcd_t *dev);
+
+/**
+ * @brief   Low level function to write a command
+ *
+ * @pre The device must have already been acquired with @ref lcd_ll_acquire
+ *      before this function can be called.
+ *
+ * @param[in]   dev     device descriptor
+ * @param[in]   cmd     command code
+ * @param[in]   data    command data to the device
+ * @param[in]   len     length of the command data
+ */
+void lcd_ll_write_cmd(const lcd_t *dev, uint8_t cmd, const uint8_t *data,
+                      size_t len);
+
+/**
+ * @brief   Low level function for read command command
+ *
+ * @note Very often the SPI MISO signal of the serial interface or the RDX
+ *       signal of the MCU 8080 parallel interface are not connected to the
+ *       display. In this case the read command does not provide valid data.
+ *
+ * @pre The device must have already been acquired with @ref lcd_ll_acquire
+ *      before this function can be called.
+ * @pre len > 0
+ *
+ * @param[in]   dev     device descriptor
+ * @param[in]   cmd     command
+ * @param[out]  data    data from the device
+ * @param[in]   len     length of the returned data
+ */
+void lcd_ll_read_cmd(const lcd_t *dev, uint8_t cmd, uint8_t *data, size_t len);
+
 /**
  * @brief   Setup an lcd display device
  *

drivers/include/st7735.h

@@ -83,7 +83,7 @@ extern "C" {
 #endif
 
 /**
- * @name    ILI9341 display rotation modes
+ * @name    ST7735 display rotation modes
  * @{
  */
 #define ST7735_ROTATION_VERT            0                   /**< Vertical mode */