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? Luca -- Luca Ceresoli, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
