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+rene...@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+rene...@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.pinch...@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
