[PATCH] drm: Pass down CONNECTOR ID for user-space to use

[PATCH] drm: Pass down CONNECTOR ID for user-space to use

[Marius Vlad]

Manually setting/forcing the connector status in sysfs does not
propagate the CONNECTOR ID. For drivers that use polling, including
manually setting it up with sysfs this would similarly to the HDP IRQ
path which can pass it through drm_connector_helper_hpd_irq_event().

Signed-off-by: Marius Vlad <marius.vlad@collabora.com>
---
 drivers/gpu/drm/drm_probe_helper.c | 28 ++++++++++++++++++++++++++--
 drivers/gpu/drm/drm_sysfs.c        |  1 +
 include/drm/drm_connector.h        |  8 ++++++++
 3 files changed, 35 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/drm_probe_helper.c b/drivers/gpu/drm/drm_probe_helper.c
index 6b3541159c0f..b472bfb43c17 100644
--- a/drivers/gpu/drm/drm_probe_helper.c
+++ b/drivers/gpu/drm/drm_probe_helper.c
@@ -843,9 +843,33 @@ static void output_poll_execute(struct work_struct *work)
 	mutex_unlock(&dev->mode_config.mutex);
 
 out:
-	if (changed)
-		drm_kms_helper_hotplug_event(dev);
+	if (changed) {
+		struct drm_connector *first_changed_connector = NULL;
+		if (!mutex_trylock(&dev->mode_config.mutex)) {
+			repoll = true;
+			goto repoll;
+		}
 
+		drm_connector_list_iter_begin(dev, &conn_iter);
+		drm_for_each_connector_iter(connector, &conn_iter) {
+			if (connector->sysfs_hotplug) {
+				drm_connector_get(connector);
+				first_changed_connector = connector;
+				connector->sysfs_hotplug = false;
+				break;
+			}
+		}
+		drm_connector_list_iter_end(&conn_iter);
+		mutex_unlock(&dev->mode_config.mutex);
+
+		if (first_changed_connector) {
+			drm_sysfs_connector_hotplug_event(connector);
+			drm_connector_put(first_changed_connector);
+		} else {
+			drm_kms_helper_hotplug_event(dev);
+		}
+	}
+repoll:
 	if (repoll)
 		schedule_delayed_work(delayed_work, DRM_OUTPUT_POLL_PERIOD);
 }
diff --git a/drivers/gpu/drm/drm_sysfs.c b/drivers/gpu/drm/drm_sysfs.c
index 60c1f26edb6f..c38b7a2accee 100644
--- a/drivers/gpu/drm/drm_sysfs.c
+++ b/drivers/gpu/drm/drm_sysfs.c
@@ -196,6 +196,7 @@ static ssize_t status_store(struct device *device,
 		return ret;
 
 	old_force = connector->force;
+	connector->sysfs_hotplug = true;
 
 	if (sysfs_streq(buf, "detect"))
 		connector->force = 0;
diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
index 137773dd138e..7a8344814219 100644
--- a/include/drm/drm_connector.h
+++ b/include/drm/drm_connector.h
@@ -2125,6 +2125,14 @@ struct drm_connector {
 	 */
 	uint8_t polled;
 
+	/**
+	 * @sysfs_hotplug:
+	 *
+	 * Set in by sysfs connector status to propagate CONNECTOR_ID to udev
+	 * and further down the stack.
+	 */
+	bool sysfs_hotplug;
+
 	/**
 	 * @dpms: Current dpms state. For legacy drivers the
 	 * &drm_connector_funcs.dpms callback must update this. For atomic
-- 
2.47.2

Re: [PATCH] drm: Pass down CONNECTOR ID for user-space to use

[Daniel Stone]

Hi Marius,

On Fri, 27 Jun 2025 at 14:18, Marius Vlad <marius.vlad@collabora.com> wrote:
> Manually setting/forcing the connector status in sysfs does not
> propagate the CONNECTOR ID. For drivers that use polling, including
> manually setting it up with sysfs this would similarly to the HDP IRQ
> path which can pass it through drm_connector_helper_hpd_irq_event().

s/HDP/HPD/g

>  out:
> -       if (changed)
> -               drm_kms_helper_hotplug_event(dev);
> +       if (changed) {
> +               struct drm_connector *first_changed_connector = NULL;
> +               if (!mutex_trylock(&dev->mode_config.mutex)) {
> +                       repoll = true;
> +                       goto repoll;
> +               }
>
> +               drm_connector_list_iter_begin(dev, &conn_iter);
> +               drm_for_each_connector_iter(connector, &conn_iter) {
> +                       if (connector->sysfs_hotplug) {
> +                               drm_connector_get(connector);
> +                               first_changed_connector = connector;
> +                               connector->sysfs_hotplug = false;
> +                               break;
> +                       }
> +               }
> +               drm_connector_list_iter_end(&conn_iter);
> +               mutex_unlock(&dev->mode_config.mutex);
> +
> +               if (first_changed_connector) {
> +                       drm_sysfs_connector_hotplug_event(connector);
> +                       drm_connector_put(first_changed_connector);
> +               } else {
> +                       drm_kms_helper_hotplug_event(dev);
> +               }
> +       }
> +repoll:
>         if (repoll)
>                 schedule_delayed_work(delayed_work, DRM_OUTPUT_POLL_PERIOD);
>  }

There are a couple of different things in here, so I think it would be
good to split into multiple commits.

This commit as-is is not correct: if we force multiple connectors to a
given status at the 'same time' (i.e. before the output-poll worker
can run in between), we will only send a hotplug event for the first
connector in the list, and not the other.

The good news is that this is easy to fix: we don't need to hold the
big mode_config mutex over this list, because it's only needed to call
the connector probe, unless I'm very much mistaken. So you could drop
the locking and just call drm_sysfs_connector_hotplug_event() for
every changed connector. But then we're introducing a specialist path
for sysfs, which is not ideal.

I think first we should  aim to let output_poll_execute() send
per-connector events for _any_ kind of HPD event it finds. To that
effect, we could have a connector->status_changed bool which was set
whenever this happened, e.g. in drm_connector_helper_hpd_irq_event() /
check_connector_changed() for the connector-aware path, or
drm_helper_probe_single_connector_modes() for the general path. That
could then be cleared by drm_kms_helper_connector_hotplug_event() for
a single connector, or drm_kms_helper_hotplug_event() for all
connectors.

In the second commit, you could change output_poll_execute() to send
per-connector events, regardless of how they arrived. That would just
be iterating the connector list (without the mode_config mutex held)
and calling drm_kms_helper_connector_hotplug_event() for every changed
connector.

That would give us a nice logical progression: first we start tracking
whether a connector's status changed in a more robust way, which would
work across the multiple detection methods we have and also be OK for
sending events for delayed work. Then we start sending fine-grained
hotplug events when they come out of the poll helper. At the end of
the series, as a happy side effect of making code more generally
robust, we get HPD events for forcing connector status via sysfs.

Cheers,
Daniel