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

[PATCH v4 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 <linus.walleij@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           | 54 +++++++++++++++------------
 drivers/gpu/drm/mcde/mcde_drv.c               | 37 +++++++++++++++++-
 drivers/gpu/drm/renesas/rcar-du/rcar_du_kms.c | 25 +++++++++++--
 include/drm/drm_atomic_helper.h               | 19 ++++++++++
 4 files changed, 108 insertions(+), 27 deletions(-)
---
base-commit: 6548d364a3e850326831799d7e3ea2d7bb97ba08
change-id: 20251120-mcde-drm-regression-thirdfix-1b0abfb52209

Best regards,
-- 
Linus Walleij <linus.walleij@linaro.org>

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

Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 drivers/gpu/drm/drm_atomic_helper.c | 54 +++++++++++++++++++++----------------
 include/drm/drm_atomic_helper.h     | 19 +++++++++++++
 2 files changed, 50 insertions(+), 23 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
index d5ebe6ea0acb..906eb4b0852c 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -1162,8 +1162,8 @@ 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)
+void
+drm_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 +1229,10 @@ encoder_bridge_disable(struct drm_device *dev, struct drm_atomic_state *state)
 		}
 	}
 }
+EXPORT_SYMBOL(drm_encoder_bridge_disable);
 
-static void
-crtc_disable(struct drm_device *dev, struct drm_atomic_state *state)
+void
+drm_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 +1283,10 @@ crtc_disable(struct drm_device *dev, struct drm_atomic_state *state)
 			drm_crtc_vblank_put(crtc);
 	}
 }
+EXPORT_SYMBOL(drm_crtc_disable);
 
-static void
-encoder_bridge_post_disable(struct drm_device *dev, struct drm_atomic_state *state)
+void
+drm_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 +1337,16 @@ encoder_bridge_post_disable(struct drm_device *dev, struct drm_atomic_state *sta
 		drm_bridge_put(bridge);
 	}
 }
