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 to and removed from a DRM card without
tearing the card down. This is possible for connectors already (used by DP
MST), it is now needed for DRM bridges as well.

As a first preliminary step, make bridges reference-counted to allow a
struct drm_bridge (along with the private driver structure embedding it) to
stay allocated even after the driver has been removed, until the last
reference is put.

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

---

Changes in v7:
 - export drm_bridge_put_void
 - struct drm_bridge: use container pointer instead of container_offset
 - remove drm_bridge_is_refcounted()
 - remove all DRM_DEBUG()s
 - drm_bridge_get/put: accept NULL pointer and return the bridge pointer to
   allow pass-through calls
 - extract to separate patches:
    - the addition of drm_bridge_alloc
    - the addition of drm_bridge_get/put() to drm_bridge_add/remove()
    - the addition of drm_bridge_get/put() to drm_bridge_attach/detach()
 - fix a typo, slightly improve kerneldoc

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 | 33 +++++++++++++++-
 include/drm/drm_bridge.h     | 91 ++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 123 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
index 96df717b2caeb41d45346ded576eaeb2806fd051..2ba0dac9bfc2dfd709d5e2457d69067c7324972c 100644
--- a/drivers/gpu/drm/drm_bridge.c
+++ b/drivers/gpu/drm/drm_bridge.c
@@ -199,23 +199,54 @@
 static DEFINE_MUTEX(bridge_lock);
 static LIST_HEAD(bridge_list);
 
+void __drm_bridge_free(struct kref *kref)
+{
+	struct drm_bridge *bridge = container_of(kref, struct drm_bridge, refcount);
+
+	kfree(bridge->container);
+}
+EXPORT_SYMBOL(__drm_bridge_free);
+
+/**
+ * drm_bridge_put_void - wrapper to drm_bridge_put() taking a void pointer
+ *
+ * @data: pointer to @struct drm_bridge, cast to a void pointer
+ *
+ * Wrapper of drm_bridge_put() to be used when a function taking a void
+ * pointer is needed, for example as a devm action.
+ */
+void drm_bridge_put_void(void *data)
+{
+	struct drm_bridge *bridge = (struct drm_bridge *)data;
+
+	drm_bridge_put(bridge);
+}
+EXPORT_SYMBOL(drm_bridge_put_void);
+
 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 = devm_kzalloc(dev, size, GFP_KERNEL);
+	container = kzalloc(size, GFP_KERNEL);
 	if (!container)
 		return ERR_PTR(-ENOMEM);
 
 	bridge = container + offset;
+	bridge->container = container;
 	bridge->funcs = funcs;
+	kref_init(&bridge->refcount);
+
+	err = devm_add_action_or_reset(dev, drm_bridge_put_void, bridge);
+	if (err)
+		return ERR_PTR(err);
 
 	return container;
 }
diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
index dae463b30542d586a595b67f7bdf5a5e898e9572..5c1e2b9cafb12eb429d1f5d3ef312e6cf9b54f47 100644
--- a/include/drm/drm_bridge.h
+++ b/include/drm/drm_bridge.h
@@ -840,6 +840,17 @@ struct drm_bridge {
 	const struct drm_bridge_timings *timings;
 	/** @funcs: control functions */
 	const struct drm_bridge_funcs *funcs;
+
+	/**
+	 * @container: Pointer to the private driver struct embedding this
+	 * @struct drm_bridge.
+	 */
+	void *container;
+	/**
+	 * @refcount: reference count of users referencing this bridge.
+	 */
+	struct kref refcount;
+
 	/** @driver_private: pointer to the bridge driver's internal context */
 	void *driver_private;
 	/** @ops: bitmask of operations supported by the bridge */
@@ -941,6 +952,82 @@ drm_priv_to_bridge(struct drm_private_obj *priv)
 	return container_of(priv, struct drm_bridge, base);
 }
 
+void __drm_bridge_free(struct kref *kref);
+
+/**
+ * drm_bridge_get - Acquire a bridge reference
+ * @bridge: DRM bridge
+ *
+ * This function increments the bridge's refcount.
+ *
+ * Returns:
+ * Pointer to @bridge.
+ */
+static inline struct drm_bridge *drm_bridge_get(struct drm_bridge *bridge)
+{
+	if (!bridge)
+		return bridge;
+
+	kref_get(&bridge->refcount);
+
+	return bridge;
+}
+
+/**
+ * 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.
+ *
+ * See also drm_bridge_put_and_clear() which is more handy in many cases.
+ *
+ * Returns:
+ * Pointer to @bridge.
+ */
+static inline struct drm_bridge *drm_bridge_put(struct drm_bridge *bridge)
+{
+	if (!bridge)
+		return bridge;
+
+	kref_put(&bridge->refcount, __drm_bridge_free);
+
+	return bridge;
+}
+
+void drm_bridge_put_void(void *data);
+
+/**
+ * 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);
 
@@ -951,6 +1038,10 @@ void *__devm_drm_bridge_alloc(struct device *dev, size_t size, size_t offset,
  * @member: the name of the &drm_bridge within @type
  * @funcs: callbacks for this bridge
  *
+ * The returned refcount is initialized to 1. This reference will be
+ * automatically dropped via devm (by calling drm_bridge_put()) when @dev
+ * is removed.
+ *
  * Returns:
  * Pointer to new bridge, or ERR_PTR on failure.
  */

