[PATCH v5 0/3] drm/atomic-helpers: Fix MCDE/R-Car DU regressions

[PATCH v5 0/3] drm/atomic-helpers: Fix MCDE/R-Car DU regressions

[Linus Walleij]

This fixes two regressions experienced in the MCDE and
R-Car DU DRM drivers after
commit c9b1150a68d9362a0827609fc0dc1664c0d8bfe1
"drm/atomic-helper: Re-order bridge chain pre-enable and post-disable"
caused a series of regressions in all panels that send
DSI commands in their .prepare() and .unprepare()
callbacks.

This series make it possible to selectively bring back the
old behaviour with explicit semantics and implements
the old behaviour as modified commit tails in MCDE and
R-Car DU.

Signed-off-by: Linus Walleij <linusw@kernel.org>
---
Changes in v5:
- Prefix all exported atomic commit helpers with drm_atomic_helper_commit_*
- Add kerneldoc to all new exported atmomic commit helpers.
- Add comments into the MCDE and Rcar DU quirks explaining what is
  altered as compared to the standard helper functions.
- Link to v4: https://lore.kernel.org/r/20251121-mcde-drm-regression-thirdfix-v4-0-d89bf8c17f85@linaro.org

Changes in v4:
- Fix a copypaste-bug in the Renesas Rcar-DU driver.
- Actually compile this using the shmobile defconfig and make
  sure it works.
- Collect Geert's Tested-by.
- Link to v3: https://lore.kernel.org/r/20251120-mcde-drm-regression-thirdfix-v3-0-24b1e9886bbf@linaro.org

Changes in v3:
- Switch to a new approach: export helper functions and create the
  special helper directly in the driver instead.
- Drop Marek's patch and write a new RFT patch with the same
  semantic content as the MCDE patch.
- Link to v2: https://lore.kernel.org/r/20251118-mcde-drm-regression-v2-0-4fedf10b18f6@linaro.org

Changes in v2:
- Queue Marek's patch first in the series for coherency.
- Add a patch to also preserve the late CRTC disablement
  semantic.
- Rename helper function to reflect the new semantic.
- Update the MCDE patch to use the new callbacks.
- Link to v1: https://lore.kernel.org/r/20251118-mcde-drm-regression-v1-1-ed5583efbd68@linaro.org

---
Linus Walleij (3):
      drm/atomic-helper: Export and namespace some functions
      drm/mcde: Create custom commit tail
      drm/rcar-du: Modify custom commit tail

 drivers/gpu/drm/drm_atomic_helper.c           | 122 +++++++++++++++++++++-----
 drivers/gpu/drm/mcde/mcde_drv.c               |  45 +++++++++-
 drivers/gpu/drm/renesas/rcar-du/rcar_du_kms.c |  33 ++++++-
 include/drm/drm_atomic_helper.h               |  22 +++++
 4 files changed, 195 insertions(+), 27 deletions(-)
---
base-commit: 6548d364a3e850326831799d7e3ea2d7bb97ba08
change-id: 20251120-mcde-drm-regression-thirdfix-1b0abfb52209

Best regards,
-- 
Linus Walleij <linusw@kernel.org>

[PATCH v5 1/3] drm/atomic-helper: Export and namespace some functions

[Linus Walleij]

Export and namespace those not prefixed with drm_* so
it becomes possible to write custom commit tail functions
in individual drivers using the helper infrastructure.

Tested-by: Marek Vasut <marek.vasut+renesas@mailbox.org>
Signed-off-by: Linus Walleij <linusw@kernel.org>
---
 drivers/gpu/drm/drm_atomic_helper.c | 122 +++++++++++++++++++++++++++++-------
 include/drm/drm_atomic_helper.h     |  22 +++++++
 2 files changed, 121 insertions(+), 23 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
index d5ebe6ea0acb..bfbe3a0ee178 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -1162,8 +1162,18 @@ crtc_needs_disable(struct drm_crtc_state *old_state,
 	       new_state->self_refresh_active;
 }
 
-static void
-encoder_bridge_disable(struct drm_device *dev, struct drm_atomic_state *state)
+/**
+ * drm_atomic_helper_commit_encoder_bridge_disable - disable bridges and encoder
+ * @dev: DRM device
+ * @state: the driver state object
+ *
+ * Loops over all connectors in the current state and if the CRTC needs
+ * it, disables the bridge chain all the way, then disables the encoder
+ * afterwards.
+ */
+void
+drm_atomic_helper_commit_encoder_bridge_disable(struct drm_device *dev,
+						struct drm_atomic_state *state)
 {
 	struct drm_connector *connector;
 	struct drm_connector_state *old_conn_state, *new_conn_state;
@@ -1229,9 +1239,18 @@ encoder_bridge_disable(struct drm_device *dev, struct drm_atomic_state *state)
 		}
 	}
 }
+EXPORT_SYMBOL(drm_atomic_helper_commit_encoder_bridge_disable);
 
-static void
-crtc_disable(struct drm_device *dev, struct drm_atomic_state *state)
+/**
+ * drm_atomic_helper_commit_crtc_disable - disable CRTSs
+ * @dev: DRM device
+ * @state: the driver state object
+ *
+ * Loops over all CRTCs in the current state and if the CRTC needs
+ * it, disables it.
+ */
+void
+drm_atomic_helper_commit_crtc_disable(struct drm_device *dev, struct drm_atomic_state *state)
 {
 	struct drm_crtc *crtc;
 	struct drm_crtc_state *old_crtc_state, *new_crtc_state;
@@ -1282,9 +1301,18 @@ crtc_disable(struct drm_device *dev, struct drm_atomic_state *state)
 			drm_crtc_vblank_put(crtc);
 	}
 }
+EXPORT_SYMBOL(drm_atomic_helper_commit_crtc_disable);
 
