On Tue, Jun 09, 2026 at 12:10:39PM +0200, Luca Ceresoli wrote: > On Mon Jun 8, 2026 at 2:10 PM CEST, Maxime Ripard wrote: > > 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? > > Good point. > > What about: > > * rename to drm_bridge_detach_from() > * drop the @encoder argument (get if from bridge->encoder) > * maybe even move to drm_bridge.c?
Yeah, sounds good Maxime
signature.asc
Description: PGP signature
