[PATCH v2] drm/panel: tdo-tl070wsh30: Use mipi_dsi_*_multi() functions

[PATCH v2] drm/panel: tdo-tl070wsh30: Use mipi_dsi_*_multi() functions

[Akash Sukhavasi]

The mipi_dsi_dcs_*() functions used by this driver are deprecated in
favor of their _multi() counterparts, as noted in
Documentation/gpu/todo.rst. The _multi() variants record the first
error in a context structure and skip every later call once an error
is set, so the return value no longer has to be checked after each
command. They also log their own failures, which makes the per-call
dev_err() calls redundant.

Convert prepare() and unprepare(), using mipi_dsi_msleep() and
mipi_dsi_usleep_range() for the delays between DSI commands. The
delays in the GPIO reset sequence stay as plain msleep() and
usleep_range(), since they run before any DSI transaction.

unprepare() now disables the regulator unconditionally and returns 0.
Previously a failure of enter_sleep_mode() returned early, leaving the
regulator enabled. drm_panel_unprepare() skips panel->prepared = false
when the callback returns an error, and drm_panel_prepare() returns
early when prepared is already set, so that path left the panel
powered and unable to be brought back up. Both functions return void,
so the error was never propagated to a caller in any case.

Signed-off-by: Akash Sukhavasi <akash.sukhavasi@gmail.com>
---
Compile tested only, no hardware available. checkpatch and a W=1 build
are clean.

Changes in v2:
- unprepare() disables the regulator unconditionally and returns 0,
  per Sashiko's review on v1. Returning an error left panel->prepared
  set, so the panel could not be prepared again.
- Link to v1: https://lore.kernel.org/r/20260725-mipi-dsi-tl070wsh30-multi-v1-1-69160b83982e@gmail.com
---
 drivers/gpu/drm/panel/panel-tdo-tl070wsh30.c | 40 ++++++++++------------------
 1 file changed, 14 insertions(+), 26 deletions(-)

