[PATCH 2/2] drm/panel: s6e63m0: Switch to DBI abstraction for SPI

[Noralf Trønnes]

Den 11.06.2021 23.42, skrev Linus Walleij:
> The SPI access to s6e63m0 is using the DBI protocol, so switch
> to using the elaborate DBI protocol implementation in the DRM
> DBI helper library.
> 
> Cc: Douglas Anderson <dianders at chromium.org>
> Cc: Noralf Trønnes <noralf at tronnes.org>
> Signed-off-by: Linus Walleij <linus.walleij at linaro.org>
> ---

> diff --git a/drivers/gpu/drm/panel/panel-samsung-s6e63m0-spi.c b/drivers/gpu/drm/panel/panel-samsung-s6e63m0-spi.c
> index 326deb3177b6..293c18ee448a 100644
> --- a/drivers/gpu/drm/panel/panel-samsung-s6e63m0-spi.c
> +++ b/drivers/gpu/drm/panel/panel-samsung-s6e63m0-spi.c
> @@ -5,62 +5,42 @@
>  #include <linux/spi/spi.h>
>  #include <linux/delay.h>
>  
> +#include <drm/drm_mipi_dbi.h>
>  #include <drm/drm_print.h>
>  
>  #include "panel-samsung-s6e63m0.h"
>  
> -#define DATA_MASK	0x100
> +static const u8 s6e63m0_dbi_read_commands[] = {
> +	MCS_READ_ID1,
> +	MCS_READ_ID2,
> +	MCS_READ_ID3,
> +	0, /* sentinel */
> +};
>  
> -static int s6e63m0_spi_dcs_read(struct device *dev, const u8 cmd, u8 *data)
> +static int s6e63m0_spi_dcs_read(struct device *dev, void *trsp,
> +				const u8 cmd, u8 *data)
>  {
> -	struct spi_device *spi = to_spi_device(dev);
> -	u16 buf[1];
> -	u16 rbuf[1];
> +	struct mipi_dbi *dbi = trsp;
>  	int ret;
>  
> -	/* SPI buffers are always in CPU order */
> -	buf[0] = (u16)cmd;
> -	ret = spi_write_then_read(spi, buf, 2, rbuf, 2);
> -	dev_dbg(dev, "READ CMD: %04x RET: %04x\n", buf[0], rbuf[0]);
> -	if (!ret)
> -		/* These high 8 bits of the 9 contains the readout */
> -		*data = (rbuf[0] & 0x1ff) >> 1;
> +	ret = mipi_dbi_command_read(dbi, cmd, data);
> +	if (ret)
> +		dev_err(dev, "error on DBI read command %02x\n", cmd);
> +	else
> +		dev_dbg(dev, "read DBI %02x\n", *data);

You could drop this dev_dbg() and set drm.debug=2 instead to get the
same info. See MIPI_DBI_DEBUG_COMMAND().

>  
>  	return ret;
>  }
>  
> -static int s6e63m0_spi_write_word(struct device *dev, u16 data)
> -{
> -	struct spi_device *spi = to_spi_device(dev);
> -
> -	/* SPI buffers are always in CPU order */
> -	return spi_write(spi, &data, 2);
> -}
> -
> -static int s6e63m0_spi_dcs_write(struct device *dev, const u8 *data, size_t len)
> +static int s6e63m0_spi_dcs_write(struct device *dev, void *trsp,
> +				 const u8 *data, size_t len)
>  {
> -	int ret = 0;
> +	struct mipi_dbi *dbi = trsp;
> +	int ret;
>  
>  	dev_dbg(dev, "SPI writing dcs seq: %*ph\n", (int)len, data);

Same here MIPI_DBI_DEBUG_COMMAND() can print this.

>  
> -	/*
> -	 * This sends 9 bits with the first bit (bit 8) set to 0
> -	 * This indicates that this is a command. Anything after the
> -	 * command is data.
> -	 */
> -	ret = s6e63m0_spi_write_word(dev, *data);
> -
> -	while (!ret && --len) {
> -		++data;
> -		/* This sends 9 bits with the first bit (bit 8) set to 1 */
> -		ret = s6e63m0_spi_write_word(dev, *data | DATA_MASK);
> -	}
> -
> -	if (ret) {
> -		dev_err(dev, "SPI error %d writing dcs seq: %*ph\n", ret,
> -			(int)len, data);
> -	}
> -
> +	ret = mipi_dbi_command_stackbuf(dbi, data[0], (data + 1), (len - 1));

_stackbuf was not meant to be used by drivers, but I see that the driver
layering gives you no choice (except open coding ofc). So:

Acked-by: Noralf Trønnes <noralf at tronnes.org>

Noralf.

>  	usleep_range(300, 310);
>  
>  	return ret;