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

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

[Akash Sukhavasi]

On Sat, Jul 25, 2026 at 05:01:33PM +0000, sashiko-bot@kernel.org wrote:
> [Severity: High]
> Does returning early here on a DSI transmission error leave the panel in an
> unrecoverable state?
>
> Because panel->prepared is never set to false on error, a subsequent call
> to drm_panel_prepare() will immediately return 0 without re-running the
> hardware initialization sequence, leaving the display permanently blank
> until a system reboot.
>
> Could we unconditionally disable the regulator and return 0 here to ensure
> the state machine remains synchronized, even if there are DSI errors?

Agreed, and this is the alternative I raised below the cut in this
patch.

Unless anyone objects, I will send a v2 that disables the regulator
unconditionally and returns 0.

Sashiko AI review: https://sashiko.dev/#/patchset/20260725-mipi-dsi-tl070wsh30-multi-v1-1-69160b83982e@gmail.com?part=1

-- 
Thanks,
Akash

[PATCH] 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.

This changes one behavior in unprepare(). A failure of
set_display_off() was previously logged and the sequence continued,
ending with the regulator disabled. A failure of enter_sleep_mode()
returned early and left the regulator enabled. Under the _multi() API
the first recorded error stops the remaining calls, so both failures
now return early with the regulator left enabled.

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

unprepare() keeps the accum_err guard, so a failed command leaves the
regulator enabled and the error propagates as before. The alternative
is to disable unconditionally and return 0, which avoids the stranded
rail, at the cost of dropping the accumulated error. Happy to send it
that way instead if that is preferred. Both approaches appear in other
panel drivers.
---
 drivers/gpu/drm/panel/panel-tdo-tl070wsh30.c | 43 +++++++++++-----------------
 1 file changed, 17 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..f7c8893af380 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,35 @@ 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);
+
+	if (dsi_ctx.accum_err)
+		return dsi_ctx.accum_err;
 
 	regulator_disable(tdo_tl070wsh30->supply);
 

---
base-commit: 0ce37745d4bfbc493f718169c3974898ffec8ee7
change-id: 20260722-mipi-dsi-tl070wsh30-multi-2095299d5b12

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