[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

[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

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

[Daniel Stone]

Hi,

On Wed, 22 Apr 2026 at 19:24, Nicolas Frattaroli
<nicolas.frattaroli@collabora.com> wrote:
> @@ -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);
> @@ -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) {

Not your doing, as it was just the same before, but doesn't this read
need to be protected by the mode_config mutex?

The series is very satisfying otherwise; both patches are:
Reviewed-by: Daniel Stone <daniels@collabora.com>

Cheers,
Daniel

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

[Nicolas Frattaroli]

+To: Simona Vetter, who wrote two of the commits relevant to this question.

On Thursday, 21 May 2026 17:00:17 Central European Summer Time Daniel Stone wrote:
> Hi,
> 
> On Wed, 22 Apr 2026 at 19:24, Nicolas Frattaroli
> <nicolas.frattaroli@collabora.com> wrote:
> > @@ -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);
> > @@ -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) {
> 
> Not your doing, as it was just the same before, but doesn't this read
> need to be protected by the mode_config mutex?

It looks like the fuzzy meaning of the mode_config.mutex came to
bite here. It is set to true with the lock held in
drm_helper_probe_single_connector_modes(), but set to false in this
function without the lock, and always read without the lock
(drm_kms_helper_poll_enable(), reschedule_output_poll_work()). Some
of these calls happen in paths where it might be inconvenient to take
a lock this coarse.

Specifically, drm_helper_probe_single_connector_modes() has this
comment right before setting delayed_event to true:

	/*
	 * The hotplug event code might call into the fb
	 * helpers, and so expects that we do not hold any
	 * locks. Fire up the poll struct instead, it will
	 * disable itself again.
	 */

So there was a problem with accessing parts under a lock before,
so if we're reintroducing the coarse lock wherever the boolean is
accessed we might be going backwards here.

The relevant commits are:

Commit 162b6a57ac50 (drm/probe-helper: don't lose hotplug event, 2015-01-21)
Commit ed293f775493 (drm/sysfs: Send out uevent when connector->force changes, 2015-11-19)

One obvious move would be to change it to an atomic so at least we
punch through any caches, but that doesn't help if the intended
design was that it's in a consistent state with the rest of
mode_config. Though I don't think that's the case?

Kind regards,
Nicolas Frattaroli

> 
> The series is very satisfying otherwise; both patches are:
> Reviewed-by: Daniel Stone <daniels@collabora.com>
> 
> Cheers,
> Daniel
> 

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

[Daniel Stone]

On Fri, 22 May 2026 at 14:06, Nicolas Frattaroli
<nicolas.frattaroli@collabora.com> wrote:
> On Thursday, 21 May 2026 17:00:17 Central European Summer Time Daniel Stone wrote:
> > On Wed, 22 Apr 2026 at 19:24, Nicolas Frattaroli
> > <nicolas.frattaroli@collabora.com> wrote:
> > > +       /* Pick up any changes detected by the probe functions. */
> > > +       if (dev->mode_config.delayed_event) {
> >
> > Not your doing, as it was just the same before, but doesn't this read
> > need to be protected by the mode_config mutex?
>
> It looks like the fuzzy meaning of the mode_config.mutex came to
> bite here. It is set to true with the lock held in
> drm_helper_probe_single_connector_modes(), but set to false in this
> function without the lock, and always read without the lock
> (drm_kms_helper_poll_enable(), reschedule_output_poll_work()). Some
> of these calls happen in paths where it might be inconvenient to take
> a lock this coarse.
>
> Specifically, drm_helper_probe_single_connector_modes() has this
> comment right before setting delayed_event to true:
>
>         /*
>          * The hotplug event code might call into the fb
>          * helpers, and so expects that we do not hold any
>          * locks. Fire up the poll struct instead, it will
>          * disable itself again.
>          */
>
> So there was a problem with accessing parts under a lock before,
> so if we're reintroducing the coarse lock wherever the boolean is
> accessed we might be going backwards here.

Yeah, that makes sense - those are definitely called from an IRQ
context so it wouldn't be safe to take the mode_config mutex.

I wonder if the safest path which guarantees eventual consistency is
to change the !trylock(mode_config) { goto out } path in
output_poll_execute() to jump directly to the schedule_delayed_work()?
That way we could read and modify delayed_event under the mode_config
mutex; the if (changed) path will not be taken if it wasn't possible
to take the mode_config mutex.

Cheers,
Daniel

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

[Ville Syrjälä]

On Fri, May 22, 2026 at 06:58:26PM +0100, Daniel Stone wrote:
> On Fri, 22 May 2026 at 14:06, Nicolas Frattaroli
> <nicolas.frattaroli@collabora.com> wrote:
> > On Thursday, 21 May 2026 17:00:17 Central European Summer Time Daniel Stone wrote:
> > > On Wed, 22 Apr 2026 at 19:24, Nicolas Frattaroli
> > > <nicolas.frattaroli@collabora.com> wrote:
> > > > +       /* Pick up any changes detected by the probe functions. */
> > > > +       if (dev->mode_config.delayed_event) {
> > >
> > > Not your doing, as it was just the same before, but doesn't this read
> > > need to be protected by the mode_config mutex?
> >
> > It looks like the fuzzy meaning of the mode_config.mutex came to
> > bite here. It is set to true with the lock held in
> > drm_helper_probe_single_connector_modes(), but set to false in this
> > function without the lock, and always read without the lock
> > (drm_kms_helper_poll_enable(), reschedule_output_poll_work()). Some
> > of these calls happen in paths where it might be inconvenient to take
> > a lock this coarse.
> >
> > Specifically, drm_helper_probe_single_connector_modes() has this
> > comment right before setting delayed_event to true:
> >
> >         /*
> >          * The hotplug event code might call into the fb
> >          * helpers, and so expects that we do not hold any
> >          * locks. Fire up the poll struct instead, it will
> >          * disable itself again.
> >          */
> >
> > So there was a problem with accessing parts under a lock before,
> > so if we're reintroducing the coarse lock wherever the boolean is
> > accessed we might be going backwards here.
> 
> Yeah, that makes sense - those are definitely called from an IRQ
> context so it wouldn't be safe to take the mode_config mutex.
> 
> I wonder if the safest path which guarantees eventual consistency is
> to change the !trylock(mode_config) { goto out } path in
> output_poll_execute() to jump directly to the schedule_delayed_work()?
> That way we could read and modify delayed_event under the mode_config
> mutex; the if (changed) path will not be taken if it wasn't possible
> to take the mode_config mutex.

I've been cursing at this delayed_event hack a couple of times.
Seems to me that the correct solution would be to make
drm_client_dev_hotplug() schedule its own work and do the actual
work there instead of this direct call we have right now.

-- 
Ville Syrjälä
Intel

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

[Nicolas Frattaroli]

On Friday, 22 May 2026 19:58:26 Central European Summer Time Daniel Stone wrote:
> On Fri, 22 May 2026 at 14:06, Nicolas Frattaroli
> <nicolas.frattaroli@collabora.com> wrote:
> > On Thursday, 21 May 2026 17:00:17 Central European Summer Time Daniel Stone wrote:
> > > On Wed, 22 Apr 2026 at 19:24, Nicolas Frattaroli
> > > <nicolas.frattaroli@collabora.com> wrote:
> > > > +       /* Pick up any changes detected by the probe functions. */
> > > > +       if (dev->mode_config.delayed_event) {
> > >
> > > Not your doing, as it was just the same before, but doesn't this read
> > > need to be protected by the mode_config mutex?
> >
> > It looks like the fuzzy meaning of the mode_config.mutex came to
> > bite here. It is set to true with the lock held in
> > drm_helper_probe_single_connector_modes(), but set to false in this
> > function without the lock, and always read without the lock
> > (drm_kms_helper_poll_enable(), reschedule_output_poll_work()). Some
> > of these calls happen in paths where it might be inconvenient to take
> > a lock this coarse.
> >
> > Specifically, drm_helper_probe_single_connector_modes() has this
> > comment right before setting delayed_event to true:
> >
> >         /*
> >          * The hotplug event code might call into the fb
> >          * helpers, and so expects that we do not hold any
> >          * locks. Fire up the poll struct instead, it will
> >          * disable itself again.
> >          */
> >
> > So there was a problem with accessing parts under a lock before,
> > so if we're reintroducing the coarse lock wherever the boolean is
> > accessed we might be going backwards here.
> 
> Yeah, that makes sense - those are definitely called from an IRQ
> context so it wouldn't be safe to take the mode_config mutex.
> 
> I wonder if the safest path which guarantees eventual consistency is
> to change the !trylock(mode_config) { goto out } path in
> output_poll_execute() to jump directly to the schedule_delayed_work()?
> That way we could read and modify delayed_event under the mode_config
> mutex; the if (changed) path will not be taken if it wasn't possible
> to take the mode_config mutex.

If the drm_kms_helper_poll module parameter is set to false (i.e. the
non-default setting), with my patch we'd then also need to think about
the behavior if delayed_event == true in that case, since it also goes
to `out` without taking the lock.

I think I need to ponder this particular orb some more. I suspect it'd
be fine to move the delayed_event clearing + sending to before the
mutex unlock, since failing to lock the mutex will just make it repoll
and try again in 0.1 seconds. The event would only be lost if the
another delayed_event arrives before it's processed.

I think Ville's on the money here in that the users of delayed_event
might want to call a drm_client_dev_hotplug that does the work
scheduling instead, but that might be a bigger thing to work towards.

Kind regards,
Nicolas Frattaroli

> 
> Cheers,
> Daniel
> 

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

[Daniel Stone]

Hi,

On Fri, 22 May 2026 at 20:28, Nicolas Frattaroli
<nicolas.frattaroli@collabora.com> wrote:
> On Friday, 22 May 2026 19:58:26 Central European Summer Time Daniel Stone wrote:
> > I wonder if the safest path which guarantees eventual consistency is
> > to change the !trylock(mode_config) { goto out } path in
> > output_poll_execute() to jump directly to the schedule_delayed_work()?
> > That way we could read and modify delayed_event under the mode_config
> > mutex; the if (changed) path will not be taken if it wasn't possible
> > to take the mode_config mutex.
>
> If the drm_kms_helper_poll module parameter is set to false (i.e. the
> non-default setting), with my patch we'd then also need to think about
> the behavior if delayed_event == true in that case, since it also goes
> to `out` without taking the lock.
>
> I think I need to ponder this particular orb some more. I suspect it'd
> be fine to move the delayed_event clearing + sending to before the
> mutex unlock, since failing to lock the mutex will just make it repoll
> and try again in 0.1 seconds. The event would only be lost if the
> another delayed_event arrives before it's processed.
>
> I think Ville's on the money here in that the users of delayed_event
> might want to call a drm_client_dev_hotplug that does the work
> scheduling instead, but that might be a bigger thing to work towards.

Yeah. That would certainly be a series (ok, multiple series - the sun
has made me optimistic I suppose) in itself, and I don't think it
makes sense to entangle the two.

Cheers,
Daniel