-static void
-encoder_bridge_post_disable(struct drm_device *dev, struct drm_atomic_state *state)
+/**
+ * drm_atomic_helper_commit_encoder_bridge_post_disable - post-disable encoder bridges
+ * @dev: DRM device
+ * @state: the driver state object
+ *
+ * Loops over all connectors in the current state and if the CRTC needs
+ * it, post-disables all encoder bridges.
+ */
+void
+drm_atomic_helper_commit_encoder_bridge_post_disable(struct drm_device *dev, struct drm_atomic_state *state)
 {
 	struct drm_connector *connector;
 	struct drm_connector_state *old_conn_state, *new_conn_state;
@@ -1335,15 +1363,16 @@ encoder_bridge_post_disable(struct drm_device *dev, struct drm_atomic_state *sta
 		drm_bridge_put(bridge);
 	}
 }
+EXPORT_SYMBOL(drm_atomic_helper_commit_encoder_bridge_post_disable);
 
 static void
 disable_outputs(struct drm_device *dev, struct drm_atomic_state *state)
 {
-	encoder_bridge_disable(dev, state);
+	drm_atomic_helper_commit_encoder_bridge_disable(dev, state);
 
-	crtc_disable(dev, state);
+	drm_atomic_helper_commit_crtc_disable(dev, state);
 
-	encoder_bridge_post_disable(dev, state);
+	drm_atomic_helper_commit_encoder_bridge_post_disable(dev, state);
 }
 
 /**
@@ -1446,8 +1475,17 @@ void drm_atomic_helper_calc_timestamping_constants(struct drm_atomic_state *stat
 }
 EXPORT_SYMBOL(drm_atomic_helper_calc_timestamping_constants);
 
-static void
-crtc_set_mode(struct drm_device *dev, struct drm_atomic_state *state)
+/**
+ * drm_atomic_helper_commit_crtc_set_mode - set the new mode
+ * @dev: DRM device
+ * @state: the driver state object
+ *
+ * Loops over all connectors in the current state and if the mode has
+ * changed, change the mode of the CRTC, then call down the bridge
+ * chain and change the mode in all bridges as well.
+ */
+void
+drm_atomic_helper_commit_crtc_set_mode(struct drm_device *dev, struct drm_atomic_state *state)
 {
 	struct drm_crtc *crtc;
 	struct drm_crtc_state *new_crtc_state;
@@ -1508,6 +1546,7 @@ crtc_set_mode(struct drm_device *dev, struct drm_atomic_state *state)
 		drm_bridge_put(bridge);
 	}
 }
+EXPORT_SYMBOL(drm_atomic_helper_commit_crtc_set_mode);
 
 /**
  * drm_atomic_helper_commit_modeset_disables - modeset commit to disable outputs
@@ -1531,12 +1570,21 @@ void drm_atomic_helper_commit_modeset_disables(struct drm_device *dev,
 	drm_atomic_helper_update_legacy_modeset_state(dev, state);
 	drm_atomic_helper_calc_timestamping_constants(state);
 
-	crtc_set_mode(dev, state);
+	drm_atomic_helper_commit_crtc_set_mode(dev, state);
 }
 EXPORT_SYMBOL(drm_atomic_helper_commit_modeset_disables);
 
-static void drm_atomic_helper_commit_writebacks(struct drm_device *dev,
-						struct drm_atomic_state *state)
+/**
+ * drm_atomic_helper_commit_writebacks - issue writebacks
+ * @dev: DRM device
+ * @state: atomic state object being committed
+ *
+ * This loops over the connectors, checks if the new state requires
+ * a writeback job to be issued and in that case issues an atomic
+ * commit on each connector.
+ */
+void drm_atomic_helper_commit_writebacks(struct drm_device *dev,
+					 struct drm_atomic_state *state)
 {
 	struct drm_connector *connector;
 	struct drm_connector_state *new_conn_state;
@@ -1555,9 +1603,18 @@ static void drm_atomic_helper_commit_writebacks(struct drm_device *dev,
 		}
 	}
 }
+EXPORT_SYMBOL(drm_atomic_helper_commit_writebacks);
 
-static void
-encoder_bridge_pre_enable(struct drm_device *dev, struct drm_atomic_state *state)
+/**
+ * drm_atomic_helper_commit_encoder_bridge_pre_enable - pre-enable bridges
+ * @dev: DRM device
+ * @state: atomic state object being committed
+ *
+ * This loops over the connectors and if the CRTC needs it, pre-enables
+ * the entire bridge chain.
+ */
+void
+drm_atomic_helper_commit_encoder_bridge_pre_enable(struct drm_device *dev, struct drm_atomic_state *state)
 {
 	struct drm_connector *connector;
 	struct drm_connector_state *new_conn_state;
@@ -1588,9 +1645,18 @@ encoder_bridge_pre_enable(struct drm_device *dev, struct drm_atomic_state *state
 		drm_bridge_put(bridge);
 	}
 }
+EXPORT_SYMBOL(drm_atomic_helper_commit_encoder_bridge_pre_enable);
 
-static void
-crtc_enable(struct drm_device *dev, struct drm_atomic_state *state)
+/**
+ * drm_atomic_helper_commit_crtc_enable - enables the CRTCs
+ * @dev: DRM device
+ * @state: atomic state object being committed
+ *
+ * This loops over CRTCs in the new state, and of the CRTC needs
+ * it, enables it.
+ */
+void
+drm_atomic_helper_commit_crtc_enable(struct drm_device *dev, struct drm_atomic_state *state)
 {
 	struct drm_crtc *crtc;
 	struct drm_crtc_state *old_crtc_state;
@@ -1619,9 +1685,18 @@ crtc_enable(struct drm_device *dev, struct drm_atomic_state *state)
 		}
 	}
 }
+EXPORT_SYMBOL(drm_atomic_helper_commit_crtc_enable);
 
