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

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

[Nicolas Frattaroli]

This series addresses a shortcoming whereby a hot plug event is sent
without it being passed the actual connector that caused it. This takes
into consideration both the polling path and the HPD (Hot Plug Detect)
path.

The motivation is that user-space applications such as Weston would
previously receive non-connector-specific hotplug events, and then have
to figure out themselves which connector needs to have a modeset
executed on. This notably did not work when the hotplug events came in
too fast, resulting in Weston missing an on-off-on transition of a
connector, seeing that its state was unchanged from "on" so can't be the
one that was hotplugged, and skipping reinitialising it as it looks
through the other connectors that could've caused it.

The real world implication is that on setups with slightly sketchy HDMI
connections, a brief flicker in the HPD signal could result in video
output bidding farewell entirely until a manual proper re-plug was
performed.

By sending connector specific hotplug events, this ambiguity is resolved
without any change to the user-space API. Future work should however
make the kernel explicitly request a modeset from userspace through a
more robust path, either by using link-status (nb. the userspace side
may not handle it properly at present) or by exposing the epoch counter
through the uAPI such that fragile state comparison code is no longer
needed.

This change results in potentially more hotplug events being sent than
previously, as n changed connectors at a time result in n hotplug
events, rather than just a single one.

Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
---
Changes in v9:
- Get rid of the kmalloc_array by splitting the hotplug event into its
  two constituent parts to avoid the deadlock.
- Link to v8: https://patch.msgid.link/20260422-hot-plug-passup-v8-0-5cfae6ba4119@collabora.com

Changes in v8:
- Drop pending_hp patch
- Add patch correcting epoch_counter documentation
- Rework implementation such that hotplug events are still sent outside
  the mode_config mutex, but don't iterate through the connector list
  without the lock.
- Link to v7: https://patch.msgid.link/20260415-hot-plug-passup-v7-0-9a27ef5e2428@collabora.com

Changes in v7 RESEND:
- None, other than removing the leftover diffstat from the cover letter
- Link to v7: https://lore.kernel.org/r/20260217-hot-plug-passup-v7-0-f8221b2aab51@collabora.com

Changes in v7:
- Drop the two vkms patches, as I don't want them to be blocked on
  review. I still think they're correct, but they're not essential and
  don't need to block this series.
- Link to v6: https://lore.kernel.org/r/20260123-hot-plug-passup-v6-0-aaaf61d960bb@collabora.com

Changes in v6:
- Rewrote cover letter to explain the motivation for this series more
  plainly
- Rename "status_changed" to "pending_hp"
- Set "pending_hp" in the existing path that would also affect
  epoch_counter
- No longer set the boolean in drm_helper_probe_single_connector_modes,
  as it does not appear to be necessary
- Reword commits to better justify the changes
- Link to v5: https://lore.kernel.org/r/20251111162338.15141-1-marius.vlad@collabora.com/

Changes in v5:
- vkms: add support for sending the CONNECTOR ID when hot-plugging through
  ConfigFS - as reported by Louis, vkms can now make use of ConfigFS to
  simulate connector status.
- vkms: add a small change to ignore previous/old drm connector status
  when sending out hot-plug uevent.
- Link to v4: https://lore.kernel.org/r/20251103174558.7709-1-marius.vlad@collabora.com/

Changes in v4:
- removed the "This patch" bit - Dmitry
- added a short note when the flag is set and cleared - Dmitry
- address double dead-locking detected - kbot: https://lore.kernel.org/dri-devel/202509251410.fdfbcac3-lkp@intel.com/
- virtual connectors do not seem have any kind of hotplug - added
  polling in vkms - as noted by Ian
- Link to v3: https://lore.kernel.org/r/20250923083636.4749-1-marius.vlad@collabora.com/

Changes in v3:
- Address comments from Dmitry:
  - guard connector status write with mode_config.mutex
  - avoid setting up the connector status and immediately unset it. Do the
    unset in drm_kms_helper_hotplug_event/drm_kms_helper_connector_hotplug_event
- Link to v2: https://lore.kernel.org/r/20250729165708.9947-1-marius.vlad@collabora.com/

Changes in 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
- Link to v1: https://lore.kernel.org/r/20250627131751.2004-1-marius.vlad@collabora.com/

---
Nicolas Frattaroli (2):
      drm/connector: Fix epoch_counter docs to reflect reality
      drm: Send per-connector hotplug events

 drivers/gpu/drm/drm_probe_helper.c | 37 ++++++++++++++++---------------------
 include/drm/drm_connector.h        |  5 ++++-
 2 files changed, 20 insertions(+), 22 deletions(-)
---
base-commit: 489e26ada57ce96a2ee3e5853cfe74981ef85bbd
change-id: 20260121-hot-plug-passup-f8ed03f7c202

Best regards,
--  
Nicolas Frattaroli <nicolas.frattaroli@collabora.com>

[PATCH v9 1/2] drm/connector: Fix epoch_counter docs to reflect reality

[Nicolas Frattaroli]

Since the very day epoch_counter in drm_connector was introduced, its
documentation was not accurate. It claims it's used to detect "any other
changes [...] besides status", when in reality, it's used to detect
changes including status, as a status change also increases the epoch
counter.

