[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <20250926-drm-bridge-alloc-encoder-chain-mutex-v1-4-23b62c47356a@bootlin.com>
Date: Fri, 26 Sep 2025 17:59:45 +0200
From: Luca Ceresoli <luca.ceresoli@...tlin.com>
To: Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
Maxime Ripard <mripard@...nel.org>, Thomas Zimmermann <tzimmermann@...e.de>,
David Airlie <airlied@...il.com>, Simona Vetter <simona@...ll.ch>,
Andrzej Hajda <andrzej.hajda@...el.com>,
Neil Armstrong <neil.armstrong@...aro.org>, Robert Foss <rfoss@...nel.org>,
Laurent Pinchart <Laurent.pinchart@...asonboard.com>,
Jonas Karlman <jonas@...boo.se>, Jernej Skrabec <jernej.skrabec@...il.com>
Cc: Hui Pu <Hui.Pu@...ealthcare.com>,
Thomas Petazzoni <thomas.petazzoni@...tlin.com>,
dri-devel@...ts.freedesktop.org, linux-kernel@...r.kernel.org,
Luca Ceresoli <luca.ceresoli@...tlin.com>
Subject: [PATCH 4/7] drm/bridge: lock the encoder chain in scoped for_each
loops
drm_for_each_bridge_in_chain_scoped() and
drm_for_each_bridge_in_chain_from() currently get/put the bridge at each
iteration. But they don't protect the encoder chain, so it could change
(bridges added/removed) while some code is iterating over the list
itself. To make iterations safe, change the logic of these for_each macros
to lock the encoder chain mutex at the beginning and unlock it at the end
of the loop (be it at the end of the list, or earlier due to a 'break'
statement).
Also remove the get/put on the current bridge because it is not needed
anymore. In fact all bridges in the encoder chain are refcounted already
thanks to the drm_bridge_get() in drm_bridge_attach() and the
drm_bridge_put() in drm_bridge_detach(). So while iterating with the mutex
held the list cannot change _and_ the refcount of all bridges in the list
cannot drop to zero.
Signed-off-by: Luca Ceresoli <luca.ceresoli@...tlin.com>
---
include/drm/drm_bridge.h | 62 ++++++++++++++++++++++++++----------------------
1 file changed, 33 insertions(+), 29 deletions(-)
diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
index 0ff7ab4aa8689a022458f935a7ffb23a2b715802..9b1aa1e29c8915648aef86ba9f8d85e843b22ca0 100644
--- a/include/drm/drm_bridge.h
+++ b/include/drm/drm_bridge.h
@@ -1440,26 +1440,29 @@ drm_bridge_chain_get_last_bridge(struct drm_encoder *encoder)
struct drm_bridge, chain_node));
}
-/**
- * drm_bridge_get_next_bridge_and_put - Get the next bridge in the chain
- * and put the previous
- * @bridge: bridge object
- *
- * Same as drm_bridge_get_next_bridge() but additionally puts the @bridge.
- *
- * RETURNS:
- * the next bridge in the chain after @bridge, or NULL if @bridge is the last.
- */
-static inline struct drm_bridge *
-drm_bridge_get_next_bridge_and_put(struct drm_bridge *bridge)
+static inline struct drm_bridge *drm_bridge_encoder_chain_lock(struct drm_bridge *bridge)
{
- struct drm_bridge *next = drm_bridge_get_next_bridge(bridge);
+ drm_encoder_chain_lock(bridge->encoder);
+
+ return bridge;
+}
- drm_bridge_put(bridge);
+/* Internal to drm_for_each_bridge_in_chain*() */
+static inline struct drm_bridge *__drm_encoder_bridge_chain_next(struct drm_bridge *bridge)
+{
+ if (list_is_last(&bridge->chain_node, &bridge->encoder->bridge_chain)) {
+ drm_encoder_chain_unlock(bridge->encoder);
- return next;
+ return NULL;
+ }
+
+ return list_next_entry(bridge, chain_node);
}
+/* Internal to drm_for_each_bridge_in_chain*() */
+DEFINE_FREE(drm_bridge_encoder_chain_unlock, struct drm_bridge *,
+ if (_T) drm_encoder_chain_unlock(_T->encoder);)
+
/**
* drm_for_each_bridge_in_chain_scoped - iterate over all bridges attached
* to an encoder
@@ -1469,14 +1472,15 @@ drm_bridge_get_next_bridge_and_put(struct drm_bridge *bridge)
*
* Iterate over all bridges present in the bridge chain attached to @encoder.
*
- * Automatically gets/puts the bridge reference while iterating, and puts
- * the reference even if returning or breaking in the middle of the loop.
+ * Automatically locks the encoder chain mutex to prevent chain
+ * modifications while iterating.
*/
-#define drm_for_each_bridge_in_chain_scoped(encoder, bridge) \
- for (struct drm_bridge *bridge __free(drm_bridge_put) = \
- drm_bridge_chain_get_first_bridge(encoder); \
- bridge; \
- bridge = drm_bridge_get_next_bridge_and_put(bridge))
+#define drm_for_each_bridge_in_chain_scoped(encoder, bridge) \
+ for (struct drm_bridge *bridge __free(drm_bridge_encoder_chain_unlock) = \
+ list_first_entry(&drm_encoder_chain_lock(encoder)->bridge_chain, \
+ struct drm_bridge, chain_node); \
+ bridge; \
+ bridge = __drm_encoder_bridge_chain_next(bridge)) \
/**
* drm_for_each_bridge_in_chain_from - iterate over all bridges starting
@@ -1488,14 +1492,14 @@ drm_bridge_get_next_bridge_and_put(struct drm_bridge *bridge)
* Iterate over all bridges in the encoder chain starting from
* @first_bridge, included.
*
- * Automatically gets/puts the bridge reference while iterating, and puts
- * the reference even if returning or breaking in the middle of the loop.
+ * Automatically locks the encoder chain mutex to prevent chain
+ * modifications while iterating.
*/
-#define drm_for_each_bridge_in_chain_from(first_bridge, bridge) \
- for (struct drm_bridge *bridge __free(drm_bridge_put) = \
- drm_bridge_get(first_bridge); \
- bridge; \
- bridge = drm_bridge_get_next_bridge_and_put(bridge))
+#define drm_for_each_bridge_in_chain_from(first_bridge, bridge) \
+ for (struct drm_bridge *bridge __free(drm_bridge_encoder_chain_unlock) = \
+ drm_bridge_encoder_chain_lock(first_bridge); \
+ bridge; \
+ bridge = __drm_encoder_bridge_chain_next(bridge)) \
enum drm_mode_status
drm_bridge_chain_mode_valid(struct drm_bridge *bridge,
--
2.51.0
Powered by blists - more mailing lists