Message-Id: <20241231-hotplug-drm-bridge-v5-3-173065a1ece1@bootlin.com>
Date: Tue, 31 Dec 2024 11:39:57 +0100
From: Luca Ceresoli <luca.ceresoli@...tlin.com>
To: Simona Vetter <simona@...ll.ch>, Inki Dae <inki.dae@...sung.com>, 
 Jagan Teki <jagan@...rulasolutions.com>, 
 Marek Szyprowski <m.szyprowski@...sung.com>, 
 Catalin Marinas <catalin.marinas@....com>, Will Deacon <will@...nel.org>, 
 Shawn Guo <shawnguo@...nel.org>, Sascha Hauer <s.hauer@...gutronix.de>, 
 Pengutronix Kernel Team <kernel@...gutronix.de>, 
 Fabio Estevam <festevam@...il.com>, Daniel Thompson <danielt@...nel.org>, 
 Andrzej Hajda <andrzej.hajda@...el.com>, Jonathan Corbet <corbet@....net>
Cc: Paul Kocialkowski <contact@...lk.fr>, 
 Maxime Ripard <mripard@...nel.org>, 
 Dmitry Baryshkov <dmitry.baryshkov@...aro.org>, 
 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>, 
 Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>, 
 Thomas Zimmermann <tzimmermann@...e.de>, David Airlie <airlied@...il.com>, 
 Hervé Codina <herve.codina@...tlin.com>, 
 Thomas Petazzoni <thomas.petazzoni@...tlin.com>, 
 linux-kernel@...r.kernel.org, dri-devel@...ts.freedesktop.org, 
 linux-doc@...r.kernel.org, 
 Paul Kocialkowski <paul.kocialkowski@...tlin.com>, 
 Luca Ceresoli <luca.ceresoli@...tlin.com>
Subject: [PATCH v5 03/10] drm/bridge: add support for refcounted DRM
 bridges

DRM bridges are currently considered as a fixed element of a DRM card, and
thus their lifetime is assumed to extend for as long as the card
exists. New use cases, such as hot-pluggable hardware with video bridges,
require DRM bridges to be added and removed to a DRM card without tearing
the card down. This is possible for connectors already (used by DP MST), so
add this possibility to DRM bridges as well.

Implementation is based on drm_connector_init() as far as it makes sense,
and differs when it doesn't. A difference is that bridges are not exposed
to userspace,hence struct drm_bridge does not embed a struct
drm_mode_object which would provide the refcount and the free_cb. So here
we add to struct drm_bridge just the refcount and free_cb fields (we don't
need other struct drm_mode_object fields here) and instead of using the
drm_mode_object_*() functions we reimplement from those functions the few
lines that drm_bridge needs for refcounting.

The function to enroll a private bridge driver data structure into
refcounting is based on drm_connector_init() and so called
drm_bridge_init() for symmetry, even though it does not initialize anything
except the refcounting and the funcs pointer which is needed to access
funcs->destroy.

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

---

This patch was added in v5.
---
 drivers/gpu/drm/drm_bridge.c |  87 ++++++++++++++++++++++++++++++++++++
 include/drm/drm_bridge.h     | 102 +++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 189 insertions(+)

diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
index b1f0d25d55e23000521ac2ac37ee410348978ed4..6255ef59f73d8041a8cb7f2c6e23e5a67d1ae926 100644
--- a/drivers/gpu/drm/drm_bridge.c
+++ b/drivers/gpu/drm/drm_bridge.c
@@ -198,6 +198,85 @@
 static DEFINE_MUTEX(bridge_lock);
 static LIST_HEAD(bridge_list);
 
