Re: [PATCH v2 3/5] drm/panel: feiyang-fy07024di26a30d: Use mipi_dsi_*_multi() functions

[neil.armstrong@linaro.org]

On 11/2/26 05:32, Hsieh Hung-En wrote:
> Replace the deprecated MIPI DSI DCS helpers with their _multi variants,
> as suggested in Documentation/gpu/todo.rst.
> 
> Replace the initialization command table and loop with seven direct
> mipi_dsi_dcs_write_seq_multi() calls. Preserve the original command
> order and values, using lowercase hexadecimal literals.
> 
> In prepare(), the first failed DCS write prevents subsequent transfers
> and its error is propagated through the cleanup path added by the
> preceding patch. The helpers also log command failures.
> 
> In unprepare(), clear accum_err between commands so that ENTER_SLEEP_MODE
> is still attempted if SET_DISPLAY_OFF fails, preserving the original
> error handling behavior.
> 
> In disable(), ignore SET_DISPLAY_OFF errors to allow drm_panel_disable()
> to clear panel->enabled, avoiding a stale enabled state after the panel
> is powered off. This follows commit 00efe8a68565 ("drm/panel:
> ronbo-rb070d30: Ignore DCS errors in disable()"). Command failures
> remain logged by the helper.
> 
> Signed-off-by: Hsieh Hung-En <hungen3108@gmail.com>
> ---
> 
> Notes:
>      Changes in v2:
>      - Replace the initialization command table with direct
>        mipi_dsi_dcs_write_seq_multi() calls, as suggested by Doug.
>      - Route initialization errors through the cleanup path added in 1/5.
>      - Convert initialization command values to lowercase hex.
>      - Drop the previous Reviewed-by tag due to the initialization rework.
>      
>      The seven initialization commands were checked against the original
>      table; their order and values are unchanged.
> 
>   .../drm/panel/panel-feiyang-fy07024di26a30d.c | 68 +++++++++----------
>   1 file changed, 33 insertions(+), 35 deletions(-)
> 
> diff --git a/drivers/gpu/drm/panel/panel-feiyang-fy07024di26a30d.c b/drivers/gpu/drm/panel/panel-feiyang-fy07024di26a30d.c
> index bb4cc1ea2ece..fa304eb7453b 100644
> --- a/drivers/gpu/drm/panel/panel-feiyang-fy07024di26a30d.c
> +++ b/drivers/gpu/drm/panel/panel-feiyang-fy07024di26a30d.c
> @@ -13,8 +13,6 @@
>   #include <linux/module.h>
>   #include <linux/regulator/consumer.h>
>   
> -#define FEIYANG_INIT_CMD_LEN	2
> -
>   struct feiyang {
>   	struct drm_panel	panel;
>   	struct mipi_dsi_device	*dsi;
> @@ -29,25 +27,21 @@ static inline struct feiyang *panel_to_feiyang(struct drm_panel *panel)
>   	return container_of(panel, struct feiyang, panel);
>   }
>   
> -struct feiyang_init_cmd {
> -	u8 data[FEIYANG_INIT_CMD_LEN];
> -};
> -
> -static const struct feiyang_init_cmd feiyang_init_cmds[] = {
> -	{ .data = { 0x80, 0x58 } },
> -	{ .data = { 0x81, 0x47 } },
> -	{ .data = { 0x82, 0xD4 } },
> -	{ .data = { 0x83, 0x88 } },
> -	{ .data = { 0x84, 0xA9 } },
> -	{ .data = { 0x85, 0xC3 } },
> -	{ .data = { 0x86, 0x82 } },
> -};
> +static void feiyang_init(struct mipi_dsi_multi_context *dsi_ctx)
> +{
> +	mipi_dsi_dcs_write_seq_multi(dsi_ctx, 0x80, 0x58);
> +	mipi_dsi_dcs_write_seq_multi(dsi_ctx, 0x81, 0x47);
> +	mipi_dsi_dcs_write_seq_multi(dsi_ctx, 0x82, 0xd4);
> +	mipi_dsi_dcs_write_seq_multi(dsi_ctx, 0x83, 0x88);
> +	mipi_dsi_dcs_write_seq_multi(dsi_ctx, 0x84, 0xa9);
> +	mipi_dsi_dcs_write_seq_multi(dsi_ctx, 0x85, 0xc3);
> +	mipi_dsi_dcs_write_seq_multi(dsi_ctx, 0x86, 0x82);
> +}
>   
>   static int feiyang_prepare(struct drm_panel *panel)
>   {
>   	struct feiyang *ctx = panel_to_feiyang(panel);
> -	struct mipi_dsi_device *dsi = ctx->dsi;
> -	unsigned int i;
> +	struct mipi_dsi_multi_context dsi_ctx = { .dsi = ctx->dsi };
>   	int ret;
>   
>   	ret = regulator_enable(ctx->dvdd);
> @@ -77,15 +71,10 @@ static int feiyang_prepare(struct drm_panel *panel)
>   	/* T12 (video & logic signal rise + backlight rise) T12 >= 200ms */
>   	msleep(200);
>   
> -	for (i = 0; i < ARRAY_SIZE(feiyang_init_cmds); i++) {
> -		const struct feiyang_init_cmd *cmd =
> -						&feiyang_init_cmds[i];
> -
> -		ret = mipi_dsi_dcs_write_buffer(dsi, cmd->data,
> -						FEIYANG_INIT_CMD_LEN);
> -		if (ret < 0)
> -			goto assert_reset;
> -	}
> +	feiyang_init(&dsi_ctx);
> +	ret = dsi_ctx.accum_err;
> +	if (ret < 0)
> +		goto assert_reset;
>   
>   	return 0;
>   
> @@ -104,32 +93,41 @@ static int feiyang_prepare(struct drm_panel *panel)
>   static int feiyang_enable(struct drm_panel *panel)
>   {
>   	struct feiyang *ctx = panel_to_feiyang(panel);
> +	struct mipi_dsi_multi_context dsi_ctx = { .dsi = ctx->dsi };
>   
>   	/* T12 (video & logic signal rise + backlight rise) T12 >= 200ms */
>   	msleep(200);
>   
> -	return mipi_dsi_dcs_set_display_on(ctx->dsi);
> +	mipi_dsi_dcs_set_display_on_multi(&dsi_ctx);
> +
> +	return dsi_ctx.accum_err;
>   }
>   
>   static int feiyang_disable(struct drm_panel *panel)
>   {
>   	struct feiyang *ctx = panel_to_feiyang(panel);
> +	struct mipi_dsi_multi_context dsi_ctx = { .dsi = ctx->dsi };
> +
> +	mipi_dsi_dcs_set_display_off_multi(&dsi_ctx);
>   
> -	return mipi_dsi_dcs_set_display_off(ctx->dsi);
> +	/*
> +	 * Returning an error would leave panel->enabled set, causing the
> +	 * next enable to be skipped even after unprepare powers off the panel.
> +	 * The helper above already logs any command failure.
> +	 */
> +	return 0;
>   }
>   
>   static int feiyang_unprepare(struct drm_panel *panel)
>   {
>   	struct feiyang *ctx = panel_to_feiyang(panel);
> -	int ret;
> +	struct mipi_dsi_multi_context dsi_ctx = { .dsi = ctx->dsi };
>   
> -	ret = mipi_dsi_dcs_set_display_off(ctx->dsi);
> -	if (ret < 0)
> -		dev_err(panel->dev, "failed to set display off: %d\n", ret);
> +	mipi_dsi_dcs_set_display_off_multi(&dsi_ctx);
>   
> -	ret = mipi_dsi_dcs_enter_sleep_mode(ctx->dsi);
> -	if (ret < 0)
> -		dev_err(panel->dev, "failed to enter sleep mode: %d\n", ret);
> +	/* Attempt to enter sleep mode even if display off failed. */
> +	dsi_ctx.accum_err = 0;
> +	mipi_dsi_dcs_enter_sleep_mode_multi(&dsi_ctx);
>   
>   	/* T13 (backlight fall + video & logic signal fall) T13 >= 200ms */
>   	msleep(200);

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

Thanks,
Neil