[PATCH v2 2/2] drm/tiny: ili9486: Do not assume 8-bit only SPI controllers
Neil Armstrong
neil.armstrong at linaro.org
Tue Nov 29 00:41:11 PST 2022
On 21/11/2022 10:42, Carlo Caione wrote:
> The pixel data for the ILI9486 is always 16-bits wide and it must be
> sent over the SPI bus. When the controller is only able to deal with
> 8-bit transfers, this 16-bits data needs to be swapped before the
> sending to account for the big endian bus, this is on the contrary not
> needed when the SPI controller already supports 16-bits transfers.
>
> The decision about swapping the pixel data or not is taken in the MIPI
> DBI code by probing the controller capabilities: if the controller only
> suppors 8-bit transfers the data is swapped, otherwise it is not.
>
> This swapping/non-swapping is relying on the assumption that when the
> controller does support 16-bit transactions then the data is sent
> unswapped in 16-bits-per-word over SPI.
>
> The problem with the ILI9486 driver is that it is forcing 8-bit
> transactions also for controllers supporting 16-bits, violating the
> assumption and corrupting the pixel data.
>
> Align the driver to what is done in the MIPI DBI code by adjusting the
> tranfer size to the maximum allowed by the SPI controller.
>
> Signed-off-by: Carlo Caione <ccaione at baylibre.com>
> ---
> drivers/gpu/drm/tiny/ili9486.c | 13 +++++++++----
> 1 file changed, 9 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/tiny/ili9486.c b/drivers/gpu/drm/tiny/ili9486.c
> index bd37dfe8dd05..4d80a413338f 100644
> --- a/drivers/gpu/drm/tiny/ili9486.c
> +++ b/drivers/gpu/drm/tiny/ili9486.c
> @@ -43,6 +43,7 @@ static int waveshare_command(struct mipi_dbi *mipi, u8 *cmd, u8 *par,
> size_t num)
> {
> struct spi_device *spi = mipi->spi;
> + unsigned int bpw = 8;
> void *data = par;
> u32 speed_hz;
> int i, ret;
> @@ -56,8 +57,6 @@ static int waveshare_command(struct mipi_dbi *mipi, u8 *cmd, u8 *par,
> * The displays are Raspberry Pi HATs and connected to the 8-bit only
> * SPI controller, so 16-bit command and parameters need byte swapping
> * before being transferred as 8-bit on the big endian SPI bus.
> - * Pixel data bytes have already been swapped before this function is
> - * called.
> */
> buf[0] = cpu_to_be16(*cmd);
> gpiod_set_value_cansleep(mipi->dc, 0);
> @@ -71,12 +70,18 @@ static int waveshare_command(struct mipi_dbi *mipi, u8 *cmd, u8 *par,
> for (i = 0; i < num; i++)
> buf[i] = cpu_to_be16(par[i]);
> num *= 2;
> - speed_hz = mipi_dbi_spi_cmd_max_speed(spi, num);
> data = buf;
> }
>
> + /*
> + * Check whether pixel data bytes needs to be swapped or not
> + */
> + if (*cmd == MIPI_DCS_WRITE_MEMORY_START && !mipi->swap_bytes)
> + bpw = 16;
> +
> gpiod_set_value_cansleep(mipi->dc, 1);
> - ret = mipi_dbi_spi_transfer(spi, speed_hz, 8, data, num);
> + speed_hz = mipi_dbi_spi_cmd_max_speed(spi, num);
> + ret = mipi_dbi_spi_transfer(spi, speed_hz, bpw, data, num);
> free:
> kfree(buf);
>
> Looks fine, but should somehow be tested on an RPi first
to check if the 8bit fallback still works.
Reviewed-by: Neil Armstrong <neil.armstrong at linaro.org>
More information about the linux-amlogic
mailing list