+static void drm_bridge_put_void(void *data)
+{
+	struct drm_bridge *bridge = (struct drm_bridge *)data;
+
+	drm_bridge_put(bridge);
+}
+
+static void drm_bridge_free(struct kref *kref)
+{
+	struct drm_bridge *bridge = container_of(kref, struct drm_bridge, refcount);
+
+	DRM_DEBUG("bridge=%p\n", bridge);
+
+	WARN_ON(!bridge->funcs->destroy);
+
+	if (bridge->funcs->destroy)
+		bridge->funcs->destroy(bridge);
+}
+
+/**
+ * drm_bridge_init - Initialize bridge and move private driver data
+ *                   lifetime management to the DRM bridge core
+ *
+ * @dev: struct device of the device whose removal shall trigger deallocation
+ * @bridge: the bridge to initialize
+ * @funcs: funcs structure for @bridge, which must have a valid .destroy func
+ *
+ * Takes over lifetime of a private bridge driver struct which embeds a
+ * struct drm_bridge. To be called by bridge drivers just after having
+ * allocated such a private structure. Initializes refcount to 1 and
+ * installs a callback which will call funcs->destroy when refcount drops
+ * to zero.
+ *
+ * After calling this function a bridge becomes a bridge with dynamic
+ * lifetime (aka a refcounted bridge).
+ *
+ * Drivers calling this function:
+ *  - must not allocate the private structure using devm_*() functions
+ *  - must not deallocate the private structure on device removal
+ *  - must deallocate the private structure in funcs->destroy
+ *
+ * Drivers not calling this function:
+ *  - must take care of freeing their private structure either by allocating
+ *    it using devm_*() functions or free it explicitly on device removal
+ *    using kfree()
+ *  - must set funcs->destroy to NULL
+ *
+ * On failure, calls funcs->destroy, thus the caller does not need to free
+ * the driver private struct in case of error.
+ *
+ * Returns:
+ * Zero on success, error code on failure.
+ */
+int drm_bridge_init(struct device *dev,
+		    struct drm_bridge *bridge,
+		    const struct drm_bridge_funcs *funcs)
+{
+	int err;
+
+	DRM_DEBUG("bridge=%p, funcs=%ps\n", bridge, funcs);
+
+	if (!(funcs && funcs->destroy)) {
+		dev_warn(dev, "Missing funcs or destroy func pointer\n");
+		return -EINVAL;
+	}
+
+	bridge->free_cb = drm_bridge_free;
+	kref_init(&bridge->refcount);
+
+	err = devm_add_action_or_reset(dev, drm_bridge_put_void, bridge);
+	if (err)
+		return err;
+
+	bridge->funcs = funcs;
+
+	return 0;
+}
+EXPORT_SYMBOL(drm_bridge_init);
+
 /**
  * drm_bridge_add - add the given bridge to the global bridge list
  *
@@ -207,6 +286,10 @@ void drm_bridge_add(struct drm_bridge *bridge)
 {
 	struct drm_bridge *br, *tmp;
 
+	DRM_DEBUG("bridge=%p\n", bridge);
+
+	drm_bridge_get(bridge);
+
 	mutex_init(&bridge->hpd_mutex);
 
 	list_for_each_entry_safe(br, tmp, &bridge_list, list)
@@ -251,6 +334,8 @@ void drm_bridge_remove(struct drm_bridge *bridge)
 {
 	struct drm_bridge *br, *tmp;
 
+	DRM_DEBUG("bridge=%p\n", bridge);
+
 	mutex_lock(&bridge_lock);
 	list_del_init(&bridge->list);
 	mutex_unlock(&bridge_lock);
@@ -260,6 +345,8 @@ void drm_bridge_remove(struct drm_bridge *bridge)
 			br->funcs->bridge_event_notify(br, DRM_EVENT_BRIDGE_REMOVE, bridge);
 
 	mutex_destroy(&bridge->hpd_mutex);
+
+	drm_bridge_put(bridge);
 }
 EXPORT_SYMBOL(drm_bridge_remove);
 
diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
index 6976bd842cedf9ce06abfb7306e7a3b4915f0378..a548a6acb02e3d70c8e34de965f648320420a7d5 100644
--- a/include/drm/drm_bridge.h
+++ b/include/drm/drm_bridge.h
@@ -31,6 +31,7 @@
 #include <drm/drm_encoder.h>
 #include <drm/drm_mode_object.h>
 #include <drm/drm_modes.h>
+#include <drm/drm_print.h>
 
 struct device_node;
 
@@ -89,6 +90,18 @@ struct drm_bridge_funcs {
 	 */
 	void (*detach)(struct drm_bridge *bridge);
 