-- 
2.48.1

On Fri, Mar 14, 2025 at 11:31:15AM +0100, Luca Ceresoli wrote:
> 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 to and removed from a DRM card without
> tearing the card down. This is possible for connectors already (used by DP
> MST), it is now needed for DRM bridges as well.
> 
> As a first preliminary step, make bridges reference-counted to allow a
> struct drm_bridge (along with the private driver structure embedding it) to
> stay allocated even after the driver has been removed, until the last
> reference is put.
> 
> Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
> 
> ---
> 
> Changes in v7:
>  - export drm_bridge_put_void
>  - struct drm_bridge: use container pointer instead of container_offset
>  - remove drm_bridge_is_refcounted()
>  - remove all DRM_DEBUG()s
>  - drm_bridge_get/put: accept NULL pointer and return the bridge pointer to
>    allow pass-through calls
>  - extract to separate patches:
>     - the addition of drm_bridge_alloc
>     - the addition of drm_bridge_get/put() to drm_bridge_add/remove()
>     - the addition of drm_bridge_get/put() to drm_bridge_attach/detach()
>  - fix a typo, slightly improve kerneldoc
> 
> 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 | 33 +++++++++++++++-
>  include/drm/drm_bridge.h     | 91 ++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 123 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
> index 96df717b2caeb41d45346ded576eaeb2806fd051..2ba0dac9bfc2dfd709d5e2457d69067c7324972c 100644
> --- a/drivers/gpu/drm/drm_bridge.c
> +++ b/drivers/gpu/drm/drm_bridge.c
> @@ -199,23 +199,54 @@
>  static DEFINE_MUTEX(bridge_lock);
>  static LIST_HEAD(bridge_list);
>  
> +void __drm_bridge_free(struct kref *kref)
> +{
> +	struct drm_bridge *bridge = container_of(kref, struct drm_bridge, refcount);
> +
> +	kfree(bridge->container);
> +}
> +EXPORT_SYMBOL(__drm_bridge_free);
> +
> +/**
> + * drm_bridge_put_void - wrapper to drm_bridge_put() taking a void pointer
> + *
> + * @data: pointer to @struct drm_bridge, cast to a void pointer
> + *
> + * Wrapper of drm_bridge_put() to be used when a function taking a void
> + * pointer is needed, for example as a devm action.
> + */
> +void drm_bridge_put_void(void *data)
> +{
> +	struct drm_bridge *bridge = (struct drm_bridge *)data;
> +
> +	drm_bridge_put(bridge);
> +}
> +EXPORT_SYMBOL(drm_bridge_put_void);
> +
>  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 = devm_kzalloc(dev, size, GFP_KERNEL);
> +	container = kzalloc(size, GFP_KERNEL);
>  	if (!container)
>  		return ERR_PTR(-ENOMEM);
>  
>  	bridge = container + offset;
> +	bridge->container = container;
>  	bridge->funcs = funcs;
> +	kref_init(&bridge->refcount);
> +
> +	err = devm_add_action_or_reset(dev, drm_bridge_put_void, bridge);
> +	if (err)
> +		return ERR_PTR(err);
>  
>  	return container;
>  }
> diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
> index dae463b30542d586a595b67f7bdf5a5e898e9572..5c1e2b9cafb12eb429d1f5d3ef312e6cf9b54f47 100644
> --- a/include/drm/drm_bridge.h
> +++ b/include/drm/drm_bridge.h
> @@ -840,6 +840,17 @@ struct drm_bridge {
>  	const struct drm_bridge_timings *timings;
>  	/** @funcs: control functions */
>  	const struct drm_bridge_funcs *funcs;
> +
> +	/**
> +	 * @container: Pointer to the private driver struct embedding this
> +	 * @struct drm_bridge.
> +	 */
> +	void *container;

newline here

> +	/**
> +	 * @refcount: reference count of users referencing this bridge.
> +	 */
> +	struct kref refcount;
> +
>  	/** @driver_private: pointer to the bridge driver's internal context */
>  	void *driver_private;
>  	/** @ops: bitmask of operations supported by the bridge */
> @@ -941,6 +952,82 @@ drm_priv_to_bridge(struct drm_private_obj *priv)
>  	return container_of(priv, struct drm_bridge, base);
>  }
>  
> +void __drm_bridge_free(struct kref *kref);
> +
> +/**
> + * drm_bridge_get - Acquire a bridge reference
> + * @bridge: DRM bridge
> + *
> + * This function increments the bridge's refcount.
> + *
> + * Returns:
> + * Pointer to @bridge.
> + */
> +static inline struct drm_bridge *drm_bridge_get(struct drm_bridge *bridge)
> +{
> +	if (!bridge)
> +		return bridge;
> +
> +	kref_get(&bridge->refcount);
> +
> +	return bridge;
> +}
> +
> +/**
> + * 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.
> + *
> + * See also drm_bridge_put_and_clear() which is more handy in many cases.
> + *
> + * Returns:
> + * Pointer to @bridge.
> + */
> +static inline struct drm_bridge *drm_bridge_put(struct drm_bridge *bridge)
> +{
> +	if (!bridge)
> +		return bridge;
> +
> +	kref_put(&bridge->refcount, __drm_bridge_free);
> +
> +	return bridge;
> +}