-static void
-encoder_bridge_enable(struct drm_device *dev, struct drm_atomic_state *state)
+/**
+ * drm_atomic_helper_commit_encoder_bridge_enable - enables the bridges
+ * @dev: DRM device
+ * @state: atomic state object being committed
+ *
+ * This loops over all connectors in the new state, and of the CRTC needs
+ * it, enables the entire bridge chain.
+ */
+void
+drm_atomic_helper_commit_encoder_bridge_enable(struct drm_device *dev, struct drm_atomic_state *state)
 {
 	struct drm_connector *connector;
 	struct drm_connector_state *new_conn_state;
@@ -1664,6 +1739,7 @@ encoder_bridge_enable(struct drm_device *dev, struct drm_atomic_state *state)
 		drm_bridge_put(bridge);
 	}
 }
+EXPORT_SYMBOL(drm_atomic_helper_commit_encoder_bridge_enable);
 
 /**
  * drm_atomic_helper_commit_modeset_enables - modeset commit to enable outputs
@@ -1682,11 +1758,11 @@ encoder_bridge_enable(struct drm_device *dev, struct drm_atomic_state *state)
 void drm_atomic_helper_commit_modeset_enables(struct drm_device *dev,
 					      struct drm_atomic_state *state)
 {
-	encoder_bridge_pre_enable(dev, state);
+	drm_atomic_helper_commit_encoder_bridge_pre_enable(dev, state);
 
-	crtc_enable(dev, state);
+	drm_atomic_helper_commit_crtc_enable(dev, state);
 
-	encoder_bridge_enable(dev, state);
+	drm_atomic_helper_commit_encoder_bridge_enable(dev, state);
 
 	drm_atomic_helper_commit_writebacks(dev, state);
 }
diff --git a/include/drm/drm_atomic_helper.h b/include/drm/drm_atomic_helper.h
index 53382fe93537..9afc2e1e24c7 100644
--- a/include/drm/drm_atomic_helper.h
+++ b/include/drm/drm_atomic_helper.h
@@ -60,6 +60,12 @@ int drm_atomic_helper_check_plane_state(struct drm_plane_state *plane_state,
 int drm_atomic_helper_check_planes(struct drm_device *dev,
 			       struct drm_atomic_state *state);
 int drm_atomic_helper_check_crtc_primary_plane(struct drm_crtc_state *crtc_state);
+void drm_atomic_helper_commit_encoder_bridge_disable(struct drm_device *dev,
+				struct drm_atomic_state *state);
+void drm_atomic_helper_commit_crtc_disable(struct drm_device *dev,
+				struct drm_atomic_state *state);
+void drm_atomic_helper_commit_encoder_bridge_post_disable(struct drm_device *dev,
+				struct drm_atomic_state *state);
 int drm_atomic_helper_check(struct drm_device *dev,
 			    struct drm_atomic_state *state);
 void drm_atomic_helper_commit_tail(struct drm_atomic_state *state);
@@ -89,8 +95,24 @@ drm_atomic_helper_update_legacy_modeset_state(struct drm_device *dev,
 void
 drm_atomic_helper_calc_timestamping_constants(struct drm_atomic_state *state);
 
+void drm_atomic_helper_commit_crtc_set_mode(struct drm_device *dev,
+					    struct drm_atomic_state *state);
+
 void drm_atomic_helper_commit_modeset_disables(struct drm_device *dev,
 					       struct drm_atomic_state *state);
+
+void drm_atomic_helper_commit_writebacks(struct drm_device *dev,
+					 struct drm_atomic_state *state);
+
+void drm_atomic_helper_commit_encoder_bridge_pre_enable(struct drm_device *dev,
+					struct drm_atomic_state *state);
+
+void drm_atomic_helper_commit_crtc_enable(struct drm_device *dev,
+					  struct drm_atomic_state *state);
+
+void drm_atomic_helper_commit_encoder_bridge_enable(struct drm_device *dev,
+						    struct drm_atomic_state *state);
+
 void drm_atomic_helper_commit_modeset_enables(struct drm_device *dev,
 					  struct drm_atomic_state *old_state);
 

-- 
2.51.1

[PATCH v5 2/3] drm/mcde: Create custom commit tail

[Linus Walleij]

commit c9b1150a68d9362a0827609fc0dc1664c0d8bfe1
"drm/atomic-helper: Re-order bridge chain pre-enable and post-disable"
caused a series of regressions in all panels that send
DSI commands in their .prepare() and .unprepare()
callbacks when used with MCDE.

As the CRTC is no longer online at bridge_pre_enable()
and gone at brige_post_disable() which maps to the panel
bridge .prepare()/.unprepare() callbacks, any CRTC that
enable/disable the DSI transmitter in it's enable/disable
callbacks will be unable to send any DSI commands in the
.prepare() and .unprepare() callbacks.

However the MCDE driver definitely need the CRTC to be
enabled during .prepare()/.unprepare().

Solve this by implementing a custom commit tail function
in the MCDE driver that always enables the CRTC first
and disables it last, using the newly exported helpers.

Link: https://lore.kernel.org/dri-devel/20251026-fix-mcde-drm-regression-v2-0-8d799e488cf9@linaro.org/
Link: https://lore.kernel.org/all/20251107230517.471894-1-marek.vasut%2Brenesas%40mailbox.org/
Fixes: c9b1150a68d9 ("drm/atomic-helper: Re-order bridge chain pre-enable and post-disable")
Tested-by: Marek Vasut <marek.vasut+renesas@mailbox.org>
Signed-off-by: Linus Walleij <linusw@kernel.org>
---
 drivers/gpu/drm/mcde/mcde_drv.c | 45 ++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 44 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/mcde/mcde_drv.c b/drivers/gpu/drm/mcde/mcde_drv.c
index 5f2c462bad7e..eb3a8bb8766b 100644
--- a/drivers/gpu/drm/mcde/mcde_drv.c
+++ b/drivers/gpu/drm/mcde/mcde_drv.c
@@ -100,13 +100,56 @@ static const struct drm_mode_config_funcs mcde_mode_config_funcs = {
 	.atomic_commit = drm_atomic_helper_commit,
 };
 
+/*
+ * This commit tail explicitly copies and changes the behaviour of
+ * the related core DRM atomic helper instead of trying to make
+ * the core helpers overly generic.
+ */
+static void mcde_atomic_commit_tail(struct drm_atomic_state *state)
+{
+	struct drm_device *dev = state->dev;
+
+	/*
+	 * Variant of drm_atomic_helper_commit_modeset_disables()
+	 * that will disable and post-disable all bridges BEFORE
+	 * disabling the CRTC.
+	 */
+	drm_atomic_helper_commit_encoder_bridge_disable(dev, state);
+	drm_atomic_helper_commit_encoder_bridge_post_disable(dev, state);
+	drm_atomic_helper_commit_crtc_disable(dev, state);
+	drm_atomic_helper_update_legacy_modeset_state(dev, state);
+	drm_atomic_helper_calc_timestamping_constants(state);
+	drm_atomic_helper_commit_crtc_set_mode(dev, state);
+
+	/*
+	 * Variant of drm_atomic_helper_commit_modeset_enables()
+	 * that will enable the CRTC BEFORE pre-enabling and
+	 * enabling the bridges.
+	 */
+	drm_atomic_helper_commit_crtc_enable(dev, state);
+	drm_atomic_helper_commit_encoder_bridge_pre_enable(dev, state);
+	drm_atomic_helper_commit_encoder_bridge_enable(dev, state);
+	drm_atomic_helper_commit_writebacks(dev, state);
+
+	drm_atomic_helper_commit_planes(dev, state,
+					DRM_PLANE_COMMIT_ACTIVE_ONLY);
+
+	drm_atomic_helper_fake_vblank(state);
+
+	drm_atomic_helper_commit_hw_done(state);
+
+	drm_atomic_helper_wait_for_vblanks(dev, state);
+
+	drm_atomic_helper_cleanup_planes(dev, state);
+}
+
 static const struct drm_mode_config_helper_funcs mcde_mode_config_helpers = {
 	/*
 	 * Using this function is necessary to commit atomic updates
 	 * that need the CRTC to be enabled before a commit, as is
 	 * the case with e.g. DSI displays.
 	 */
-	.atomic_commit_tail = drm_atomic_helper_commit_tail_rpm,
+	.atomic_commit_tail = mcde_atomic_commit_tail,
 };
 
 static irqreturn_t mcde_irq(int irq, void *data)

