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

[Louis Chauvet <louis.chauvet@bootlin.com>]

On 4/2/26 18:27, 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>
> 
> ---
> 
> Changes in v4:
> - fixed conflict after commit 3bce3fdd1ff2 ("drm: rcar-du: Don't leak
>    device_link to CMM")
> - Use ARRAY_SIZE() instead of define
> - Unsigned variable for never-negative variable
> - Added comment to clarify the additional drm_bridge_get() in the
>    panel_bridge case
> - Coding style (C comments, line wrap at 80)
> ---
>   drivers/gpu/drm/renesas/rcar-du/rcar_du_encoder.c | 35 +++++++++++++++++++----
>   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, 32 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..0ae06edf3066 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,33 @@ 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;

Same as before, can you use no_free_ptr?

With or without this modification:

Reviewed-by: Louis Chauvet <louis.chauvet@bootlin.com>

> +		}
> +
> +		/*
> +		 * 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);
>   
>   		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 +148,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)
> +{
> +	unsigned int i;
> +
> +	for (i = 0; i < ARRAY_SIZE(rcdu->lvds); i++)
> +		drm_bridge_put(rcdu->lvds[i]);
> +	for (i = 0; i < ARRAY_SIZE(rcdu->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 b2d0e4651e35..1119c84e5fe9 100644
> --- a/drivers/gpu/drm/renesas/rcar-du/rcar_du_kms.c
> +++ b/drivers/gpu/drm/renesas/rcar-du/rcar_du_kms.c
> @@ -832,6 +832,8 @@ static void rcar_du_modeset_cleanup(struct drm_device *dev, void *res)
>   
>   		put_device(cmm->dev);
>   	}
> +
> +	rcar_du_encoder_cleanup(rcdu);
>   }
>   
>   int rcar_du_modeset_init(struct rcar_du_device *rcdu)
>