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

[Luca Ceresoli]

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

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