1. Patchew
  2. linux
  3. [PATCH v3 0/4] drm/bridge: convert users of of_drm_find_bridge(), part 4
  4. View patch

[PATCH v3 3/4] drm: rcar-du: encoder: convert to of_drm_find_and_get_bridge()

Luca Ceresoli posted 4 patches 2 weeks, 1 day ago
[PATCH v3 3/4] drm: rcar-du: encoder: convert to of_drm_find_and_get_bridge()
Posted by Luca Ceresoli 2 weeks, 1 day ago 
of_drm_find_bridge() is deprecated. Move to its replacement
of_drm_find_and_get_bridge() which gets a bridge reference, and ensure it
is put when done.

We need to handle the two cases: when a panel_bridge is added and when it
isn't. So:

 * in the 'else' case a panel_bridge is not added and bridge is found: use
   of_drm_find_and_get_bridge() to get a reference to the found bridge
 * in the 'then' case a panel_bridge is found using a devm function which
   already takes a refcount and will put it on removal, but we need to take
   another so the following code in this function always get exactly one
   reference that it needs to put

In order to put the reference, add the needed drm_bridge_put() calls in the
existing cleanup function.

Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
---
 drivers/gpu/drm/renesas/rcar-du/rcar_du_encoder.c | 28 ++++++++++++++++++-----
 drivers/gpu/drm/renesas/rcar-du/rcar_du_encoder.h |  1 +
 drivers/gpu/drm/renesas/rcar-du/rcar_du_kms.c     |  2 ++
 3 files changed, 25 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/renesas/rcar-du/rcar_du_encoder.c b/drivers/gpu/drm/renesas/rcar-du/rcar_du_encoder.c
