* [PATCH v5 0/4] Pass down hot-plug CONNECTOR ID to user-space
@ 2025-11-11 16:23 Marius Vlad
2025-11-11 16:23 ` [PATCH 1/4] drm: Introduce a new connector status Marius Vlad
` (3 more replies)
0 siblings, 4 replies; 9+ messages in thread
From: Marius Vlad @ 2025-11-11 16:23 UTC (permalink / raw)
To: dri-devel
Cc: dmitry.baryshkov, tzimmermann, simona.vetter, jani.nikula,
ian.forbes, louis.chauvet, daniel.stone
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.
It also adds support for the vkms driver (using ConfigFS) for
propagating the CONNECTOR ID when changing the connector's status.
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.
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
- v4 is at https://lore.kernel.org/dri-devel/20251103174558.7709-1-marius.vlad@collabora.com/
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
- v3 is at https://lore.kernel.org/dri-devel/20250923083636.4749-1-marius.vlad@collabora.com/
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
- v2 is at https://lore.kernel.org/dri-devel/20250729165708.9947-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
^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH 1/4] drm: Introduce a new connector status
2025-11-11 16:23 [PATCH v5 0/4] Pass down hot-plug CONNECTOR ID to user-space Marius Vlad
@ 2025-11-11 16:23 ` Marius Vlad
2025-11-11 18:54 ` Ville Syrjälä
2025-11-11 16:23 ` [PATCH 2/4] drm: Propagate connector status change Marius Vlad
` (2 subsequent siblings)
3 siblings, 1 reply; 9+ messages in thread
From: Marius Vlad @ 2025-11-11 16:23 UTC (permalink / raw)
To: dri-devel
Cc: dmitry.baryshkov, tzimmermann, simona.vetter, jani.nikula,
ian.forbes, louis.chauvet, daniel.stone
Introduce 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 change would make use of this connector status to propagate
per-connector udev hotplug events.
The connector status is set in the drm_connector_funcs.fill_modes/vkms
ConfigFS connector's status and cleared out when firing out KMS uevents.
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 | 17 +++++++++++++++++
drivers/gpu/drm/drm_sysfs.c | 1 +
drivers/gpu/drm/vkms/vkms_configfs.c | 6 ++++++
include/drm/drm_connector.h | 3 +++
5 files changed, 28 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 09b12c30df69..f0474368e98d 100644
--- a/drivers/gpu/drm/drm_probe_helper.c
+++ b/drivers/gpu/drm/drm_probe_helper.c
@@ -629,6 +629,8 @@ 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;
}
/*
@@ -732,6 +734,17 @@ 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;
+
+ mutex_lock(&dev->mode_config.mutex);
+ 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);
+ mutex_unlock(&dev->mode_config.mutex);
+
drm_sysfs_hotplug_event(dev);
drm_client_dev_hotplug(dev);
}
@@ -748,6 +761,10 @@ void drm_kms_helper_connector_hotplug_event(struct drm_connector *connector)
{
struct drm_device *dev = connector->dev;
+ mutex_lock(&dev->mode_config.mutex);
+ connector->status_changed = false;
+ mutex_unlock(&dev->mode_config.mutex);
+
drm_sysfs_connector_hotplug_event(connector);
drm_client_dev_hotplug(dev);
}
diff --git a/drivers/gpu/drm/drm_sysfs.c b/drivers/gpu/drm/drm_sysfs.c
index b01ffa4d6509..bd9161490116 100644
--- a/drivers/gpu/drm/drm_sysfs.c
+++ b/drivers/gpu/drm/drm_sysfs.c
@@ -199,6 +199,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/drivers/gpu/drm/vkms/vkms_configfs.c b/drivers/gpu/drm/vkms/vkms_configfs.c
index 506666e21c91..6d6dd1a2c3a6 100644
--- a/drivers/gpu/drm/vkms/vkms_configfs.c
+++ b/drivers/gpu/drm/vkms/vkms_configfs.c
@@ -537,8 +537,14 @@ static ssize_t connector_status_store(struct config_item *item,
{
struct vkms_configfs_connector *connector;
enum drm_connector_status status;
+ struct vkms_connector *vkms_connector;
+ struct vkms_device *vkms_dev;
connector = connector_item_to_vkms_configfs_connector(item);
+ vkms_connector = connector->config->connector;
+ vkms_dev = connector->config->config->dev;
+ scoped_guard(mutex, &vkms_dev->drm.mode_config.mutex)
+ vkms_connector->base.status_changed = true;
if (kstrtouint(page, 10, &status))
return -EINVAL;
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
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 2/4] drm: Propagate connector status change
2025-11-11 16:23 [PATCH v5 0/4] Pass down hot-plug CONNECTOR ID to user-space Marius Vlad
2025-11-11 16:23 ` [PATCH 1/4] drm: Introduce a new connector status Marius Vlad
@ 2025-11-11 16:23 ` Marius Vlad
2025-11-11 16:23 ` [PATCH 3/4] vkms: Do not send hotplug events for same connector status Marius Vlad
2025-11-11 16:23 ` [PATCH 4/4] vkms: Pass the vkms connector as opposed to the vkms device Marius Vlad
3 siblings, 0 replies; 9+ messages in thread
From: Marius Vlad @ 2025-11-11 16:23 UTC (permalink / raw)
To: dri-devel
Cc: dmitry.baryshkov, tzimmermann, simona.vetter, jani.nikula,
ian.forbes, louis.chauvet, daniel.stone
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 hotplug 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 f0474368e98d..63960f589b6b 100644
--- a/drivers/gpu/drm/drm_probe_helper.c
+++ b/drivers/gpu/drm/drm_probe_helper.c
@@ -861,8 +861,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);
@@ -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->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
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 3/4] vkms: Do not send hotplug events for same connector status
2025-11-11 16:23 [PATCH v5 0/4] Pass down hot-plug CONNECTOR ID to user-space Marius Vlad
2025-11-11 16:23 ` [PATCH 1/4] drm: Introduce a new connector status Marius Vlad
2025-11-11 16:23 ` [PATCH 2/4] drm: Propagate connector status change Marius Vlad
@ 2025-11-11 16:23 ` Marius Vlad
2025-11-11 16:23 ` [PATCH 4/4] vkms: Pass the vkms connector as opposed to the vkms device Marius Vlad
3 siblings, 0 replies; 9+ messages in thread
From: Marius Vlad @ 2025-11-11 16:23 UTC (permalink / raw)
To: dri-devel
Cc: dmitry.baryshkov, tzimmermann, simona.vetter, jani.nikula,
ian.forbes, louis.chauvet, daniel.stone
Rather than sending the same hotplug events just do it when the new drm
connector status has been changed.
Signed-off-by: Marius Vlad <marius.vlad@collabora.com>
---
drivers/gpu/drm/vkms/vkms_configfs.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/vkms/vkms_configfs.c b/drivers/gpu/drm/vkms/vkms_configfs.c
index 6d6dd1a2c3a6..2b0ca61da62e 100644
--- a/drivers/gpu/drm/vkms/vkms_configfs.c
+++ b/drivers/gpu/drm/vkms/vkms_configfs.c
@@ -555,9 +555,11 @@ static ssize_t connector_status_store(struct config_item *item,
return -EINVAL;
scoped_guard(mutex, &connector->dev->lock) {
+ enum drm_connector_status old_status =
+ vkms_config_connector_get_status(connector->config);
vkms_config_connector_set_status(connector->config, status);
- if (connector->dev->enabled)
+ if (connector->dev->enabled && old_status != status)
vkms_trigger_connector_hotplug(connector->dev->config->dev);
}
--
2.47.2
^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH 4/4] vkms: Pass the vkms connector as opposed to the vkms device
2025-11-11 16:23 [PATCH v5 0/4] Pass down hot-plug CONNECTOR ID to user-space Marius Vlad
` (2 preceding siblings ...)
2025-11-11 16:23 ` [PATCH 3/4] vkms: Do not send hotplug events for same connector status Marius Vlad
@ 2025-11-11 16:23 ` Marius Vlad
3 siblings, 0 replies; 9+ messages in thread
From: Marius Vlad @ 2025-11-11 16:23 UTC (permalink / raw)
To: dri-devel
Cc: dmitry.baryshkov, tzimmermann, simona.vetter, jani.nikula,
ian.forbes, louis.chauvet, daniel.stone
This way we can pass straight in the CONNECTOR ID.
Signed-off-by: Marius Vlad <marius.vlad@collabora.com>
---
drivers/gpu/drm/vkms/vkms_configfs.c | 2 +-
drivers/gpu/drm/vkms/vkms_connector.c | 6 +++---
drivers/gpu/drm/vkms/vkms_connector.h | 4 ++--
3 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/vkms/vkms_configfs.c b/drivers/gpu/drm/vkms/vkms_configfs.c
index 2b0ca61da62e..709bcc8bebc1 100644
--- a/drivers/gpu/drm/vkms/vkms_configfs.c
+++ b/drivers/gpu/drm/vkms/vkms_configfs.c
@@ -560,7 +560,7 @@ static ssize_t connector_status_store(struct config_item *item,
vkms_config_connector_set_status(connector->config, status);
if (connector->dev->enabled && old_status != status)
- vkms_trigger_connector_hotplug(connector->dev->config->dev);
+ vkms_trigger_connector_hotplug(vkms_connector);
}
return (ssize_t)count;
diff --git a/drivers/gpu/drm/vkms/vkms_connector.c b/drivers/gpu/drm/vkms/vkms_connector.c
index b0a6b212d3f4..cad64eff72ea 100644
--- a/drivers/gpu/drm/vkms/vkms_connector.c
+++ b/drivers/gpu/drm/vkms/vkms_connector.c
@@ -88,9 +88,9 @@ struct vkms_connector *vkms_connector_init(struct vkms_device *vkmsdev)
return connector;
}
-void vkms_trigger_connector_hotplug(struct vkms_device *vkmsdev)
+void vkms_trigger_connector_hotplug(struct vkms_connector *vkms_connector)
{
- struct drm_device *dev = &vkmsdev->drm;
+ struct drm_connector *connector = &vkms_connector->base;
- drm_kms_helper_hotplug_event(dev);
+ drm_kms_helper_connector_hotplug_event(connector);
}
diff --git a/drivers/gpu/drm/vkms/vkms_connector.h b/drivers/gpu/drm/vkms/vkms_connector.h
index ed312f4eff3a..7cd76d01b10b 100644
--- a/drivers/gpu/drm/vkms/vkms_connector.h
+++ b/drivers/gpu/drm/vkms/vkms_connector.h
@@ -28,8 +28,8 @@ struct vkms_connector *vkms_connector_init(struct vkms_device *vkmsdev);
/**
* vkms_trigger_connector_hotplug() - Update the device's connectors status
- * @vkmsdev: VKMS device to update
+ * @vkms_connector: VKMS connector to update
*/
-void vkms_trigger_connector_hotplug(struct vkms_device *vkmsdev);
+void vkms_trigger_connector_hotplug(struct vkms_connector *vkms_connector);
#endif /* _VKMS_CONNECTOR_H_ */
--
2.47.2
^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH 1/4] drm: Introduce a new connector status
2025-11-11 16:23 ` [PATCH 1/4] drm: Introduce a new connector status Marius Vlad
@ 2025-11-11 18:54 ` Ville Syrjälä
2025-11-11 19:13 ` Ville Syrjälä
2025-11-17 12:48 ` Marius Vlad
0 siblings, 2 replies; 9+ messages in thread
From: Ville Syrjälä @ 2025-11-11 18:54 UTC (permalink / raw)
To: Marius Vlad
Cc: dri-devel, dmitry.baryshkov, tzimmermann, simona.vetter,
jani.nikula, ian.forbes, louis.chauvet, daniel.stone
On Tue, Nov 11, 2025 at 06:23:35PM +0200, Marius Vlad wrote:
> Introduce 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 change would make use of this connector status to propagate
> per-connector udev hotplug events.
>
> The connector status is set in the drm_connector_funcs.fill_modes/vkms
> ConfigFS connector's status and cleared out when firing out KMS uevents.
>
> 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 | 17 +++++++++++++++++
> drivers/gpu/drm/drm_sysfs.c | 1 +
> drivers/gpu/drm/vkms/vkms_configfs.c | 6 ++++++
> include/drm/drm_connector.h | 3 +++
> 5 files changed, 28 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 09b12c30df69..f0474368e98d 100644
> --- a/drivers/gpu/drm/drm_probe_helper.c
> +++ b/drivers/gpu/drm/drm_probe_helper.c
> @@ -629,6 +629,8 @@ 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;
.fill_modes() gets exectued from the getconnector() ioctl which userspace
issues in response to the uevent. Not the other way around. So looks
like you have the chicken and egg the wrong way around here.
> }
>
> /*
> @@ -732,6 +734,17 @@ 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;
> +
> + mutex_lock(&dev->mode_config.mutex);
> + 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);
> + mutex_unlock(&dev->mode_config.mutex);
> +
> drm_sysfs_hotplug_event(dev);
> drm_client_dev_hotplug(dev);
> }
> @@ -748,6 +761,10 @@ void drm_kms_helper_connector_hotplug_event(struct drm_connector *connector)
> {
> struct drm_device *dev = connector->dev;
>
> + mutex_lock(&dev->mode_config.mutex);
> + connector->status_changed = false;
> + mutex_unlock(&dev->mode_config.mutex);
> +
> drm_sysfs_connector_hotplug_event(connector);
> drm_client_dev_hotplug(dev);
> }
> diff --git a/drivers/gpu/drm/drm_sysfs.c b/drivers/gpu/drm/drm_sysfs.c
> index b01ffa4d6509..bd9161490116 100644
> --- a/drivers/gpu/drm/drm_sysfs.c
> +++ b/drivers/gpu/drm/drm_sysfs.c
> @@ -199,6 +199,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/drivers/gpu/drm/vkms/vkms_configfs.c b/drivers/gpu/drm/vkms/vkms_configfs.c
> index 506666e21c91..6d6dd1a2c3a6 100644
> --- a/drivers/gpu/drm/vkms/vkms_configfs.c
> +++ b/drivers/gpu/drm/vkms/vkms_configfs.c
> @@ -537,8 +537,14 @@ static ssize_t connector_status_store(struct config_item *item,
> {
> struct vkms_configfs_connector *connector;
> enum drm_connector_status status;
> + struct vkms_connector *vkms_connector;
> + struct vkms_device *vkms_dev;
>
> connector = connector_item_to_vkms_configfs_connector(item);
> + vkms_connector = connector->config->connector;
> + vkms_dev = connector->config->config->dev;
> + scoped_guard(mutex, &vkms_dev->drm.mode_config.mutex)
> + vkms_connector->base.status_changed = true;
>
> if (kstrtouint(page, 10, &status))
> return -EINVAL;
> 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
--
Ville Syrjälä
Intel
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/4] drm: Introduce a new connector status
2025-11-11 18:54 ` Ville Syrjälä
@ 2025-11-11 19:13 ` Ville Syrjälä
2025-11-17 13:06 ` Marius Vlad
2025-11-17 12:48 ` Marius Vlad
1 sibling, 1 reply; 9+ messages in thread
From: Ville Syrjälä @ 2025-11-11 19:13 UTC (permalink / raw)
To: Marius Vlad
Cc: dri-devel, dmitry.baryshkov, tzimmermann, simona.vetter,
jani.nikula, ian.forbes, louis.chauvet, daniel.stone
On Tue, Nov 11, 2025 at 08:54:59PM +0200, Ville Syrjälä wrote:
> On Tue, Nov 11, 2025 at 06:23:35PM +0200, Marius Vlad wrote:
> > Introduce 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 change would make use of this connector status to propagate
> > per-connector udev hotplug events.
> >
> > The connector status is set in the drm_connector_funcs.fill_modes/vkms
> > ConfigFS connector's status and cleared out when firing out KMS uevents.
> >
> > 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 | 17 +++++++++++++++++
> > drivers/gpu/drm/drm_sysfs.c | 1 +
> > drivers/gpu/drm/vkms/vkms_configfs.c | 6 ++++++
> > include/drm/drm_connector.h | 3 +++
> > 5 files changed, 28 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 09b12c30df69..f0474368e98d 100644
> > --- a/drivers/gpu/drm/drm_probe_helper.c
> > +++ b/drivers/gpu/drm/drm_probe_helper.c
> > @@ -629,6 +629,8 @@ 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;
>
> .fill_modes() gets exectued from the getconnector() ioctl which userspace
> issues in response to the uevent. Not the other way around. So looks
> like you have the chicken and egg the wrong way around here.
Oh, and we already have a connector->epoch_counter which is supposed
track whether anything on the connector has changed. So I'm not sure
what extra this new boolean is supposed to achieve on top of that?
> > }
> >
> > /*
> > @@ -732,6 +734,17 @@ 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;
> > +
> > + mutex_lock(&dev->mode_config.mutex);
> > + 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);
> > + mutex_unlock(&dev->mode_config.mutex);
> > +
> > drm_sysfs_hotplug_event(dev);
> > drm_client_dev_hotplug(dev);
> > }
> > @@ -748,6 +761,10 @@ void drm_kms_helper_connector_hotplug_event(struct drm_connector *connector)
> > {
> > struct drm_device *dev = connector->dev;
> >
> > + mutex_lock(&dev->mode_config.mutex);
> > + connector->status_changed = false;
> > + mutex_unlock(&dev->mode_config.mutex);
> > +
> > drm_sysfs_connector_hotplug_event(connector);
> > drm_client_dev_hotplug(dev);
> > }
> > diff --git a/drivers/gpu/drm/drm_sysfs.c b/drivers/gpu/drm/drm_sysfs.c
> > index b01ffa4d6509..bd9161490116 100644
> > --- a/drivers/gpu/drm/drm_sysfs.c
> > +++ b/drivers/gpu/drm/drm_sysfs.c
> > @@ -199,6 +199,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/drivers/gpu/drm/vkms/vkms_configfs.c b/drivers/gpu/drm/vkms/vkms_configfs.c
> > index 506666e21c91..6d6dd1a2c3a6 100644
> > --- a/drivers/gpu/drm/vkms/vkms_configfs.c
> > +++ b/drivers/gpu/drm/vkms/vkms_configfs.c
> > @@ -537,8 +537,14 @@ static ssize_t connector_status_store(struct config_item *item,
> > {
> > struct vkms_configfs_connector *connector;
> > enum drm_connector_status status;
> > + struct vkms_connector *vkms_connector;
> > + struct vkms_device *vkms_dev;
> >
> > connector = connector_item_to_vkms_configfs_connector(item);
> > + vkms_connector = connector->config->connector;
> > + vkms_dev = connector->config->config->dev;
> > + scoped_guard(mutex, &vkms_dev->drm.mode_config.mutex)
> > + vkms_connector->base.status_changed = true;
> >
> > if (kstrtouint(page, 10, &status))
> > return -EINVAL;
> > 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
>
> --
> Ville Syrjälä
> Intel
--
Ville Syrjälä
Intel
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/4] drm: Introduce a new connector status
2025-11-11 18:54 ` Ville Syrjälä
2025-11-11 19:13 ` Ville Syrjälä
@ 2025-11-17 12:48 ` Marius Vlad
1 sibling, 0 replies; 9+ messages in thread
From: Marius Vlad @ 2025-11-17 12:48 UTC (permalink / raw)
To: Ville Syrjälä
Cc: dri-devel, dmitry.baryshkov, tzimmermann, simona.vetter,
jani.nikula, ian.forbes, louis.chauvet, daniel.stone
[-- Attachment #1: Type: text/plain, Size: 5571 bytes --]
Hi Ville,
On Tue, Nov 11, 2025 at 08:54:59PM +0200, Ville Syrjälä wrote:
> On Tue, Nov 11, 2025 at 06:23:35PM +0200, Marius Vlad wrote:
> > Introduce 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 change would make use of this connector status to propagate
> > per-connector udev hotplug events.
> >
> > The connector status is set in the drm_connector_funcs.fill_modes/vkms
> > ConfigFS connector's status and cleared out when firing out KMS uevents.
> >
> > 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 | 17 +++++++++++++++++
> > drivers/gpu/drm/drm_sysfs.c | 1 +
> > drivers/gpu/drm/vkms/vkms_configfs.c | 6 ++++++
> > include/drm/drm_connector.h | 3 +++
> > 5 files changed, 28 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 09b12c30df69..f0474368e98d 100644
> > --- a/drivers/gpu/drm/drm_probe_helper.c
> > +++ b/drivers/gpu/drm/drm_probe_helper.c
> > @@ -629,6 +629,8 @@ 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;
>
> .fill_modes() gets exectued from the getconnector() ioctl which userspace
> issues in response to the uevent. Not the other way around. So looks
> like you have the chicken and egg the wrong way around here.
.fill_modes() is for drm_sysfs path to simulate a hot-plug event.
There's no getconnector in that case.
>
> > }
> >
> > /*
> > @@ -732,6 +734,17 @@ 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;
> > +
> > + mutex_lock(&dev->mode_config.mutex);
> > + 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);
> > + mutex_unlock(&dev->mode_config.mutex);
> > +
> > drm_sysfs_hotplug_event(dev);
> > drm_client_dev_hotplug(dev);
> > }
> > @@ -748,6 +761,10 @@ void drm_kms_helper_connector_hotplug_event(struct drm_connector *connector)
> > {
> > struct drm_device *dev = connector->dev;
> >
> > + mutex_lock(&dev->mode_config.mutex);
> > + connector->status_changed = false;
> > + mutex_unlock(&dev->mode_config.mutex);
> > +
> > drm_sysfs_connector_hotplug_event(connector);
> > drm_client_dev_hotplug(dev);
> > }
> > diff --git a/drivers/gpu/drm/drm_sysfs.c b/drivers/gpu/drm/drm_sysfs.c
> > index b01ffa4d6509..bd9161490116 100644
> > --- a/drivers/gpu/drm/drm_sysfs.c
> > +++ b/drivers/gpu/drm/drm_sysfs.c
> > @@ -199,6 +199,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/drivers/gpu/drm/vkms/vkms_configfs.c b/drivers/gpu/drm/vkms/vkms_configfs.c
> > index 506666e21c91..6d6dd1a2c3a6 100644
> > --- a/drivers/gpu/drm/vkms/vkms_configfs.c
> > +++ b/drivers/gpu/drm/vkms/vkms_configfs.c
> > @@ -537,8 +537,14 @@ static ssize_t connector_status_store(struct config_item *item,
> > {
> > struct vkms_configfs_connector *connector;
> > enum drm_connector_status status;
> > + struct vkms_connector *vkms_connector;
> > + struct vkms_device *vkms_dev;
> >
> > connector = connector_item_to_vkms_configfs_connector(item);
> > + vkms_connector = connector->config->connector;
> > + vkms_dev = connector->config->config->dev;
> > + scoped_guard(mutex, &vkms_dev->drm.mode_config.mutex)
> > + vkms_connector->base.status_changed = true;
> >
> > if (kstrtouint(page, 10, &status))
> > return -EINVAL;
> > 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
>
> --
> Ville Syrjälä
> Intel
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH 1/4] drm: Introduce a new connector status
2025-11-11 19:13 ` Ville Syrjälä
@ 2025-11-17 13:06 ` Marius Vlad
0 siblings, 0 replies; 9+ messages in thread
From: Marius Vlad @ 2025-11-17 13:06 UTC (permalink / raw)
To: Ville Syrjälä
Cc: dri-devel, dmitry.baryshkov, tzimmermann, simona.vetter,
jani.nikula, ian.forbes, louis.chauvet, daniel.stone
[-- Attachment #1: Type: text/plain, Size: 6564 bytes --]
On Tue, Nov 11, 2025 at 09:13:06PM +0200, Ville Syrjälä wrote:
> On Tue, Nov 11, 2025 at 08:54:59PM +0200, Ville Syrjälä wrote:
> > On Tue, Nov 11, 2025 at 06:23:35PM +0200, Marius Vlad wrote:
> > > Introduce 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 change would make use of this connector status to propagate
> > > per-connector udev hotplug events.
> > >
> > > The connector status is set in the drm_connector_funcs.fill_modes/vkms
> > > ConfigFS connector's status and cleared out when firing out KMS uevents.
> > >
> > > 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 | 17 +++++++++++++++++
> > > drivers/gpu/drm/drm_sysfs.c | 1 +
> > > drivers/gpu/drm/vkms/vkms_configfs.c | 6 ++++++
> > > include/drm/drm_connector.h | 3 +++
> > > 5 files changed, 28 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 09b12c30df69..f0474368e98d 100644
> > > --- a/drivers/gpu/drm/drm_probe_helper.c
> > > +++ b/drivers/gpu/drm/drm_probe_helper.c
> > > @@ -629,6 +629,8 @@ 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;
> >
> > .fill_modes() gets exectued from the getconnector() ioctl which userspace
> > issues in response to the uevent. Not the other way around. So looks
> > like you have the chicken and egg the wrong way around here.
>
> Oh, and we already have a connector->epoch_counter which is supposed
> track whether anything on the connector has changed. So I'm not sure
> what extra this new boolean is supposed to achieve on top of that?
epoch_counter doesn't take all cases into consideration. For the
instance the connector list loop in output_poll_execute will be a no-op
if we try to simulate a hot-plug event with drm_sysfs. I suppose we
could've have used it for (real) HW hot-plug.
There's a few use-cases:
- 2 with real HW hot-plug - one for polling and one for HPD
- 2 for with virtual connectors - one for VKMS and one for drm_sysfs
The bool helps to know -- in case you have multiple connectors -- which
of them had a hot-plug event.
>
> > > }
> > >
> > > /*
> > > @@ -732,6 +734,17 @@ 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;
> > > +
> > > + mutex_lock(&dev->mode_config.mutex);
> > > + 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);
> > > + mutex_unlock(&dev->mode_config.mutex);
> > > +
> > > drm_sysfs_hotplug_event(dev);
> > > drm_client_dev_hotplug(dev);
> > > }
> > > @@ -748,6 +761,10 @@ void drm_kms_helper_connector_hotplug_event(struct drm_connector *connector)
> > > {
> > > struct drm_device *dev = connector->dev;
> > >
> > > + mutex_lock(&dev->mode_config.mutex);
> > > + connector->status_changed = false;
> > > + mutex_unlock(&dev->mode_config.mutex);
> > > +
> > > drm_sysfs_connector_hotplug_event(connector);
> > > drm_client_dev_hotplug(dev);
> > > }
> > > diff --git a/drivers/gpu/drm/drm_sysfs.c b/drivers/gpu/drm/drm_sysfs.c
> > > index b01ffa4d6509..bd9161490116 100644
> > > --- a/drivers/gpu/drm/drm_sysfs.c
> > > +++ b/drivers/gpu/drm/drm_sysfs.c
> > > @@ -199,6 +199,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/drivers/gpu/drm/vkms/vkms_configfs.c b/drivers/gpu/drm/vkms/vkms_configfs.c
> > > index 506666e21c91..6d6dd1a2c3a6 100644
> > > --- a/drivers/gpu/drm/vkms/vkms_configfs.c
> > > +++ b/drivers/gpu/drm/vkms/vkms_configfs.c
> > > @@ -537,8 +537,14 @@ static ssize_t connector_status_store(struct config_item *item,
> > > {
> > > struct vkms_configfs_connector *connector;
> > > enum drm_connector_status status;
> > > + struct vkms_connector *vkms_connector;
> > > + struct vkms_device *vkms_dev;
> > >
> > > connector = connector_item_to_vkms_configfs_connector(item);
> > > + vkms_connector = connector->config->connector;
> > > + vkms_dev = connector->config->config->dev;
> > > + scoped_guard(mutex, &vkms_dev->drm.mode_config.mutex)
> > > + vkms_connector->base.status_changed = true;
> > >
> > > if (kstrtouint(page, 10, &status))
> > > return -EINVAL;
> > > 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
> >
> > --
> > Ville Syrjälä
> > Intel
>
> --
> Ville Syrjälä
> Intel
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2025-11-17 13:07 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-11-11 16:23 [PATCH v5 0/4] Pass down hot-plug CONNECTOR ID to user-space Marius Vlad
2025-11-11 16:23 ` [PATCH 1/4] drm: Introduce a new connector status Marius Vlad
2025-11-11 18:54 ` Ville Syrjälä
2025-11-11 19:13 ` Ville Syrjälä
2025-11-17 13:06 ` Marius Vlad
2025-11-17 12:48 ` Marius Vlad
2025-11-11 16:23 ` [PATCH 2/4] drm: Propagate connector status change Marius Vlad
2025-11-11 16:23 ` [PATCH 3/4] vkms: Do not send hotplug events for same connector status Marius Vlad
2025-11-11 16:23 ` [PATCH 4/4] vkms: Pass the vkms connector as opposed to the vkms device Marius Vlad
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.