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

[Laurent Pinchart]

On Wed, Apr 08, 2026 at 06:03:01PM +0200, Louis Chauvet wrote:
> 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@xxxxxxxxxxx>
> > 
> > ---
> > 
> > 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?

That would be nice. You could then drop the comment, as no_free_ptr
makes it explicit.

Reviewed-by: Laurent Pinchart <laurent.pinchart+renesas@xxxxxxxxxxxxxxxx>

> With or without this modification:
> 
> Reviewed-by: Louis Chauvet <louis.chauvet@xxxxxxxxxxx>
> 
> > +		}
> > +
> > +		/*
> > +		 * 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)
> > 

-- 
Regards,

Laurent Pinchart