index 7ecec7b04a8d..5789fc75092f 100644
--- a/drivers/gpu/drm/renesas/rcar-du/rcar_du_encoder.c
+++ b/drivers/gpu/drm/renesas/rcar-du/rcar_du_encoder.c
@@ -51,7 +51,7 @@ int rcar_du_encoder_init(struct rcar_du_device *rcdu,
 {
 	struct rcar_du_encoder *renc;
 	struct drm_connector *connector;
-	struct drm_bridge *bridge;
+	struct drm_bridge *bridge __free(drm_bridge_put) = NULL;
 	int ret;
 
 	/*
@@ -69,20 +69,26 @@ int rcar_du_encoder_init(struct rcar_du_device *rcdu,
 
 		bridge = devm_drm_panel_bridge_add_typed(rcdu->dev, panel,
 							 DRM_MODE_CONNECTOR_DPI);
-		if (IS_ERR(bridge))
-			return PTR_ERR(bridge);
+		if (IS_ERR(bridge)) {
+			// Inhibit the cleanup action on an ERR_PTR
+			ret = PTR_ERR(bridge);
+			bridge = NULL;
+			return ret;
+		}
+
+		drm_bridge_get(bridge);
 	} else {
-		bridge = of_drm_find_bridge(enc_node);
+		bridge = of_drm_find_and_get_bridge(enc_node);
 		if (!bridge)
 			return -EPROBE_DEFER;
 
 		if (output == RCAR_DU_OUTPUT_LVDS0 ||
 		    output == RCAR_DU_OUTPUT_LVDS1)
-			rcdu->lvds[output - RCAR_DU_OUTPUT_LVDS0] = bridge;
+			rcdu->lvds[output - RCAR_DU_OUTPUT_LVDS0] = drm_bridge_get(bridge);
 
 		if (output == RCAR_DU_OUTPUT_DSI0 ||
 		    output == RCAR_DU_OUTPUT_DSI1)
-			rcdu->dsi[output - RCAR_DU_OUTPUT_DSI0] = bridge;
+			rcdu->dsi[output - RCAR_DU_OUTPUT_DSI0] = drm_bridge_get(bridge);
 	}
 
 	/*
@@ -135,3 +141,13 @@ int rcar_du_encoder_init(struct rcar_du_device *rcdu,
 
 	return drm_connector_attach_encoder(connector, &renc->base);
 }
+
+void rcar_du_encoder_cleanup(struct rcar_du_device *rcdu)
+{
+	int i;
+
+	for (i = 0; i < RCAR_DU_MAX_LVDS; i++)
+		drm_bridge_put(rcdu->lvds[i]);
+	for (i = 0; i < RCAR_DU_MAX_DSI; i++)
+		drm_bridge_put(rcdu->dsi[i]);
+}
diff --git a/drivers/gpu/drm/renesas/rcar-du/rcar_du_encoder.h b/drivers/gpu/drm/renesas/rcar-du/rcar_du_encoder.h
index e5ec8fbb3979..b2b5e93f30f8 100644
--- a/drivers/gpu/drm/renesas/rcar-du/rcar_du_encoder.h
+++ b/drivers/gpu/drm/renesas/rcar-du/rcar_du_encoder.h
@@ -25,5 +25,6 @@ struct rcar_du_encoder {
 int rcar_du_encoder_init(struct rcar_du_device *rcdu,
 			 enum rcar_du_output output,
 			 struct device_node *enc_node);
+void rcar_du_encoder_cleanup(struct rcar_du_device *rcdu);
 
 #endif /* __RCAR_DU_ENCODER_H__ */
diff --git a/drivers/gpu/drm/renesas/rcar-du/rcar_du_kms.c b/drivers/gpu/drm/renesas/rcar-du/rcar_du_kms.c
index 60e6f43b8ab2..82d16a999ee6 100644
--- a/drivers/gpu/drm/renesas/rcar-du/rcar_du_kms.c
+++ b/drivers/gpu/drm/renesas/rcar-du/rcar_du_kms.c
@@ -836,6 +836,8 @@ static void rcar_du_modeset_cleanup(struct drm_device *dev, void *res)
 
 	for (i = 0; i < ARRAY_SIZE(rcdu->cmms); ++i)
 		platform_device_put(rcdu->cmms[i]);
+
+	rcar_du_encoder_cleanup(rcdu);
 }
 
 int rcar_du_modeset_init(struct rcar_du_device *rcdu)

-- 
2.53.0
Re: [PATCH v3 3/4] drm: rcar-du: encoder: convert to of_drm_find_and_get_bridge()
Posted by Laurent Pinchart 2 weeks, 1 day ago 
Hi Luca,

Thank you for the patch.

On Wed, Mar 18, 2026 at 10:39:37AM +0100, Luca Ceresoli wrote:
> of_drm_find_bridge() is deprecated. Move to its replacement
> of_drm_find_and_get_bridge() which gets a bridge reference, and ensure it
> is put when done.
> 
> We need to handle the two cases: when a panel_bridge is added and when it
> isn't. So:
> 
>  * in the 'else' case a panel_bridge is not added and bridge is found: use
>    of_drm_find_and_get_bridge() to get a reference to the found bridge
>  * in the 'then' case a panel_bridge is found using a devm function which
>    already takes a refcount and will put it on removal, but we need to take
>    another so the following code in this function always get exactly one
>    reference that it needs to put
> 
> In order to put the reference, add the needed drm_bridge_put() calls in the
> existing cleanup function.
> 
> Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
> ---
>  drivers/gpu/drm/renesas/rcar-du/rcar_du_encoder.c | 28 ++++++++++++++++++-----
>  drivers/gpu/drm/renesas/rcar-du/rcar_du_encoder.h |  1 +
>  drivers/gpu/drm/renesas/rcar-du/rcar_du_kms.c     |  2 ++
>  3 files changed, 25 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/renesas/rcar-du/rcar_du_encoder.c b/drivers/gpu/drm/renesas/rcar-du/rcar_du_encoder.c
> index 7ecec7b04a8d..5789fc75092f 100644
> --- a/drivers/gpu/drm/renesas/rcar-du/rcar_du_encoder.c
> +++ b/drivers/gpu/drm/renesas/rcar-du/rcar_du_encoder.c
> @@ -51,7 +51,7 @@ int rcar_du_encoder_init(struct rcar_du_device *rcdu,
>  {
>  	struct rcar_du_encoder *renc;
>  	struct drm_connector *connector;
> -	struct drm_bridge *bridge;
> +	struct drm_bridge *bridge __free(drm_bridge_put) = NULL;
>  	int ret;
>  
>  	/*
> @@ -69,20 +69,26 @@ int rcar_du_encoder_init(struct rcar_du_device *rcdu,
>  
>  		bridge = devm_drm_panel_bridge_add_typed(rcdu->dev, panel,
>  							 DRM_MODE_CONNECTOR_DPI);
> -		if (IS_ERR(bridge))
> -			return PTR_ERR(bridge);
> +		if (IS_ERR(bridge)) {
> +			// Inhibit the cleanup action on an ERR_PTR

C-style comments.

Shouldn't drm_bridge_put() be extended to be a no-op when called on an
ERR_PTR ?

> +			ret = PTR_ERR(bridge);
> +			bridge = NULL;
> +			return ret;
> +		}
> +

A comment here would be good.

		/*
		 * The reference taken by devm_drm_panel_bridge_add_typed() is
		 * released automatically. Take a second one for the __free()
		 * when this function will return.
		 */

> +		drm_bridge_get(bridge);
>  	} else {
> -		bridge = of_drm_find_bridge(enc_node);
> +		bridge = of_drm_find_and_get_bridge(enc_node);
>  		if (!bridge)
>  			return -EPROBE_DEFER;
>  
>  		if (output == RCAR_DU_OUTPUT_LVDS0 ||
>  		    output == RCAR_DU_OUTPUT_LVDS1)
> -			rcdu->lvds[output - RCAR_DU_OUTPUT_LVDS0] = bridge;
> +			rcdu->lvds[output - RCAR_DU_OUTPUT_LVDS0] = drm_bridge_get(bridge);

Line wrap.

>  
>  		if (output == RCAR_DU_OUTPUT_DSI0 ||
>  		    output == RCAR_DU_OUTPUT_DSI1)
> -			rcdu->dsi[output - RCAR_DU_OUTPUT_DSI0] = bridge;
> +			rcdu->dsi[output - RCAR_DU_OUTPUT_DSI0] = drm_bridge_get(bridge);

Same.

>  	}
>  
>  	/*
> @@ -135,3 +141,13 @@ int rcar_du_encoder_init(struct rcar_du_device *rcdu,
>  
>  	return drm_connector_attach_encoder(connector, &renc->base);
>  }
> +
> +void rcar_du_encoder_cleanup(struct rcar_du_device *rcdu)
> +{
> +	int i;

i is never negative, make it unsigned.

> +
> +	for (i = 0; i < RCAR_DU_MAX_LVDS; i++)

Use ARRAY_SIZE() here.

> +		drm_bridge_put(rcdu->lvds[i]);
> +	for (i = 0; i < RCAR_DU_MAX_DSI; i++)

Same here.

I have tested the patch and it seems to behave fine.

> +		drm_bridge_put(rcdu->dsi[i]);
> +}
> diff --git a/drivers/gpu/drm/renesas/rcar-du/rcar_du_encoder.h b/drivers/gpu/drm/renesas/rcar-du/rcar_du_encoder.h
> index e5ec8fbb3979..b2b5e93f30f8 100644
> --- a/drivers/gpu/drm/renesas/rcar-du/rcar_du_encoder.h
> +++ b/drivers/gpu/drm/renesas/rcar-du/rcar_du_encoder.h
> @@ -25,5 +25,6 @@ struct rcar_du_encoder {
>  int rcar_du_encoder_init(struct rcar_du_device *rcdu,
>  			 enum rcar_du_output output,
>  			 struct device_node *enc_node);
> +void rcar_du_encoder_cleanup(struct rcar_du_device *rcdu);
>  
>  #endif /* __RCAR_DU_ENCODER_H__ */
> diff --git a/drivers/gpu/drm/renesas/rcar-du/rcar_du_kms.c b/drivers/gpu/drm/renesas/rcar-du/rcar_du_kms.c
> index 60e6f43b8ab2..82d16a999ee6 100644
> --- a/drivers/gpu/drm/renesas/rcar-du/rcar_du_kms.c
> +++ b/drivers/gpu/drm/renesas/rcar-du/rcar_du_kms.c
> @@ -836,6 +836,8 @@ static void rcar_du_modeset_cleanup(struct drm_device *dev, void *res)
>  
>  	for (i = 0; i < ARRAY_SIZE(rcdu->cmms); ++i)
>  		platform_device_put(rcdu->cmms[i]);
> +
> +	rcar_du_encoder_cleanup(rcdu);
>  }
>  
>  int rcar_du_modeset_init(struct rcar_du_device *rcdu)
> 

-- 
Regards,

Laurent Pinchart
Re: [PATCH v3 3/4] drm: rcar-du: encoder: convert to of_drm_find_and_get_bridge()
Posted by Luca Ceresoli 2 weeks ago 
Hi Laurent,

On Wed Mar 18, 2026 at 4:25 PM CET, Laurent Pinchart wrote:
> Hi Luca,
>
> Thank you for the patch.
>
> On Wed, Mar 18, 2026 at 10:39:37AM +0100, Luca Ceresoli wrote:
>> of_drm_find_bridge() is deprecated. Move to its replacement
>> of_drm_find_and_get_bridge() which gets a bridge reference, and ensure it
>> is put when done.
>>
>> We need to handle the two cases: when a panel_bridge is added and when it
>> isn't. So:
>>
>>  * in the 'else' case a panel_bridge is not added and bridge is found: use
>>    of_drm_find_and_get_bridge() to get a reference to the found bridge
>>  * in the 'then' case a panel_bridge is found using a devm function which
>>    already takes a refcount and will put it on removal, but we need to take
>>    another so the following code in this function always get exactly one
>>    reference that it needs to put
>>
>> In order to put the reference, add the needed drm_bridge_put() calls in the
>> existing cleanup function.
>>
>> Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
>> ---
>>  drivers/gpu/drm/renesas/rcar-du/rcar_du_encoder.c | 28 ++++++++++++++++++-----
>>  drivers/gpu/drm/renesas/rcar-du/rcar_du_encoder.h |  1 +
>>  drivers/gpu/drm/renesas/rcar-du/rcar_du_kms.c     |  2 ++
>>  3 files changed, 25 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/renesas/rcar-du/rcar_du_encoder.c b/drivers/gpu/drm/renesas/rcar-du/rcar_du_encoder.c
>> index 7ecec7b04a8d..5789fc75092f 100644
>> --- a/drivers/gpu/drm/renesas/rcar-du/rcar_du_encoder.c
>> +++ b/drivers/gpu/drm/renesas/rcar-du/rcar_du_encoder.c
>> @@ -51,7 +51,7 @@ int rcar_du_encoder_init(struct rcar_du_device *rcdu,
>>  {
>>  	struct rcar_du_encoder *renc;
>>  	struct drm_connector *connector;
>> -	struct drm_bridge *bridge;
>> +	struct drm_bridge *bridge __free(drm_bridge_put) = NULL;
>>  	int ret;
>>
>>  	/*
>> @@ -69,20 +69,26 @@ int rcar_du_encoder_init(struct rcar_du_device *rcdu,
>>
>>  		bridge = devm_drm_panel_bridge_add_typed(rcdu->dev, panel,
>>  							 DRM_MODE_CONNECTOR_DPI);
>> -		if (IS_ERR(bridge))
>> -			return PTR_ERR(bridge);
>> +		if (IS_ERR(bridge)) {
>> +			// Inhibit the cleanup action on an ERR_PTR
>
> C-style comments.

OK

> Shouldn't drm_bridge_put() be extended to be a no-op when called on an
> ERR_PTR ?

Uhm, maybe, even though I think I haven't encountered many cases like this
where this was troublesome, and all those I can remember are related to
panel_bridge functions that are meant to be reworked.

Also, drm_bridge_put() deliberately mimicks of_node_put() which also checks
for NULL but not for IS_ERR.

Maxime, your opinion about Laurent's suggestion?

>> +			ret = PTR_ERR(bridge);
>> +			bridge = NULL;
>> +			return ret;
>> +		}
>> +
>
> A comment here would be good.
>
> 		/*
> 		 * The reference taken by devm_drm_panel_bridge_add_typed() is
> 		 * released automatically. Take a second one for the __free()
> 		 * when this function will return.
> 		 */

Sure, makes sense. I'll add one to other patches of this series where
relevant.

>
>> +		drm_bridge_get(bridge);
>>  	} else {
>> -		bridge = of_drm_find_bridge(enc_node);
>> +		bridge = of_drm_find_and_get_bridge(enc_node);
>>  		if (!bridge)
>>  			return -EPROBE_DEFER;
>>
>>  		if (output == RCAR_DU_OUTPUT_LVDS0 ||
>>  		    output == RCAR_DU_OUTPUT_LVDS1)
>> -			rcdu->lvds[output - RCAR_DU_OUTPUT_LVDS0] = bridge;
>> +			rcdu->lvds[output - RCAR_DU_OUTPUT_LVDS0] = drm_bridge_get(bridge);
>
> Line wrap.

I wish the 80-VS-100 policy were clear and valid for all the kernel. But no
big deal here, and you maintain this driver, so I'll split.

> I have tested the patch and it seems to behave fine.

thanks for reviewing and testing!

Luca

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

Patchew 2.1-devel-ea86937

© 2016 - 2026 Red Hat, Inc.