diff --git a/drivers/gpu/drm/panel/panel-tdo-tl070wsh30.c b/drivers/gpu/drm/panel/panel-tdo-tl070wsh30.c
index 13cfe252a838..cd846e5ab2e7 100644
--- a/drivers/gpu/drm/panel/panel-tdo-tl070wsh30.c
+++ b/drivers/gpu/drm/panel/panel-tdo-tl070wsh30.c
@@ -35,6 +35,7 @@ struct tdo_tl070wsh30_panel *to_tdo_tl070wsh30_panel(struct drm_panel *panel)
 static int tdo_tl070wsh30_panel_prepare(struct drm_panel *panel)
 {
 	struct tdo_tl070wsh30_panel *tdo_tl070wsh30 = to_tdo_tl070wsh30_panel(panel);
+	struct mipi_dsi_multi_context dsi_ctx = { .dsi = tdo_tl070wsh30->link };
 	int err;
 
 	err = regulator_enable(tdo_tl070wsh30->supply);
@@ -51,45 +52,32 @@ static int tdo_tl070wsh30_panel_prepare(struct drm_panel *panel)
 
 	msleep(200);
 
-	err = mipi_dsi_dcs_exit_sleep_mode(tdo_tl070wsh30->link);
-	if (err < 0) {
-		dev_err(panel->dev, "failed to exit sleep mode: %d\n", err);
-		regulator_disable(tdo_tl070wsh30->supply);
-		return err;
-	}
+	mipi_dsi_dcs_exit_sleep_mode_multi(&dsi_ctx);
 
-	msleep(200);
+	mipi_dsi_msleep(&dsi_ctx, 200);
 
-	err = mipi_dsi_dcs_set_display_on(tdo_tl070wsh30->link);
-	if (err < 0) {
-		dev_err(panel->dev, "failed to set display on: %d\n", err);
-		regulator_disable(tdo_tl070wsh30->supply);
-		return err;
-	}
+	mipi_dsi_dcs_set_display_on_multi(&dsi_ctx);
 
-	msleep(20);
+	mipi_dsi_msleep(&dsi_ctx, 20);
 
-	return 0;
+	if (dsi_ctx.accum_err)
+		regulator_disable(tdo_tl070wsh30->supply);
+
+	return dsi_ctx.accum_err;
 }
 
 static int tdo_tl070wsh30_panel_unprepare(struct drm_panel *panel)
 {
 	struct tdo_tl070wsh30_panel *tdo_tl070wsh30 = to_tdo_tl070wsh30_panel(panel);
-	int err;
+	struct mipi_dsi_multi_context dsi_ctx = { .dsi = tdo_tl070wsh30->link };
 
-	err = mipi_dsi_dcs_set_display_off(tdo_tl070wsh30->link);
-	if (err < 0)
-		dev_err(panel->dev, "failed to set display off: %d\n", err);
+	mipi_dsi_dcs_set_display_off_multi(&dsi_ctx);
 
-	usleep_range(10000, 11000);
+	mipi_dsi_usleep_range(&dsi_ctx, 10000, 11000);
 
-	err = mipi_dsi_dcs_enter_sleep_mode(tdo_tl070wsh30->link);
-	if (err < 0) {
-		dev_err(panel->dev, "failed to enter sleep mode: %d\n", err);
-		return err;
-	}
+	mipi_dsi_dcs_enter_sleep_mode_multi(&dsi_ctx);
 
-	usleep_range(10000, 11000);
+	mipi_dsi_usleep_range(&dsi_ctx, 10000, 11000);
 
 	regulator_disable(tdo_tl070wsh30->supply);
 

---
base-commit: 62cc90241548d5570ee68e01aaba6506964e9811
change-id: 20260722-mipi-dsi-tl070wsh30-multi-2095299d5b12

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

Re: [PATCH v2] drm/panel: tdo-tl070wsh30: Use mipi_dsi_*_multi() functions

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Unintended short-circuiting of the panel power-down sequence on error.
--

--- Patch [1]: [PATCH v2] drm/panel: tdo-tl070wsh30: Use mipi_dsi_*_multi() functions ---
commit dccc547d72f30cdc8106a575cdc689ca0fbc4142
Author: Akash Sukhavasi <akash.sukhavasi@gmail.com>

drm/panel: tdo-tl070wsh30: Use mipi_dsi_*_multi() functions

This commit converts the tdo-tl070wsh30 panel driver to use the
mipi_dsi_*_multi() API. It also updates unprepare() to disable the
regulator unconditionally and return 0.

Link: https://lore.kernel.org/r/20260725-mipi-dsi-tl070wsh30-multi-v1-1-69160b83982e@gmail.com

> diff --git a/drivers/gpu/drm/panel/panel-tdo-tl070wsh30.c b/drivers/gpu/drm/panel/panel-tdo-tl070wsh30.c
> index 13cfe252a838d..cd846e5ab2e7e 100644
> --- a/drivers/gpu/drm/panel/panel-tdo-tl070wsh30.c
> +++ b/drivers/gpu/drm/panel/panel-tdo-tl070wsh30.c

[ ... ]

>  static int tdo_tl070wsh30_panel_unprepare(struct drm_panel *panel)
>  {
>  	struct tdo_tl070wsh30_panel *tdo_tl070wsh30 = to_tdo_tl070wsh30_panel(panel);
> -	int err;
> +	struct mipi_dsi_multi_context dsi_ctx = { .dsi = tdo_tl070wsh30->link };
>  
> -	err = mipi_dsi_dcs_set_display_off(tdo_tl070wsh30->link);
> -	if (err < 0)
> -		dev_err(panel->dev, "failed to set display off: %d\n", err);
> +	mipi_dsi_dcs_set_display_off_multi(&dsi_ctx);
>  
> -	usleep_range(10000, 11000);
> +	mipi_dsi_usleep_range(&dsi_ctx, 10000, 11000);
>  
> -	err = mipi_dsi_dcs_enter_sleep_mode(tdo_tl070wsh30->link);
> -	if (err < 0) {
> -		dev_err(panel->dev, "failed to enter sleep mode: %d\n", err);
> -		return err;
> -	}
> +	mipi_dsi_dcs_enter_sleep_mode_multi(&dsi_ctx);
>  
> -	usleep_range(10000, 11000);
> +	mipi_dsi_usleep_range(&dsi_ctx, 10000, 11000);
>  
>  	regulator_disable(tdo_tl070wsh30->supply);

[Severity: Medium]
Does this unintentionally short-circuit the panel power-down sequence on error?

In the original code, if mipi_dsi_dcs_set_display_off fails, the error is
logged but execution continues. This ensures the 10ms delays and the
enter_sleep_mode command are still processed before disabling the regulator.

With the multi API, if mipi_dsi_dcs_set_display_off_multi fails, it sets
dsi_ctx.accum_err. The subsequent mipi_dsi_usleep_range and
mipi_dsi_dcs_enter_sleep_mode_multi calls will check this accumulated error
and skip their execution entirely.

Could this immediate jump to regulator_disable violate the panel's
power-down timing requirements by skipping the mandatory delays?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260727-mipi-dsi-tl070wsh30-multi-v2-1-f47b937631d3@gmail.com?part=1

Re: [PATCH v2] drm/panel: tdo-tl070wsh30: Use mipi_dsi_*_multi() functions

[Doug Anderson]

Hi,

On Mon, Jul 27, 2026 at 6:45 PM Akash Sukhavasi
<akash.sukhavasi@gmail.com> wrote:
>
> The mipi_dsi_dcs_*() functions used by this driver are deprecated in
> favor of their _multi() counterparts, as noted in
> Documentation/gpu/todo.rst. The _multi() variants record the first
> error in a context structure and skip every later call once an error
> is set, so the return value no longer has to be checked after each
> command. They also log their own failures, which makes the per-call
> dev_err() calls redundant.
>
> Convert prepare() and unprepare(), using mipi_dsi_msleep() and
> mipi_dsi_usleep_range() for the delays between DSI commands. The
> delays in the GPIO reset sequence stay as plain msleep() and
> usleep_range(), since they run before any DSI transaction.
>
> unprepare() now disables the regulator unconditionally and returns 0.
> Previously a failure of enter_sleep_mode() returned early, leaving the
> regulator enabled. drm_panel_unprepare() skips panel->prepared = false
> when the callback returns an error, and drm_panel_prepare() returns
> early when prepared is already set, so that path left the panel
> powered and unable to be brought back up. Both functions return void,
> so the error was never propagated to a caller in any case.
>
> Signed-off-by: Akash Sukhavasi <akash.sukhavasi@gmail.com>
> ---
> Compile tested only, no hardware available. checkpatch and a W=1 build
> are clean.
>
> Changes in v2:
> - unprepare() disables the regulator unconditionally and returns 0,
>   per Sashiko's review on v1. Returning an error left panel->prepared
>   set, so the panel could not be prepared again.
> - Link to v1: https://lore.kernel.org/r/20260725-mipi-dsi-tl070wsh30-multi-v1-1-69160b83982e@gmail.com
> ---
>  drivers/gpu/drm/panel/panel-tdo-tl070wsh30.c | 40 ++++++++++------------------
>  1 file changed, 14 insertions(+), 26 deletions(-)

Just as a point of note, something about your original email to me and
your original patch triggered gmail's SPAM filters, so I actually
didn't see anything from you until your first reply to Sashiko [1],
then I've been a bit backlogged. Sorry about that!

[1] https://lore.kernel.org/r/amU_GppAwt3XXPX-@gmail.com


Also: since your patch contains some minor bugfixes, it's probably
good not to hide under a code cleanup. Maybe a better subject line:

drm/panel: tdo-tl070wsh30: Use mipi_dsi_*_multi(); fix minor bugs


> diff --git a/drivers/gpu/drm/panel/panel-tdo-tl070wsh30.c b/drivers/gpu/drm/panel/panel-tdo-tl070wsh30.c
> index 13cfe252a838..cd846e5ab2e7 100644
> --- a/drivers/gpu/drm/panel/panel-tdo-tl070wsh30.c
> +++ b/drivers/gpu/drm/panel/panel-tdo-tl070wsh30.c
> @@ -35,6 +35,7 @@ struct tdo_tl070wsh30_panel *to_tdo_tl070wsh30_panel(struct drm_panel *panel)
>  static int tdo_tl070wsh30_panel_prepare(struct drm_panel *panel)
>  {
>         struct tdo_tl070wsh30_panel *tdo_tl070wsh30 = to_tdo_tl070wsh30_panel(panel);
> +       struct mipi_dsi_multi_context dsi_ctx = { .dsi = tdo_tl070wsh30->link };
>         int err;
>
>         err = regulator_enable(tdo_tl070wsh30->supply);
> @@ -51,45 +52,32 @@ static int tdo_tl070wsh30_panel_prepare(struct drm_panel *panel)
>
>         msleep(200);
>
> -       err = mipi_dsi_dcs_exit_sleep_mode(tdo_tl070wsh30->link);
> -       if (err < 0) {
> -               dev_err(panel->dev, "failed to exit sleep mode: %d\n", err);
> -               regulator_disable(tdo_tl070wsh30->supply);
> -               return err;
> -       }
> +       mipi_dsi_dcs_exit_sleep_mode_multi(&dsi_ctx);
>
> -       msleep(200);
> +       mipi_dsi_msleep(&dsi_ctx, 200);
>
> -       err = mipi_dsi_dcs_set_display_on(tdo_tl070wsh30->link);
> -       if (err < 0) {
> -               dev_err(panel->dev, "failed to set display on: %d\n", err);
> -               regulator_disable(tdo_tl070wsh30->supply);
> -               return err;
> -       }
> +       mipi_dsi_dcs_set_display_on_multi(&dsi_ctx);
>
> -       msleep(20);
> +       mipi_dsi_msleep(&dsi_ctx, 20);
>
> -       return 0;
> +       if (dsi_ctx.accum_err)
> +               regulator_disable(tdo_tl070wsh30->supply);
> +
> +       return dsi_ctx.accum_err;

Style nit: in general, there are way too many blank lines. You don't
need a blank line between every statement.

>  }
>
>  static int tdo_tl070wsh30_panel_unprepare(struct drm_panel *panel)
>  {
>         struct tdo_tl070wsh30_panel *tdo_tl070wsh30 = to_tdo_tl070wsh30_panel(panel);
> -       int err;
> +       struct mipi_dsi_multi_context dsi_ctx = { .dsi = tdo_tl070wsh30->link };
>
> -       err = mipi_dsi_dcs_set_display_off(tdo_tl070wsh30->link);
> -       if (err < 0)
> -               dev_err(panel->dev, "failed to set display off: %d\n", err);
> +       mipi_dsi_dcs_set_display_off_multi(&dsi_ctx);
>
> -       usleep_range(10000, 11000);
> +       mipi_dsi_usleep_range(&dsi_ctx, 10000, 11000);
>
> -       err = mipi_dsi_dcs_enter_sleep_mode(tdo_tl070wsh30->link);
> -       if (err < 0) {
> -               dev_err(panel->dev, "failed to enter sleep mode: %d\n", err);
> -               return err;
> -       }
> +       mipi_dsi_dcs_enter_sleep_mode_multi(&dsi_ctx);
>
> -       usleep_range(10000, 11000);
> +       mipi_dsi_usleep_range(&dsi_ctx, 10000, 11000);
>
>         regulator_disable(tdo_tl070wsh30->supply);

I support fixing `unprepare` to always return 0, and I'm glad you
mentioned that fix in the commit message.

...and I think it should be fine to disable the regulator even if the
MIPI commands fail. ...at least, I hope so.

However, I think the Sashiko bot had a point [2] when it noted that
the old code used to still sleep for ~10ms and try to enter sleep mode
even if the "display off" failed. That behavior is probably worth
preserving.

[2] http://lore.kernel.org/r/20260728015051.97FA71F000E9@smtp.kernel.org

-Doug

Re: [PATCH v2] drm/panel: tdo-tl070wsh30: Use mipi_dsi_*_multi() functions

[Akash Sukhavasi]

On Wed, Jul 29, 2026 at 06:55:53PM -0700, Doug Anderson wrote:
> Style nit: in general, there are way too many blank lines. You don't
> need a blank line between every statement.

Will pack them in v3.

> However, I think the Sashiko bot had a point [2] when it noted that
> the old code used to still sleep for ~10ms and try to enter sleep mode
> even if the "display off" failed. That behavior is probably worth
> preserving.

Understood. I will preserve the behaviour in v3.
-- 
Thanks,
Akash