I'm not sure we discussed it already, but why do you need to put both
drm_bridge_get and drm_bridge_put into the header, and thus export
__drm_bridge_free?

I'd expect both to be in drm_bridge.c?

> +
> +void drm_bridge_put_void(void *data);
> +
> +/**
> + * 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);
> +}

I'm not convinced we need that one for now, and it doesn't look like
there's a user for it in your series, so I'd rather introduce it once we
actually need it.

>  void *__devm_drm_bridge_alloc(struct device *dev, size_t size, size_t offset,
>  			      const struct drm_bridge_funcs *funcs);
>  
> @@ -951,6 +1038,10 @@ void *__devm_drm_bridge_alloc(struct device *dev, size_t size, size_t offset,
>   * @member: the name of the &drm_bridge within @type
>   * @funcs: callbacks for this bridge
>   *
> + * The returned refcount is initialized to 1.

I'm confused, there's no returned refcount here? Or did you mean the
returned bridge refcount?

Maxime

Hello Maxime,

thanks for the very prompt feedback!

On Fri, 14 Mar 2025 19:04:40 +0100
Maxime Ripard <mripard@kernel.org> wrote:

> On Fri, Mar 14, 2025 at 11:31:15AM +0100, Luca Ceresoli wrote:
> > 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 to and removed from a DRM card without
> > tearing the card down. This is possible for connectors already (used by DP
> > MST), it is now needed for DRM bridges as well.
> > 
> > As a first preliminary step, make bridges reference-counted to allow a
> > struct drm_bridge (along with the private driver structure embedding it) to
> > stay allocated even after the driver has been removed, until the last
> > reference is put.
> > 
> > Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
> > 
> > ---
> > 
> > Changes in v7:
> >  - export drm_bridge_put_void
> >  - struct drm_bridge: use container pointer instead of container_offset
> >  - remove drm_bridge_is_refcounted()
> >  - remove all DRM_DEBUG()s
> >  - drm_bridge_get/put: accept NULL pointer and return the bridge pointer to
> >    allow pass-through calls
> >  - extract to separate patches:
> >     - the addition of drm_bridge_alloc
> >     - the addition of drm_bridge_get/put() to drm_bridge_add/remove()
> >     - the addition of drm_bridge_get/put() to drm_bridge_attach/detach()
> >  - fix a typo, slightly improve kerneldoc
> > 
> > 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 | 33 +++++++++++++++-
> >  include/drm/drm_bridge.h     | 91 ++++++++++++++++++++++++++++++++++++++++++++
> >  2 files changed, 123 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
> > index 96df717b2caeb41d45346ded576eaeb2806fd051..2ba0dac9bfc2dfd709d5e2457d69067c7324972c 100644
> > --- a/drivers/gpu/drm/drm_bridge.c
> > +++ b/drivers/gpu/drm/drm_bridge.c
> > @@ -199,23 +199,54 @@
> >  static DEFINE_MUTEX(bridge_lock);
> >  static LIST_HEAD(bridge_list);
> >  
> > +void __drm_bridge_free(struct kref *kref)
> > +{
> > +	struct drm_bridge *bridge = container_of(kref, struct drm_bridge, refcount);
> > +
> > +	kfree(bridge->container);
> > +}
> > +EXPORT_SYMBOL(__drm_bridge_free);
> > +
> > +/**
> > + * drm_bridge_put_void - wrapper to drm_bridge_put() taking a void pointer
> > + *
> > + * @data: pointer to @struct drm_bridge, cast to a void pointer
> > + *
> > + * Wrapper of drm_bridge_put() to be used when a function taking a void
> > + * pointer is needed, for example as a devm action.
> > + */
> > +void drm_bridge_put_void(void *data)
> > +{
> > +	struct drm_bridge *bridge = (struct drm_bridge *)data;
> > +
> > +	drm_bridge_put(bridge);
> > +}
> > +EXPORT_SYMBOL(drm_bridge_put_void);
> > +
> >  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 = devm_kzalloc(dev, size, GFP_KERNEL);
> > +	container = kzalloc(size, GFP_KERNEL);
> >  	if (!container)
> >  		return ERR_PTR(-ENOMEM);
> >  
> >  	bridge = container + offset;
> > +	bridge->container = container;
> >  	bridge->funcs = funcs;
> > +	kref_init(&bridge->refcount);
> > +
> > +	err = devm_add_action_or_reset(dev, drm_bridge_put_void, bridge);
> > +	if (err)
> > +		return ERR_PTR(err);
> >  
> >  	return container;
> >  }
> > diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
> > index dae463b30542d586a595b67f7bdf5a5e898e9572..5c1e2b9cafb12eb429d1f5d3ef312e6cf9b54f47 100644
> > --- a/include/drm/drm_bridge.h
> > +++ b/include/drm/drm_bridge.h
> > @@ -840,6 +840,17 @@ struct drm_bridge {
> >  	const struct drm_bridge_timings *timings;
> >  	/** @funcs: control functions */
> >  	const struct drm_bridge_funcs *funcs;
> > +
> > +	/**
> > +	 * @container: Pointer to the private driver struct embedding this
> > +	 * @struct drm_bridge.
> > +	 */
> > +	void *container;  
> 
> newline here
> 
> > +	/**
> > +	 * @refcount: reference count of users referencing this bridge.
> > +	 */
> > +	struct kref refcount;
> > +
> >  	/** @driver_private: pointer to the bridge driver's internal context */
> >  	void *driver_private;
> >  	/** @ops: bitmask of operations supported by the bridge */
> > @@ -941,6 +952,82 @@ drm_priv_to_bridge(struct drm_private_obj *priv)
> >  	return container_of(priv, struct drm_bridge, base);
> >  }
> >  
> > +void __drm_bridge_free(struct kref *kref);
> > +
> > +/**
> > + * drm_bridge_get - Acquire a bridge reference
> > + * @bridge: DRM bridge
> > + *
> > + * This function increments the bridge's refcount.
> > + *
> > + * Returns:
> > + * Pointer to @bridge.
> > + */
> > +static inline struct drm_bridge *drm_bridge_get(struct drm_bridge *bridge)
> > +{
> > +	if (!bridge)
> > +		return bridge;
> > +
> > +	kref_get(&bridge->refcount);
> > +
> > +	return bridge;
> > +}
> > +
> > +/**
> > + * 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.
> > + *
> > + * See also drm_bridge_put_and_clear() which is more handy in many cases.
> > + *
> > + * Returns:
> > + * Pointer to @bridge.
> > + */
> > +static inline struct drm_bridge *drm_bridge_put(struct drm_bridge *bridge)
> > +{
> > +	if (!bridge)
> > +		return bridge;
> > +
> > +	kref_put(&bridge->refcount, __drm_bridge_free);
> > +
> > +	return bridge;
> > +}  
> 
> I'm not sure we discussed it already, but why do you need to put both
> drm_bridge_get and drm_bridge_put into the header, and thus export
> __drm_bridge_free?
> 
> I'd expect both to be in drm_bridge.c?

I don't think we had discussed this before. There is no strong reason,
just a few small ones, and one is related to the DRM_DEBUG() calls that
I have removed from this series, so it's fine to move back
drm_bridge_get/put() in the .c file, and make __drm_bridge_free()
private again.

> > +/**
> > + * 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);
> > +}  
> 
> I'm not convinced we need that one for now, and it doesn't look like
> there's a user for it in your series, so I'd rather introduce it once we
> actually need it.

Indeed, that's a leftover I missed when extracting this series for the
branch with the entire work. I'm definitely removing it in the next
iteration.

> >  void *__devm_drm_bridge_alloc(struct device *dev, size_t size, size_t offset,
> >  			      const struct drm_bridge_funcs *funcs);
> >  
> > @@ -951,6 +1038,10 @@ void *__devm_drm_bridge_alloc(struct device *dev, size_t size, size_t offset,
> >   * @member: the name of the &drm_bridge within @type
> >   * @funcs: callbacks for this bridge
> >   *
> > + * The returned refcount is initialized to 1.  
> 
> I'm confused, there's no returned refcount here? Or did you mean the
> returned bridge refcount?

Indeed, good catch!

Luca

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