On Fri, Aug 14, 2026 at 04:05:53PM +0200, Luca Ceresoli wrote: > Adding a panel does currently not add a panel_bridge wrapping it. Usually > the panel_bridge creation happens later, when some other driver (e.g. the > previous bridge or the encoder) calls *_of_get_bridge() and the following > element in the pipeline is a panel. > > This has some drawbacks: > > * hte bridge API is currently the best practice to access various > components of the pipeline, especially with complex cards where bridges > can be combined in different ways on different hardware > * the panel_bridge is not created in the context of the driver of the > underlying physical device (the panel driver), but of some other driver > * that other driver is not aware of whether the returned drm_bridge > pointer is a panel_bridge created on the fly, a pre-existing > panel_bridge or a non-panel bridge > * removal of a panel_bridge requires calling drm_panel_bridge_remove(), > but the other driver doesn't know whether this is needed because it > doesn't know whether it has created a panel_bridge or not > > So far the current approach has been working because devm and drmm ensure > the panel bridge would be dealloacted at some later point. However with the > upcoming implementation of dynamic bridge lifetime this will get more > complicated. > > Switch to the new approach: always create a panel_bridge with a drm_panel, > thus matching the lifetime of the drm_panel and the panel_bridge wrapping > it. This makes lifetime much more straightforward to understand and to > further develop on. > > As a consequence devm_drm_of_get_bridge() and drmm_of_get_bridge() don't > need to look for a panel anymore and become simple wrappers to > of_drm_get_bridge_by_endpoint(). Also deprecate them as they can be > replaced by of_drm_get_bridge_by_endpoint() which also handles refcount. > > Signed-off-by: Luca Ceresoli <[email protected]> > --- > drivers/gpu/drm/bridge/panel.c | 47 > +++++++++++++++++++++--------------------- > drivers/gpu/drm/drm_panel.c | 3 +++ > include/drm/drm_panel.h | 8 +++++++ > 3 files changed, 34 insertions(+), 24 deletions(-) > > diff --git a/drivers/gpu/drm/bridge/panel.c b/drivers/gpu/drm/bridge/panel.c > index 02388a3de626..d86555254aa9 100644 > --- a/drivers/gpu/drm/bridge/panel.c > +++ b/drivers/gpu/drm/bridge/panel.c > @@ -412,6 +412,11 @@ struct drm_bridge > *devm_drm_panel_bridge_add_typed(struct device *dev, > { > struct drm_bridge **ptr, *bridge; > > + if (panel->bridge) { > + dev_dbg(dev, "returning existing bridge=%p\n", panel->bridge); > + return panel->bridge; > + } > + > ptr = devres_alloc(devm_drm_panel_bridge_release, sizeof(*ptr), > GFP_KERNEL); > if (!ptr) > @@ -495,9 +500,12 @@ EXPORT_SYMBOL(drm_panel_bridge_connector); > * @port: port in the device tree node > * @endpoint: endpoint in the device tree node > * > + * This function is deprecated and should not be used in new drivers. The > + * returned bridge refcount is not incremented! Replace by > + * of_drm_get_bridge_by_endpoint() and handle bridge refcount. > + * > * Given a DT node's port and endpoint number, finds the connected node > - * and returns the associated bridge if any, or creates and returns a > - * drm panel bridge instance if a panel is connected. > + * and returns the associated bridge if any. > * > * Returns a pointer to the bridge if successful, or an error pointer > * otherwise. > @@ -507,18 +515,12 @@ struct drm_bridge *devm_drm_of_get_bridge(struct device > *dev, > u32 port, u32 endpoint) > { > struct drm_bridge *bridge; > - struct drm_panel *panel; > - int ret; > > - ret = drm_of_find_panel_or_bridge(np, port, endpoint, > - &panel, &bridge); > - if (ret) > - return ERR_PTR(ret); > + bridge = of_drm_get_bridge_by_endpoint(np, port, endpoint); > + if (IS_ERR(bridge)) > + return bridge; > > - if (panel) { > - bridge = devm_drm_panel_bridge_add(dev, panel); > - drm_panel_put(panel); > - } > + drm_bridge_put(bridge); > > return bridge; > } > @@ -531,9 +533,12 @@ EXPORT_SYMBOL(devm_drm_of_get_bridge); > * @port: port in the device tree node > * @endpoint: endpoint in the device tree node > * > + * This function is deprecated and should not be used in new drivers. The > + * returned bridge refcount is not incremented! Replace by > + * of_drm_get_bridge_by_endpoint() and handle bridge refcount. > + * > * Given a DT node's port and endpoint number, finds the connected node > - * and returns the associated bridge if any, or creates and returns a > - * drm panel bridge instance if a panel is connected. > + * and returns the associated bridge if any. > * > * Returns a drmm managed pointer to the bridge if successful, or an error > * pointer otherwise. > @@ -543,18 +548,12 @@ struct drm_bridge *drmm_of_get_bridge(struct drm_device > *drm, > u32 port, u32 endpoint) > { > struct drm_bridge *bridge; > - struct drm_panel *panel; > - int ret; > > - ret = drm_of_find_panel_or_bridge(np, port, endpoint, > - &panel, &bridge); > - if (ret) > - return ERR_PTR(ret); > + bridge = of_drm_get_bridge_by_endpoint(np, port, endpoint); > + if (IS_ERR(bridge)) > + return bridge; > > - if (panel) { > - bridge = drmm_panel_bridge_add(drm, panel); > - drm_panel_put(panel); > - } > + drm_bridge_put(bridge); > > return bridge; > } > diff --git a/drivers/gpu/drm/drm_panel.c b/drivers/gpu/drm/drm_panel.c > index f8f6082e637f..95fc95d1b4b7 100644 > --- a/drivers/gpu/drm/drm_panel.c > +++ b/drivers/gpu/drm/drm_panel.c > @@ -453,6 +453,9 @@ void *__devm_drm_panel_alloc(struct device *dev, size_t > size, size_t offset, > > drm_panel_init(panel, dev, funcs, connector_type); > > + panel->bridge = devm_drm_panel_bridge_add(dev, panel); > + WARN_ON(!panel->bridge); > +
Having two refcounted dynamic allocations yet with the exact same lifetime makes me a bit uneasy: we'll introduce lifetime / reference count issue because we would then have to keep the same reference count everywhere. I wonder if we shouldn't embed the drm_bridge struct into drm_panel, and provide a way for the drm_panel and drm_bridge to share the same refcount, maybe by making drm_panel_get take drm_bridge.refcount instead of duplicating the refcount in both drm_panel and drm_bridge? Maxime
signature.asc
Description: PGP signature