-- 
2.51.1

[PATCH v5 3/3] drm/rcar-du: Modify custom commit tail

[Linus Walleij]

commit c9b1150a68d9362a0827609fc0dc1664c0d8bfe1
"drm/atomic-helper: Re-order bridge chain pre-enable and post-disable"
caused regressions in all bridges that e.g. send DSI commands in
their .prepare() and .unprepare() callbacks when used with R-Car DU.

This is needed on R-Car DU, where the CRTC provides clock to LVDS
and DSI, and has to be started before a bridge may call .prepare,
which may trigger e.g. a DSI transfer.

This specifically fixes the case where ILI9881C is connected to R-Car
DU DSI. The ILI9881C panel driver does DSI command transfer in its
struct drm_panel_funcs .prepare function, which is currently called
before R-Car DU rcar_du_crtc_atomic_enable() rcar_mipi_dsi_pclk_enable()
and the DSI command transfer times out.

Fixes: c9b1150a68d9 ("drm/atomic-helper: Re-order bridge chain pre-enable and post-disable")
Link: https://lore.kernel.org/all/20251107230517.471894-1-marek.vasut%2Brenesas%40mailbox.org/
Tested-by: Geert Uytterhoeven <geert@linux-m68k.org>
Co-developed-by: Marek Vasut <marek.vasut+renesas@mailbox.org>
Signed-off-by: Marek Vasut <marek.vasut+renesas@mailbox.org>
Tested-by: Marek Vasut <marek.vasut+renesas@mailbox.org>
Signed-off-by: Linus Walleij <linusw@kernel.org>
---
This is a modified version of Marek's patch using the approach
from MCDE. I'm pretty sure this driver also needs the original
semantic ording during disablement, and it surely doesn't hurt
to restore it too.
---
 drivers/gpu/drm/renesas/rcar-du/rcar_du_kms.c | 33 ++++++++++++++++++++++++---
 1 file changed, 30 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/renesas/rcar-du/rcar_du_kms.c b/drivers/gpu/drm/renesas/rcar-du/rcar_du_kms.c
index 216219accfd9..299d14ec486f 100644
--- a/drivers/gpu/drm/renesas/rcar-du/rcar_du_kms.c
+++ b/drivers/gpu/drm/renesas/rcar-du/rcar_du_kms.c
@@ -540,11 +540,38 @@ static void rcar_du_atomic_commit_tail(struct drm_atomic_state *old_state)
 			rcdu->dpad1_source = rcrtc->index;
 	}
 
-	/* Apply the atomic update. */
-	drm_atomic_helper_commit_modeset_disables(dev, old_state);
+	/*
+	 * Apply the atomic update.
+	 *
+	 * We need special ordering to make sure the CRTC disabled last
+	 * and enabled first. We do this with modified versions of the
+	 * common modeset_disables/enables functions.
+	 */
+
+	/*
+	 * Variant of drm_atomic_helper_commit_modeset_disables()
+	 * that will disable and post-disable all bridges BEFORE
+	 * disabling the CRTC.
+	 */
+	drm_atomic_helper_commit_encoder_bridge_disable(dev, old_state);
+	drm_atomic_helper_commit_encoder_bridge_post_disable(dev, old_state);
+	drm_atomic_helper_commit_crtc_disable(dev, old_state);
+	drm_atomic_helper_update_legacy_modeset_state(dev, old_state);
+	drm_atomic_helper_calc_timestamping_constants(old_state);
+	drm_atomic_helper_commit_crtc_set_mode(dev, old_state);
+
 	drm_atomic_helper_commit_planes(dev, old_state,
 					DRM_PLANE_COMMIT_ACTIVE_ONLY);
