[PATCH 1/3] drm/bridge: Fix drm_bridge_chain_pre_enable()

[Laurent Pinchart]

Hi Boris,

Thank you for the patch.

On Fri, Dec 27, 2019 at 03:41:22PM +0100, Boris Brezillon wrote:
> Stop iterating on the bridge chain when we reach the bridge element.
> That's what other helpers do and should allow bridge implementations
> to execute a pre_enable operation on a sub-chain.

The code looks fine to me, but I think you should update the
documentation to explain this. It currently states:

 * Calls &drm_bridge_funcs.pre_enable op for all the bridges in the encoder
 * chain, starting from the last bridge to the first. These are called
 * before calling the encoder's commit op.
 *
 * Note: the bridge passed should be the one closest to the encoder

I suggest stating instead that the operation is called from the last
bridge to the bridge passed as the argument. The note should then either
be removed, or updated to state that bridge is usually the bridge
closest to the encoder, but can be any other bridge if the caller only
wants to execute the operation on a subset of the chain. It's also
probably worth it updating the other functions accordingly.

> Fixes: 05193dc38197 ("drm/bridge: Make the bridge chain a double-linked list")
> Signed-off-by: Boris Brezillon <boris.brezillon at collabora.com>
> ---
>  drivers/gpu/drm/drm_bridge.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
> index c2cf0c90fa26..b3b269ec6a39 100644
> --- a/drivers/gpu/drm/drm_bridge.c
> +++ b/drivers/gpu/drm/drm_bridge.c
> @@ -357,6 +357,9 @@ void drm_bridge_chain_pre_enable(struct drm_bridge *bridge)
>  	list_for_each_entry_reverse(iter, &encoder->bridge_chain, chain_node) {
>  		if (iter->funcs->pre_enable)
>  			iter->funcs->pre_enable(iter);
> +
> +		if (iter == bridge)
> +			break;
>  	}
>  }
>  EXPORT_SYMBOL(drm_bridge_chain_pre_enable);

-- 
Regards,

Laurent Pinchart