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

[Laurent Pinchart]

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@xxxxxxxxxxx>
> ---
>  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