+	/**
+	 * @destroy:
+	 *
+	 * Clean up bridge resources for bridges with dynamic
+	 * lifetime. This is called when the bridge refcount drops to
+	 * zero. It is never called for bridges without dynamic lifetime.
+	 *
+	 * See drm_bridge_init() for info about bridges with dynamic
+	 * lifetime.
+	 */
+	void (*destroy)(struct drm_bridge *bridge);
+
 	/**
 	 * @mode_valid:
 	 *
@@ -810,6 +823,18 @@ struct drm_bridge {
 	const struct drm_bridge_timings *timings;
 	/** @funcs: control functions */
 	const struct drm_bridge_funcs *funcs;
+
+	/**
+	 * @refcount: reference count for bridges with dynamic lifetime
+	 * (see drm_bridge_init)
+	 */
+	struct kref refcount;
+	/**
+	 * @free_cb: free function callback, only set for bridges with
+	 * dynamic lifetime
+	 */
+	void (*free_cb)(struct kref *kref);
+
 	/** @driver_private: pointer to the bridge driver's internal context */
 	void *driver_private;
 	/** @ops: bitmask of operations supported by the bridge */
@@ -890,6 +915,83 @@ drm_priv_to_bridge(struct drm_private_obj *priv)
 	return container_of(priv, struct drm_bridge, base);
 }
 
+static inline bool drm_bridge_is_refcounted(struct drm_bridge *bridge)
+{
+	return bridge->free_cb;
+}
+
+/**
+ * drm_bridge_get - Acquire a bridge reference
+ * @bridge: DRM bridge
+ *
+ * This function increments the bridge's refcount.
+ *
+ * It does nothing on non-refcounted bridges. See drm_bridge_init().
+ */
+static inline void drm_bridge_get(struct drm_bridge *bridge)
+{
+	if (!drm_bridge_is_refcounted(bridge))
+		return;
+
+	DRM_DEBUG("bridge=%p GET\n", bridge);
+
+	kref_get(&bridge->refcount);
+}
+
+/**
+ * drm_bridge_put - Release a bridge reference
+ * @bridge: DRM bridge
+ *
+ * This function decrements the bridge's reference count and frees the
+ * object if the reference count drops to zero.
+ *
+ * It does nothing on non-refcounted bridges. See drm_bridge_init().
+ *
+ * See also drm_bridge_put_and_clear() which is more handy in many cases.
+ */
+static inline void drm_bridge_put(struct drm_bridge *bridge)
+{
+	if (!drm_bridge_is_refcounted(bridge))
+		return;
+
+	DRM_DEBUG("bridge=%p PUT\n", bridge);
+
+	kref_put(&bridge->refcount, bridge->free_cb);
+}
+
+/**
+ * drm_bridge_put_and_clear - Given a bridge pointer, clear the pointer
+ *                            then put the bridge
+ *
+ * @br_ptr: pointer to a struct drm_bridge (must be != NULL)
+ *
+ * Helper to put a DRM bridge (whose pointer is passed), but only after
+ * setting its pointer to NULL. Useful for drivers having struct drm_bridge
+ * pointers they need to dispose of, without leaving a use-after-free
+ * window where the pointed bridge might have been freed while still
+ * holding a pointer to it.
+ *
+ * For example a driver having this private struct::
+ *
+ *     struct my_bridge {
+ *         struct drm_bridge *remote_bridge;
+ *         ...
+ *     };
+ *
+ * can dispose of remote_bridge using::
+ *
+ *     drm_bridge_put_and_clear(my_bridge->remote_bridge);
+ */
+#define drm_bridge_put_and_clear(br_ptr) do { \
+	struct drm_bridge **brpp = &(br_ptr); \
+	struct drm_bridge *brp = *brpp; \
+	*brpp = NULL; \
+	drm_bridge_put(brp); \
+} while (0)
+
+int drm_bridge_init(struct device *dev,
+		    struct drm_bridge *bridge,
+		    const struct drm_bridge_funcs *funcs);
 void drm_bridge_add(struct drm_bridge *bridge);
 int devm_drm_bridge_add(struct device *dev, struct drm_bridge *bridge);
 void drm_bridge_remove(struct drm_bridge *bridge);

-- 
2.34.1

Powered by blists - more mailing lists