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 <[email protected]>
> ---
>  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

Reply via email to