Re: [PATCH v5 7/7] drm/bridge: prevent encoder chain changes in pre_enable/post_disable

[Louis Chauvet <louis.chauvet@bootlin.com>]

On 3/24/26 9:58 AM, Luca Ceresoli wrote:
> Take the encoder chain mutex while iterating over the encoder chain in
> drm_atomic_bridge_chain_pre_enable() and
> drm_atomic_bridge_chain_post_disable() to ensure the lists won't change
> while being inspected.
> 
> These functions have nested list_for_each_*() loops, which makes them
> complicated. list_for_each_entry_from() loops could be replaced by
> drm_for_each_bridge_in_chain_from(), but it would not work in a nested way
> in its current implementation. Besides, there is no "_reverse" variant of
> drm_for_each_bridge_in_chain_from().
> 
> Keep code simple and readable by explicitly locking around the outer
> loop. Thankfully there are no break or return points inside the loops, so
> the change is trivial and readable.
> 
> Reviewed-by: Maxime Ripard <mripard@kernel.org>
> Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>

Reviewed-by: Louis Chauvet <louis.chauvet@bootlin.com>

> ---
> 
> Changes in v3:
> - Lock encoder->bridge_chain_mutex directly, no wrappers
> - Improved commit message
> 
> Changes in v2:
> - Improved commit message
> ---
>   drivers/gpu/drm/drm_bridge.c | 4 ++++
>   1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
> index c76f3451af39..29231730e1f8 100644
> --- a/drivers/gpu/drm/drm_bridge.c
> +++ b/drivers/gpu/drm/drm_bridge.c
> @@ -867,6 +867,7 @@ void drm_atomic_bridge_chain_post_disable(struct drm_bridge *bridge,
>   
>   	encoder = bridge->encoder;
>   
> +	mutex_lock(&encoder->bridge_chain_mutex);
>   	list_for_each_entry_from(bridge, &encoder->bridge_chain, chain_node) {
>   		limit = NULL;
>   
> @@ -915,6 +916,7 @@ void drm_atomic_bridge_chain_post_disable(struct drm_bridge *bridge,
>   			/* Jump all bridges that we have already post_disabled */
>   			bridge = limit;
>   	}
> +	mutex_unlock(&encoder->bridge_chain_mutex);
>   }
>   EXPORT_SYMBOL(drm_atomic_bridge_chain_post_disable);
>   
> @@ -961,6 +963,7 @@ void drm_atomic_bridge_chain_pre_enable(struct drm_bridge *bridge,
>   
>   	encoder = bridge->encoder;
>   
> +	mutex_lock(&encoder->bridge_chain_mutex);
>   	list_for_each_entry_reverse(iter, &encoder->bridge_chain, chain_node) {
>   		if (iter->pre_enable_prev_first) {
>   			next = iter;
> @@ -1003,6 +1006,7 @@ void drm_atomic_bridge_chain_pre_enable(struct drm_bridge *bridge,
>   		if (iter == bridge)
>   			break;
>   	}
> +	mutex_unlock(&encoder->bridge_chain_mutex);
>   }
>   EXPORT_SYMBOL(drm_atomic_bridge_chain_pre_enable);
>   
>