[PATCH] drm/panel: samsung-s6d16d0: Use mipi_dsi_*_multi(); fix minor bugs

[PATCH] drm/panel: samsung-s6d16d0: Use mipi_dsi_*_multi(); fix minor bugs

[Akash Sukhavasi]

The mipi_dsi_dcs_*() functions used by this driver are deprecated
in favour of their _multi() counterparts, as noted in
Documentation/gpu/todo.rst. The _multi() variants record the
first error in a context structure and skip subsequent calls once
an error is set, removing the need to check the return value
after each command. They also log failures internally, making the
per-call dev_err() calls redundant.

Convert prepare(), enable(), disable(), and unprepare() to use
mipi_dsi_dcs_*_multi().

unprepare() previously returned an error if
mipi_dsi_dcs_enter_sleep_mode() failed, skipping RESET assertion
and regulator_disable(). Because drm_panel_unprepare() does not
clear panel->prepared when the callback returns an error,
drm_panel_prepare() would then return early on the next call,
leaving the panel powered and unable to be re-initialised. The
converted code always asserts RESET, disables the regulator, and
returns 0.

Also fix a typo in a comment ("Enabe" -> "Enable").

Signed-off-by: Akash Sukhavasi <akash.sukhavasi@gmail.com>
---
Compile tested only, no hardware available. checkpatch and a W=1
build are clean.
---
 drivers/gpu/drm/panel/panel-samsung-s6d16d0.c | 56 ++++++++-------------------
 1 file changed, 16 insertions(+), 40 deletions(-)

