Re: [PATCH 05/11] drm/bridge: panel: add a panel_bridge to every panel

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