Re: [PATCH 04/26] drm/bridge: make of_drm_find_bridge() a wrapper of drm_of_find_bridge()

[Maxime Ripard <mripard@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 2800 bytes --]

On Mon, Nov 24, 2025 at 05:44:09PM +0100, Luca Ceresoli wrote:
> On Mon Nov 24, 2025 at 11:22 AM CET, Maxime Ripard wrote:
> > Hi,
> >
> > On Wed, Nov 19, 2025 at 02:05:35PM +0100, Luca Ceresoli wrote:
> >> of_drm_find_bridge() is identical to drm_of_find_bridge() except it does
> >> not increment the refcount. Rewrite it as a wrapper and put the bridge
> >> being returned so the behaviour is still the same.
> >>
> >> Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
> >
> > Kind of the same comment than on the TODO. Is it worth doing that patch
> > when we could just remove it at the end of the series?
> 
> This series is not converting all users I'm afraid.
> 
> There are still some drivers to convert, but not a big deal.

Oh, ok, my bad then :)

> The main user to be converted is drm_of_find_panel_or_bridge(), which is
> very tricky, and in turn it is used by devm_drm_of_get_bridge(). We
> discussed this in the past and the conclusion was a rework of the drm_panel
> lifetime was needed to be able to properly replace
> drm_of_find_panel_or_bridge().

Yeah. I wonder, now that we have a proper allocation scheme for panels
too, if we shouldn't just create a panel_bridge for every panel we
allocate.

> A drm_panel rework had started very well with devm_drm_panel_alloc() that
> got upstreamed by Anusha, but I'm not sure if it has made further progress
> after that. So AFAICT the plan is still "People will gradually switch to
> the new API over time", and the deprecated of_drm_find_bridge() will be
> removed after that.
> 
> Does it still make sense to you?

Yep.

> Maxime, Anusha, are you aware of any steps forward about dynamic panel
> lifetime, after devm_drm_panel_alloc()?

AFAIK, Anusha stopped working on it. I'm fairly busy at the moment, but
early next year I'll try to revive that effort.

> >> @@ -1460,19 +1460,11 @@ EXPORT_SYMBOL(drm_of_find_bridge);
> >>   */
> >>  struct drm_bridge *of_drm_find_bridge(struct device_node *np)
> >>  {
> >> -	struct drm_bridge *bridge;
> >> -
> >> -	mutex_lock(&bridge_lock);
> >> +	struct drm_bridge *bridge = drm_of_find_bridge(np);
> >>
> >> -	list_for_each_entry(bridge, &bridge_list, list) {
> >> -		if (bridge->of_node == np) {
> >> -			mutex_unlock(&bridge_lock);
> >> -			return bridge;
> >> -		}
> >> -	}
> >> +	drm_bridge_put(bridge);
> >
> > And if it does make sense to keep that patch, we should add a comment
> > here to document why we are doing this.
> 
> OK, what about:
> 
> /**
>  * We need to emulate the original semantice of of_drm_find_bridge(), which
>  * was not getting any bridge reference. Being now based on
>  * drm_of_find_bridge() which gets a reference, put it before returning.
>  */

Yep, sounds good

Maxime

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 273 bytes --]