+EXPORT_SYMBOL(drm_encoder_bridge_post_disable);
 
 static void
 disable_outputs(struct drm_device *dev, struct drm_atomic_state *state)
 {
-	encoder_bridge_disable(dev, state);
+	drm_encoder_bridge_disable(dev, state);
 
-	crtc_disable(dev, state);
+	drm_crtc_disable(dev, state);
 
-	encoder_bridge_post_disable(dev, state);
+	drm_encoder_bridge_post_disable(dev, state);
 }
 
 /**
@@ -1446,8 +1449,8 @@ 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)
+void
+drm_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 +1511,7 @@ crtc_set_mode(struct drm_device *dev, struct drm_atomic_state *state)
 		drm_bridge_put(bridge);
 	}
 }
+EXPORT_SYMBOL(drm_crtc_set_mode);
 
 /**
  * drm_atomic_helper_commit_modeset_disables - modeset commit to disable outputs
@@ -1531,12 +1535,12 @@ 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_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)
+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 +1559,10 @@ 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)
+void
+drm_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 +1593,10 @@ encoder_bridge_pre_enable(struct drm_device *dev, struct drm_atomic_state *state
 		drm_bridge_put(bridge);
 	}
 }
+EXPORT_SYMBOL(drm_encoder_bridge_pre_enable);
 
-static void
-crtc_enable(struct drm_device *dev, struct drm_atomic_state *state)
+void
+drm_crtc_enable(struct drm_device *dev, struct drm_atomic_state *state)
 {
 	struct drm_crtc *crtc;
 	struct drm_crtc_state *old_crtc_state;
@@ -1619,9 +1625,10 @@ crtc_enable(struct drm_device *dev, struct drm_atomic_state *state)
 		}
 	}
 }
+EXPORT_SYMBOL(drm_crtc_enable);
 
-static void
-encoder_bridge_enable(struct drm_device *dev, struct drm_atomic_state *state)
+void
+drm_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 +1671,7 @@ encoder_bridge_enable(struct drm_device *dev, struct drm_atomic_state *state)
 		drm_bridge_put(bridge);
 	}
 }
+EXPORT_SYMBOL(drm_encoder_bridge_enable);
 
 /**
  * drm_atomic_helper_commit_modeset_enables - modeset commit to enable outputs
@@ -1682,11 +1690,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_encoder_bridge_pre_enable(dev, state);
 
-	crtc_enable(dev, state);
+	drm_crtc_enable(dev, state);
 
-	encoder_bridge_enable(dev, state);
+	drm_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..39878aa485c3 100644
--- a/include/drm/drm_atomic_helper.h
+++ b/include/drm/drm_atomic_helper.h
@@ -60,6 +60,11 @@ 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_encoder_bridge_disable(struct drm_device *dev,
+				struct drm_atomic_state *state);
+void drm_crtc_disable(struct drm_device *dev, struct drm_atomic_state *state);
+void drm_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 +94,22 @@ 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_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_encoder_bridge_pre_enable(struct drm_device *dev,
+				   struct drm_atomic_state *state);
+
+void drm_crtc_enable(struct drm_device *dev, struct drm_atomic_state *state);
+
+void drm_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

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

[Tomi Valkeinen]

Hi,

On 21/11/2025 16:08, 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.
> 
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
>  drivers/gpu/drm/drm_atomic_helper.c | 54 +++++++++++++++++++++----------------
>  include/drm/drm_atomic_helper.h     | 19 +++++++++++++
>  2 files changed, 50 insertions(+), 23 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> index d5ebe6ea0acb..906eb4b0852c 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -1162,8 +1162,8 @@ 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)
> +void
> +drm_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 +1229,10 @@ encoder_bridge_disable(struct drm_device *dev, struct drm_atomic_state *state)
>  		}
>  	}
>  }
> +EXPORT_SYMBOL(drm_encoder_bridge_disable);

I'm not sure if it's a hard rule, but probably exported functions should
have a kernel doc.

 Tomi

>  
> -static void
> -crtc_disable(struct drm_device *dev, struct drm_atomic_state *state)
> +void
> +drm_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 +1283,10 @@ crtc_disable(struct drm_device *dev, struct drm_atomic_state *state)
>  			drm_crtc_vblank_put(crtc);
>  	}
>  }
> +EXPORT_SYMBOL(drm_crtc_disable);
>  
> -static void
> -encoder_bridge_post_disable(struct drm_device *dev, struct drm_atomic_state *state)
> +void
> +drm_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 +1337,16 @@ encoder_bridge_post_disable(struct drm_device *dev, struct drm_atomic_state *sta
>  		drm_bridge_put(bridge);
>  	}
>  }
> +EXPORT_SYMBOL(drm_encoder_bridge_post_disable);
>  
>  static void
>  disable_outputs(struct drm_device *dev, struct drm_atomic_state *state)
>  {
> -	encoder_bridge_disable(dev, state);
> +	drm_encoder_bridge_disable(dev, state);
>  
> -	crtc_disable(dev, state);
> +	drm_crtc_disable(dev, state);
>  
> -	encoder_bridge_post_disable(dev, state);
> +	drm_encoder_bridge_post_disable(dev, state);
>  }
>  
>  /**
> @@ -1446,8 +1449,8 @@ 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)
> +void
> +drm_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 +1511,7 @@ crtc_set_mode(struct drm_device *dev, struct drm_atomic_state *state)
>  		drm_bridge_put(bridge);
>  	}
>  }
> +EXPORT_SYMBOL(drm_crtc_set_mode);
>  
>  /**
>   * drm_atomic_helper_commit_modeset_disables - modeset commit to disable outputs
> @@ -1531,12 +1535,12 @@ 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_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)
> +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 +1559,10 @@ 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)
> +void
> +drm_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 +1593,10 @@ encoder_bridge_pre_enable(struct drm_device *dev, struct drm_atomic_state *state
>  		drm_bridge_put(bridge);
>  	}
>  }
> +EXPORT_SYMBOL(drm_encoder_bridge_pre_enable);
>  
> -static void
> -crtc_enable(struct drm_device *dev, struct drm_atomic_state *state)
> +void
> +drm_crtc_enable(struct drm_device *dev, struct drm_atomic_state *state)
>  {
>  	struct drm_crtc *crtc;
>  	struct drm_crtc_state *old_crtc_state;
> @@ -1619,9 +1625,10 @@ crtc_enable(struct drm_device *dev, struct drm_atomic_state *state)
>  		}
>  	}
>  }
> +EXPORT_SYMBOL(drm_crtc_enable);
>  
> -static void
> -encoder_bridge_enable(struct drm_device *dev, struct drm_atomic_state *state)
> +void
> +drm_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 +1671,7 @@ encoder_bridge_enable(struct drm_device *dev, struct drm_atomic_state *state)
>  		drm_bridge_put(bridge);
>  	}
>  }
> +EXPORT_SYMBOL(drm_encoder_bridge_enable);
>  
>  /**
>   * drm_atomic_helper_commit_modeset_enables - modeset commit to enable outputs
> @@ -1682,11 +1690,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_encoder_bridge_pre_enable(dev, state);
>  
> -	crtc_enable(dev, state);
> +	drm_crtc_enable(dev, state);
>  
> -	encoder_bridge_enable(dev, state);
> +	drm_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..39878aa485c3 100644
> --- a/include/drm/drm_atomic_helper.h
> +++ b/include/drm/drm_atomic_helper.h
> @@ -60,6 +60,11 @@ 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_encoder_bridge_disable(struct drm_device *dev,
> +				struct drm_atomic_state *state);
> +void drm_crtc_disable(struct drm_device *dev, struct drm_atomic_state *state);
> +void drm_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 +94,22 @@ 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_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_encoder_bridge_pre_enable(struct drm_device *dev,
> +				   struct drm_atomic_state *state);
> +
> +void drm_crtc_enable(struct drm_device *dev, struct drm_atomic_state *state);
> +
> +void drm_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);
>  
> 

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

[Maxime Ripard]

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

Hi Linus,

Thanks for following-up on that

On Fri, Nov 21, 2025 at 03:08:31PM +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.
> 
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
>  drivers/gpu/drm/drm_atomic_helper.c | 54 +++++++++++++++++++++----------------
>  include/drm/drm_atomic_helper.h     | 19 +++++++++++++
>  2 files changed, 50 insertions(+), 23 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> index d5ebe6ea0acb..906eb4b0852c 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -1162,8 +1162,8 @@ 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)
> +void
> +drm_encoder_bridge_disable(struct drm_device *dev, struct drm_atomic_state *state)

I think the prefix should rather be drm_atomic_helper_commit_* to remain
consistent, so drm_atomic_helper_commit_encoder_bridge_disable in this
particular case.

This applies to the other functions here too, but it looks good to me
otherwise.

Maxime

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

[PATCH v4 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")
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
 drivers/gpu/drm/mcde/mcde_drv.c | 37 ++++++++++++++++++++++++++++++++++++-
 1 file changed, 36 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/mcde/mcde_drv.c b/drivers/gpu/drm/mcde/mcde_drv.c
index 5f2c462bad7e..290082c86100 100644
--- a/drivers/gpu/drm/mcde/mcde_drv.c
+++ b/drivers/gpu/drm/mcde/mcde_drv.c
@@ -100,13 +100,48 @@ 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() */
+	drm_encoder_bridge_disable(dev, state);
+	drm_encoder_bridge_post_disable(dev, state);
+	drm_crtc_disable(dev, state);
+	drm_atomic_helper_update_legacy_modeset_state(dev, state);
+	drm_atomic_helper_calc_timestamping_constants(state);
+	drm_crtc_set_mode(dev, state);
+
+	/* Variant of drm_atomic_helper_commit_modeset_enables() */
+	drm_crtc_enable(dev, state);
+	drm_encoder_bridge_pre_enable(dev, state);
+	drm_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

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

