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

From: Boris Brezillon <boris.brezillon@collabora.com>
To: Andrzej Hajda <a.hajda@samsung.com>
Cc: Jernej Skrabec <jernej.skrabec@siol.net>,
	Jonas Karlman <jonas@kwiboo.se>,
	Neil Armstrong <narmstrong@baylibre.com>,
	Seung-Woo Kim <sw0312.kim@samsung.com>,
	dri-devel@lists.freedesktop.org,
	Kyungmin Park <kyungmin.park@samsung.com>,
	Laurent Pinchart <laurent.pinchart@ideasonboard.com>,
	Marek Szyprowski <m.szyprowski@samsung.com>
Subject: Re: [PATCH 1/3] drm/bridge: Fix drm_bridge_chain_pre_enable()
Date: Tue, 7 Jan 2020 16:33:28 +0100	[thread overview]
Message-ID: <20200107163328.2134e428@collabora.com> (raw)
In-Reply-To: <9ec04432-c727-2309-ec4a-6527d9fc3e7a@samsung.com>

On Tue, 7 Jan 2020 16:27:10 +0100
Andrzej Hajda <a.hajda@samsung.com> wrote:

> 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.

You mean passing an encoder instead of a bridge? I think that's what I
initially did and was told we might want to execute operations on a
sub-chain at some point.

> 
> Moreover after patches 2 and 3 drm_bridge_chain_* helpers have no users.

Well, the core is still using it, but there's no external users, you're
right. Do you want me to stop exporting those helpers?

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel