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

Luca Ceresoli
2026-03-18T16:53:20Z

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