diff --git a/drivers/gpu/drm/panel/panel-samsung-s6d16d0.c b/drivers/gpu/drm/panel/panel-samsung-s6d16d0.c
index 54a65abf7e89..85b4515f443d 100644
--- a/drivers/gpu/drm/panel/panel-samsung-s6d16d0.c
+++ b/drivers/gpu/drm/panel/panel-samsung-s6d16d0.c
@@ -47,15 +47,10 @@ static inline struct s6d16d0 *panel_to_s6d16d0(struct drm_panel *panel)
 static int s6d16d0_unprepare(struct drm_panel *panel)
 {
 	struct s6d16d0 *s6 = panel_to_s6d16d0(panel);
-	struct mipi_dsi_device *dsi = to_mipi_dsi_device(s6->dev);
-	int ret;
+	struct mipi_dsi_multi_context dsi_ctx = { .dsi = to_mipi_dsi_device(s6->dev) };
 
 	/* Enter sleep mode */
-	ret = mipi_dsi_dcs_enter_sleep_mode(dsi);
-	if (ret) {
-		dev_err(s6->dev, "failed to enter sleep mode (%d)\n", ret);
-		return ret;
-	}
+	mipi_dsi_dcs_enter_sleep_mode_multi(&dsi_ctx);
 
 	/* Assert RESET */
 	gpiod_set_value_cansleep(s6->reset_gpio, 1);
@@ -67,7 +62,7 @@ static int s6d16d0_unprepare(struct drm_panel *panel)
 static int s6d16d0_prepare(struct drm_panel *panel)
 {
 	struct s6d16d0 *s6 = panel_to_s6d16d0(panel);
-	struct mipi_dsi_device *dsi = to_mipi_dsi_device(s6->dev);
+	struct mipi_dsi_multi_context dsi_ctx = { .dsi = to_mipi_dsi_device(s6->dev) };
 	int ret;
 
 	ret = regulator_enable(s6->supply);
@@ -83,57 +78,38 @@ static int s6d16d0_prepare(struct drm_panel *panel)
 	gpiod_set_value_cansleep(s6->reset_gpio, 0);
 	msleep(120);
 
-	/* Enabe tearing mode: send TE (tearing effect) at VBLANK */
-	ret = mipi_dsi_dcs_set_tear_on(dsi,
+	/* Enable tearing mode: send TE (tearing effect) at VBLANK */
+	mipi_dsi_dcs_set_tear_on_multi(&dsi_ctx,
 				       MIPI_DSI_DCS_TEAR_MODE_VBLANK);
-	if (ret) {
-		dev_err(s6->dev, "failed to enable vblank TE (%d)\n", ret);
-		goto err_power_off;
-	}
 	/* Exit sleep mode and power on */
-	ret = mipi_dsi_dcs_exit_sleep_mode(dsi);
-	if (ret) {
-		dev_err(s6->dev, "failed to exit sleep mode (%d)\n", ret);
-		goto err_power_off;
+	mipi_dsi_dcs_exit_sleep_mode_multi(&dsi_ctx);
+	if (dsi_ctx.accum_err) {
+		gpiod_set_value_cansleep(s6->reset_gpio, 1);
+		regulator_disable(s6->supply);
+		return dsi_ctx.accum_err;
 	}
 
 	return 0;
-
-err_power_off:
-	gpiod_set_value_cansleep(s6->reset_gpio, 1);
-	regulator_disable(s6->supply);
-
-	return ret;
 }
 
 static int s6d16d0_enable(struct drm_panel *panel)
 {
 	struct s6d16d0 *s6 = panel_to_s6d16d0(panel);
-	struct mipi_dsi_device *dsi = to_mipi_dsi_device(s6->dev);
-	int ret;
+	struct mipi_dsi_multi_context dsi_ctx = { .dsi = to_mipi_dsi_device(s6->dev) };
 
-	ret = mipi_dsi_dcs_set_display_on(dsi);
-	if (ret) {
-		dev_err(s6->dev, "failed to turn display on (%d)\n", ret);
-		return ret;
-	}
+	mipi_dsi_dcs_set_display_on_multi(&dsi_ctx);
 
-	return 0;
+	return dsi_ctx.accum_err;
 }
 
 static int s6d16d0_disable(struct drm_panel *panel)
 {
 	struct s6d16d0 *s6 = panel_to_s6d16d0(panel);
-	struct mipi_dsi_device *dsi = to_mipi_dsi_device(s6->dev);
-	int ret;
+	struct mipi_dsi_multi_context dsi_ctx = { .dsi = to_mipi_dsi_device(s6->dev) };
 
-	ret = mipi_dsi_dcs_set_display_off(dsi);
-	if (ret) {
-		dev_err(s6->dev, "failed to turn display off (%d)\n", ret);
-		return ret;
-	}
+	mipi_dsi_dcs_set_display_off_multi(&dsi_ctx);
 
-	return 0;
+	return dsi_ctx.accum_err;
 }
 
 static int s6d16d0_get_modes(struct drm_panel *panel,

---
base-commit: dc2f9f7fed1a8ea5290f9f60c6310d497e85e666
change-id: 20260807-mipi-dsi-s6d16d0-multi-167e236e9666

Best regards,
-- 
Akash Sukhavasi <akash.sukhavasi@gmail.com>

Re: [PATCH] drm/panel: samsung-s6d16d0: Use mipi_dsi_*_multi(); fix minor bugs

[Neil Armstrong]

On 8/8/26 00:55, Akash Sukhavasi wrote:
> The mipi_dsi_dcs_*() functions used by this driver are deprecated
> in favour of their _multi() counterparts, as noted in
> Documentation/gpu/todo.rst. The _multi() variants record the
> first error in a context structure and skip subsequent calls once
> an error is set, removing the need to check the return value
> after each command. They also log failures internally, making the
> per-call dev_err() calls redundant.
> 
> Convert prepare(), enable(), disable(), and unprepare() to use
> mipi_dsi_dcs_*_multi().
> 
> unprepare() previously returned an error if
> mipi_dsi_dcs_enter_sleep_mode() failed, skipping RESET assertion
> and regulator_disable(). Because drm_panel_unprepare() does not
> clear panel->prepared when the callback returns an error,
> drm_panel_prepare() would then return early on the next call,
> leaving the panel powered and unable to be re-initialised. The
> converted code always asserts RESET, disables the regulator, and
> returns 0.
> 
> Also fix a typo in a comment ("Enabe" -> "Enable").
> 
> Signed-off-by: Akash Sukhavasi <akash.sukhavasi@gmail.com>
> ---
> Compile tested only, no hardware available. checkpatch and a W=1
> build are clean.
> ---
>   drivers/gpu/drm/panel/panel-samsung-s6d16d0.c | 56 ++++++++-------------------
>   1 file changed, 16 insertions(+), 40 deletions(-)
> 
> diff --git a/drivers/gpu/drm/panel/panel-samsung-s6d16d0.c b/drivers/gpu/drm/panel/panel-samsung-s6d16d0.c
> index 54a65abf7e89..85b4515f443d 100644
> --- a/drivers/gpu/drm/panel/panel-samsung-s6d16d0.c
> +++ b/drivers/gpu/drm/panel/panel-samsung-s6d16d0.c
> @@ -47,15 +47,10 @@ static inline struct s6d16d0 *panel_to_s6d16d0(struct drm_panel *panel)
>   static int s6d16d0_unprepare(struct drm_panel *panel)
>   {
>   	struct s6d16d0 *s6 = panel_to_s6d16d0(panel);
> -	struct mipi_dsi_device *dsi = to_mipi_dsi_device(s6->dev);
> -	int ret;
> +	struct mipi_dsi_multi_context dsi_ctx = { .dsi = to_mipi_dsi_device(s6->dev) };

Ok why not keeping the original:

	struct mipi_dsi_device *dsi = to_mipi_dsi_device(s6->dev);
	struct mipi_dsi_multi_context dsi_ctx = { .dsi = dsi };


It's much easier to read, for me at least, but I won't block it...

>   
>   	/* Enter sleep mode */
> -	ret = mipi_dsi_dcs_enter_sleep_mode(dsi);
> -	if (ret) {
> -		dev_err(s6->dev, "failed to enter sleep mode (%d)\n", ret);
> -		return ret;
> -	}
> +	mipi_dsi_dcs_enter_sleep_mode_multi(&dsi_ctx);

Add a comment you ignore the the dsi_ctx.accum_err and it's fine

>   
>   	/* Assert RESET */
>   	gpiod_set_value_cansleep(s6->reset_gpio, 1);
> @@ -67,7 +62,7 @@ static int s6d16d0_unprepare(struct drm_panel *panel)
>   static int s6d16d0_prepare(struct drm_panel *panel)
>   {
>   	struct s6d16d0 *s6 = panel_to_s6d16d0(panel);
> -	struct mipi_dsi_device *dsi = to_mipi_dsi_device(s6->dev);
> +	struct mipi_dsi_multi_context dsi_ctx = { .dsi = to_mipi_dsi_device(s6->dev) };

Ditto

>   	int ret;
>   
>   	ret = regulator_enable(s6->supply);
> @@ -83,57 +78,38 @@ static int s6d16d0_prepare(struct drm_panel *panel)
>   	gpiod_set_value_cansleep(s6->reset_gpio, 0);
>   	msleep(120);
>   
> -	/* Enabe tearing mode: send TE (tearing effect) at VBLANK */
> -	ret = mipi_dsi_dcs_set_tear_on(dsi,
> +	/* Enable tearing mode: send TE (tearing effect) at VBLANK */
> +	mipi_dsi_dcs_set_tear_on_multi(&dsi_ctx,
>   				       MIPI_DSI_DCS_TEAR_MODE_VBLANK);
> -	if (ret) {
> -		dev_err(s6->dev, "failed to enable vblank TE (%d)\n", ret);
> -		goto err_power_off;
> -	}
>   	/* Exit sleep mode and power on */
> -	ret = mipi_dsi_dcs_exit_sleep_mode(dsi);
> -	if (ret) {
> -		dev_err(s6->dev, "failed to exit sleep mode (%d)\n", ret);
> -		goto err_power_off;
> +	mipi_dsi_dcs_exit_sleep_mode_multi(&dsi_ctx);
> +	if (dsi_ctx.accum_err) {
> +		gpiod_set_value_cansleep(s6->reset_gpio, 1);
> +		regulator_disable(s6->supply);
> +		return dsi_ctx.accum_err;
>   	}
>   
>   	return 0;

You could write:

	if (dsi_ctx.accum_err) {
		gpiod_set_value_cansleep(s6->reset_gpio, 1);
		regulator_disable(s6->supply);
	}

	return dsi_ctx.accum_err;

> -
> -err_power_off:
> -	gpiod_set_value_cansleep(s6->reset_gpio, 1);
> -	regulator_disable(s6->supply);
> -
> -	return ret;
>   }
>   
>   static int s6d16d0_enable(struct drm_panel *panel)
>   {
>   	struct s6d16d0 *s6 = panel_to_s6d16d0(panel);
> -	struct mipi_dsi_device *dsi = to_mipi_dsi_device(s6->dev);
> -	int ret;
> +	struct mipi_dsi_multi_context dsi_ctx = { .dsi = to_mipi_dsi_device(s6->dev) };

Ditto

>   
> -	ret = mipi_dsi_dcs_set_display_on(dsi);
> -	if (ret) {
> -		dev_err(s6->dev, "failed to turn display on (%d)\n", ret);
> -		return ret;
> -	}
> +	mipi_dsi_dcs_set_display_on_multi(&dsi_ctx);
>   
> -	return 0;
> +	return dsi_ctx.accum_err;
>   }
>   
>   static int s6d16d0_disable(struct drm_panel *panel)
>   {
>   	struct s6d16d0 *s6 = panel_to_s6d16d0(panel);
> -	struct mipi_dsi_device *dsi = to_mipi_dsi_device(s6->dev);
> -	int ret;
> +	struct mipi_dsi_multi_context dsi_ctx = { .dsi = to_mipi_dsi_device(s6->dev) };

Ditto

>   
> -	ret = mipi_dsi_dcs_set_display_off(dsi);
> -	if (ret) {
> -		dev_err(s6->dev, "failed to turn display off (%d)\n", ret);
> -		return ret;
> -	}
> +	mipi_dsi_dcs_set_display_off_multi(&dsi_ctx);
>   
> -	return 0;
> +	return dsi_ctx.accum_err;
>   }
>   
>   static int s6d16d0_get_modes(struct drm_panel *panel,
> 
> ---
> base-commit: dc2f9f7fed1a8ea5290f9f60c6310d497e85e666
> change-id: 20260807-mipi-dsi-s6d16d0-multi-167e236e9666
> 
> Best regards,

Thanks,
Neil

Re: [PATCH] drm/panel: samsung-s6d16d0: Use mipi_dsi_*_multi(); fix minor bugs

[Akash Sukhavasi]

On Mon, Aug 10, 2026 at 11:57:06AM +0200, Neil Armstrong wrote:
> > +	struct mipi_dsi_multi_context dsi_ctx = { .dsi = to_mipi_dsi_device(s6->dev) };
> 
> Ok why not keeping the original:
> 
> 	struct mipi_dsi_device *dsi = to_mipi_dsi_device(s6->dev);
> 	struct mipi_dsi_multi_context dsi_ctx = { .dsi = dsi };
> 
> 
> It's much easier to read, for me at least, but I won't block it...
> 
> >   	/* Enter sleep mode */
> > -	ret = mipi_dsi_dcs_enter_sleep_mode(dsi);
> > -	if (ret) {
> > -		dev_err(s6->dev, "failed to enter sleep mode (%d)\n", ret);
> > -		return ret;
> > -	}
> > +	mipi_dsi_dcs_enter_sleep_mode_multi(&dsi_ctx);
> 
> Add a comment you ignore the the dsi_ctx.accum_err and it's fine
 
> You could write:
> 
> 	if (dsi_ctx.accum_err) {
> 		gpiod_set_value_cansleep(s6->reset_gpio, 1);
> 		regulator_disable(s6->supply);
> 	}
> 
> 	return dsi_ctx.accum_err;

Thanks for the review, Neil. Will address all three in v2.

On the intermediate dsi variable, I prefer inlining it to keep one fewer
variable in scope, but I see the readability argument. Will revert it.
-- 
Thanks,
Akash