[PATCH v8 0/3] drm/bridge: debugfs: show refcount and list removed bridges

[PATCH v8 0/3] drm/bridge: debugfs: show refcount and list removed bridges

[Luca Ceresoli]

This series shows removed bridges to the global <debugfs>/dri/bridges file.
Removed bridges are bridges after drm_bridges_remove() but before they are
eventually freed on the last drm_bridge_put().

This is part of the work towards removal of bridges from a still existing
DRM pipeline without use-after-free. The grand plan was discussed in [1].
Here's the work breakdown (➜ marks the current series):

This is part of the work towards removal of bridges from a still existing
DRM pipeline without use-after-free. The grand plan was discussed in [1].
Here's the work breakdown (➜ marks the current series):

 1. ➜ add refcounting to DRM bridges (struct drm_bridge)
    (based on devm_drm_bridge_alloc() [0])
    A. ✔ add new alloc API and refcounting (v6.16)
    B. ✔ convert all bridge drivers to new API (v6.17-rc1)
    C. ✔ kunit tests (v6.17-rc1)
    D. ✔ add get/put to drm_bridge_add/remove() + attach/detach()
         and warn on old allocation pattern (v6.17-rc1)
    E. … add get/put on drm_bridge accessors
       1. ✔ drm_bridge_chain_get_first_bridge() + add a cleanup action
            (drm-misc-next)
       2. ✔ drm_bridge_get_prev_bridge() (drm-misc-next)
       3. …✔ drm_bridge_get_next_bridge() (partially in drm-misc-next)
       4. …✔ drm_for_each_bridge_in_chain() (partially in drm-misc-next)
       5. drm_bridge_connector_init
       6. of_drm_find_bridge
       7. drm_of_find_panel_or_bridge, *_of_get_bridge
    F. ➜ debugfs improvements
       1. ✔ add top-level 'bridges' file (v6.16)
       2. ➜ show refcount and list removed bridges
 2. … handle gracefully atomic updates during bridge removal
 3. … DSI host-device driver interaction
 4. finish the hotplug bridge work, removing the "always-disconnected"
    connector, moving code to the core and potentially removing the
    hotplug-bridge itself (this needs to be clarified as points 1-3 are
    developed)

To show the removed bridges we need to keep track of them, thus add a new
global list to store them between drm_bridge_remove() and the eventual
free. This is bit tricky in case a bridge is removed and then re-added
before being freed. This is handled in patch 2.

This series  depends on one other series:
 * [PATCH v2 0/9] drm/bridge: get/put the bridge when looping over the encoder chain
   Link: https://lore.kernel.org/r/20250808-drm-bridge-alloc-getput-for_each_bridge-v2-0-edb6ee81edf1@bootlin.com
   Reason: trivial conflict in context (drm_for_each_bridge_in_chain_scoped())

[0] https://gitlab.freedesktop.org/drm/misc/kernel/-/commit/0cc6aadd7fc1e629b715ea3d1ba537ef2da95eec
[1] https://lore.kernel.org/lkml/20250206-hotplug-drm-bridge-v6-0-9d6f2c9c3058@bootlin.com/t/#u

Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
---
Changes in v8:
- Removed patch 1, now in drm-misc-next
- Renamed "removed" -> "lingering"
- Rewrote documentation changes to use "register" in lieu of "publish",
  split them to a separate patch
- Link to v7: https://lore.kernel.org/r/20250819-drm-bridge-debugfs-removed-v7-0-970702579978@bootlin.com

This series was initially part of v6 of this other series:
- Link to v6: https://lore.kernel.org/dri-devel/20250206-hotplug-drm-bridge-v6-0-9d6f2c9c3058@bootlin.com/

---
Luca Ceresoli (3):
      drm/bridge: add list of removed refcounted bridges
      drm/debugfs: show removed bridges
      drm/bridge: adapt drm_bridge_add/remove() docs, mention the lingering list

 drivers/gpu/drm/drm_bridge.c | 46 ++++++++++++++++++++++++++++++++++++--------
 1 file changed, 38 insertions(+), 8 deletions(-)
---
base-commit: 27ed0d64a0f37224ca865fbf8f0aacdc46a3f481
change-id: 20250408-drm-bridge-debugfs-removed-2579f892e4b3

