On Tue, May 05, 2020 at 05:03:29PM +0100, Emil Velikov wrote:
> From: Emil Velikov <emil.velikov@collabora.com>
> 
> The helper uses the MIPI_DCS_SET_TEAR_SCANLINE, although it's currently
> using the generic write. This does not look right.
> 
> Perhaps some platforms don't distinguish between the two writers?

I don't think platforms usually care about this level of detail. The
Tegra driver for example doesn't really look at the packet type and just
packets the data in the standard DSI format and then sends it off on the
bus. It does inspect some fields of the packet, but none that are
related to the packet type, I think.

So it's possible that the panel will accept the packet irrespective of
type and handle it correctly. I can imagine that the decoding logic in
these panels might be rather primitive, so perhaps it's not very strict
as to what exactly the type is as long as it can do something with the
data.

In any case, it does make sense to send DCS commands using the DCS type,
so I'd say let's merge this and see if somebody complains:

Reviewed-by: Thierry Reding <treding@nvidia.com>

> Cc: Robert Chiras <robert.chiras@nxp.com>
> Cc: Vinay Simha BN <simhavcs@gmail.com>
> Cc: Jani Nikula <jani.nikula@intel.com>
> Cc: Thierry Reding <treding@nvidia.com>
> Fixes: e83950816367 ("drm/dsi: Implement set tear scanline")
> Signed-off-by: Emil Velikov <emil.velikov@collabora.com>
> ---
> Robert, can you please test this against the only user - the Raydium
> RM67191 panel driver that you introduced.
> 
> Thanks
> 
> Vinay, can you confirm if this is a genuine typo or there's something
> really subtle happening.
> ---
>  drivers/gpu/drm/drm_mipi_dsi.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_mipi_dsi.c b/drivers/gpu/drm/drm_mipi_dsi.c
> index b96d5b4629d7..07102d8da58f 100644
> --- a/drivers/gpu/drm/drm_mipi_dsi.c
> +++ b/drivers/gpu/drm/drm_mipi_dsi.c
> @@ -1082,11 +1082,11 @@ EXPORT_SYMBOL(mipi_dsi_dcs_set_pixel_format);
>   */
>  int mipi_dsi_dcs_set_tear_scanline(struct mipi_dsi_device *dsi, u16 scanline)
>  {
> -	u8 payload[3] = { MIPI_DCS_SET_TEAR_SCANLINE, scanline >> 8,
> -			  scanline & 0xff };
> +	u8 payload[2] = { scanline >> 8, scanline & 0xff };
>  	ssize_t err;
>  
> -	err = mipi_dsi_generic_write(dsi, payload, sizeof(payload));
> +	err = mipi_dsi_dcs_write(dsi, MIPI_DCS_SET_TEAR_SCANLINE, payload,
> +				 sizeof(payload));
>  	if (err < 0)
>  		return err;
>  
> -- 
> 2.25.1
>