[PATCH 0/3] drm/bridge: get/put the bridge returned by drm_bridge_get_prev_bridge()

[PATCH 0/3] drm/bridge: get/put the bridge returned by drm_bridge_get_prev_bridge()

[Luca Ceresoli]

This series adds drm_bridge_get/put() calls for DRM bridges returned by
drm_bridge_get_prev_bridge().

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 (in v6.16-rc1)
    B. ✔ convert all bridge drivers to new API (now in drm-misc-next)
    C. ✔ kunit tests (now in drm-misc-next)
    D. ✔ add get/put to drm_bridge_add/remove() + attach/detach()
         and warn on old allocation pattern (now in drm-misc-next)
    E. ➜ add get/put on drm_bridge accessors
       1. … drm_bridge_chain_get_first_bridge() + add a cleanup action
       2. drm_bridge_chain_get_last_bridge()
       3. ➜ drm_bridge_get_prev_bridge()
       4. drm_bridge_get_next_bridge()
       5. drm_for_each_bridge_in_chain()
       6. drm_bridge_connector_init
       7. of_drm_find_bridge
       8. drm_of_find_panel_or_bridge, *_of_get_bridge
    F. debugfs improvements
 2. handle gracefully atomic updates during bridge removal
 3. … avoid DSI host drivers to have dangling pointers to DSI devices
 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)