Best regards,
-- 
Luca Ceresoli <luca.ceresoli@bootlin.com>

[PATCH v8 1/3] drm/bridge: add list of removed refcounted bridges

[Luca Ceresoli]

Between drm_bridge_add() and drm_bridge_remove() bridges are registered to
the DRM core via the global bridge_list and visible in
/sys/kernel/debug/dri/bridges. However between drm_bridge_remove() and the
last drm_bridge_put() memory is still allocated even though the bridge is
not registered, i.e. not in bridges_list, and also not visible in
debugfs. This prevents debugging refcounted bridges lifetime, especially
leaks due to a missing drm_bridge_put().

In order to allow debugfs to also show the removed bridges, move such
bridges into a new ad-hoc list until they are eventually freed.

Note this requires adding INIT_LIST_HEAD(&bridge->list) in the bridge
initialization code. The lack of such init was not exposing any bug so far,
but it would with the new code, for example when a bridge is allocated and
then freed without calling drm_bridge_add(), which is common on probe
errors.

drm_bridge_add() needs special care for bridges being added after having
been previously added and then removed.  This happens for example for many
non-DCS DSI host bridge drivers like samsung-dsim which
drm_bridge_add/remove() themselves every time the DSI device does a DSI
attaches/detach. When the DSI device is hot-pluggable this happens multiple
times in the lifetime of the DSI host bridge.  On every attach after the
first one, drm_bridge_add() finds bridge->list in the removed list, not at
the initialized state as drm_bridge_add() currently expects. Add a
list_del_init() to remove the bridge from the lingering list and bring
bridge->list back to the initialized state.

Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>

---

Changes in v8:
- split the documentation changes to a separate patch
- rename "removed" to "lingering"
- improve commits message about the special care needed for "un-removed"
  bridges

Changes in v7:
- rebase on current drm-misc-next
- remove if (drm_bridge_is_refcounted(bridge)), refcounting is now
  mandatory
- add check to detect when re-adding a bridge that is in the removed list
- improve commit message
- fix typo

This patch was added in v6.
---
 drivers/gpu/drm/drm_bridge.c | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
index 5ec2ff77da79e51f614f98ebe58b6171c611867a..9491ae7c884d355be4a82fb02a43a42d17fa8e0c 100644
--- a/drivers/gpu/drm/drm_bridge.c
+++ b/drivers/gpu/drm/drm_bridge.c
@@ -197,15 +197,22 @@
  * driver.
  */
 
+/* Protect bridge_list and bridge_lingering_list */
 static DEFINE_MUTEX(bridge_lock);
 static LIST_HEAD(bridge_list);
+static LIST_HEAD(bridge_lingering_list);
 
 static void __drm_bridge_free(struct kref *kref)
 {
 	struct drm_bridge *bridge = container_of(kref, struct drm_bridge, refcount);
 
+	mutex_lock(&bridge_lock);
+	list_del(&bridge->list);
+	mutex_unlock(&bridge_lock);
+
 	if (bridge->funcs->destroy)
 		bridge->funcs->destroy(bridge);
+
 	kfree(bridge->container);
 }
 
