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 <[email protected]> > > > > 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
signature.asc
Description: PGP signature
