| Message ID | 20241231-hotplug-drm-bridge-v5-3-173065a1ece1@bootlin.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Add support for hot-pluggable DRM bridges | expand |
On Tue, 31 Dec 2024, Luca Ceresoli <luca.ceresoli@bootlin.com> 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 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@bootlin.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); Please don't add new DRM_DEBUG or WARN_ON where you can use the drm_dbg_* or drm_WARN_ON variants. > + > + 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);
Hello Jani, thanks for your review. On Tue, 31 Dec 2024 13:11:31 +0200 Jani Nikula <jani.nikula@linux.intel.com> wrote: > On Tue, 31 Dec 2024, Luca Ceresoli <luca.ceresoli@bootlin.com> 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 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@bootlin.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); > > Please don't add new DRM_DEBUG or WARN_ON where you can use the > drm_dbg_* or drm_WARN_ON variants. Good point. However drm_WARN_ON() cannot be used because it needs a non-NULL struct drm_drm_device pointer which is not always available here: in case of -EPROBE_DEFER it usually isn't. I guess I'll go for drm_dbg_core() or drm_warn[_once](), even though none of them prints a stack trace and I find that would be useful. This is raising a loosely-related question about the DRM_DEBUG()s this patch is adding, such as the one quoted above: would it make sense to add a new drm_debug_category value for the bridge refcounting functions? Or for bridges altogether? They are pretty different from the core messages, and it may be useful to see only the refcounting messages or only the core messages. DRM_UT_BRIDGE? DRM_UT_BRIDGE_REFCOUNT? Luca
On Thu, 02 Jan 2025, Luca Ceresoli <luca.ceresoli@bootlin.com> wrote: > Hello Jani, > > thanks for your review. > > On Tue, 31 Dec 2024 13:11:31 +0200 > Jani Nikula <jani.nikula@linux.intel.com> wrote: > >> On Tue, 31 Dec 2024, Luca Ceresoli <luca.ceresoli@bootlin.com> 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 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@bootlin.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); >> >> Please don't add new DRM_DEBUG or WARN_ON where you can use the >> drm_dbg_* or drm_WARN_ON variants. > > Good point. However drm_WARN_ON() cannot be used because it needs a > non-NULL struct drm_drm_device pointer which is not always available > here: in case of -EPROBE_DEFER it usually isn't. I guess I'll go for > drm_dbg_core() or drm_warn[_once](), even though none of them prints a > stack trace and I find that would be useful. drm_dbg_* can handle NULL drm device; maybe drm_WARN* should be modified to do so as well? > This is raising a loosely-related question about the DRM_DEBUG()s this > patch is adding, such as the one quoted above: would it make sense to > add a new drm_debug_category value for the bridge refcounting > functions? Or for bridges altogether? They are pretty different from > the core messages, and it may be useful to see only the refcounting > messages or only the core messages. > > DRM_UT_BRIDGE? > DRM_UT_BRIDGE_REFCOUNT? IMO the biggest benefit of new categories would be for very noisy logging that you really only want enabled when debugging a specific issue. Otherwise, the hard part about adding new categories is their adoption. For example, we now have DRM_UT_DP, but it's only haphazardly used here and there. I don't really see a lot of point in having that separated from DRM_UT_KMS. When would you want one but not the other? How would you go about converting some KMS to DP logging, and why? What's special about DP, why don't we have an HDMI category? Etc. OTOH, DRM_UT_DP is also used for DP AUX transfer debug logging. I think that would've been a good category on its own: Do you want noisy logging about DPCD access or not? But not used for anything else. Oh, having written the above, I looked up a18b21929453 ("drm/dp_helper: Add DP aux channel tracing"). DRM_UT_DP *was* intended only for DP AUX message tracing, but its naming unfortunately suggests a broader category, and here we are. BR, Jani.
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);
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@bootlin.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(+)