@@ -275,6 +282,7 @@ void *__devm_drm_bridge_alloc(struct device *dev, size_t size, size_t offset,
 		return ERR_PTR(-ENOMEM);
 
 	bridge = container + offset;
+	INIT_LIST_HEAD(&bridge->list);
 	bridge->container = container;
 	bridge->funcs = funcs;
 	kref_init(&bridge->refcount);
@@ -304,6 +312,14 @@ void drm_bridge_add(struct drm_bridge *bridge)
 
 	drm_bridge_get(bridge);
 
+	/*
+	 * If the bridge was previously added and then removed, it is now
+	 * in bridge_lingering_list. Remove it or bridge_lingering_list will be
+	 * corrupted when adding this bridge to bridge_list below.
+	 */
+	if (!list_empty(&bridge->list))
+		list_del_init(&bridge->list);
+
 	mutex_init(&bridge->hpd_mutex);
 
 	if (bridge->ops & DRM_BRIDGE_OP_HDMI)
@@ -357,7 +373,7 @@ void drm_bridge_remove(struct drm_bridge *bridge)
 			br->funcs->bridge_event_notify(br, DRM_EVENT_BRIDGE_REMOVING, bridge);
 
 	mutex_lock(&bridge_lock);
-	list_del_init(&bridge->list);
+	list_move_tail(&bridge->list, &bridge_lingering_list);
 	mutex_unlock(&bridge_lock);
 
 	mutex_destroy(&bridge->hpd_mutex);

-- 
2.51.0

[PATCH v8 2/3] drm/debugfs: show removed bridges

[Luca Ceresoli]

The usefulness of /sys/kernel/debug/dri/bridges is limited as it only shows
bridges between drm_bridge_add() and drm_bridge_remove(). However
refcounted bridges can stay allocated for a long time after
drm_bridge_remove(), and a memory leak due to a missing drm_bridge_put()
would not be visible in this debugfs file.

Add removed bridges to the /sys/kernel/debug/dri/bridges output.

Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>

---

Changes in v8:
- updated to rename bridge_removed_list -> bridge_lingering_list

Changes in v7:
- rebased on current code which is in drm_bridge.c now
- removed if (drm_bridge_is_refcounted(bridge)), refcounting is not
  optional
- don't show bridge address
- improve commit message

This patch was added in v6.
---
 drivers/gpu/drm/drm_bridge.c | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
index 9491ae7c884d355be4a82fb02a43a42d17fa8e0c..fccc42017fd4df6ecfb596325df2dc4d17566f39 100644
--- a/drivers/gpu/drm/drm_bridge.c
+++ b/drivers/gpu/drm/drm_bridge.c
@@ -1491,17 +1491,20 @@ EXPORT_SYMBOL(devm_drm_put_bridge);
 
 static void drm_bridge_debugfs_show_bridge(struct drm_printer *p,
 					   struct drm_bridge *bridge,
-					   unsigned int idx)
+					   unsigned int idx,
+					   bool removed)
 {
 	drm_printf(p, "bridge[%u]: %ps\n", idx, bridge->funcs);
 
-	drm_printf(p, "\trefcount: %u\n", kref_read(&bridge->refcount));
+	drm_printf(p, "\trefcount: %u%s\n", kref_read(&bridge->refcount),
+		   removed ? " [removed]" : "");
 
 	drm_printf(p, "\ttype: [%d] %s\n",
 		   bridge->type,
 		   drm_get_connector_type_name(bridge->type));
 
-	if (bridge->of_node)
+	/* The OF node could be freed after drm_bridge_remove() */
+	if (bridge->of_node && !removed)
 		drm_printf(p, "\tOF: %pOFfc\n", bridge->of_node);
 
 	drm_printf(p, "\tops: [0x%x]", bridge->ops);
@@ -1527,7 +1530,10 @@ static int allbridges_show(struct seq_file *m, void *data)
 	mutex_lock(&bridge_lock);
 
 	list_for_each_entry(bridge, &bridge_list, list)
-		drm_bridge_debugfs_show_bridge(&p, bridge, idx++);
+		drm_bridge_debugfs_show_bridge(&p, bridge, idx++, false);
+
+	list_for_each_entry(bridge, &bridge_lingering_list, list)
+		drm_bridge_debugfs_show_bridge(&p, bridge, idx++, true);
 
 	mutex_unlock(&bridge_lock);
 
@@ -1542,7 +1548,7 @@ static int encoder_bridges_show(struct seq_file *m, void *data)
 	unsigned int idx = 0;
 
 	drm_for_each_bridge_in_chain_scoped(encoder, bridge)
-		drm_bridge_debugfs_show_bridge(&p, bridge, idx++);
+		drm_bridge_debugfs_show_bridge(&p, bridge, idx++, false);
 
 	return 0;
 }

-- 
2.51.0

Re: [PATCH v8 2/3] drm/debugfs: show removed bridges

[Maxime Ripard]

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

