[PATCH v5 04/10] drm/bridge: add documentation of refcounted bridges

[Dmitry Baryshkov]

On Mon, Jan 06, 2025 at 03:49:48PM +0100, Maxime Ripard wrote:
> On Mon, Jan 06, 2025 at 02:24:00PM +0200, Dmitry Baryshkov wrote:
> > On Mon, 6 Jan 2025 at 12:39, Maxime Ripard <mripard at kernel.org> wrote:
> > >
> > > Hi,
> > >
> > > Most of these comments affect your earlier patches, but let's work on
> > > the API-level view.
> > >
> > > On Tue, Dec 31, 2024 at 11:39:58AM +0100, Luca Ceresoli wrote:

> > >         if (IS_ERR(priv))
> > >            return ERR_PTR(priv);
> > >         bridge = &priv->bridge;
> > >
> > >         ...
> > >
> > >         drm_bridge_add(bridge);
> > > }
> > >
> > > Would work just as well.
> > >
> > > I also don't think we need explicit (at least for the common case)
> > > drm_bridge_get and drm_bridge_put calls for bridge users.
> > > drm_bridge_attach and drm_bridge_detach can get/put the reference
> > > directly.
> > 
> > As I wrote previously, I think drm_bridge_attach() might be too late for that.
> > It sounds like drm_of_get_panel_or_bridge() and of_drm_find_bridge
> > should increment the refcount, possibly adding a devres action to put
> > the reference.
> 
> We probably need both. drm_bridge_attach adds the bridge pointer to the
> encoder bridge_chain list, so if we had something like
> 
> bridge = drm_of_find_bridge();
> drm_bridge_attach(encoder, bridge);
> drm_bridge_put(bridge);
> 
> We could have a dangling pointer.

Yes... So, both drm_bridge_attach and drm_of_find_bridge() should take
the refcount.

Just as an idea, it might be nice to add refcounting to bridges_show(),
so that we can easily verify that refcounting works correctly.

-- 
With best wishes
Dmitry