Re: [PATCH] drm/bridge: Fix refcount shown via debugfs for encoder_bridges_show()

["Luca Ceresoli" <luca.ceresoli@bootlin.com>]

Hello Liu,

On Thu Mar 12, 2026 at 7:05 AM CET, Liu Ying wrote:
> A typical bridge refcount value is 3 after a bridge chain is formed:
> - devm_drm_bridge_alloc() initializes the refcount value to be 1.
> - drm_bridge_add() gets an additional reference hence 2.
> - drm_bridge_attach() gets the third reference hence 3.
>
> This typical refcount value aligns with allbridges_show()'s behaviour.
> However, since encoder_bridges_show() uses
> drm_for_each_bridge_in_chain_scoped() to automatically get/put the
> bridge reference while iterating, a bogus reference is accidentally
> got when showing the wrong typical refcount value as 4 to users via
> debugfs.  Fix this by caching the refcount value returned from
> kref_read() while iterating and explicitly decreasing the cached
> refcount value by 1 before showing it to users.
>
> Fixes: bd57048e4576 ("drm/bridge: use drm_for_each_bridge_in_chain_scoped()")
> Signed-off-by: Liu Ying <victor.liu@nxp.com>
> ---
>  drivers/gpu/drm/drm_bridge.c | 32 ++++++++++++++++++++++++++------
>  1 file changed, 26 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
> index f8b0333a0a3b..84fc3cfd17e0 100644
> --- a/drivers/gpu/drm/drm_bridge.c
> +++ b/drivers/gpu/drm/drm_bridge.c
> @@ -1567,14 +1567,18 @@ void devm_drm_put_bridge(struct device *dev, struct drm_bridge *bridge)
>  }
>  EXPORT_SYMBOL(devm_drm_put_bridge);
>
> -static void drm_bridge_debugfs_show_bridge(struct drm_printer *p,
> -					   struct drm_bridge *bridge,
> -					   unsigned int idx,
> -					   bool lingering)
> +static void __drm_bridge_debugfs_show_bridge(struct drm_printer *p,
> +					     struct drm_bridge *bridge,
> +					     unsigned int idx,
> +					     bool lingering,
> +					     bool scoped)
>  {
> +	unsigned int refcount = kref_read(&bridge->refcount);
> +
>  	drm_printf(p, "bridge[%u]: %ps\n", idx, bridge->funcs);
>
> -	drm_printf(p, "\trefcount: %u%s\n", kref_read(&bridge->refcount),
> +	drm_printf(p, "\trefcount: %u%s\n",
> +		   scoped ? --refcount : refcount,

I'd s/--refcount/refcount - 1/ here, no point in modifying the value while
printing it.

> @@ -1599,6 +1603,22 @@ static void drm_bridge_debugfs_show_bridge(struct drm_printer *p,
>  	drm_puts(p, "\n");
>  }
>
> +static void drm_bridge_debugfs_show_bridge(struct drm_printer *p,
> +					   struct drm_bridge *bridge,
> +					   unsigned int idx,
> +					   bool lingering)
> +{
> +	__drm_bridge_debugfs_show_bridge(p, bridge, idx, lingering, false);
> +}
> +
> +static void drm_bridge_debugfs_show_bridge_scoped(struct drm_printer *p,
> +						  struct drm_bridge *bridge,
> +						  unsigned int idx,
> +						  bool lingering)
> +{
> +	__drm_bridge_debugfs_show_bridge(p, bridge, idx, lingering, true);
> +}

I think this should be much simpler and avoid a lot of the boilerplate
code: just add a 'bool scoped' argument to drm_bridge_debugfs_show_bridge()
and pass true/false as applicable.

Or maybe an 'int offset' with an integer to be subtracted from the refcount
for diplaying, but that's probably overkill.

Best regards,
Luca

--
Luca Ceresoli, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com