On Tue, Nov 18, 2025 at 05:01:28PM +0200, Laurent Pinchart wrote:
> 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.

Agreed, and I'd go even further than that: we don't want every odd order
in the core. And if some driver has to break the order we document in
some way it should be very obvious.

If you have a single user for all these new helpers variants, you
shouldn't be using the helpers at all. Go with a driver specific
implementation.

Maxime

Attachment: signature.asc
Description: PGP signature

Reply via email to