-	drm_atomic_helper_commit_modeset_enables(dev, old_state);
+
+	/*
+	 * Variant of drm_atomic_helper_commit_modeset_enables()
+	 * that will enable the CRTC BEFORE pre-enabling and
+	 * enabling the bridges.
+	 */
+	drm_atomic_helper_commit_crtc_enable(dev, old_state);
+	drm_atomic_helper_commit_encoder_bridge_pre_enable(dev, old_state);
+	drm_atomic_helper_commit_encoder_bridge_enable(dev, old_state);
+	drm_atomic_helper_commit_writebacks(dev, old_state);
 
 	drm_atomic_helper_commit_hw_done(old_state);
 	drm_atomic_helper_wait_for_flip_done(dev, old_state);

-- 
2.51.1

Re: [PATCH v5 1/3] drm/atomic-helper: Export and namespace some functions

[Maxime Ripard]

On Sun, 30 Nov 2025 13:11:17 +0100, Linus Walleij wrote:
> Export and namespace those not prefixed with drm_* so
> it becomes possible to write custom commit tail functions
> in individual drivers using the helper infrastructure.
> 
> Tested-by: Marek Vasut <marek.vasut+renesas@mailbox.org>
> 
> [ ... ]

Reviewed-by: Maxime Ripard <mripard@kernel.org>

Thanks!
Maxime

Re: [PATCH v5 2/3] drm/mcde: Create custom commit tail

[Maxime Ripard]

On Sun, 30 Nov 2025 13:11:18 +0100, Linus Walleij wrote:
> commit c9b1150a68d9362a0827609fc0dc1664c0d8bfe1
> "drm/atomic-helper: Re-order bridge chain pre-enable and post-disable"
> caused a series of regressions in all panels that send
> DSI commands in their .prepare() and .unprepare()
> callbacks when used with MCDE.
> 
> [ ... ]

Reviewed-by: Maxime Ripard <mripard@kernel.org>

Thanks!
Maxime

Re: [PATCH v5 3/3] drm/rcar-du: Modify custom commit tail

[Maxime Ripard]

On Sun, 30 Nov 2025 13:11:19 +0100, Linus Walleij wrote:
> commit c9b1150a68d9362a0827609fc0dc1664c0d8bfe1
> "drm/atomic-helper: Re-order bridge chain pre-enable and post-disable"
> caused regressions in all bridges that e.g. send DSI commands in
> their .prepare() and .unprepare() callbacks when used with R-Car DU.
> 
> 
> [ ... ]

Reviewed-by: Maxime Ripard <mripard@kernel.org>

Thanks!
Maxime

Re: [PATCH v5 0/3] drm/atomic-helpers: Fix MCDE/R-Car DU regressions

[Laurent Pinchart]

Hi Linus,

On Sun, Nov 30, 2025 at 01:11:16PM +0100, Linus Walleij wrote:
> This fixes two regressions experienced in the MCDE and
> R-Car DU DRM drivers after
> commit c9b1150a68d9362a0827609fc0dc1664c0d8bfe1
> "drm/atomic-helper: Re-order bridge chain pre-enable and post-disable"
> caused a series of regressions in all panels that send
> DSI commands in their .prepare() and .unprepare()
> callbacks.
> 
> This series make it possible to selectively bring back the
> old behaviour with explicit semantics and implements
> the old behaviour as modified commit tails in MCDE and
> R-Car DU.

We now have a third platform broken by
c9b1150a68d9362a0827609fc0dc1664c0d8bfe1, see [1]. I think this calls
for a revert, to give us the time to implement a correct solution.

[1] https://lore.kernel.org/r/CAAMcf8Di8sc_XVZAnzQ9sUiUf-Ayvg2yjhx2dWmvvCnfF3pBRA@mail.gmail.com

> Signed-off-by: Linus Walleij <linusw@kernel.org>
> ---
> Changes in v5:
> - Prefix all exported atomic commit helpers with drm_atomic_helper_commit_*
> - Add kerneldoc to all new exported atmomic commit helpers.
> - Add comments into the MCDE and Rcar DU quirks explaining what is
>   altered as compared to the standard helper functions.
> - Link to v4: https://lore.kernel.org/r/20251121-mcde-drm-regression-thirdfix-v4-0-d89bf8c17f85@linaro.org
> 
> Changes in v4:
> - Fix a copypaste-bug in the Renesas Rcar-DU driver.
> - Actually compile this using the shmobile defconfig and make
>   sure it works.
> - Collect Geert's Tested-by.
> - Link to v3: https://lore.kernel.org/r/20251120-mcde-drm-regression-thirdfix-v3-0-24b1e9886bbf@linaro.org
> 
> Changes in v3:
> - Switch to a new approach: export helper functions and create the
>   special helper directly in the driver instead.
> - Drop Marek's patch and write a new RFT patch with the same
>   semantic content as the MCDE patch.
> - Link to v2: https://lore.kernel.org/r/20251118-mcde-drm-regression-v2-0-4fedf10b18f6@linaro.org
> 
> Changes in v2:
> - Queue Marek's patch first in the series for coherency.
> - Add a patch to also preserve the late CRTC disablement
>   semantic.
> - Rename helper function to reflect the new semantic.
> - Update the MCDE patch to use the new callbacks.
> - Link to v1: https://lore.kernel.org/r/20251118-mcde-drm-regression-v1-1-ed5583efbd68@linaro.org
> 
> ---
> Linus Walleij (3):
>       drm/atomic-helper: Export and namespace some functions
>       drm/mcde: Create custom commit tail
>       drm/rcar-du: Modify custom commit tail
> 
>  drivers/gpu/drm/drm_atomic_helper.c           | 122 +++++++++++++++++++++-----
>  drivers/gpu/drm/mcde/mcde_drv.c               |  45 +++++++++-
>  drivers/gpu/drm/renesas/rcar-du/rcar_du_kms.c |  33 ++++++-
>  include/drm/drm_atomic_helper.h               |  22 +++++
>  4 files changed, 195 insertions(+), 27 deletions(-)
> ---
> base-commit: 6548d364a3e850326831799d7e3ea2d7bb97ba08
> change-id: 20251120-mcde-drm-regression-thirdfix-1b0abfb52209

