[v1] drm/bridge: add drm_of_find_bridge(), deprecate of_drm_find_bridge()

[PATCH 04/26] drm/bridge: make of_drm_find_bridge() a wrapper of drm_of_find_bridge()

Posted by Luca Ceresoli 1 week, 5 days ago

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 <luca.ceresoli@bootlin.com>
---
 drivers/gpu/drm/drm_bridge.c | 14 +++-----------
 1 file changed, 3 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
index 6debbf20aaa8..09ad825f9cb8 100644
--- a/drivers/gpu/drm/drm_bridge.c
+++ b/drivers/gpu/drm/drm_bridge.c
@@ -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);
 
-	mutex_unlock(&bridge_lock);
-	return NULL;
+	return bridge;
 }
 EXPORT_SYMBOL(of_drm_find_bridge);
 #endif

-- 
2.51.1

Re: [PATCH 04/26] drm/bridge: make of_drm_find_bridge() a wrapper of drm_of_find_bridge()

Posted by Maxime Ripard 1 week ago

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 <luca.ceresoli@bootlin.com>

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?

> ---
>  drivers/gpu/drm/drm_bridge.c | 14 +++-----------
>  1 file changed, 3 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
> index 6debbf20aaa8..09ad825f9cb8 100644
> --- a/drivers/gpu/drm/drm_bridge.c
> +++ b/drivers/gpu/drm/drm_bridge.c
> @@ -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.

Maxime

Re: [PATCH 04/26] drm/bridge: make of_drm_find_bridge() a wrapper of drm_of_find_bridge()

Posted by Luca Ceresoli 1 week ago

Hi,

+Cc Anusha

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 <luca.ceresoli@bootlin.com>
>
> 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.

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().

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?

Maxime, Anusha, are you aware of any steps forward about dynamic panel
lifetime, after devm_drm_panel_alloc()?

>> @@ -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.
 */

Luca

--
Luca Ceresoli, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH 04/26] drm/bridge: make of_drm_find_bridge() a wrapper of drm_of_find_bridge()

Posted by Maxime Ripard 9 hours ago

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 <luca.ceresoli@bootlin.com>
> >
> > 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