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/[email protected]/ > 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 <[email protected]> > --- > 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
signature.asc
Description: PGP signature