[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>
---
Luca Ceresoli (3):
      drm/bridge: get the bridge returned by drm_bridge_get_prev_bridge()
      drm/bridge: select_bus_fmt_recursive(): put the bridge obtained by drm_bridge_get_prev_bridge()
      drm/bridge: display-connector: put the bridge obtained by drm_bridge_get_prev_bridge()

 drivers/gpu/drm/bridge/display-connector.c | 4 ++--
 drivers/gpu/drm/drm_bridge.c               | 4 ++--
 include/drm/drm_bridge.h                   | 9 ++++++++-
 3 files changed, 12 insertions(+), 5 deletions(-)
---
base-commit: 0f168e7be696a17487e83d1d47e5a408a181080f
change-id: 20250709-drm-bridge-alloc-getput-drm_bridge_get_prev_bridge-9470dca81bb3

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

[PATCH 1/3] drm/bridge: get the bridge returned by drm_bridge_get_prev_bridge()

[Luca Ceresoli]

drm_bridge_get_prev_bridge() returns a bridge pointer that the
caller could hold for a long time. Increment the refcount of the returned
bridge and document it must be put by the caller.

Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
---
 include/drm/drm_bridge.h | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
index d2454ba83db36f8f0d475b0b37468c2ebe7e921d..1963fa61b7f3aa41c7112cb9902790962aff8808 100644
--- a/include/drm/drm_bridge.h
+++ b/include/drm/drm_bridge.h
@@ -1316,6 +1316,13 @@ drm_bridge_get_next_bridge(struct drm_bridge *bridge)
  * drm_bridge_get_prev_bridge() - Get the previous bridge in the chain
  * @bridge: bridge object
  *
+ * The caller is responsible of having a reference to @bridge via
+ * drm_bridge_get() or equivalent. This function leaves the refcount of
+ * @bridge unmodified.
+ *
+ * The refcount of the returned bridge is incremented. Use drm_bridge_put()
+ * when done with it.
+ *
  * RETURNS:
  * the previous bridge in the chain, or NULL if @bridge is the first.
  */
@@ -1325,7 +1332,7 @@ drm_bridge_get_prev_bridge(struct drm_bridge *bridge)
 	if (list_is_first(&bridge->chain_node, &bridge->encoder->bridge_chain))
 		return NULL;
 
-	return list_prev_entry(bridge, chain_node);
+	return drm_bridge_get(list_prev_entry(bridge, chain_node));
 }
 
 /**

-- 
2.50.0

[PATCH 2/3] drm/bridge: select_bus_fmt_recursive(): put the bridge obtained by drm_bridge_get_prev_bridge()

[Luca Ceresoli]

The bridge returned by drm_bridge_get_prev_bridge() is refcounted. Put it
when done.

select_bus_fmt_recursive() has several return points, and ensuring
drm_bridge_put() is always called in the right place would be error-prone
(especially with future changes to the select_bus_fmt_recursive() code) and
make code uglier. Instead use a scope-based free, which is future-proof and
a lot cleaner.

Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
---
 drivers/gpu/drm/drm_bridge.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
index 0b450b334afd82e0460f18fdd248f79d0a2b153d..e33ccc5cabab72505b839d23bfa5ae644d2105c4 100644
--- a/drivers/gpu/drm/drm_bridge.c
+++ b/drivers/gpu/drm/drm_bridge.c
@@ -941,11 +941,11 @@ static int select_bus_fmt_recursive(struct drm_bridge *first_bridge,
 {
 	unsigned int i, num_in_bus_fmts = 0;
 	struct drm_bridge_state *cur_state;
-	struct drm_bridge *prev_bridge;
+	struct drm_bridge *prev_bridge __free(drm_bridge_put) =
+		drm_bridge_get_prev_bridge(cur_bridge);
 	u32 *in_bus_fmts;
 	int ret;
 
-	prev_bridge = drm_bridge_get_prev_bridge(cur_bridge);
 	cur_state = drm_atomic_get_new_bridge_state(crtc_state->state,
 						    cur_bridge);
 

-- 
2.50.0

[PATCH 3/3] drm/bridge: display-connector: put the bridge obtained by drm_bridge_get_prev_bridge()

[Luca Ceresoli]

The bridge returned by drm_bridge_get_prev_bridge() is refcounted. Put it
when done.

To keep the code clean and future-proof use a scope-based free.

Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
---
 drivers/gpu/drm/bridge/display-connector.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/bridge/display-connector.c b/drivers/gpu/drm/bridge/display-connector.c
index badd2c7f91a186e9a47c5a4ddc870d269f3798ab..b33f67d1ce49a8bd0b0ac3f13d15d332d9c95cf1 100644
--- a/drivers/gpu/drm/bridge/display-connector.c
+++ b/drivers/gpu/drm/bridge/display-connector.c
@@ -103,7 +103,7 @@ static u32 *display_connector_get_output_bus_fmts(struct drm_bridge *bridge,
 					struct drm_connector_state *conn_state,
 					unsigned int *num_output_fmts)
 {
-	struct drm_bridge *prev_bridge = drm_bridge_get_prev_bridge(bridge);
+	struct drm_bridge *prev_bridge __free(drm_bridge_put) = drm_bridge_get_prev_bridge(bridge);
 	struct drm_bridge_state *prev_bridge_state;
 
 	if (!prev_bridge || !prev_bridge->funcs->atomic_get_output_bus_fmts) {
@@ -146,7 +146,7 @@ static u32 *display_connector_get_input_bus_fmts(struct drm_bridge *bridge,
 					u32 output_fmt,
 					unsigned int *num_input_fmts)
 {
-	struct drm_bridge *prev_bridge = drm_bridge_get_prev_bridge(bridge);
+	struct drm_bridge *prev_bridge __free(drm_bridge_put) = drm_bridge_get_prev_bridge(bridge);
 	struct drm_bridge_state *prev_bridge_state;
 
 	if (!prev_bridge || !prev_bridge->funcs->atomic_get_input_bus_fmts) {

-- 
2.50.0

Re: [PATCH 1/3] drm/bridge: get the bridge returned by drm_bridge_get_prev_bridge()

[Maxime Ripard]

On Wed, 9 Jul 2025 17:59:37 +0200, Luca Ceresoli wrote:
> drm_bridge_get_prev_bridge() returns a bridge pointer that the
> caller could hold for a long time. Increment the refcount of the returned
> bridge and document it must be put by the caller.
> 
> Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
> 
> [ ... ]

Reviewed-by: Maxime Ripard <mripard@kernel.org>

Thanks!
Maxime

Re: [PATCH 2/3] drm/bridge: select_bus_fmt_recursive(): put the bridge obtained by drm_bridge_get_prev_bridge()

[Maxime Ripard]

On Wed, 9 Jul 2025 17:59:38 +0200, Luca Ceresoli wrote:
> The bridge returned by drm_bridge_get_prev_bridge() is refcounted. Put it
> when done.
> 
> select_bus_fmt_recursive() has several return points, and ensuring
> drm_bridge_put() is always called in the right place would be error-prone
> 
> [ ... ]

Reviewed-by: Maxime Ripard <mripard@kernel.org>

Thanks!
Maxime

Re: [PATCH 3/3] drm/bridge: display-connector: put the bridge obtained by drm_bridge_get_prev_bridge()

[Maxime Ripard]

On Wed, 9 Jul 2025 17:59:39 +0200, Luca Ceresoli wrote:
> The bridge returned by drm_bridge_get_prev_bridge() is refcounted. Put it
> when done.
> 
> To keep the code clean and future-proof use a scope-based free.
> 
> 
> [ ... ]

Reviewed-by: Maxime Ripard <mripard@kernel.org>

Thanks!
Maxime

Re: [PATCH 0/3] drm/bridge: get/put the bridge returned by drm_bridge_get_prev_bridge()

[Luca Ceresoli]

On Wed, 09 Jul 2025 17:59:36 +0200, Luca Ceresoli wrote:
> This series adds drm_bridge_get/put() calls for DRM bridges returned by
> drm_bridge_get_prev_bridge().
> 
> 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):
> 
> [...]

Applied, thanks!

[1/3] drm/bridge: get the bridge returned by drm_bridge_get_prev_bridge()
      commit: 9b75346e3c2b8ecb5b90b132c2fc185ddd30ecf3
[2/3] drm/bridge: select_bus_fmt_recursive(): put the bridge obtained by drm_bridge_get_prev_bridge()
      commit: d4eecb4c24dc160f4a003c804602c746fb8fec58
[3/3] drm/bridge: display-connector: put the bridge obtained by drm_bridge_get_prev_bridge()
      commit: c571cb70e1ed43ee543c70151e61a001ab2eefa2

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