Hi Linus, Thank you for the patch.
On Tue, Nov 18, 2025 at 03:36:05PM +0100, Linus Walleij wrote: > This adds (yet another) variant of the > drm_atomic_helper_commit_tail_crtc_early_late() helper function > to deal with regressions caused by reordering the bridge > prepare/enablement sequence. > > 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. > > 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(). > > This patch from Marek Vasut: > https://lore.kernel.org/all/20251107230517.471894-1-marek.vasut%2Brenesas%40mailbox.org/ > solves part of the problem for drivers using custom > tail functions, since MCDE is using helpers only, we > add a new helper function that exploits the new > drm_atomic_helper_commit_modeset_enables_crtc_early() > and use that in MCDE. > > 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/drm_atomic_helper.c | 35 +++++++++++++++++++++++++++++++++++ > drivers/gpu/drm/mcde/mcde_drv.c | 6 ++++-- > include/drm/drm_atomic_helper.h | 1 + > 3 files changed, 40 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/drm_atomic_helper.c > b/drivers/gpu/drm/drm_atomic_helper.c > index eb47883be153..23fa27f21c4e 100644 > --- a/drivers/gpu/drm/drm_atomic_helper.c > +++ b/drivers/gpu/drm/drm_atomic_helper.c > @@ -2005,6 +2005,41 @@ void drm_atomic_helper_commit_tail_rpm(struct > drm_atomic_state *state) > } > EXPORT_SYMBOL(drm_atomic_helper_commit_tail_rpm); > > +/** > + * drm_atomic_helper_commit_tail_crtc_early_late - commit atomic update Based on the function name, it feels that the nem commit tail and modeset enable/disable helpers reached a point where we may want to reconsider the design instead of adding new functions with small differences in behaviour that will end up confusing driver developers. > + * @state: new modeset state to be committed > + * > + * This is an alternative implementation for the > + * &drm_mode_config_helper_funcs.atomic_commit_tail hook, for drivers > + * that support runtime_pm or need the CRTC to be enabled to perform a > + * commit, and also need the CRTC to be enabled before preparing any > + * bridges, as well as leaving the CRTC enabled while unpreparing > + * any bridges. > + * > + * Otherwise, one should use the default implementation > + * drm_atomic_helper_commit_tail(). > + */ > +void drm_atomic_helper_commit_tail_crtc_early_late(struct drm_atomic_state > *state) > +{ > + struct drm_device *dev = state->dev; > + > + drm_atomic_helper_commit_modeset_disables_crtc_late(dev, state); > + > + drm_atomic_helper_commit_modeset_enables_crtc_early(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); > +} > +EXPORT_SYMBOL(drm_atomic_helper_commit_tail_crtc_early_late); > + > static void commit_tail(struct drm_atomic_state *state) > { > struct drm_device *dev = state->dev; > diff --git a/drivers/gpu/drm/mcde/mcde_drv.c b/drivers/gpu/drm/mcde/mcde_drv.c > index 5f2c462bad7e..f3833d20c0fa 100644 > --- a/drivers/gpu/drm/mcde/mcde_drv.c > +++ b/drivers/gpu/drm/mcde/mcde_drv.c > @@ -104,9 +104,11 @@ 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. > + * the case with e.g. DSI displays, and also make sure that the > + * CRTC is enabled before any bridges are prepared and disabled > + * after any bridges are disabled. > */ > - .atomic_commit_tail = drm_atomic_helper_commit_tail_rpm, > + .atomic_commit_tail = drm_atomic_helper_commit_tail_crtc_early_late, > }; > > static irqreturn_t mcde_irq(int irq, void *data) > diff --git a/include/drm/drm_atomic_helper.h b/include/drm/drm_atomic_helper.h > index d479afcd1637..1e85df5eea4f 100644 > --- a/include/drm/drm_atomic_helper.h > +++ b/include/drm/drm_atomic_helper.h > @@ -64,6 +64,7 @@ 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); > void drm_atomic_helper_commit_tail_rpm(struct drm_atomic_state *state); > +void drm_atomic_helper_commit_tail_crtc_early_late(struct drm_atomic_state > *state); > int drm_atomic_helper_commit(struct drm_device *dev, > struct drm_atomic_state *state, > bool nonblock); -- Regards, Laurent Pinchart
