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 <[email protected]>
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
signature.asc
Description: PGP signature