-- 
Regards,

Laurent Pinchart

Re: [PATCH v5 0/3] drm/atomic-helpers: Fix MCDE/R-Car DU regressions

[Tomi Valkeinen]

Hi,

On 02/12/2025 08:34, Laurent Pinchart wrote:
> Hi Linus,
> 
> On Sun, Nov 30, 2025 at 01:11:16PM +0100, Linus Walleij wrote:
>> This fixes two regressions experienced in the MCDE and
>> R-Car DU DRM drivers after
>> commit c9b1150a68d9362a0827609fc0dc1664c0d8bfe1
>> "drm/atomic-helper: Re-order bridge chain pre-enable and post-disable"
>> caused a series of regressions in all panels that send
>> DSI commands in their .prepare() and .unprepare()
>> callbacks.
>>
>> This series make it possible to selectively bring back the
>> old behaviour with explicit semantics and implements
>> the old behaviour as modified commit tails in MCDE and
>> R-Car DU.
> 
> We now have a third platform broken by
> c9b1150a68d9362a0827609fc0dc1664c0d8bfe1, see [1]. I think this calls
> for a revert, to give us the time to implement a correct solution.
Perhaps... It's been very slow or not possible to get feedback regarding
(some) of the broken platforms, so I don't think we have a safe way of
changing the enable/disable sequence. I think the "correct" solution
then is to make this new enable/disable sequence either opt-in, offered
by the framework, or just implement it as a custom sequence in the
specific drm driver.

Reverting c9b1150a68d9362a0827609fc0dc1664c0d8bfe1 will break DSI and
OLDI outputs on TI platforms, so we need to implement a fix for those
platforms before the revert, and there has been one or two fixes merged
for other platforms for this, which most likely also need to get reverted.

 Tomi

> [1] https://lore.kernel.org/r/CAAMcf8Di8sc_XVZAnzQ9sUiUf-Ayvg2yjhx2dWmvvCnfF3pBRA@mail.gmail.com
> 
>> Signed-off-by: Linus Walleij <linusw@kernel.org>
>> ---
>> Changes in v5:
>> - Prefix all exported atomic commit helpers with drm_atomic_helper_commit_*
>> - Add kerneldoc to all new exported atmomic commit helpers.
>> - Add comments into the MCDE and Rcar DU quirks explaining what is
>>   altered as compared to the standard helper functions.
>> - Link to v4: https://lore.kernel.org/r/20251121-mcde-drm-regression-thirdfix-v4-0-d89bf8c17f85@linaro.org
>>
>> Changes in v4:
>> - Fix a copypaste-bug in the Renesas Rcar-DU driver.
>> - Actually compile this using the shmobile defconfig and make
>>   sure it works.
>> - Collect Geert's Tested-by.
>> - Link to v3: https://lore.kernel.org/r/20251120-mcde-drm-regression-thirdfix-v3-0-24b1e9886bbf@linaro.org
>>
>> Changes in v3:
>> - Switch to a new approach: export helper functions and create the
>>   special helper directly in the driver instead.
>> - Drop Marek's patch and write a new RFT patch with the same
>>   semantic content as the MCDE patch.
>> - Link to v2: https://lore.kernel.org/r/20251118-mcde-drm-regression-v2-0-4fedf10b18f6@linaro.org
>>
>> Changes in v2:
>> - Queue Marek's patch first in the series for coherency.
>> - Add a patch to also preserve the late CRTC disablement
>>   semantic.
>> - Rename helper function to reflect the new semantic.
>> - Update the MCDE patch to use the new callbacks.
>> - Link to v1: https://lore.kernel.org/r/20251118-mcde-drm-regression-v1-1-ed5583efbd68@linaro.org
>>
>> ---
>> Linus Walleij (3):
>>       drm/atomic-helper: Export and namespace some functions
>>       drm/mcde: Create custom commit tail
>>       drm/rcar-du: Modify custom commit tail
>>
>>  drivers/gpu/drm/drm_atomic_helper.c           | 122 +++++++++++++++++++++-----
>>  drivers/gpu/drm/mcde/mcde_drv.c               |  45 +++++++++-
>>  drivers/gpu/drm/renesas/rcar-du/rcar_du_kms.c |  33 ++++++-
>>  include/drm/drm_atomic_helper.h               |  22 +++++
>>  4 files changed, 195 insertions(+), 27 deletions(-)
>> ---
>> base-commit: 6548d364a3e850326831799d7e3ea2d7bb97ba08
>> change-id: 20251120-mcde-drm-regression-thirdfix-1b0abfb52209
> 

Re: [PATCH v5 0/3] drm/atomic-helpers: Fix MCDE/R-Car DU regressions

[Linus Walleij]

On Tue, Dec 2, 2025 at 7:34 AM Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:

> We now have a third platform broken by
> c9b1150a68d9362a0827609fc0dc1664c0d8bfe1, see [1]. I think this calls
> for a revert, to give us the time to implement a correct solution.
>
> [1] https://lore.kernel.org/r/CAAMcf8Di8sc_XVZAnzQ9sUiUf-Ayvg2yjhx2dWmvvCnfF3pBRA@mail.gmail.com

This is a mess.

But OK let's see if we can deal with the mess.

I will make an RFT patch for Rockchip as well and resend at the
tail of the patch series if we want to try to fix also that one.

Yours,
Linus Walleij

Re: [PATCH v5 0/3] drm/atomic-helpers: Fix MCDE/R-Car DU regressions

[Laurent Pinchart]

