[PATCH 0/2] drm/panel/visionox-rm69299: Split DSI commands out into enable/disable

[PATCH 0/2] drm/panel/visionox-rm69299: Split DSI commands out into enable/disable

[Guido Günther]

The DSI commands should be sent in enable/disable, fix that which also
fixes various DSI commands failing on screen blank/unblank like:

  msm_dsi ae94000.dsi: [drm:dsi_cmds2buf_tx] *ERROR* wait for video done timed out
  dsi_cmds2buf_tx: cmd dma tx failed, type=0x5, data0=0x28, len=4, ret=-110
  panel-visionox-rm69299 ae94000.dsi.0: sending DCS SET_DISPLAY_OFF failed: -110

Even with this applied I'm seeing some

   disp_cc_mdss_pclk0_clk status stuck at 'on'
   disp_cc_mdss_byte0_intf_clk status stuck at 'on'
   disp_cc_mdss_byte0_clk status stuck at 'on'

on SHIFT6mq once during boot but all DSI timeouts are gone.

Signed-off-by: Guido Günther <agx@sigxcpu.org>
---
Guido Günther (2):
      drm/panel: visionox-rm69299: Split DSI commands out into enable/disable
      drm/panel: visionox-rm69299: Move power_on/off into prepare/unprepare

 drivers/gpu/drm/panel/panel-visionox-rm69299.c | 113 ++++++++++++-------------
 1 file changed, 56 insertions(+), 57 deletions(-)
---
base-commit: 254f49634ee16a731174d2ae34bc50bd5f45e731
change-id: 20260427-visionox-rm69299-enable-06f3d736f664

Best regards,
-- 
Guido Günther <agx@sigxcpu.org>

[PATCH 1/2] drm/panel: visionox-rm69299: Split DSI commands out into enable/disable

[Guido Günther]

It's when DSI commands should be sent and it also fixes these DSI errors
on every screen blank/unblank on the SHIFT6mq:

dmesg:
  msm_dsi ae94000.dsi: [drm:dsi_cmds2buf_tx] *ERROR* wait for video done timed out
  dsi_cmds2buf_tx: cmd dma tx failed, type=0x5, data0=0x28, len=4, ret=-110
  panel-visionox-rm69299 ae94000.dsi.0: sending DCS SET_DISPLAY_OFF failed: -110

Signed-off-by: Guido Günther <agx@sigxcpu.org>
---
 drivers/gpu/drm/panel/panel-visionox-rm69299.c | 77 +++++++++++++++-----------
 1 file changed, 44 insertions(+), 33 deletions(-)

diff --git a/drivers/gpu/drm/panel/panel-visionox-rm69299.c b/drivers/gpu/drm/panel/panel-visionox-rm69299.c
index f1430370ff94..3c92a6ceb8df 100644
--- a/drivers/gpu/drm/panel/panel-visionox-rm69299.c
+++ b/drivers/gpu/drm/panel/panel-visionox-rm69299.c
@@ -158,6 +158,46 @@ static inline struct visionox_rm69299 *panel_to_ctx(struct drm_panel *panel)
 	return container_of(panel, struct visionox_rm69299, panel);
 }
 