[Maxime Ripard]

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

On Fri, Nov 21, 2025 at 03:08:32PM +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.
> 
> 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")
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
>  drivers/gpu/drm/mcde/mcde_drv.c | 37 ++++++++++++++++++++++++++++++++++++-
>  1 file changed, 36 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/mcde/mcde_drv.c b/drivers/gpu/drm/mcde/mcde_drv.c
> index 5f2c462bad7e..290082c86100 100644
> --- a/drivers/gpu/drm/mcde/mcde_drv.c
> +++ b/drivers/gpu/drm/mcde/mcde_drv.c
> @@ -100,13 +100,48 @@ 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() */
> +	drm_encoder_bridge_disable(dev, state);
> +	drm_encoder_bridge_post_disable(dev, state);
> +	drm_crtc_disable(dev, state);
> +	drm_atomic_helper_update_legacy_modeset_state(dev, state);
> +	drm_atomic_helper_calc_timestamping_constants(state);
> +	drm_crtc_set_mode(dev, state);
> +
> +	/* Variant of drm_atomic_helper_commit_modeset_enables() */
> +	drm_crtc_enable(dev, state);
> +	drm_encoder_bridge_pre_enable(dev, state);
> +	drm_encoder_bridge_enable(dev, state);
> +	drm_atomic_helper_commit_writebacks(dev, state);