On Tue, Dec 02, 2025 at 09:35:26PM +0100, Linus Walleij wrote:
> On Tue, Dec 2, 2025 at 7:34 AM Laurent Pinchart wrote:
> > We now have a third platform broken by
> > c9b1150a68d9362a0827609fc0dc1664c0d8bfe1, see [1]. I think this calls
> > for a revert, to give us the time to implement a correct solution.
> >
> > [1] https://lore.kernel.org/r/CAAMcf8Di8sc_XVZAnzQ9sUiUf-Ayvg2yjhx2dWmvvCnfF3pBRA@mail.gmail.com
> 
> This is a mess.
> 
> But OK let's see if we can deal with the mess.
> 
> I will make an RFT patch for Rockchip as well and resend at the
> tail of the patch series if we want to try to fix also that one.

What bothers me here is that we're rushing to fix three different
platforms, with others possibly broken as well, in a way that nobody
really likes. That's why I think a revert would be better, to give us
the time to then design a proper solution.

-- 
Regards,

Laurent Pinchart

Re: [PATCH v5 0/3] drm/atomic-helpers: Fix MCDE/R-Car DU regressions

[Laurent Pinchart]

On Tue, Dec 02, 2025 at 10:58:50AM +0200, Tomi Valkeinen wrote:
> On 02/12/2025 08:34, Laurent Pinchart wrote:
> > On Sun, Nov 30, 2025 at 01:11:16PM +0100, Linus Walleij wrote:
> >> This fixes two regressions experienced in the MCDE and
> >> R-Car DU DRM drivers after
> >> commit c9b1150a68d9362a0827609fc0dc1664c0d8bfe1
> >> "drm/atomic-helper: Re-order bridge chain pre-enable and post-disable"
> >> caused a series of regressions in all panels that send
> >> DSI commands in their .prepare() and .unprepare()
> >> callbacks.
> >>
> >> This series make it possible to selectively bring back the
> >> old behaviour with explicit semantics and implements
> >> the old behaviour as modified commit tails in MCDE and
> >> R-Car DU.
> > 
> > We now have a third platform broken by
> > c9b1150a68d9362a0827609fc0dc1664c0d8bfe1, see [1]. I think this calls
> > for a revert, to give us the time to implement a correct solution.
>
> Perhaps... It's been very slow or not possible to get feedback regarding
> (some) of the broken platforms, so I don't think we have a safe way of
> changing the enable/disable sequence. I think the "correct" solution
> then is to make this new enable/disable sequence either opt-in, offered
> by the framework, or just implement it as a custom sequence in the
> specific drm driver.

I don't think that's right, sorry. We need to improve the bridge API to
handle ordering properly. Changes to the commit tail handlers in display
controller drivers are hacks, they handle issues with the internal DSI
transmitters, but if you had a LVDS-to-DSI bridge in the pipeline things
would still break.

> Reverting c9b1150a68d9362a0827609fc0dc1664c0d8bfe1 will break DSI and
> OLDI outputs on TI platforms, so we need to implement a fix for those
> platforms before the revert, and there has been one or two fixes merged
> for other platforms for this, which most likely also need to get reverted.

That's 3 vs. 1, so I think breaking DSI and OLDI with a revert is better
than not reverting the commit. If we can merge a propert solution at the
same time that's great, but the first target is to restore operation of
the drivers that got broken.

> > [1] https://lore.kernel.org/r/CAAMcf8Di8sc_XVZAnzQ9sUiUf-Ayvg2yjhx2dWmvvCnfF3pBRA@mail.gmail.com
> > 
> >> Signed-off-by: Linus Walleij <linusw@kernel.org>
> >> ---
> >> Changes in v5:
> >> - Prefix all exported atomic commit helpers with drm_atomic_helper_commit_*
> >> - Add kerneldoc to all new exported atmomic commit helpers.
> >> - Add comments into the MCDE and Rcar DU quirks explaining what is
> >>   altered as compared to the standard helper functions.
> >> - Link to v4: https://lore.kernel.org/r/20251121-mcde-drm-regression-thirdfix-v4-0-d89bf8c17f85@linaro.org
> >>
> >> Changes in v4:
> >> - Fix a copypaste-bug in the Renesas Rcar-DU driver.
> >> - Actually compile this using the shmobile defconfig and make
> >>   sure it works.
> >> - Collect Geert's Tested-by.
> >> - Link to v3: https://lore.kernel.org/r/20251120-mcde-drm-regression-thirdfix-v3-0-24b1e9886bbf@linaro.org
> >>
> >> Changes in v3:
> >> - Switch to a new approach: export helper functions and create the
> >>   special helper directly in the driver instead.
> >> - Drop Marek's patch and write a new RFT patch with the same
> >>   semantic content as the MCDE patch.
> >> - Link to v2: https://lore.kernel.org/r/20251118-mcde-drm-regression-v2-0-4fedf10b18f6@linaro.org
> >>
> >> Changes in v2:
> >> - Queue Marek's patch first in the series for coherency.
> >> - Add a patch to also preserve the late CRTC disablement
> >>   semantic.
> >> - Rename helper function to reflect the new semantic.
> >> - Update the MCDE patch to use the new callbacks.
> >> - Link to v1: https://lore.kernel.org/r/20251118-mcde-drm-regression-v1-1-ed5583efbd68@linaro.org
> >>
> >> ---
> >> Linus Walleij (3):
> >>       drm/atomic-helper: Export and namespace some functions
> >>       drm/mcde: Create custom commit tail
> >>       drm/rcar-du: Modify custom commit tail
> >>
> >>  drivers/gpu/drm/drm_atomic_helper.c           | 122 +++++++++++++++++++++-----
> >>  drivers/gpu/drm/mcde/mcde_drv.c               |  45 +++++++++-
> >>  drivers/gpu/drm/renesas/rcar-du/rcar_du_kms.c |  33 ++++++-
> >>  include/drm/drm_atomic_helper.h               |  22 +++++
> >>  4 files changed, 195 insertions(+), 27 deletions(-)
> >> ---
> >> base-commit: 6548d364a3e850326831799d7e3ea2d7bb97ba08
> >> change-id: 20251120-mcde-drm-regression-thirdfix-1b0abfb52209

