On Thu, Feb 06, 2025 at 07:14:27PM +0100, Luca Ceresoli wrote:
> In preparation for allowing bridges to be added to and removed from a DRM
> card without destroying the whole card, add a new DRM bridge function
> called on addition and removal of bridges.
>
> Signed-off-by: Luca Ceresoli <luca.ceresoli@xxxxxxxxxxx>
>
> ---
>
> Changed in v6:
> - rebased fixing conflicts
>
> Changed in v5:
> - fixed kerneldoc errors
>
> This patch was added in v4.
> ---
> drivers/gpu/drm/drm_bridge.c | 12 ++++++++++++
> include/drm/drm_bridge.h | 23 +++++++++++++++++++++++
> 2 files changed, 35 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
> index b0834b8644284e5f7751cec81724af849b4180e7..1955a231378050abf1071d74a145831b425c47c5 100644
> --- a/drivers/gpu/drm/drm_bridge.c
> +++ b/drivers/gpu/drm/drm_bridge.c
> @@ -207,12 +207,18 @@ LIST_HEAD(bridge_list);
> */
> void drm_bridge_add(struct drm_bridge *bridge)
> {
> + struct drm_bridge *br, *tmp;
> +
> mutex_init(&bridge->hpd_mutex);
>
> if (bridge->ops & DRM_BRIDGE_OP_HDMI)
> bridge->ycbcr_420_allowed = !!(bridge->supported_formats &
> BIT(HDMI_COLORSPACE_YUV420));
>
> + list_for_each_entry_safe(br, tmp, &bridge_list, list)
> + if (br->funcs->bridge_event_notify)
> + br->funcs->bridge_event_notify(br, DRM_EVENT_BRIDGE_ADD, bridge);
> +
> mutex_lock(&bridge_lock);
> list_add_tail(&bridge->list, &bridge_list);
> mutex_unlock(&bridge_lock);
> @@ -249,10 +255,16 @@ EXPORT_SYMBOL(devm_drm_bridge_add);
> */
> void drm_bridge_remove(struct drm_bridge *bridge)
> {
> + struct drm_bridge *br, *tmp;
> +
> mutex_lock(&bridge_lock);
> list_del_init(&bridge->list);
> mutex_unlock(&bridge_lock);
>
> + list_for_each_entry_safe(br, tmp, &bridge_list, list)
> + if (br->funcs->bridge_event_notify)
> + br->funcs->bridge_event_notify(br, DRM_EVENT_BRIDGE_REMOVE, bridge);
> +
I think the order should be different: notify about the added bridge
after adding to the list, notify about bridge removal before removing it
from the list.
> mutex_destroy(&bridge->hpd_mutex);
> }
> EXPORT_SYMBOL(drm_bridge_remove);
> diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
> index 1561347c4991dac6022319774510f9560c9283c3..ad7ba444a13e5ecf16f996de3742e4ac67dc21f1 100644
> --- a/include/drm/drm_bridge.h
> +++ b/include/drm/drm_bridge.h
> @@ -56,6 +56,11 @@ enum drm_bridge_attach_flags {
> DRM_BRIDGE_ATTACH_NO_CONNECTOR = BIT(0),
> };
>
> +enum drm_bridge_event_type {
> + DRM_EVENT_BRIDGE_ADD,
> + DRM_EVENT_BRIDGE_REMOVE,
> +};
> +
> /**
> * struct drm_bridge_funcs - drm_bridge control functions
> */
> @@ -729,6 +734,24 @@ struct drm_bridge_funcs {
> struct drm_bridge *bridge,
> bool enable, int direction);
>
> + /**
> + * @bridge_event_notify:
> + *
> + * Notify that another bridge is being added or removed.
> + *
> + * This callback is optional. Bridges implementing it must always
> + * check whether the event refers to a bridge they actually need to
> + * interact with.
> + *
> + * @bridge: bridge being notified
> + * @event: event happened (add/remove bridge)
> + * @event_bridge: the bridge mentioned by the event (i.e. the
> + * bridge being added or removed)
> + */
> + void (*bridge_event_notify)(struct drm_bridge *bridge,
> + enum drm_bridge_event_type event,
> + struct drm_bridge *event_bridge);
> +
This creates a small issue. It requires drivers to have a bridge, even
if they don't need one - e.g. the drm_encoder doesn't get notifications
about the added bridges.
I'm not sure if that can be solved in an efficient way.
> /**
> * @debugfs_init:
> *
>
> --
> 2.34.1
>
--
With best wishes
Dmitry
