[PATCH v2 2/4] drm: rcar-du: dsi: Improve DSI shutdown

[Laurent Pinchart]

Hi Tomi,

Thank you for the patch.

On Mon, Aug 22, 2022 at 04:05:10PM +0300, Tomi Valkeinen wrote:
> From: Tomi Valkeinen <tomi.valkeinen+renesas at ideasonboard.com>
> 
> Improve the DSI shutdown procedure by clearing various bits that were
> set while enabling the DSI output. There has been no clear issues caused
> by these, but it's safer to ensure that the features are disabled at the
> start of the next DSI enable.
> 
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen+renesas at ideasonboard.com>
> ---
>  drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c b/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c
> index 7f2be490fcf8..6a10a35f1122 100644
> --- a/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c
> +++ b/drivers/gpu/drm/rcar-du/rcar_mipi_dsi.c
> @@ -441,9 +441,21 @@ static int rcar_mipi_dsi_startup(struct rcar_mipi_dsi *dsi,
>  
>  static void rcar_mipi_dsi_shutdown(struct rcar_mipi_dsi *dsi)
>  {
> +	/* Disable VCLKEN */
> +	rcar_mipi_dsi_clr(dsi, VCLKEN, VCLKEN_CKEN);
> +
> +	/* Disable DOT clock */
> +	rcar_mipi_dsi_clr(dsi, VCLKSET, VCLKSET_CKEN);

I think you can write 0 to those two registers, this will also be safer.
With this,

Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

I think there's a bug in rcar_mipi_dsi_startup() related to this by the
way, the function only uses rcar_mipi_dsi_set() to set bits, so if the
DSI format is modified between two starts, bad things will happen.

> +
>  	rcar_mipi_dsi_clr(dsi, PHYSETUP, PHYSETUP_RSTZ);
>  	rcar_mipi_dsi_clr(dsi, PHYSETUP, PHYSETUP_SHUTDOWNZ);
>  
> +	/* CFGCLK disable */
> +	rcar_mipi_dsi_clr(dsi, CFGCLKSET, CFGCLKSET_CKEN);
> +
> +	/* LPCLK disable */
> +	rcar_mipi_dsi_clr(dsi, LPCLKSET, LPCLKSET_CKEN);
> +
>  	dev_dbg(dsi->dev, "DSI device is shutdown\n");
>  }
>  

-- 
Regards,

Laurent Pinchart