-- 
Regards,

Laurent Pinchart

Re: [PATCH v5 0/3] drm/atomic-helpers: Fix MCDE/R-Car DU regressions

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 2583 bytes --]

On Wed, Dec 03, 2025 at 11:22:29AM +0900, Laurent Pinchart wrote:
> On Tue, Dec 02, 2025 at 10:58:50AM +0200, Tomi Valkeinen wrote:
> > On 02/12/2025 08:34, Laurent Pinchart wrote:
> > > On Sun, Nov 30, 2025 at 01:11:16PM +0100, Linus Walleij wrote:
> > >> This fixes two regressions experienced in the MCDE and
> > >> R-Car DU DRM drivers after
> > >> commit c9b1150a68d9362a0827609fc0dc1664c0d8bfe1
> > >> "drm/atomic-helper: Re-order bridge chain pre-enable and post-disable"
> > >> caused a series of regressions in all panels that send
> > >> DSI commands in their .prepare() and .unprepare()
> > >> callbacks.
> > >>
> > >> This series make it possible to selectively bring back the
> > >> old behaviour with explicit semantics and implements
> > >> the old behaviour as modified commit tails in MCDE and
> > >> R-Car DU.
> > > 
> > > We now have a third platform broken by
> > > c9b1150a68d9362a0827609fc0dc1664c0d8bfe1, see [1]. I think this calls
> > > for a revert, to give us the time to implement a correct solution.
> >
> > Perhaps... It's been very slow or not possible to get feedback regarding
> > (some) of the broken platforms, so I don't think we have a safe way of
> > changing the enable/disable sequence. I think the "correct" solution
> > then is to make this new enable/disable sequence either opt-in, offered
> > by the framework, or just implement it as a custom sequence in the
> > specific drm driver.
> 
> I don't think that's right, sorry. We need to improve the bridge API to
> handle ordering properly. Changes to the commit tail handlers in display
> controller drivers are hacks, they handle issues with the internal DSI
> transmitters, but if you had a LVDS-to-DSI bridge in the pipeline things
> would still break.
> 
> > Reverting c9b1150a68d9362a0827609fc0dc1664c0d8bfe1 will break DSI and
> > OLDI outputs on TI platforms, so we need to implement a fix for those
> > platforms before the revert, and there has been one or two fixes merged
> > for other platforms for this, which most likely also need to get reverted.
> 
> That's 3 vs. 1, so I think breaking DSI and OLDI with a revert is better
> than not reverting the commit. If we can merge a propert solution at the
> same time that's great, but the first target is to restore operation of
> the drivers that got broken.

Yeah, I agree. Could it be possible to flip the custom commit_tail
implementation and instead implement it into tidss while the core
changes are reverted to avoid the regressions and keeping tidss
functional?

Maxime

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 273 bytes --]

Re: [PATCH v5 0/3] drm/atomic-helpers: Fix MCDE/R-Car DU regressions

[Tomi Valkeinen]

Hi,

On 05/12/2025 10:32, Maxime Ripard wrote:
> On Wed, Dec 03, 2025 at 11:22:29AM +0900, Laurent Pinchart wrote:
>> On Tue, Dec 02, 2025 at 10:58:50AM +0200, Tomi Valkeinen wrote:
>>> On 02/12/2025 08:34, Laurent Pinchart wrote:
>>>> On Sun, Nov 30, 2025 at 01:11:16PM +0100, Linus Walleij wrote:
>>>>> This fixes two regressions experienced in the MCDE and
>>>>> R-Car DU DRM drivers after
>>>>> commit c9b1150a68d9362a0827609fc0dc1664c0d8bfe1
>>>>> "drm/atomic-helper: Re-order bridge chain pre-enable and post-disable"
>>>>> caused a series of regressions in all panels that send
>>>>> DSI commands in their .prepare() and .unprepare()
>>>>> callbacks.
>>>>>
>>>>> This series make it possible to selectively bring back the
>>>>> old behaviour with explicit semantics and implements
>>>>> the old behaviour as modified commit tails in MCDE and
>>>>> R-Car DU.
>>>>
>>>> We now have a third platform broken by
>>>> c9b1150a68d9362a0827609fc0dc1664c0d8bfe1, see [1]. I think this calls
>>>> for a revert, to give us the time to implement a correct solution.
>>>
>>> Perhaps... It's been very slow or not possible to get feedback regarding
>>> (some) of the broken platforms, so I don't think we have a safe way of
>>> changing the enable/disable sequence. I think the "correct" solution
>>> then is to make this new enable/disable sequence either opt-in, offered
>>> by the framework, or just implement it as a custom sequence in the
>>> specific drm driver.
>>
>> I don't think that's right, sorry. We need to improve the bridge API to
>> handle ordering properly. Changes to the commit tail handlers in display
>> controller drivers are hacks, they handle issues with the internal DSI
>> transmitters, but if you had a LVDS-to-DSI bridge in the pipeline things
>> would still break.
>>
>>> Reverting c9b1150a68d9362a0827609fc0dc1664c0d8bfe1 will break DSI and
>>> OLDI outputs on TI platforms, so we need to implement a fix for those
>>> platforms before the revert, and there has been one or two fixes merged
>>> for other platforms for this, which most likely also need to get reverted.
>>
>> That's 3 vs. 1, so I think breaking DSI and OLDI with a revert is better
>> than not reverting the commit. If we can merge a propert solution at the
>> same time that's great, but the first target is to restore operation of
>> the drivers that got broken.
> 
> Yeah, I agree. Could it be possible to flip the custom commit_tail
> implementation and instead implement it into tidss while the core
> changes are reverted to avoid the regressions and keeping tidss
> functional?
Yep, I have a WIP series. I'll send it today.

 Tomi