I'd like to have a mention of *what* changes here too, not only that it
changes.

This also applies to the other patch.

But aside from these two comments, the prefix change on the first patch,
and the doc comment Tomi had, I think it's very reasonable and looks
good overall.

Maxime

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

[PATCH v4 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>
Signed-off-by: Linus Walleij <linus.walleij@linaro.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 | 25 ++++++++++++++++++++++---
 1 file changed, 22 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..704f5e067357 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,30 @@ 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() */
+	drm_encoder_bridge_disable(dev, old_state);
+	drm_encoder_bridge_post_disable(dev, old_state);
+	drm_crtc_disable(dev, old_state);
+	drm_atomic_helper_update_legacy_modeset_state(dev, old_state);
+	drm_atomic_helper_calc_timestamping_constants(old_state);
+	drm_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() */
+	drm_crtc_enable(dev, old_state);
+	drm_encoder_bridge_pre_enable(dev, old_state);
+	drm_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 v4 0/3] drm/atomic-helpers: Fix MCDE/R-Car DU regressions

[Linus Walleij]

On Fri, Nov 21, 2025 at 3:08 PM Linus Walleij <linus.walleij@linaro.org> 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.

So what's the take on this patch series?

Maxime: is this what you had in mind? Acked-by?

Marek: do you think you have an opportunity to test this on the R-Car DU?

I am happy to develop a fifth version using the approach outlined by
Tomi (special codepaths in the helpers) if there is some consensus
that this is better in the long run.

Yours,
Linus Walleij

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

[Marek Vasut]

On 11/25/25 5:13 PM, Linus Walleij wrote:

Hello everyone,

>> 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.
> 
> So what's the take on this patch series?
> 
> Maxime: is this what you had in mind? Acked-by?
> 
> Marek: do you think you have an opportunity to test this on the R-Car DU?

I'm sorry, I admittedly was happy that Tomi took over and I could focus 
on other topics. Now, I tested this series on R-Car V4H Retronix Sparrow 
Hawk board with RPi 7" Display 2 with ILI9881C controller, on current 
next-20251125 . For this series:

Tested-by: Marek Vasut <marek.vasut+renesas@mailbox.org>

Thank you for taking care of this and your hard work !

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

[Tomi Valkeinen]

Hi,

On 25/11/2025 18:13, Linus Walleij wrote:
> On Fri, Nov 21, 2025 at 3:08 PM Linus Walleij <linus.walleij@linaro.org> 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.
> 
> So what's the take on this patch series?
> 
> Maxime: is this what you had in mind? Acked-by?
> 
> Marek: do you think you have an opportunity to test this on the R-Car DU?
> 
> I am happy to develop a fifth version using the approach outlined by
> Tomi (special codepaths in the helpers) if there is some consensus
> that this is better in the long run.
I'm ok with this, it looks quite clean solution to a messy problem. My
only comment was about the missing kernel docs.

 Tomi

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

[Linus Walleij]

On Wed, Nov 26, 2025 at 12:08 PM Tomi Valkeinen
<tomi.valkeinen@ideasonboard.com> wrote:

> > I am happy to develop a fifth version using the approach outlined by
> > Tomi (special codepaths in the helpers) if there is some consensus
> > that this is better in the long run.
>
> I'm ok with this, it looks quite clean solution to a messy problem. My
> only comment was about the missing kernel docs.

It will become a rather big linecount set for a regression fix, but
OK!

Yours,
Linus Walleij