On Tue, May 19, 2026 at 12:37:40PM +0200, Luca Ceresoli wrote: > Supporting hardware whose final part of the DRM pipeline can be physically > removed requires the ability to detach all bridges from a given point to > the end of the pipeline. > > Introduce a variant of drm_encoder_cleanup() for this. > > Take care to not try detaching non-attached bridges. This is needed because > when two or more bridges are removed not in the backwards order, > drm_encoder_cleanup_from() is called more than once for bridges closer to > the panel. > > Signed-off-by: Luca Ceresoli <[email protected]> > > --- > > Note: in theory drm_encoder_cleanup() is now a superset of > drm_encoder_cleanup_from() and may be simplified to just call > drm_encoder_cleanup_from() and then do the extra actions. However the > common code is subtly different in terms of locking and checks, so this > would complicate the code in this patch and has thus been kept separate for > the time being to make reviewing sompler. Reimplementing > drm_encoder_cleanup() by using drm_encoder_cleanup_from() cvacn be done > later on. > > A much simpler and now obsolete version of this patch (missing locking and > checks) previously appeared in > https://lore.kernel.org/lkml/[email protected]/ > --- > drivers/gpu/drm/drm_encoder.c | 38 ++++++++++++++++++++++++++++++++++++++ > include/drm/drm_encoder.h | 1 + > 2 files changed, 39 insertions(+) > > diff --git a/drivers/gpu/drm/drm_encoder.c b/drivers/gpu/drm/drm_encoder.c > index 0d5dbed06db4..40ece477b302 100644 > --- a/drivers/gpu/drm/drm_encoder.c > +++ b/drivers/gpu/drm/drm_encoder.c > @@ -179,6 +179,44 @@ int drm_encoder_init(struct drm_device *dev, > } > EXPORT_SYMBOL(drm_encoder_init); > > +/** > + * drm_encoder_cleanup_from - remove a given bridge and all the following > + * @encoder: encoder whole list of bridges shall be pruned > + * @bridge: first bridge to remove > + * > + * Removes from an encoder all the bridges starting with a given bridge > + * and until the end of the chain. > + * > + * Does nothing if the bridge is not attached to an encoder chain. > + * > + * This should not be used in "normal" DRM pipelines. It is only useful for > + * devices whose final part of the DRM chain can be physically removed and > + * later reconnected (possibly with different hardware). > + */ > +void drm_encoder_cleanup_from(struct drm_encoder *encoder, struct drm_bridge > *bridge) > +{ > + struct drm_bridge *next; > + LIST_HEAD(tmplist); > + > + /* > + * We need the bridge_chain_mutex to modify the chain, but > + * drm_bridge_detach() will call DRM_MODESET_LOCK_ALL_BEGIN() (in > + * drm_modeset_lock_fini()), resulting in a possible ABBA circular > + * deadlock. Avoid it by first moving all the bridges to a > + * temporary list holding the lock, and then calling > + * drm_bridge_detach() without the lock. > + */ > + mutex_lock(&encoder->bridge_chain_mutex); > + if (!list_empty(&bridge->chain_node)) > + list_for_each_entry_safe_from(bridge, next, > &encoder->bridge_chain, chain_node) > + list_move_tail(&bridge->chain_node, &tmplist); > + mutex_unlock(&encoder->bridge_chain_mutex); > + > + while (!list_empty(&tmplist)) > + drm_bridge_detach(list_first_entry(&tmplist, struct drm_bridge, > chain_node)); > +} > +EXPORT_SYMBOL(drm_encoder_cleanup_from); > +
The name is super confusing, because it doesn't clean up anything. drm_encoder_cleanup is called that way because it cleans up the encoder. This function doesn't. Unlike what's being documented, it doesn't remove any bridge either. It just detaches bridge, so let's just call it that way? Maxime
signature.asc
Description: PGP signature
