[PATCH v6 14/26] drm/bridge: add support for refcounted DRM bridges

Luca Ceresoli
2025-02-06T18:15:16Z

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. Instead we add to struct
drm_bridge a refcount field (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.

Also add a new devm_drm_bridge_alloc() macro to allocate a new refcounted
bridge.

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

---

Changes in v6:
- use drm_warn, not WARN_ON (Jani Nikula)
- Add devm_drm_bridge_alloc() to replace drm_bridge_init() (similar to
drmm_encoder_alloc)
- Remove .destroy func: deallocation is done via the struct offset
computed by the devm_drm_bridge_alloc() macro
- use fixed free callback, as the same callback is used in all cases
anyway (remove free_cb, add bool is_refcounted)
- add drm_bridge_get/put() to drm_bridge_attach/detach() (add the bridge
to a list)
- make some DRM_DEBUG() strings more informative

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

diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
index 1955a231378050abf1071d74a145831b425c47c5..f694b32ca59cb91c32846bc00b43360df41cc1ad 100644
--- a/drivers/gpu/drm/drm_bridge.c
+++ b/drivers/gpu/drm/drm_bridge.c
@@ -200,6 +200,57 @@
DEFINE_MUTEX(bridge_lock);
LIST_HEAD(bridge_list);

+/* Internal function (for refcounted bridges) */
+void __drm_bridge_free(struct kref *kref)
+{
+ struct drm_bridge *bridge = container_of(kref, struct drm_bridge, refcount);
+ void *container = ((void *)bridge) - bridge->container_offset;
+
+ DRM_DEBUG("bridge=%p, container=%p FREE\n", bridge, container);
+
+ kfree(container);
+}
+EXPORT_SYMBOL(__drm_bridge_free);
+
+static void drm_bridge_put_void(void *data)
+{
+ struct drm_bridge *bridge = (struct drm_bridge *)data;
+
+ drm_bridge_put(bridge);
+}
+
+void *__devm_drm_bridge_alloc(struct device *dev, size_t size, size_t offset,
+ const struct drm_bridge_funcs *funcs)
+{
+ void *container;
+ struct drm_bridge *bridge;
+ int err;
+
+ if (!funcs) {
+ dev_warn(dev, "Missing funcs pointer\n");
+ return ERR_PTR(-EINVAL);
+ }
+
+ container = kzalloc(size, GFP_KERNEL);
+ if (!container)
+ return ERR_PTR(-ENOMEM);
+
+ bridge = container + offset;
+ bridge->container_offset = offset;
+ bridge->funcs = funcs;
+ kref_init(&bridge->refcount);
+ bridge->is_refcounted = 1;
+
+ err = devm_add_action_or_reset(dev, drm_bridge_put_void, bridge);
+ if (err)
+ return ERR_PTR(err);
+
+ DRM_DEBUG("bridge=%p, container=%p, funcs=%ps ALLOC\n", bridge, container, funcs);
+
+ return container;
+}
+EXPORT_SYMBOL(__devm_drm_bridge_alloc);
+
/**
* drm_bridge_add - add the given bridge to the global bridge list
*
@@ -209,6 +260,10 @@ void drm_bridge_add(struct drm_bridge *bridge)
{
struct drm_bridge *br, *tmp;

+ DRM_DEBUG("bridge=%p ADD\n", bridge);
+
+ drm_bridge_get(bridge);
+
mutex_init(&bridge->hpd_mutex);

if (bridge->ops & DRM_BRIDGE_OP_HDMI)
@@ -257,6 +312,8 @@ void drm_bridge_remove(struct drm_bridge *bridge)
{
struct drm_bridge *br, *tmp;

+ DRM_DEBUG("bridge=%p REMOVE\n", bridge);
+
mutex_lock(&bridge_lock);
list_del_init(&bridge->list);
mutex_unlock(&bridge_lock);
@@ -266,6 +323,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);

@@ -326,11 +385,17 @@ int drm_bridge_attach(struct drm_encoder *encoder, struct drm_bridge *bridge,
if (!encoder || !bridge)
return -EINVAL;

- if (previous && (!previous->dev || previous->encoder != encoder))
- return -EINVAL;
+ drm_bridge_get(bridge);

- if (bridge->dev)
- return -EBUSY;
+ if (previous && (!previous->dev || previous->encoder != encoder)) {
+ ret = -EINVAL;
+ goto err_put_bridge;
+ }
+
+ if (bridge->dev) {
+ ret = -EBUSY;
+ goto err_put_bridge;
+ }

bridge->dev = encoder->dev;
bridge->encoder = encoder;
@@ -379,6 +444,8 @@ int drm_bridge_attach(struct drm_encoder *encoder, struct drm_bridge *bridge,
"failed to attach bridge %pOF to encoder %s\n",
bridge->of_node, encoder->name);

+err_put_bridge:
+ drm_bridge_put(bridge);
return ret;
}
EXPORT_SYMBOL(drm_bridge_attach);
@@ -399,6 +466,7 @@ void drm_bridge_detach(struct drm_bridge *bridge)

list_del(&bridge->chain_node);
bridge->dev = NULL;
+ drm_bridge_put(bridge);
}

/**
diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
index ad7ba444a13e5ecf16f996de3742e4ac67dc21f1..43cef0f6ccd36034f64ad2babfebea62db1d9e43 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;

@@ -863,6 +864,22 @@ struct drm_bridge {
const struct drm_bridge_timings *timings;
/** @funcs: control functions */
const struct drm_bridge_funcs *funcs;
+
+ /**
+ * @container_offset: Offset of this struct within the container
+ * struct embedding it. Used for refcounted bridges to free the
+ * embeddeing struct when the refcount drops to zero. Unused on
+ * legacy bridges.
+ */
+ size_t container_offset;
+ /**
+ * @refcount: reference count for bridges with dynamic lifetime
+ * (see drm_bridge_init)
+ */
+ struct kref refcount;
+ /** @is_refcounted: this bridge has dynamic lifetime management */
+ bool is_refcounted;
+
/** @driver_private: pointer to the bridge driver's internal context */
void *driver_private;
/** @ops: bitmask of operations supported by the bridge */
@@ -964,6 +981,106 @@ 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->is_refcounted;
+}
+
+void __drm_bridge_free(struct kref *kref);
+
+/**
+ * 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, __drm_bridge_free);
+}
+
+/**
+ * drm_bridge_put_and_clear - Given a bridge pointer, clear the pointer
+ * then put the bridge
+ *
+ * @bridge_pp: pointer to pointer to a struct drm_bridge
+ *
+ * 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);
+ */
+static inline void drm_bridge_put_and_clear(struct drm_bridge **bridge_pp)
+{
+ struct drm_bridge *bridge = *bridge_pp;
+
+ *bridge_pp = NULL;
+ drm_bridge_put(bridge);
+}
+
+void *__devm_drm_bridge_alloc(struct device *dev, size_t size, size_t offset,
+ const struct drm_bridge_funcs *funcs);
+
+/**
+ * devm_drm_bridge_alloc - Allocate and initialize an refcounted bridge
+ * @dev: struct device of the bridge device
+ * @type: the type of the struct which contains struct &drm_bridge
+ * @member: the name of the &drm_bridge within @type
+ * @funcs: callbacks for this bridge
+ *
+ * The bridge will have a dynamic lifetime (aka a refcounted bridge).
+ *
+ * The returned refcount is initialized to 1. This reference will be
+ * automatically dropped via devm (by calling drm_bridge_put()) when the
+ * device is removed.
+ *
+ * Returns:
+ * Pointer to new bridge, or ERR_PTR on failure.
+ */
+#define devm_drm_bridge_alloc(dev, type, member, funcs) \
+ ((type *)__devm_drm_bridge_alloc(dev, sizeof(type), \
+ offsetof(type, member), 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