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

Maxime Ripard
2025-12-01T16:34:23Z

[-- 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 --]