Re: [PATCH v5 6/7] drm/bridge: prevent encoder chain changes while iterating with list_for_each_entry_reverse()

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

On 3/24/26 9:58 AM, Luca Ceresoli wrote:
> These loops in drm_bridge.c iterate over the encoder chain using
> list_for_each_entry_reverse(), which does not prevent changes to the bridge
> chain while iterating over it.
> 
> Take the encoder chain mutex while iterating to avoid chain changes while
> iterating.
> 
> All the "simple" loops are converted. drm_atomic_bridge_chain_pre_enable()
> and drm_atomic_bridge_chain_post_disable() are handled by a separate
> commit.
> 
> 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
> 
> Changes in v2: none
> ---
>   drivers/gpu/drm/drm_bridge.c | 40 ++++++++++++++++++++++------------------
>   1 file changed, 22 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
> index 37feb8143de4..c76f3451af39 100644
> --- a/drivers/gpu/drm/drm_bridge.c
> +++ b/drivers/gpu/drm/drm_bridge.c
> @@ -808,6 +808,7 @@ void drm_atomic_bridge_chain_disable(struct drm_bridge *bridge,
>   		return;
>   
>   	encoder = bridge->encoder;
> +	mutex_lock(&encoder->bridge_chain_mutex);
>   	list_for_each_entry_reverse(iter, &encoder->bridge_chain, chain_node) {
>   		if (iter->funcs->atomic_disable) {
>   			iter->funcs->atomic_disable(iter, state);
> @@ -818,6 +819,7 @@ void drm_atomic_bridge_chain_disable(struct drm_bridge *bridge,
>   		if (iter == bridge)
>   			break;
>   	}
> +	mutex_unlock(&encoder->bridge_chain_mutex);
>   }
>   EXPORT_SYMBOL(drm_atomic_bridge_chain_disable);
>   
> @@ -1322,25 +1324,27 @@ int drm_atomic_bridge_chain_check(struct drm_bridge *bridge,
>   		return ret;
>   
>   	encoder = bridge->encoder;
> -	list_for_each_entry_reverse(iter, &encoder->bridge_chain, chain_node) {
> -		int ret;
> -
> -		/*
> -		 * Bus flags are propagated by default. If a bridge needs to
> -		 * tweak the input bus flags for any reason, it should happen
> -		 * in its &drm_bridge_funcs.atomic_check() implementation such
> -		 * that preceding bridges in the chain can propagate the new
> -		 * bus flags.
> -		 */
> -		drm_atomic_bridge_propagate_bus_flags(iter, conn,
> -						      crtc_state->state);
> -
> -		ret = drm_atomic_bridge_check(iter, crtc_state, conn_state);
> -		if (ret)
> -			return ret;
> +	scoped_guard(mutex, &encoder->bridge_chain_mutex) {
> +		list_for_each_entry_reverse(iter, &encoder->bridge_chain, chain_node) {
> +			int ret;
> +
> +			/*
> +			 * Bus flags are propagated by default. If a bridge needs to
> +			 * tweak the input bus flags for any reason, it should happen
> +			 * in its &drm_bridge_funcs.atomic_check() implementation such
> +			 * that preceding bridges in the chain can propagate the new
> +			 * bus flags.
> +			 */
> +			drm_atomic_bridge_propagate_bus_flags(iter, conn,
> +							      crtc_state->state);
> +
> +			ret = drm_atomic_bridge_check(iter, crtc_state, conn_state);
> +			if (ret)
> +				return ret;
>   
> -		if (iter == bridge)
> -			break;
> +			if (iter == bridge)
> +				break;
> +		}
>   	}
>   
>   	return 0;
>