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