Re: [PATCH 2/4] drm/panel: st7703: move init commands to prepare()

[Neil Armstrong <neil.armstrong@linaro.org>]

On 10/7/26 22:18, Marc-Olivier Champagne wrote:
> The panel init sequence is currently sent from enable(), i.e. after
> the DSI host has started the video stream. Not all DSI hosts accept
> command packets in that state, so the init sequence may never reach
> the panel.
> 
> Move the init sequence to prepare(), right after the panel reset, to
> ensure it is sent before the host enables the video stream.
> 
> Since the init sequence can now fail in prepare(), power the panel
> back down on error: the panel core does not call unprepare() when
> prepare() fails, leaving the power supplies enabled.
> 
> Both patterns are already used by other DSI panel drivers.
> 
> Suggested-by: Hugo Villeneuve <hvilleneuve@dimonoff.com>
> Signed-off-by: Marc-Olivier Champagne <marc-olivier.champagne@savoirfairelinux.com>
> ---
>   drivers/gpu/drm/panel/panel-sitronix-st7703.c | 18 +++++++++++++++---
>   1 file changed, 15 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/panel/panel-sitronix-st7703.c b/drivers/gpu/drm/panel/panel-sitronix-st7703.c
> index 368c57591817..f4635a7327de 100644
> --- a/drivers/gpu/drm/panel/panel-sitronix-st7703.c
> +++ b/drivers/gpu/drm/panel/panel-sitronix-st7703.c
> @@ -774,8 +774,6 @@ static int st7703_enable(struct drm_panel *panel)
>   	struct mipi_dsi_device *dsi = to_mipi_dsi_device(ctx->dev);
>   	struct mipi_dsi_multi_context dsi_ctx = {.dsi = dsi};
>   
> -	ctx->desc->init_sequence(&dsi_ctx);
> -
>   	mipi_dsi_dcs_exit_sleep_mode_multi(&dsi_ctx);
>   
>   	/* It takes the controller 120 msec to wake up after sleep. */
> @@ -784,7 +782,7 @@ static int st7703_enable(struct drm_panel *panel)
>   	mipi_dsi_dcs_set_display_on_multi(&dsi_ctx);
>   
>   	if (!dsi_ctx.accum_err)
> -		dev_dbg(ctx->dev, "Panel init sequence done\n");
> +		dev_dbg(ctx->dev, "Panel enable done\n");
>   
>   	return dsi_ctx.accum_err;
>   }
> @@ -819,6 +817,8 @@ static int st7703_unprepare(struct drm_panel *panel)
>   static int st7703_prepare(struct drm_panel *panel)
>   {
>   	struct st7703 *ctx = panel_to_st7703(panel);
> +	struct mipi_dsi_device *dsi = to_mipi_dsi_device(ctx->dev);
> +	struct mipi_dsi_multi_context dsi_ctx = {.dsi = dsi};
>   	int ret;
>   
>   	dev_dbg(ctx->dev, "Resetting the panel\n");
> @@ -843,6 +843,18 @@ static int st7703_prepare(struct drm_panel *panel)
>   	gpiod_set_value_cansleep(ctx->reset_gpio, 0);
>   	usleep_range(15000, 20000);
>   
> +	ctx->desc->init_sequence(&dsi_ctx);
> +
> +	if (dsi_ctx.accum_err) {
> +		gpiod_set_value_cansleep(ctx->reset_gpio, 1);
> +		regulator_disable(ctx->iovcc);
> +		regulator_disable(ctx->vcc);
> +
> +		return dsi_ctx.accum_err;
> +	}
> +
> +	dev_dbg(ctx->dev, "Panel init sequence done\n");
> +
>   	return 0;
>   }
>   

Reviewed-by: Neil Armstrong <neil.armstrong@linaro.org>

Thanks,
Neil