+static int visionox_rm69299_enable(struct drm_panel *panel)
+{
+	struct visionox_rm69299 *ctx = panel_to_ctx(panel);
+	struct mipi_dsi_multi_context dsi_ctx = { .dsi = ctx->dsi };
+
+	ctx->dsi->mode_flags |= MIPI_DSI_MODE_LPM;
+
+	for (int i = 0; i < ctx->desc->init_seq_len; i++)
+		mipi_dsi_dcs_write_buffer_multi(&dsi_ctx, &ctx->desc->init_seq[i * 2], 2);
+
+	mipi_dsi_dcs_exit_sleep_mode_multi(&dsi_ctx);
+
+	/* Per DSI spec wait 120ms after sending exit sleep DCS command */
+	mipi_dsi_msleep(&dsi_ctx, 120);
+
+	mipi_dsi_dcs_set_display_on_multi(&dsi_ctx);
+
+	/* Per DSI spec wait 120ms after sending set_display_on DCS command */
+	mipi_dsi_msleep(&dsi_ctx, 120);
+
+	return dsi_ctx.accum_err;
+}
+
+static int visionox_rm69299_disable(struct drm_panel *panel)
+{
+	struct visionox_rm69299 *ctx = panel_to_ctx(panel);
+	struct mipi_dsi_multi_context dsi_ctx = { .dsi = ctx->dsi };
+
+	ctx->dsi->mode_flags &= ~MIPI_DSI_MODE_LPM;
+
+	mipi_dsi_dcs_set_display_off_multi(&dsi_ctx);
+
+	/* 120ms delay required here as per DCS spec */
+	mipi_dsi_msleep(&dsi_ctx, 120);
+
+	mipi_dsi_dcs_enter_sleep_mode_multi(&dsi_ctx);
+
+	return dsi_ctx.accum_err;
+}
+
 static int visionox_rm69299_power_on(struct visionox_rm69299 *ctx)
 {
 	int ret;
@@ -193,16 +233,6 @@ static int visionox_rm69299_power_off(struct visionox_rm69299 *ctx)
 static int visionox_rm69299_unprepare(struct drm_panel *panel)
 {
 	struct visionox_rm69299 *ctx = panel_to_ctx(panel);
-	struct mipi_dsi_multi_context dsi_ctx = { .dsi = ctx->dsi };
-
-	ctx->dsi->mode_flags &= ~MIPI_DSI_MODE_LPM;
-
-	mipi_dsi_dcs_set_display_off_multi(&dsi_ctx);
-
-	/* 120ms delay required here as per DCS spec */
-	mipi_dsi_msleep(&dsi_ctx, 120);
-
-	mipi_dsi_dcs_enter_sleep_mode_multi(&dsi_ctx);
 
 	return visionox_rm69299_power_off(ctx);
 }
@@ -210,29 +240,8 @@ static int visionox_rm69299_unprepare(struct drm_panel *panel)
 static int visionox_rm69299_prepare(struct drm_panel *panel)
 {
 	struct visionox_rm69299 *ctx = panel_to_ctx(panel);
-	struct mipi_dsi_multi_context dsi_ctx = { .dsi = ctx->dsi };
-	int ret, i;
 
-	ret = visionox_rm69299_power_on(ctx);
-	if (ret < 0)
-		return ret;
-
-	ctx->dsi->mode_flags |= MIPI_DSI_MODE_LPM;
-
-	for (i = 0; i < ctx->desc->init_seq_len; i++)
-		mipi_dsi_dcs_write_buffer_multi(&dsi_ctx, &ctx->desc->init_seq[i * 2], 2);
-
-	mipi_dsi_dcs_exit_sleep_mode_multi(&dsi_ctx);
-
-	/* Per DSI spec wait 120ms after sending exit sleep DCS command */
-	mipi_dsi_msleep(&dsi_ctx, 120);
-
-	mipi_dsi_dcs_set_display_on_multi(&dsi_ctx);
-
-	/* Per DSI spec wait 120ms after sending set_display_on DCS command */
-	mipi_dsi_msleep(&dsi_ctx, 120);
-
-	return dsi_ctx.accum_err;
+	return visionox_rm69299_power_on(ctx);
 }
 
 static const struct drm_display_mode visionox_rm69299_1080x2248_60hz = {
@@ -284,7 +293,9 @@ static int visionox_rm69299_get_modes(struct drm_panel *panel,
 
 static const struct drm_panel_funcs visionox_rm69299_drm_funcs = {
 	.unprepare = visionox_rm69299_unprepare,
-	.prepare = visionox_rm69299_prepare,
+	.disable   = visionox_rm69299_disable,
+	.prepare   = visionox_rm69299_prepare,
+	.enable	   = visionox_rm69299_enable,
 	.get_modes = visionox_rm69299_get_modes,
 };
 

-- 
2.53.0

Claude review: drm/panel: visionox-rm69299: Split DSI commands out into enable/disable

[Claude Code Review Bot]

Patch Review

This is the substantive fix. The init sequence, `exit_sleep_mode`, `set_display_on` move from `prepare` to a new `enable` callback, and `set_display_off`, `enter_sleep_mode` move from `unprepare` to a new `disable` callback. The remaining `prepare`/`unprepare` become thin wrappers around `power_on`/`power_off`.

The code motion is faithful — no logic changes, same delays, same LPM flag manipulation. The fix is correct: DSI commands must be sent after the video pipeline is up (`enable`) and before it goes down (`disable`), not during power sequencing.

One minor style observation on the funcs struct:

```c
static const struct drm_panel_funcs visionox_rm69299_drm_funcs = {
	.unprepare = visionox_rm69299_unprepare,
	.disable   = visionox_rm69299_disable,
	.prepare   = visionox_rm69299_prepare,
	.enable	   = visionox_rm69299_enable,
	.get_modes = visionox_rm69299_get_modes,
};
```

The alignment with extra spaces (`.disable   =`, `.prepare   =`) is inconsistent with the rest of the file and not standard kernel style. Tabs-only alignment or no padding would be more conventional. Very minor.

No functional issues.

---
Generated by Claude Code Patch Reviewer

[PATCH 2/2] drm/panel: visionox-rm69299: Move power_on/off into prepare/unprepare

[Guido Günther]

No need to have separate functions now that prepare/unprepare only
handle powering on & off.

Signed-off-by: Guido Günther <agx@sigxcpu.org>
---
 drivers/gpu/drm/panel/panel-visionox-rm69299.c | 36 +++++++++-----------------
 1 file changed, 12 insertions(+), 24 deletions(-)

diff --git a/drivers/gpu/drm/panel/panel-visionox-rm69299.c b/drivers/gpu/drm/panel/panel-visionox-rm69299.c
index 3c92a6ceb8df..5c869d353368 100644
--- a/drivers/gpu/drm/panel/panel-visionox-rm69299.c
+++ b/drivers/gpu/drm/panel/panel-visionox-rm69299.c
@@ -198,15 +198,25 @@ static int visionox_rm69299_disable(struct drm_panel *panel)
 	return dsi_ctx.accum_err;
 }
 
-static int visionox_rm69299_power_on(struct visionox_rm69299 *ctx)
+static int visionox_rm69299_unprepare(struct drm_panel *panel)
+{
+	struct visionox_rm69299 *ctx = panel_to_ctx(panel);
+
+	gpiod_set_value(ctx->reset_gpio, 0);
+
+	return regulator_bulk_disable(ARRAY_SIZE(visionox_rm69299_supplies),
+				      ctx->supplies);
+}
+
+static int visionox_rm69299_prepare(struct drm_panel *panel)
 {
 	int ret;
+	struct visionox_rm69299 *ctx = panel_to_ctx(panel);
 
 	ret = regulator_bulk_enable(ARRAY_SIZE(visionox_rm69299_supplies),
 				    ctx->supplies);
 	if (ret < 0)
 		return ret;
-
 	/*
 	 * Reset sequence of visionox panel requires the panel to be
 	 * out of reset for 10ms, followed by being held in reset
@@ -222,28 +232,6 @@ static int visionox_rm69299_power_on(struct visionox_rm69299 *ctx)
 	return 0;
 }
 
-static int visionox_rm69299_power_off(struct visionox_rm69299 *ctx)
-{
-	gpiod_set_value(ctx->reset_gpio, 0);
-
-	return regulator_bulk_disable(ARRAY_SIZE(visionox_rm69299_supplies),
-				      ctx->supplies);
-}
-
-static int visionox_rm69299_unprepare(struct drm_panel *panel)
-{
-	struct visionox_rm69299 *ctx = panel_to_ctx(panel);
-
-	return visionox_rm69299_power_off(ctx);
-}
-
-static int visionox_rm69299_prepare(struct drm_panel *panel)
-{
-	struct visionox_rm69299 *ctx = panel_to_ctx(panel);
-
-	return visionox_rm69299_power_on(ctx);
-}
-
 static const struct drm_display_mode visionox_rm69299_1080x2248_60hz = {
 	.name = "1080x2248",
 	.clock = 158695,

-- 
2.53.0

Claude review: drm/panel: visionox-rm69299: Move power_on/off into prepare/unprepare

[Claude Code Review Bot]

Patch Review

A clean simplification. After patch 1, `prepare` just called `power_on` and `unprepare` just called `power_off`, so inlining them is the right call.

One minor reverse-christmas-tree nit in the new `prepare`:

```c
static int visionox_rm69299_prepare(struct drm_panel *panel)
{
	int ret;
	struct visionox_rm69299 *ctx = panel_to_ctx(panel);
```

Kernel convention puts longer declarations first. This should be:

```c
	struct visionox_rm69299 *ctx = panel_to_ctx(panel);
	int ret;
```

Also, the diff removes a blank line between the error check and the subsequent block comment:

```c
	if (ret < 0)
		return ret;
	/*
	 * Reset sequence of visionox panel requires the panel to be
```

The original had a blank line separating those, which aided readability. Very minor.

No functional issues.

---
Generated by Claude Code Patch Reviewer

Claude review: drm/panel/visionox-rm69299: Split DSI commands out into enable/disable

[Claude Code Review Bot]

Overall Series Review

Subject: drm/panel/visionox-rm69299: Split DSI commands out into enable/disable
Author: =?utf-8?q?Guido_G=C3=BCnther?= <agx@sigxcpu.org>
Patches: 3
Reviewed: 2026-04-28T14:33:55.368214

---

This is a clean, well-motivated 2-patch series that fixes DSI command timeouts on the SHIFT6mq by moving DSI commands from `prepare`/`unprepare` into the correct `enable`/`disable` lifecycle callbacks. The DRM panel API contract is that `prepare`/`unprepare` handle power sequencing (regulators, GPIOs) while `enable`/`disable` handle DSI command traffic. The original driver sent DSI commands in `prepare`/`unprepare`, which caused commands to be issued while the video pipeline was not yet ready (or already shut down), leading to the reported `-ETIMEDOUT` errors.

The series is logically split: patch 1 does the functional fix (moving DSI commands), and patch 2 is a follow-up cleanup (inlining the now-trivial `power_on`/`power_off` helpers). Both patches are straightforward code motion with no behavioral surprises. The final result reads well.

**Verdict: The series looks good. Two minor style nits below, neither blocking.**

---

---
Generated by Claude Code Patch Reviewer