On Fri, Sep 12, 2025 at 07:03:42PM +0200, Luca Ceresoli wrote:
> The usefulness of /sys/kernel/debug/dri/bridges is limited as it only shows
> bridges between drm_bridge_add() and drm_bridge_remove(). However
> refcounted bridges can stay allocated for a long time after
> drm_bridge_remove(), and a memory leak due to a missing drm_bridge_put()
> would not be visible in this debugfs file.
> 
> Add removed bridges to the /sys/kernel/debug/dri/bridges output.
> 
> Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
> 
> ---
> 
> Changes in v8:
> - updated to rename bridge_removed_list -> bridge_lingering_list
> 
> Changes in v7:
> - rebased on current code which is in drm_bridge.c now
> - removed if (drm_bridge_is_refcounted(bridge)), refcounting is not
>   optional
> - don't show bridge address
> - improve commit message
> 
> This patch was added in v6.
> ---
>  drivers/gpu/drm/drm_bridge.c | 16 +++++++++++-----
>  1 file changed, 11 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
> index 9491ae7c884d355be4a82fb02a43a42d17fa8e0c..fccc42017fd4df6ecfb596325df2dc4d17566f39 100644
> --- a/drivers/gpu/drm/drm_bridge.c
> +++ b/drivers/gpu/drm/drm_bridge.c
> @@ -1491,17 +1491,20 @@ EXPORT_SYMBOL(devm_drm_put_bridge);
>  
>  static void drm_bridge_debugfs_show_bridge(struct drm_printer *p,
>  					   struct drm_bridge *bridge,
> -					   unsigned int idx)
> +					   unsigned int idx,
> +					   bool removed)
>  {
>  	drm_printf(p, "bridge[%u]: %ps\n", idx, bridge->funcs);
>  
> -	drm_printf(p, "\trefcount: %u\n", kref_read(&bridge->refcount));
> +	drm_printf(p, "\trefcount: %u%s\n", kref_read(&bridge->refcount),
> +		   removed ? " [removed]" : "");

I would use lingering here too, and your commit logs haven't been
updated either.

Maxime

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

[PATCH v8 3/3] drm/bridge: adapt drm_bridge_add/remove() docs, mention the lingering list

[Luca Ceresoli]

The role of drm_bridge_add/remove() is more complex now after having added
the lingering list. Update the kdoc accordingly.

Also stop mentioning the global list(s) in the first line of the docs: the
most important thing to mention here is that bridges are registered and
deregistered, lists are just the type of container used to implement such
(de)registration.

Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>

---

Changes in v8:
- extracted to an ad-hoc patch from the v7 patch "drm/bridge: add list of
  removed refcounted bridges"
---
 drivers/gpu/drm/drm_bridge.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
index fccc42017fd4df6ecfb596325df2dc4d17566f39..9a3db8f5adc8a4d1265679335d7b05d0705194b7 100644
--- a/drivers/gpu/drm/drm_bridge.c
+++ b/drivers/gpu/drm/drm_bridge.c
@@ -296,10 +296,13 @@ void *__devm_drm_bridge_alloc(struct device *dev, size_t size, size_t offset,
 EXPORT_SYMBOL(__devm_drm_bridge_alloc);
 
 /**
- * drm_bridge_add - add the given bridge to the global bridge list
+ * drm_bridge_add - register a bridge
  *
  * @bridge: bridge control structure
  *
+ * Add the given bridge to the global list of bridges, where they can be
+ * found by users via of_drm_find_bridge().
+ *
  * The bridge to be added must have been allocated by
  * devm_drm_bridge_alloc().
  */
@@ -360,9 +363,14 @@ int devm_drm_bridge_add(struct device *dev, struct drm_bridge *bridge)
 EXPORT_SYMBOL(devm_drm_bridge_add);
 
 /**
- * drm_bridge_remove - remove the given bridge from the global bridge list
+ * drm_bridge_remove - unregister a bridge
  *
  * @bridge: bridge control structure
+ *
+ * Remove the given bridge from the global list of registered bridges, so
+ * it won't be found by users via of_drm_find_bridge(), and add it to the
+ * lingering bridge list, to keep track of it until its allocated memory is
+ * eventually freed.
  */
 void drm_bridge_remove(struct drm_bridge *bridge)
 {

-- 
2.51.0

Re: [PATCH v8 0/3] drm/bridge: debugfs: show refcount and list removed bridges

[Maxime Ripard]

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

Hi,

On Fri, Sep 12, 2025 at 07:03:40PM +0200, Luca Ceresoli wrote:
> This series shows removed bridges to the global <debugfs>/dri/bridges file.
> Removed bridges are bridges after drm_bridges_remove() but before they are
> eventually freed on the last drm_bridge_put().
> 
> This is part of the work towards removal of bridges from a still existing
> DRM pipeline without use-after-free. The grand plan was discussed in [1].
> Here's the work breakdown (➜ marks the current series):
> 
> This is part of the work towards removal of bridges from a still existing
> DRM pipeline without use-after-free. The grand plan was discussed in [1].
> Here's the work breakdown (➜ marks the current series):
> 
>  1. ➜ add refcounting to DRM bridges (struct drm_bridge)
>     (based on devm_drm_bridge_alloc() [0])
>     A. ✔ add new alloc API and refcounting (v6.16)
>     B. ✔ convert all bridge drivers to new API (v6.17-rc1)
>     C. ✔ kunit tests (v6.17-rc1)
>     D. ✔ add get/put to drm_bridge_add/remove() + attach/detach()
>          and warn on old allocation pattern (v6.17-rc1)
>     E. … add get/put on drm_bridge accessors
>        1. ✔ drm_bridge_chain_get_first_bridge() + add a cleanup action
>             (drm-misc-next)
>        2. ✔ drm_bridge_get_prev_bridge() (drm-misc-next)
>        3. …✔ drm_bridge_get_next_bridge() (partially in drm-misc-next)
>        4. …✔ drm_for_each_bridge_in_chain() (partially in drm-misc-next)
>        5. drm_bridge_connector_init
>        6. of_drm_find_bridge
>        7. drm_of_find_panel_or_bridge, *_of_get_bridge
>     F. ➜ debugfs improvements
>        1. ✔ add top-level 'bridges' file (v6.16)
>        2. ➜ show refcount and list removed bridges
>  2. … handle gracefully atomic updates during bridge removal
>  3. … DSI host-device driver interaction
>  4. finish the hotplug bridge work, removing the "always-disconnected"
>     connector, moving code to the core and potentially removing the
>     hotplug-bridge itself (this needs to be clarified as points 1-3 are
>     developed)
> 
> To show the removed bridges we need to keep track of them, thus add a new
> global list to store them between drm_bridge_remove() and the eventual
> free. This is bit tricky in case a bridge is removed and then re-added
> before being freed. This is handled in patch 2.

Once the minor issue I've reported is fixed,
Reviewed-by: Maxime Ripard <mripard@kernel.org>

Maxime

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

Re: [PATCH v8 0/3] drm/bridge: debugfs: show refcount and list removed bridges

[Luca Ceresoli]

Hi Maxime,

On Mon, 15 Sep 2025 10:13:16 +0200
Maxime Ripard <mripard@kernel.org> wrote:

> Once the minor issue I've reported is fixed,
> Reviewed-by: Maxime Ripard <mripard@kernel.org>

Ok, thanks!

v9 sent.

Note that this series depends on [0] which is waiting for discussion to
continue, so any feedback on drm_for_each_bridge_in_chain_scoped()
would be very welcome to unlock both this and other series from being
applied/sent.

[0]
https://lore.kernel.org/all/20250808-drm-bridge-alloc-getput-for_each_bridge-v2-3-edb6ee81edf1@bootlin.com/

Luca

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

Re: [PATCH v8 0/3] drm/bridge: debugfs: show refcount and list removed bridges

[Maxime Ripard]

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

On Mon, Sep 15, 2025 at 12:30:10PM +0200, Luca Ceresoli wrote:
> Hi Maxime,
> 
> On Mon, 15 Sep 2025 10:13:16 +0200
> Maxime Ripard <mripard@kernel.org> wrote:
> 
> > Once the minor issue I've reported is fixed,
> > Reviewed-by: Maxime Ripard <mripard@kernel.org>
> 
> Ok, thanks!
> 
> v9 sent.
> 
> Note that this series depends on [0] which is waiting for discussion to
> continue, so any feedback on drm_for_each_bridge_in_chain_scoped()
> would be very welcome to unlock both this and other series from being
> applied/sent.

Thanks for the reminder, I've reviewed it

Maxime

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