Re: [PATCH 8/9] drm/bridge: put the bridge returned by drm_bridge_get_next_bridge()

[Maxime Ripard <mripard@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 1486 bytes --]

Hi,

On Wed, Jul 09, 2025 at 06:48:07PM +0200, Luca Ceresoli wrote:
> The bridge returned by drm_bridge_get_next_bridge() is refcounted. Put it
> when done.
> 
> Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>

You should really expand a bit more your commit logs, and provide the
context of why you think putting drm_bridge_put where you do is a good idea.

> ---
>  drivers/gpu/drm/drm_bridge.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
> index 0b450b334afd82e0460f18fdd248f79d0a2b153d..05e85457099ab1e0a23ea7842c9654c9a6881dfb 100644
> --- a/drivers/gpu/drm/drm_bridge.c
> +++ b/drivers/gpu/drm/drm_bridge.c
> @@ -1147,6 +1147,8 @@ drm_atomic_bridge_propagate_bus_flags(struct drm_bridge *bridge,
>  	} else {
>  		next_bridge_state = drm_atomic_get_new_bridge_state(state,
>  								next_bridge);
> +		drm_bridge_put(next_bridge);
> +
>  		/*
>  		 * No bridge state attached to the next bridge, just leave the
>  		 * flags to 0.

In particular, I don't think it is here.

You still have a variable in scope after that branch that you would have
given up the reference for, which is pretty dangerous.

Also, the bridge state lifetime is shorter than the bridge lifetime
itself, so we probably want to have the drm_bridge_put after we're done
with next_bridge_state too.

Overall, I think using __free here is probably the most robust solution.

Maxime

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 273 bytes --]