[PATCH v2 0/2] Pass down hot plug CONNECTOR ID to user-space

[PATCH v2 0/2] Pass down hot plug CONNECTOR ID to user-space

[Marius Vlad]

Patch series addresses a shortcoming where we're sending a hot plug event
without passing the actual CONNECTOR that caused it. This takes into
consideration both the polling path and the HPD (Hot Plug Detect) path.

v2: Address comments from Daniel

- split patch into 2, one that introduces a bool to track connector
  connection status change and a patch that uses that to be able to send
  hot plug events with the proper CONNECTOR ID to udev and further pass
  that down to user-space
- nuke out mutex when iterating connector list
- fix typo

v1 is at https://lore.kernel.org/dri-devel/20250627131751.2004-1-marius.vlad@collabora.com/T/#m9e35be35b70dc0d799dd950ce3cb4ef9c910e9c5

Marius Vlad (2):
  drm: Introduce a new connector status
  drm: Propagate connector status change

 drivers/gpu/drm/drm_connector.c    |  1 +
 drivers/gpu/drm/drm_probe_helper.c | 34 +++++++++++++++++++++++++-----
 drivers/gpu/drm/drm_sysfs.c        |  1 +
 include/drm/drm_connector.h        |  3 +++
 4 files changed, 34 insertions(+), 5 deletions(-)

-- 
2.47.2

[PATCH v2 1/2] drm: Introduce a new connector status

[Marius Vlad]

This patch introduces a new boolean variable used to track connector's
connect/disconnect status and it is being used on both polling and
the HPD (Hot Plug Detect) paths.

A subsequent patch would make use of this connector status to propagate
per-connector udev hot plug events. This allows user-space to receive
the connector's ID, rather than having a generic hot-plug event for all
connectors, or in the HPD path, just the first one found with a
connection status change.

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

diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index 272d6254ea47..3c6628ee3096 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -274,6 +274,7 @@ static int drm_connector_init_only(struct drm_device *dev,
 
 	/* provide ddc symlink in sysfs */
 	connector->ddc = ddc;
+	connector->status_changed = false;
 
 	INIT_LIST_HEAD(&connector->head);
 	INIT_LIST_HEAD(&connector->global_connector_list_entry);
diff --git a/drivers/gpu/drm/drm_probe_helper.c b/drivers/gpu/drm/drm_probe_helper.c
index 6b3541159c0f..761766181e99 100644
--- a/drivers/gpu/drm/drm_probe_helper.c
+++ b/drivers/gpu/drm/drm_probe_helper.c
@@ -628,6 +628,7 @@ int drm_helper_probe_single_connector_modes(struct drm_connector *connector,
 			mod_delayed_work(system_wq,
 					 &dev->mode_config.output_poll_work,
 					 0);
+		connector->status_changed = true;
 	}
 
 	/*
@@ -731,6 +732,15 @@ EXPORT_SYMBOL(drm_helper_probe_single_connector_modes);
  */
 void drm_kms_helper_hotplug_event(struct drm_device *dev)
 {
+	struct drm_connector *connector;
+	struct drm_connector_list_iter conn_iter;
+
+	drm_connector_list_iter_begin(dev, &conn_iter);
+	drm_for_each_connector_iter(connector, &conn_iter) {
+		connector->status_changed = false;
+	}
+	drm_connector_list_iter_end(&conn_iter);
+
 	drm_sysfs_hotplug_event(dev);
 	drm_client_dev_hotplug(dev);
 }
@@ -747,6 +757,7 @@ void drm_kms_helper_connector_hotplug_event(struct drm_connector *connector)
 {
 	struct drm_device *dev = connector->dev;
 
+	connector->status_changed = false;
 	drm_sysfs_connector_hotplug_event(connector);
 	drm_client_dev_hotplug(dev);
 }
@@ -1041,6 +1052,7 @@ bool drm_connector_helper_hpd_irq_event(struct drm_connector *connector)
 	mutex_unlock(&dev->mode_config.mutex);
 
 	if (changed) {
+		connector->status_changed = true;
 		drm_kms_helper_connector_hotplug_event(connector);
 		drm_dbg_kms(dev, "[CONNECTOR:%d:%s] Sent hotplug event\n",
 			    connector->base.id,
diff --git a/drivers/gpu/drm/drm_sysfs.c b/drivers/gpu/drm/drm_sysfs.c
index 60c1f26edb6f..77f880654d6a 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->status_changed = true;
 
 	if (sysfs_streq(buf, "detect"))
 		connector->force = 0;
diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
index 8f34f4b8183d..e4310df3d55c 100644
--- a/include/drm/drm_connector.h
+++ b/include/drm/drm_connector.h
@@ -2146,6 +2146,9 @@ struct drm_connector {
 	/** @force: a DRM_FORCE_<foo> state for forced mode sets */
 	enum drm_connector_force force;
 
+	/** @status_changed: if the old status doesn't match current connection status */
+	bool status_changed;
+
 	/**
 	 * @edid_override: Override EDID set via debugfs.
 	 *
-- 
2.47.2

[PATCH v2 2/2] drm: Propagate connector status change

[Marius Vlad]

On the HPD (Hot Plug Detect) path this change makes use of the connector
status to notify all connectors, rather than just first one found that
suffered a status change.

Similarly on the polling side, this also takes into consideration
sending per-connector udev hot plug events.

Signed-off-by: Marius Vlad <marius.vlad@collabora.com>
---
 drivers/gpu/drm/drm_probe_helper.c | 22 +++++++++++++++++-----
 1 file changed, 17 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/drm_probe_helper.c b/drivers/gpu/drm/drm_probe_helper.c
index 761766181e99..52761ca34460 100644
--- a/drivers/gpu/drm/drm_probe_helper.c
+++ b/drivers/gpu/drm/drm_probe_helper.c
@@ -854,8 +854,14 @@ 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) {
+		drm_connector_list_iter_begin(dev, &conn_iter);
+		drm_for_each_connector_iter(connector, &conn_iter) {
+			if (connector->status_changed)
+				drm_kms_helper_connector_hotplug_event(connector);
+		}
+		drm_connector_list_iter_end(&conn_iter);
+	}
 
 	if (repoll)
 		schedule_delayed_work(delayed_work, DRM_OUTPUT_POLL_PERIOD);
@@ -1118,10 +1124,16 @@ bool drm_helper_hpd_irq_event(struct drm_device *dev)
 	drm_connector_list_iter_end(&conn_iter);
 	mutex_unlock(&dev->mode_config.mutex);
 
-	if (changed == 1)
+	if (changed == 1) {
 		drm_kms_helper_connector_hotplug_event(first_changed_connector);
-	else if (changed > 0)
-		drm_kms_helper_hotplug_event(dev);
+	} else if (changed > 0) {
+		drm_connector_list_iter_begin(dev, &conn_iter);
+		drm_for_each_connector_iter(connector, &conn_iter) {
+			if (connector->status_changed)
+				drm_kms_helper_connector_hotplug_event(connector);
+		}
+		drm_connector_list_iter_end(&conn_iter);
+	}
 
 	if (first_changed_connector)
 		drm_connector_put(first_changed_connector);
-- 
2.47.2

Re: [PATCH v2 1/2] drm: Introduce a new connector status

[Dmitry Baryshkov]

On Tue, Jul 29, 2025 at 07:57:07PM +0300, Marius Vlad wrote:
> This patch introduces a new boolean variable used to track connector's
> connect/disconnect status and it is being used on both polling and
> the HPD (Hot Plug Detect) paths.
> 
> A subsequent patch would make use of this connector status to propagate
> per-connector udev hot plug events. This allows user-space to receive
> the connector's ID, rather than having a generic hot-plug event for all
> connectors, or in the HPD path, just the first one found with a
> connection status change.

This looks good, the main question would be, what prevents a races
during modifications of this field? I think it should be toggled under
the dev->mode_config.mutex.

> 
> Signed-off-by: Marius Vlad <marius.vlad@collabora.com>
> ---
>  drivers/gpu/drm/drm_connector.c    |  1 +
>  drivers/gpu/drm/drm_probe_helper.c | 12 ++++++++++++
>  drivers/gpu/drm/drm_sysfs.c        |  1 +
>  include/drm/drm_connector.h        |  3 +++
>  4 files changed, 17 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> index 272d6254ea47..3c6628ee3096 100644
> --- a/drivers/gpu/drm/drm_connector.c
> +++ b/drivers/gpu/drm/drm_connector.c
> @@ -274,6 +274,7 @@ static int drm_connector_init_only(struct drm_device *dev,
>  
>  	/* provide ddc symlink in sysfs */
>  	connector->ddc = ddc;
> +	connector->status_changed = false;
>  
>  	INIT_LIST_HEAD(&connector->head);
>  	INIT_LIST_HEAD(&connector->global_connector_list_entry);
> diff --git a/drivers/gpu/drm/drm_probe_helper.c b/drivers/gpu/drm/drm_probe_helper.c
> index 6b3541159c0f..761766181e99 100644
> --- a/drivers/gpu/drm/drm_probe_helper.c
> +++ b/drivers/gpu/drm/drm_probe_helper.c
> @@ -628,6 +628,7 @@ int drm_helper_probe_single_connector_modes(struct drm_connector *connector,
>  			mod_delayed_work(system_wq,
>  					 &dev->mode_config.output_poll_work,
>  					 0);
> +		connector->status_changed = true;
>  	}
>  
>  	/*
> @@ -731,6 +732,15 @@ EXPORT_SYMBOL(drm_helper_probe_single_connector_modes);
>   */
>  void drm_kms_helper_hotplug_event(struct drm_device *dev)
>  {
> +	struct drm_connector *connector;
> +	struct drm_connector_list_iter conn_iter;
> +
> +	drm_connector_list_iter_begin(dev, &conn_iter);
> +	drm_for_each_connector_iter(connector, &conn_iter) {
> +		connector->status_changed = false;
> +	}
> +	drm_connector_list_iter_end(&conn_iter);
> +
>  	drm_sysfs_hotplug_event(dev);
>  	drm_client_dev_hotplug(dev);
>  }
> @@ -747,6 +757,7 @@ void drm_kms_helper_connector_hotplug_event(struct drm_connector *connector)
>  {
>  	struct drm_device *dev = connector->dev;
>  
> +	connector->status_changed = false;
>  	drm_sysfs_connector_hotplug_event(connector);
>  	drm_client_dev_hotplug(dev);


What would be the rule? Should it be unset before or after calling all
the notifiers? Otherwise it's really strange. In the chunk below you set
the flag, then it calls this function and the flags gets immediately
unset.

>  }
> @@ -1041,6 +1052,7 @@ bool drm_connector_helper_hpd_irq_event(struct drm_connector *connector)
>  	mutex_unlock(&dev->mode_config.mutex);
>  
>  	if (changed) {
> +		connector->status_changed = true;
>  		drm_kms_helper_connector_hotplug_event(connector);
>  		drm_dbg_kms(dev, "[CONNECTOR:%d:%s] Sent hotplug event\n",
>  			    connector->base.id,
> diff --git a/drivers/gpu/drm/drm_sysfs.c b/drivers/gpu/drm/drm_sysfs.c
> index 60c1f26edb6f..77f880654d6a 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->status_changed = true;
>  
>  	if (sysfs_streq(buf, "detect"))
>  		connector->force = 0;
> diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
> index 8f34f4b8183d..e4310df3d55c 100644
> --- a/include/drm/drm_connector.h
> +++ b/include/drm/drm_connector.h
> @@ -2146,6 +2146,9 @@ struct drm_connector {
>  	/** @force: a DRM_FORCE_<foo> state for forced mode sets */
>  	enum drm_connector_force force;
>  
> +	/** @status_changed: if the old status doesn't match current connection status */
> +	bool status_changed;
> +
>  	/**
>  	 * @edid_override: Override EDID set via debugfs.
>  	 *
> -- 
> 2.47.2
> 

-- 
With best wishes
Dmitry