Adjust the documentation to rectify this discrepancy.

Fixes: 5186421cbfe2 ("drm: Introduce epoch counter to drm_connector")
Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
---
 include/drm/drm_connector.h | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
index 3e422a4f2e72..446385a12f4b 100644
--- a/include/drm/drm_connector.h
+++ b/include/drm/drm_connector.h
@@ -2264,7 +2264,10 @@ struct drm_connector {
 	 */
 	struct mutex edid_override_mutex;
 
-	/** @epoch_counter: used to detect any other changes in connector, besides status */
+	/**
+	 * @epoch_counter: Used to detect changes in connector. Increased when
+	 * the connector, including its status, is changed.
+	 */
 	u64 epoch_counter;
 
 	/**

-- 
2.53.0

Claude review: drm/connector: Fix epoch_counter docs to reflect reality

[Claude Code Review Bot]

Patch Review

**Verdict: Looks good.**

This is a straightforward documentation fix. The old comment:
```c
/** @epoch_counter: used to detect any other changes in connector, besides status */
```
is replaced with:
```c
/**
 * @epoch_counter: Used to detect changes in connector. Increased when
 * the connector, including its status, is changed.
 */
```

The fix is correct. The `epoch_counter` is bumped in `drm_connector_set_status_locked()` (among other places), so it very much does track status changes. The Fixes tag referencing the original commit `5186421cbfe2` is appropriate since the doc was wrong from the start.

No issues.

---
Generated by Claude Code Patch Reviewer

[PATCH v9 2/2] drm: Send per-connector hotplug events

[Nicolas Frattaroli]

Try to send per-connector hotplug events as often as possible, rather
than connector-less global hotplug events. This does result in more
hotplug events if multiple connectors changed at the same time, but
give userspace more actionable information.

Since the hotplug event needs to be sent outside of the mode_config
mutex to avoid a deadlock, the drm_client_dev_hotplug() call is split
off from the drm_sysfs_(connector_)?hotplug_event calls.

Co-developed-by: Marius Vlad <marius.vlad@collabora.com>
Signed-off-by: Marius Vlad <marius.vlad@collabora.com>
Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
---
 drivers/gpu/drm/drm_probe_helper.c | 37 ++++++++++++++++---------------------
 1 file changed, 16 insertions(+), 21 deletions(-)

diff --git a/drivers/gpu/drm/drm_probe_helper.c b/drivers/gpu/drm/drm_probe_helper.c
index d4dc8cb45bce..2daa4242ce7e 100644
--- a/drivers/gpu/drm/drm_probe_helper.c
+++ b/drivers/gpu/drm/drm_probe_helper.c
@@ -760,16 +760,12 @@ static void output_poll_execute(struct work_struct *work)
 	struct drm_connector *connector;
 	struct drm_connector_list_iter conn_iter;
 	enum drm_connector_status old_status;
-	bool repoll = false, changed;
+	bool repoll = false, changed = false;
 	u64 old_epoch_counter;
 
 	if (!dev->mode_config.poll_enabled)
 		return;
 
-	/* Pick up any changes detected by the probe functions. */
-	changed = dev->mode_config.delayed_event;
-	dev->mode_config.delayed_event = false;
-
 	if (!drm_kms_helper_poll) {
 		if (dev->mode_config.poll_running) {
 			drm_kms_helper_disable_hpd(dev);
@@ -836,6 +832,7 @@ static void output_poll_execute(struct work_struct *work)
 				    connector->base.id, connector->name,
 				    old_epoch_counter, connector->epoch_counter);
 
+			drm_sysfs_connector_hotplug_event(connector);
 			changed = true;
 		}
 	}
@@ -844,8 +841,15 @@ static void output_poll_execute(struct work_struct *work)
 	mutex_unlock(&dev->mode_config.mutex);
 
 out:
+	/* Pick up any changes detected by the probe functions. */
+	if (dev->mode_config.delayed_event) {
+		dev->mode_config.delayed_event = false;
+		changed = true;
+		drm_sysfs_hotplug_event(dev);
+	}
+
 	if (changed)
-		drm_kms_helper_hotplug_event(dev);
+		drm_client_dev_hotplug(dev);
 
 	if (repoll)
 		schedule_delayed_work(delayed_work, DRM_OUTPUT_POLL_PERIOD);
@@ -1081,9 +1085,9 @@ EXPORT_SYMBOL(drm_connector_helper_hpd_irq_event);
  */
 bool drm_helper_hpd_irq_event(struct drm_device *dev)
 {
-	struct drm_connector *connector, *first_changed_connector = NULL;
 	struct drm_connector_list_iter conn_iter;
-	int changed = 0;
+	struct drm_connector *connector;
+	bool changed = false;
 
 	if (!dev->mode_config.poll_enabled)
 		return false;
@@ -1096,24 +1100,15 @@ bool drm_helper_hpd_irq_event(struct drm_device *dev)
 			continue;
 
 		if (check_connector_changed(connector)) {
-			if (!first_changed_connector) {
-				drm_connector_get(connector);
-				first_changed_connector = connector;
-			}
-
-			changed++;
+			changed = true;
+			drm_sysfs_connector_hotplug_event(connector);
 		}
 	}
 	drm_connector_list_iter_end(&conn_iter);
 	mutex_unlock(&dev->mode_config.mutex);
 
-	if (changed == 1)
-		drm_kms_helper_connector_hotplug_event(first_changed_connector);
-	else if (changed > 0)
-		drm_kms_helper_hotplug_event(dev);
-
-	if (first_changed_connector)
-		drm_connector_put(first_changed_connector);
+	if (changed)
+		drm_client_dev_hotplug(dev);
 
 	return changed;
 }

-- 
2.53.0

Claude review: drm: Send per-connector hotplug events

[Claude Code Review Bot]

Patch Review

**Verdict: Looks good, with minor observations.**

**Design approach — correct and clean:**

The key insight in v9 is decomposing `drm_kms_helper_hotplug_event()` (which calls `drm_sysfs_hotplug_event()` + `drm_client_dev_hotplug()`) into its two parts. The `drm_sysfs_connector_hotplug_event()` calls are safe to make inside the `mode_config.mutex` since they only do `kobject_uevent_env()`, while `drm_client_dev_hotplug()` is deferred to after the lock is released. This avoids the deadlock from earlier versions and avoids needing a temporary allocation for connector IDs.

**`output_poll_execute` changes:**

The poll path now calls `drm_sysfs_connector_hotplug_event(connector)` inline within the loop (under the mutex), and defers the `drm_client_dev_hotplug(dev)` call to after `mutex_unlock()`:

```c
+		drm_sysfs_connector_hotplug_event(connector);
 		changed = true;
```

and later:

```c
+	if (dev->mode_config.delayed_event) {
+		dev->mode_config.delayed_event = false;
+		changed = true;
+		drm_sysfs_hotplug_event(dev);
+	}
+
 	if (changed)
-		drm_kms_helper_hotplug_event(dev);
+		drm_client_dev_hotplug(dev);
```

This is correct. The `delayed_event` handling has been moved from before the lock acquisition to after the lock release in the `out:` label. This is a subtle but important change:

- **Observation 1 (minor):** In the original code, `delayed_event` was consumed before the `goto out` on `!drm_kms_helper_poll`, meaning a delayed event would still fire `drm_kms_helper_hotplug_event()`. In the new code, the `delayed_event` check is at the `out:` label, so it's also reached from that path — behavior is preserved. Good.

- **Observation 2 (minor):** In the original code, `delayed_event` was consumed before the `mutex_trylock` failure path (`goto out` with `repoll = true`). In the new code, the same applies — if the trylock fails, we still check `delayed_event` at `out:`, which is correct and actually slightly better since we don't lose the delayed event on trylock failure. In the old code, a trylock failure after consuming `delayed_event` would fire the hotplug event. The new code does the same via the `out:` label path. Correct.

- **Observation 3 (race on `delayed_event`):** The `delayed_event` flag is read and cleared without holding the `mode_config.mutex` (it's in the `out:` block after the unlock). This is the same as the original code (which also read it before acquiring the lock), so it's not a regression. The flag is set under the poll work context which is single-threaded, and races with `drm_helper_probe_single_connector_modes` setting it are benign (worst case: an extra poll cycle). Fine.

**`drm_helper_hpd_irq_event` changes:**

The old code tracked `first_changed_connector` and a `changed` count, then dispatched either `drm_kms_helper_connector_hotplug_event()` for single-connector changes or `drm_kms_helper_hotplug_event()` for multi-connector changes. The new code simply calls `drm_sysfs_connector_hotplug_event()` for each changed connector within the loop, then calls `drm_client_dev_hotplug()` once after the lock is released:

```c
 		if (check_connector_changed(connector)) {
-			if (!first_changed_connector) {
-				drm_connector_get(connector);
-				first_changed_connector = connector;
-			}
-			changed++;
+			changed = true;
+			drm_sysfs_connector_hotplug_event(connector);
 		}
```

This is a nice simplification — the `first_changed_connector` / refcount dance is eliminated entirely. The `drm_sysfs_connector_hotplug_event()` call is safe under `mode_config.mutex` since it just does a `kobject_uevent_env()`.

- **Observation 4 (behavioral change in multi-connector case):** Previously, if 3 connectors changed simultaneously, userspace got 1 generic `HOTPLUG=1` uevent (without `CONNECTOR=`). Now it gets 3 separate `HOTPLUG=1 CONNECTOR=<id>` uevents plus a single `drm_client_dev_hotplug()`. This is intentional per the cover letter and is strictly better for userspace — more information, no loss of the old signal. Well-designed compositors already handle `CONNECTOR=` uevents. Older compositors that ignore `CONNECTOR=` still see `HOTPLUG=1` and re-probe everything, so backwards compatibility is maintained.

- **Observation 5 (multiple `drm_client_dev_hotplug` calls):** In both paths, `drm_client_dev_hotplug()` is called exactly once if anything changed, which is correct and avoids redundant fbdev/fbcon restores.

**No issues found.** The patch is a clean improvement that simplifies the code while providing better information to userspace. The locking split is correct — uevent delivery inside the lock, client notification outside.

---
Generated by Claude Code Patch Reviewer

Claude review: Pass down hot-plug CONNECTOR ID to user-space

[Claude Code Review Bot]

Overall Series Review

Subject: Pass down hot-plug CONNECTOR ID to user-space
Author: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
Patches: 3
Reviewed: 2026-04-23T07:22:52.089066

---

This is a well-motivated 2-patch series at v9 that improves DRM hotplug event reporting by sending per-connector `CONNECTOR=<id>` uevents instead of generic global hotplug events. The cover letter clearly explains the real-world problem: compositors like Weston can miss rapid on-off-on HPD transitions because they can't identify which connector changed. The approach is sound — split the sysfs uevent (which is safe to call with the mutex held) from `drm_client_dev_hotplug()` (which needs to be called without mode_config locks), thus avoiding the deadlock that plagued earlier versions.

The series is clean and simple at 2 patches / net -1 lines changed. The first patch is a trivial doc fix, and the second implements the actual behavioral change. The code is a simplification over the current baseline which already had partial per-connector support (the `first_changed_connector` logic in `drm_helper_hpd_irq_event`). I have a few observations but nothing blocking.

---
Generated by Claude Code Patch Reviewer

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

[Nicolas Frattaroli]

This series addresses a shortcoming whereby a hot plug event is sent
without it being passed the actual connector that caused it. This takes
into consideration both the polling path and the HPD (Hot Plug Detect)
path.

The motivation is that user-space applications such as Weston would
previously receive non-connector-specific hotplug events, and then have
to figure out themselves which connector needs to have a modeset
executed on. This notably did not work when the hotplug events came in
too fast, resulting in Weston missing an on-off-on transition of a
connector, seeing that its state was unchanged from "on" so can't be the
one that was hotplugged, and skipping reinitialising it as it looks
through the other connectors that could've caused it.

The real world implication is that on setups with slightly sketchy HDMI
connections, a brief flicker in the HPD signal could result in video
output bidding farewell entirely until a manual proper re-plug was
performed.

By sending connector specific hotplug events, this ambiguity is resolved
without any change to the user-space API. Future work should however
make the kernel explicitly request a modeset from userspace through a
more robust path, either by using link-status (nb. the userspace side
may not handle it properly at present) or by exposing the epoch counter
through the uAPI such that fragile state comparison code is no longer
needed.

This change results in potentially more hotplug events being sent than
previously, as n changed connectors at a time result in n hotplug
events, rather than just a single one.

Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
---
Changes in v8:
- Drop pending_hp patch
- Add patch correcting epoch_counter documentation
- Rework implementation such that hotplug events are still sent outside
  the mode_config mutex, but don't iterate through the connector list
  without the lock.
- Link to v7: https://patch.msgid.link/20260415-hot-plug-passup-v7-0-9a27ef5e2428@collabora.com

Changes in v7 RESEND:
- None, other than removing the leftover diffstat from the cover letter
- Link to v7: https://lore.kernel.org/r/20260217-hot-plug-passup-v7-0-f8221b2aab51@collabora.com

Changes in v7:
- Drop the two vkms patches, as I don't want them to be blocked on
  review. I still think they're correct, but they're not essential and
  don't need to block this series.
- Link to v6: https://lore.kernel.org/r/20260123-hot-plug-passup-v6-0-aaaf61d960bb@collabora.com

Changes in v6:
- Rewrote cover letter to explain the motivation for this series more
  plainly
- Rename "status_changed" to "pending_hp"
- Set "pending_hp" in the existing path that would also affect
  epoch_counter
- No longer set the boolean in drm_helper_probe_single_connector_modes,
  as it does not appear to be necessary
- Reword commits to better justify the changes
- Link to v5: https://lore.kernel.org/r/20251111162338.15141-1-marius.vlad@collabora.com/

Changes in v5:
- vkms: add support for sending the CONNECTOR ID when hot-plugging through
  ConfigFS - as reported by Louis, vkms can now make use of ConfigFS to
  simulate connector status.
- vkms: add a small change to ignore previous/old drm connector status
  when sending out hot-plug uevent.
- Link to v4: https://lore.kernel.org/r/20251103174558.7709-1-marius.vlad@collabora.com/

Changes in v4:
- removed the "This patch" bit - Dmitry
- added a short note when the flag is set and cleared - Dmitry
- address double dead-locking detected - kbot: https://lore.kernel.org/dri-devel/202509251410.fdfbcac3-lkp@intel.com/
- virtual connectors do not seem have any kind of hotplug - added
  polling in vkms - as noted by Ian
- Link to v3: https://lore.kernel.org/r/20250923083636.4749-1-marius.vlad@collabora.com/

Changes in v3:
- Address comments from Dmitry:
  - guard connector status write with mode_config.mutex
  - avoid setting up the connector status and immediately unset it. Do the
    unset in drm_kms_helper_hotplug_event/drm_kms_helper_connector_hotplug_event
- Link to v2: https://lore.kernel.org/r/20250729165708.9947-1-marius.vlad@collabora.com/

Changes in 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
- Link to v1: https://lore.kernel.org/r/20250627131751.2004-1-marius.vlad@collabora.com/

---
Nicolas Frattaroli (2):
      drm/connector: Fix epoch_counter docs to reflect reality
      drm: Send per-connector hotplug events

 drivers/gpu/drm/drm_probe_helper.c | 68 ++++++++++++++++++++++++--------------
 include/drm/drm_connector.h        |  5 ++-
 2 files changed, 47 insertions(+), 26 deletions(-)
---
base-commit: 28247e192ca66016a4b374ff06acdf9dbb6cfe7c
change-id: 20260121-hot-plug-passup-f8ed03f7c202

Best regards,
--  
Nicolas Frattaroli <nicolas.frattaroli@collabora.com>

[PATCH v8 2/2] drm: Send per-connector hotplug events

[Nicolas Frattaroli]

Try to send per-connector hotplug events as often as possible, rather
than connector-less global hotplug events. This does result in more
hotplug events if multiple connectors changed at the same time, but
give userspace more actionable information.

Since the hotplug event needs to be sent outside of the mode_config
mutex to avoid a deadlock, pointers to all the changed connectors are
stored in a newly allocated array.

The "changed" boolean in output_poll_execute now only serves to signal
that a non-connector-specific hotplug event needs to be sent from a
prior event that was delayed. So, rename it from "changed" to
"delayed_hp" to avoid any confusion.

Co-developed-by: Marius Vlad <marius.vlad@collabora.com>
Signed-off-by: Marius Vlad <marius.vlad@collabora.com>
Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
---
 drivers/gpu/drm/drm_probe_helper.c | 68 ++++++++++++++++++++++++--------------
 1 file changed, 43 insertions(+), 25 deletions(-)

diff --git a/drivers/gpu/drm/drm_probe_helper.c b/drivers/gpu/drm/drm_probe_helper.c
index d4dc8cb45bce..3beed8aa32fe 100644
--- a/drivers/gpu/drm/drm_probe_helper.c
+++ b/drivers/gpu/drm/drm_probe_helper.c
@@ -757,17 +757,19 @@ static void output_poll_execute(struct work_struct *work)
 {
 	struct delayed_work *delayed_work = to_delayed_work(work);
 	struct drm_device *dev = container_of(delayed_work, struct drm_device, mode_config.output_poll_work);
+	struct drm_connector **changed_conns;
 	struct drm_connector *connector;
 	struct drm_connector_list_iter conn_iter;
 	enum drm_connector_status old_status;
-	bool repoll = false, changed;
+	unsigned int num_changed = 0, i;
+	bool repoll = false, delayed_hp;
 	u64 old_epoch_counter;
 
 	if (!dev->mode_config.poll_enabled)
 		return;
 
 	/* Pick up any changes detected by the probe functions. */
-	changed = dev->mode_config.delayed_event;
+	delayed_hp = dev->mode_config.delayed_event;
 	dev->mode_config.delayed_event = false;
 
 	if (!drm_kms_helper_poll) {
@@ -783,6 +785,13 @@ static void output_poll_execute(struct work_struct *work)
 		goto out;
 	}
 
+	changed_conns = kmalloc_array(dev->mode_config.num_connector,
+				sizeof(*changed_conns), GFP_KERNEL);
+	if (!changed_conns) {
+		repoll = true;
+		goto out;
+	}
+
 	drm_connector_list_iter_begin(dev, &conn_iter);
 	drm_for_each_connector_iter(connector, &conn_iter) {
 		/* Ignore forced connectors. */
@@ -836,15 +845,23 @@ static void output_poll_execute(struct work_struct *work)
 				    connector->base.id, connector->name,
 				    old_epoch_counter, connector->epoch_counter);
 
-			changed = true;
+			drm_connector_get(connector);
+			changed_conns[num_changed++] = connector;
 		}
 	}
 	drm_connector_list_iter_end(&conn_iter);
 
 	mutex_unlock(&dev->mode_config.mutex);
 
+	for (i = 0; i < num_changed; i++) {
+		drm_kms_helper_connector_hotplug_event(changed_conns[i]);
+		drm_connector_put(changed_conns[i]);
+	}
+
+	kfree(changed_conns);
+
 out:
-	if (changed)
+	if (delayed_hp)
 		drm_kms_helper_hotplug_event(dev);
 
 	if (repoll)
@@ -1081,39 +1098,40 @@ EXPORT_SYMBOL(drm_connector_helper_hpd_irq_event);
  */
 bool drm_helper_hpd_irq_event(struct drm_device *dev)
 {
-	struct drm_connector *connector, *first_changed_connector = NULL;
 	struct drm_connector_list_iter conn_iter;
-	int changed = 0;
+	struct drm_connector **changed_conns;
+	struct drm_connector *connector;
+	unsigned int changed = 0, i;
 
 	if (!dev->mode_config.poll_enabled)
 		return false;
 
-	mutex_lock(&dev->mode_config.mutex);
-	drm_connector_list_iter_begin(dev, &conn_iter);
-	drm_for_each_connector_iter(connector, &conn_iter) {
-		/* Only handle HPD capable connectors. */
-		if (!(connector->polled & DRM_CONNECTOR_POLL_HPD))
-			continue;
+	scoped_guard(mutex, &dev->mode_config.mutex) {
+		changed_conns = kmalloc_array(dev->mode_config.num_connector,
+					sizeof(*changed_conns), GFP_KERNEL);
+		if (!changed_conns)
+			return false;
 
-		if (check_connector_changed(connector)) {
-			if (!first_changed_connector) {
+		drm_connector_list_iter_begin(dev, &conn_iter);
+		drm_for_each_connector_iter(connector, &conn_iter) {
+			/* Only handle HPD capable connectors. */
+			if (!(connector->polled & DRM_CONNECTOR_POLL_HPD))
+				continue;
+
+			if (check_connector_changed(connector)) {
 				drm_connector_get(connector);
-				first_changed_connector = connector;
+				changed_conns[changed++] = connector;
 			}
-
-			changed++;
 		}
+		drm_connector_list_iter_end(&conn_iter);
 	}
-	drm_connector_list_iter_end(&conn_iter);
-	mutex_unlock(&dev->mode_config.mutex);
 
-	if (changed == 1)
-		drm_kms_helper_connector_hotplug_event(first_changed_connector);
-	else if (changed > 0)
-		drm_kms_helper_hotplug_event(dev);
+	for (i = 0; i < changed; i++) {
+		drm_kms_helper_connector_hotplug_event(changed_conns[i]);
+		drm_connector_put(changed_conns[i]);
+	}
 
-	if (first_changed_connector)
-		drm_connector_put(first_changed_connector);
+	kfree(changed_conns);
 
 	return changed;
 }

-- 
2.53.0

Claude review: drm: Send per-connector hotplug events

[Claude Code Review Bot]

Patch Review

**`output_poll_execute` changes:**

The core change replaces the `changed` boolean with an array of connector pointers:

```c
changed_conns = kmalloc_array(dev->mode_config.num_connector,
                              sizeof(*changed_conns), GFP_KERNEL);
```

**Concern: `num_connector` is protected by `connector_list_lock` (a spinlock), not by `mode_config.mutex`.** The allocation uses `num_connector` read under `mode_config.mutex` (the `mutex_trylock` above), but `num_connector` is modified under `connector_list_lock`. A connector could be added between the `kmalloc_array` and the iteration, causing a write past the end of `changed_conns`. In practice, `drm_connector_list_iter` uses RCU + `connector_list_lock` and could see a connector that was added after the array was sized. This is a pre-existing window in the old code too (the old code iterated the same list), but the old code didn't index into a fixed-size array — it just set a boolean. The new code writes `changed_conns[num_changed++]` without any bounds check.

The probability is extremely low in practice (connectors are rarely added at runtime during polling), but for correctness, either:
- Read `num_connector` under `connector_list_lock`, or
- Add a bounds check `if (num_changed < array_size)` before storing, or
- Use a dynamically-growing list.

This is the most significant issue in the series.

**`kmalloc_array` failure handling:**

```c
if (!changed_conns) {
    repoll = true;
    goto out;
}
```

On allocation failure, the function jumps to `out:` which checks `delayed_hp`. This means if both `delayed_hp` is true AND the allocation fails, the delayed hotplug event will still fire. That's fine. And `repoll = true` ensures we'll retry. Reasonable.

**Connector refcounting pattern:**

```c
drm_connector_get(connector);
changed_conns[num_changed++] = connector;
```

Then outside the lock:
```c
for (i = 0; i < num_changed; i++) {
    drm_kms_helper_connector_hotplug_event(changed_conns[i]);
    drm_connector_put(changed_conns[i]);
}
```

This is correct. The `drm_connector_get` inside the iter loop (which itself holds a reference) ensures the connector survives past the lock release. The `drm_connector_put` after sending the event balances it.

**Rename `changed` to `delayed_hp`:**

```c
delayed_hp = dev->mode_config.delayed_event;
```

Good — clarifies that this variable only covers the case where a prior delayed event was pending, not connectors found changed during this poll cycle.

**`drm_helper_hpd_irq_event` changes:**

The old code had special handling: if exactly 1 connector changed, send a per-connector event; if >1 changed, send a global event. The new code always sends per-connector events:

```c
for (i = 0; i < changed; i++) {
    drm_kms_helper_connector_hotplug_event(changed_conns[i]);
    drm_connector_put(changed_conns[i]);
}
```

This is a clear improvement — every changed connector gets its own per-connector event regardless of count.

**Use of `scoped_guard`:**

```c
scoped_guard(mutex, &dev->mode_config.mutex) {
    changed_conns = kmalloc_array(...);
    ...
    drm_connector_list_iter_end(&conn_iter);
}
```

Clean use of `scoped_guard`. The `kmalloc_array` is called under mutex with `GFP_KERNEL`, which is fine (the mutex is not a spinlock).

**Same `num_connector` race concern applies here** — and arguably more so, since `drm_helper_hpd_irq_event` is called from interrupt-related paths where dynamic connector registration is more plausible.

**Return value change:**

```c
return changed;
```

The old function returned `bool` from `int changed` — so `changed >= 1` became `true`. The new code returns `unsigned int changed` implicitly converted to `bool`, which works the same way. No issue.

**Minor nit:** The `kmalloc_array` indentation in both functions uses a tab for the second line of args (`sizeof(*changed_conns), GFP_KERNEL`), which should align with the first argument per kernel style. Looking at the diff context, the `output_poll_execute` version aligns to the tab stop after `kmalloc_array(`, while the `drm_helper_hpd_irq_event` version has:

```c
changed_conns = kmalloc_array(dev->mode_config.num_connector,
                              sizeof(*changed_conns), GFP_KERNEL);
```

vs:

```c
changed_conns = kmalloc_array(dev->mode_config.num_connector,
			sizeof(*changed_conns), GFP_KERNEL);
```

The latter (in `output_poll_execute`) uses tabs which doesn't align to the opening paren. The `drm_helper_hpd_irq_event` version looks better-aligned but both are within the range of acceptable kernel style.

**Summary for Patch 2:** The design is sound and the code is mostly correct. The main concern is the theoretical array overflow if `num_connector` increases between the allocation and the iteration. A simple fix would be to track the allocated size and add a bounds check, or to read `num_connector` under the appropriate lock. Given this is v8 and the window is tiny in practice, it may be acceptable with a comment noting the assumption, but it would be cleaner to guard against it.

---
Generated by Claude Code Patch Reviewer

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

[Nicolas Frattaroli]

I will be taking over this series from Marius Vlad. 

This series addresses a shortcoming whereby a hot plug event is sent
without it being passed the actual connector that caused it. This takes
into consideration both the polling path and the HPD (Hot Plug Detect)
path. It also adds support for the vkms driver (using ConfigFS) for
propagating the connector ID when changing the connector's status.

The motivation is that user-space applications such as Weston would
previously receive non-connector-specific hotplug events, and then have
to figure out themselves which connector needs to have a modeset
executed on. This notably did not work when the hotplug events came in
too fast, resulting in Weston missing an on-off-on transition of a
connector, seeing that its state was unchanged from "on" so can't be the
one that was hotplugged, and skipping reinitialising it as it looks
through the other connectors that could've caused it.

The real world implication is that on setups with slightly sketchy HDMI
connections, a brief flicker in the HPD signal could result in video
output bidding farewell entirely until a manual proper re-plug was
performed.

By sending connector specific hotplug events, this ambiguity is
resolved without any change to the user-space API.

Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
---
Changes in v7:
- Drop the two vkms patches, as I don't want them to be blocked on
  review. I still think they're correct, but they're not essential and
  don't need to block this series.
- Link to v6: https://lore.kernel.org/r/20260123-hot-plug-passup-v6-0-aaaf61d960bb@collabora.com

Changes in v6:
- Rewrote cover letter to explain the motivation for this series more
  plainly
- Rename "status_changed" to "pending_hp"
- Set "pending_hp" in the existing path that would also affect
  epoch_counter
- No longer set the boolean in drm_helper_probe_single_connector_modes,
  as it does not appear to be necessary
- Reword commits to better justify the changes
- Link to v5: https://lore.kernel.org/r/20251111162338.15141-1-marius.vlad@collabora.com/

Changes in v5:
- vkms: add support for sending the CONNECTOR ID when hot-plugging through
  ConfigFS - as reported by Louis, vkms can now make use of ConfigFS to
  simulate connector status.
- vkms: add a small change to ignore previous/old drm connector status
  when sending out hot-plug uevent.
- Link to v4: https://lore.kernel.org/r/20251103174558.7709-1-marius.vlad@collabora.com/

Changes in v4:
- removed the "This patch" bit - Dmitry
- added a short note when the flag is set and cleared - Dmitry
- address double dead-locking detected - kbot: https://lore.kernel.org/dri-devel/202509251410.fdfbcac3-lkp@intel.com/
- virtual connectors do not seem have any kind of hotplug - added
  polling in vkms - as noted by Ian
- Link to v3: https://lore.kernel.org/r/20250923083636.4749-1-marius.vlad@collabora.com/

Changes in v3:
- Address comments from Dmitry:
  - guard connector status write with mode_config.mutex
  - avoid setting up the connector status and immediately unset it. Do the
    unset in drm_kms_helper_hotplug_event/drm_kms_helper_connector_hotplug_event
- Link to v2: https://lore.kernel.org/r/20250729165708.9947-1-marius.vlad@collabora.com/

Changes in 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
- Link to v1: https://lore.kernel.org/r/20250627131751.2004-1-marius.vlad@collabora.com/

Marius Vlad (4):
  drm: Introduce a new connector status
  drm: Propagate connector status change
  vkms: Do not send hotplug events for same connector status
  vkms: Pass the vkms connector as opposed to the vkms device

 drivers/gpu/drm/drm_connector.c       |  1 +
 drivers/gpu/drm/drm_probe_helper.c    | 39 +++++++++++++++++++++++----
 drivers/gpu/drm/drm_sysfs.c           |  1 +
 drivers/gpu/drm/vkms/vkms_configfs.c  | 12 +++++++--
 drivers/gpu/drm/vkms/vkms_connector.c |  6 ++---
 drivers/gpu/drm/vkms/vkms_connector.h |  4 +--
 include/drm/drm_connector.h           |  3 +++
 7 files changed, 54 insertions(+), 12 deletions(-)

--
2.47.2

---
Marius Vlad (2):
      drm: Introduce pending_hp to drm_connector
      drm: Send per-connector hotplug events

 drivers/gpu/drm/drm_connector.c    |  1 +
 drivers/gpu/drm/drm_probe_helper.c | 39 +++++++++++++++++++++++++++++++++-----
 drivers/gpu/drm/drm_sysfs.c        |  2 ++
 include/drm/drm_connector.h        |  3 +++
 4 files changed, 40 insertions(+), 5 deletions(-)
---
base-commit: e66c64e4f12e55cad35c680f1e328a7c7ee92cab
change-id: 20260121-hot-plug-passup-f8ed03f7c202

Best regards,
-- 
Nicolas Frattaroli <nicolas.frattaroli@collabora.com>

[PATCH v7 2/2] drm: Send per-connector hotplug events

[Nicolas Frattaroli]

From: Marius Vlad <marius.vlad@collabora.com>

Use the new pending_hp member of drm_connector to always send
per-connector hotplug events for those connectors that need it, rather
than sending a global event, or only an event for one connector.

On the HPD (Hot Plug Detect) path this change notifies all connectors,
rather than just first changed connector.

The polling path is changed to no longer send a connector-less hotplug
event, but similarly send a hotplug event for each changed connector.

Signed-off-by: Marius Vlad <marius.vlad@collabora.com>
Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@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 f8cbd6713960..5c39f27ada1d 100644
--- a/drivers/gpu/drm/drm_probe_helper.c
+++ b/drivers/gpu/drm/drm_probe_helper.c
@@ -860,8 +860,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->pending_hp)
+				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);
@@ -1124,10 +1130,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->pending_hp)
+				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.53.0

Claude review: drm: Send per-connector hotplug events

[Claude Code Review Bot]

Patch Review

This patch converts both the polling and HPD paths to send per-connector hotplug events instead of global ones.

**Bug: `delayed_event` path drops hotplug notifications**

> -	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->pending_hp)
> +				drm_kms_helper_connector_hotplug_event(connector);
> +		}
> +		drm_connector_list_iter_end(&conn_iter);
> +	}

In `output_poll_execute`, `changed` can become true via two paths: (1) the connector iteration loop detects an epoch counter change and sets both `changed = true` and `connector->pending_hp = true`, or (2) `delayed_event` was already set at the top of the function:

```c
changed = dev->mode_config.delayed_event;
dev->mode_config.delayed_event = false;
```

The `delayed_event` path is triggered from `drm_helper_probe_single_connector_modes` when userspace probes a connector and finds its status changed. In this case, `changed` is true but no connector has `pending_hp` set, so the new loop will iterate all connectors, find none with `pending_hp`, and send no hotplug event at all. The old code called `drm_kms_helper_hotplug_event(dev)` which would have sent a global uevent.

This is a regression: connector status changes detected through userspace-initiated probing will no longer generate hotplug events.

One possible fix: set `pending_hp = true` on the connector in `drm_helper_probe_single_connector_modes` where `delayed_event` is set. Alternatively, fall back to the global `drm_kms_helper_hotplug_event(dev)` when `changed` is true but the connector loop sends zero events.

**Performance: `drm_client_dev_hotplug` called per-connector**

Each call to `drm_kms_helper_connector_hotplug_event` calls `drm_client_dev_hotplug(dev)`, which iterates all DRM clients and invokes their hotplug callback. When multiple connectors change simultaneously, this means the client hotplug callbacks (e.g., fbdev emulation) run N times instead of once. For the common case of a dock with multiple outputs, this is redundant work. It is not a correctness bug, but it may be worth noting.

**HPD path: the `changed == 1` fast path**

> -	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->pending_hp)
> +				drm_kms_helper_connector_hotplug_event(connector);
> +		}
> +		drm_connector_list_iter_end(&conn_iter);
> +	}

The HPD path in `drm_helper_hpd_irq_event` looks correct. When `changed == 1`, it uses `first_changed_connector` directly (which also has `pending_hp` set, so `drm_kms_helper_connector_hotplug_event` will clear it). When `changed > 1`, it iterates and sends individual events. The `pending_hp` flag was set under `mode_config.mutex` and read here after the mutex was dropped -- this is fine because this is single-threaded within this function and the writes are already committed.

**Reading `pending_hp` without the mutex**

In both the polling and HPD paths, `connector->pending_hp` is read without holding `mode_config.mutex`. The write happened under the mutex and the read happens on the same thread after the mutex was dropped, so the value will be visible. Concurrent writers (e.g., `status_store` on another CPU) could theoretically set `pending_hp` on a connector between the mutex drop and the iteration, but this is benign -- it just means an extra connector event may fire, which is harmless.

No other issues found in this patch beyond the `delayed_event` bug described above.

---
Generated by Claude Code Patch Reviewer