Hi Maxime,
On Mon Aug 17, 2026 at 10:42 AM CEST, Maxime Ripard wrote:
> 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.
It may look twisty but I don't see why it should not work.
> 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?
That is the other option I evaluated, and I must agree it makes
sense. Basically the resulting bridge refcount would be:
bridge.refcount == number of drm_bridge_get() calls
+ number of drm_panel_get() calls
- number of drm_bridge_put() calls
- number of drm_panel_put() calls
I think that's an implementation detail, even though an important one. The
API for both panel and bridge users would be the same, and with the same
guarantee: memory won't be freed before (get - put) reaches zero, but with
the second idea it could be freed even later.
I'll try implementing this, and see what happens when going into the
details.
Luca
--
Luca Ceresoli, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
