On 06.01.2020 11:29, Laurent Pinchart wrote:
> 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.
Apparently drm_(atomic_)bridge_chain_* helpers are always called on the
1st bridge so you can try to remove bridge argument, if it is true.
Moreover after patches 2 and 3 drm_bridge_chain_* helpers have no users.
Regards
Andrzej
>
>> Fixes: 05193dc38197 ("drm/bridge: Make the bridge chain a double-linked list")
>> Signed-off-by: Boris Brezillon <boris.brezillon@xxxxxxxxxxxxx>
>> ---
>> 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);
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
