[PATCH v3 0/6] drm: add per-connector hotplug events

[PATCH v3 0/6] drm: add per-connector hotplug events

[Simon Ser]

When a uevent only updates a single connector, add a CONNECTOR property
to the uevent. This allows user-space to ignore other connectors when
handling the uevent. This is purely an optimization, drivers can still
send a uevent without the CONNECTOR property.

The CONNECTOR property is already set when sending HDCP property update
uevents, see drm_sysfs_connector_status_event.

This has been tested with a wlroots patch [1].

amdgpu and the probe-helper has been updated to use these new fine-grained
uevents.

Changes in v3: rebase

[1]: https://github.com/swaywm/wlroots/pull/2959

Simon Ser (6):
  drm/sysfs: introduce drm_sysfs_connector_hotplug_event
  drm/probe-helper: add drm_kms_helper_connector_hotplug_event
  drm/connector: use drm_sysfs_connector_hotplug_event
  amdgpu: use drm_kms_helper_connector_hotplug_event
  drm/probe-helper: use drm_kms_helper_connector_hotplug_event
  i915/display/dp: send a more fine-grained link-status uevent

 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c |  8 ++--
 .../amd/display/amdgpu_dm/amdgpu_dm_debugfs.c |  4 +-
 drivers/gpu/drm/drm_connector.c               |  2 +-
 drivers/gpu/drm/drm_probe_helper.c            | 43 +++++++++++++++++--
 drivers/gpu/drm/drm_sysfs.c                   | 25 +++++++++++
 drivers/gpu/drm/i915/display/intel_dp.c       |  2 +
 include/drm/drm_probe_helper.h                |  1 +
 include/drm/drm_sysfs.h                       |  1 +
 8 files changed, 75 insertions(+), 11 deletions(-)


base-commit: f6632721cd6231e1bf28b5317dcc7543e43359f7
-- 
2.33.1

[PATCH v3 1/6] drm/sysfs: introduce drm_sysfs_connector_hotplug_event

[Simon Ser]

This function sends a hotplug uevent with a CONNECTOR property.

Signed-off-by: Simon Ser <contact@emersion.fr>
---
 drivers/gpu/drm/drm_sysfs.c | 25 +++++++++++++++++++++++++
 include/drm/drm_sysfs.h     |  1 +
 2 files changed, 26 insertions(+)

diff --git a/drivers/gpu/drm/drm_sysfs.c b/drivers/gpu/drm/drm_sysfs.c
index 76ff6ec3421b..430e00b16eec 100644
--- a/drivers/gpu/drm/drm_sysfs.c
+++ b/drivers/gpu/drm/drm_sysfs.c
@@ -409,6 +409,31 @@ void drm_sysfs_hotplug_event(struct drm_device *dev)
 }
 EXPORT_SYMBOL(drm_sysfs_hotplug_event);
 
+/**
+ * drm_sysfs_connector_hotplug_event - generate a DRM uevent for any connector
+ * change
+ * @connector: connector which has changed
+ *
+ * Send a uevent for the DRM connector specified by @connector. This will send
+ * a uevent with the properties HOTPLUG=1 and CONNECTOR.
+ */
+void drm_sysfs_connector_hotplug_event(struct drm_connector *connector)
+{
+	struct drm_device *dev = connector->dev;
+	char hotplug_str[] = "HOTPLUG=1", conn_id[21];
+	char *envp[] = { hotplug_str, conn_id, NULL };
+
+	snprintf(conn_id, sizeof(conn_id),
+		 "CONNECTOR=%u", connector->base.id);
+
+	drm_dbg_kms(connector->dev,
+		    "[CONNECTOR:%d:%s] generating connector hotplug event\n",
+		    connector->base.id, connector->name);
+
+	kobject_uevent_env(&dev->primary->kdev->kobj, KOBJ_CHANGE, envp);
+}
+EXPORT_SYMBOL(drm_sysfs_connector_hotplug_event);
+
 /**
  * drm_sysfs_connector_status_event - generate a DRM uevent for connector
  * property status change
diff --git a/include/drm/drm_sysfs.h b/include/drm/drm_sysfs.h
index d454ef617b2c..6273cac44e47 100644
--- a/include/drm/drm_sysfs.h
+++ b/include/drm/drm_sysfs.h
@@ -11,6 +11,7 @@ int drm_class_device_register(struct device *dev);
 void drm_class_device_unregister(struct device *dev);
 
 void drm_sysfs_hotplug_event(struct drm_device *dev);
+void drm_sysfs_connector_hotplug_event(struct drm_connector *connector);
 void drm_sysfs_connector_status_event(struct drm_connector *connector,
 				      struct drm_property *property);
 #endif
-- 
2.33.1

[PATCH v3 2/6] drm/probe-helper: add drm_kms_helper_connector_hotplug_event

[Simon Ser]

This function is the same as drm_kms_helper_hotplug_event, but takes
a connector instead of a device.

Signed-off-by: Simon Ser <contact@emersion.fr>
---
 drivers/gpu/drm/drm_probe_helper.c | 23 +++++++++++++++++++++++
 include/drm/drm_probe_helper.h     |  1 +
 2 files changed, 24 insertions(+)

diff --git a/drivers/gpu/drm/drm_probe_helper.c b/drivers/gpu/drm/drm_probe_helper.c
index 61d5c57f23e1..3aef3b188c99 100644
--- a/drivers/gpu/drm/drm_probe_helper.c
+++ b/drivers/gpu/drm/drm_probe_helper.c
@@ -604,6 +604,9 @@ EXPORT_SYMBOL(drm_helper_probe_single_connector_modes);
  *
  * This function must be called from process context with no mode
  * setting locks held.
+ *
+ * If only a single connector has changed, consider calling
+ * drm_kms_helper_connector_hotplug_event() instead.
  */
 void drm_kms_helper_hotplug_event(struct drm_device *dev)
 {
@@ -616,6 +619,26 @@ void drm_kms_helper_hotplug_event(struct drm_device *dev)
 }
 EXPORT_SYMBOL(drm_kms_helper_hotplug_event);
 
+/**
+ * drm_kms_helper_connector_hotplug_event - fire off a KMS connector hotplug event
+ * @connector: drm_connector which has changed
+ *
+ * This is the same as drm_kms_helper_hotplug_event(), except it fires a more
+ * fine-grained uevent for a single connector.
+ */
+void drm_kms_helper_connector_hotplug_event(struct drm_connector *connector)
+{
+	struct drm_device *dev = connector->dev;
+
+	/* send a uevent + call fbdev */
+	drm_sysfs_connector_hotplug_event(connector);
+	if (dev->mode_config.funcs->output_poll_changed)
+		dev->mode_config.funcs->output_poll_changed(dev);
+
+	drm_client_dev_hotplug(dev);
+}
+EXPORT_SYMBOL(drm_kms_helper_connector_hotplug_event);
+
 static void output_poll_execute(struct work_struct *work)
 {
 	struct delayed_work *delayed_work = to_delayed_work(work);
diff --git a/include/drm/drm_probe_helper.h b/include/drm/drm_probe_helper.h
index 04c57564c397..48300aa6ca71 100644
--- a/include/drm/drm_probe_helper.h
+++ b/include/drm/drm_probe_helper.h
@@ -20,6 +20,7 @@ void drm_kms_helper_poll_fini(struct drm_device *dev);
 bool drm_helper_hpd_irq_event(struct drm_device *dev);
 bool drm_connector_helper_hpd_irq_event(struct drm_connector *connector);
 void drm_kms_helper_hotplug_event(struct drm_device *dev);
+void drm_kms_helper_connector_hotplug_event(struct drm_connector *connector);
 
 void drm_kms_helper_poll_disable(struct drm_device *dev);
 void drm_kms_helper_poll_enable(struct drm_device *dev);
-- 
2.33.1

[PATCH v3 3/6] drm/connector: use drm_sysfs_connector_hotplug_event

[Simon Ser]

In drm_connector_register, use drm_sysfs_connector_hotplug_event
instead of drm_sysfs_hotplug_event, because the hotplug event
only updates a single connector.

Signed-off-by: Simon Ser <contact@emersion.fr>
---
 drivers/gpu/drm/drm_connector.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index ec3973e8963c..a50c82bc2b2f 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -547,7 +547,7 @@ int drm_connector_register(struct drm_connector *connector)
 	connector->registration_state = DRM_CONNECTOR_REGISTERED;
 
 	/* Let userspace know we have a new connector */
-	drm_sysfs_hotplug_event(connector->dev);
+	drm_sysfs_connector_hotplug_event(connector);
 
 	if (connector->privacy_screen)
 		drm_privacy_screen_register_notifier(connector->privacy_screen,
-- 
2.33.1

[PATCH v3 4/6] amdgpu: use drm_kms_helper_connector_hotplug_event

[Simon Ser]

When updating a single connector, use
drm_kms_helper_connector_hotplug_event instead of
drm_kms_helper_hotplug_event.

Signed-off-by: Simon Ser <contact@emersion.fr>
---
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c         | 8 ++++----
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c | 4 ++--
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index 9b1fc54555ee..c261e57d9a22 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -2652,7 +2652,7 @@ static void handle_hpd_irq(void *param)
 		drm_modeset_unlock_all(dev);
 
 		if (aconnector->base.force == DRM_FORCE_UNSPECIFIED)
-			drm_kms_helper_hotplug_event(dev);
+			drm_kms_helper_connector_hotplug_event(connector);
 
 	} else if (dc_link_detect(aconnector->dc_link, DETECT_REASON_HPD)) {
 		if (new_connection_type == dc_connection_none &&
@@ -2666,7 +2666,7 @@ static void handle_hpd_irq(void *param)
 		drm_modeset_unlock_all(dev);
 
 		if (aconnector->base.force == DRM_FORCE_UNSPECIFIED)
-			drm_kms_helper_hotplug_event(dev);
+			drm_kms_helper_connector_hotplug_event(connector);
 	}
 	mutex_unlock(&aconnector->hpd_lock);
 
@@ -2833,7 +2833,7 @@ static void handle_hpd_rx_irq(void *param)
 			dm_restore_drm_connector_state(dev, connector);
 			drm_modeset_unlock_all(dev);
 
-			drm_kms_helper_hotplug_event(dev);
+			drm_kms_helper_connector_hotplug_event(connector);
 		} else if (dc_link_detect(dc_link, DETECT_REASON_HPDRX)) {
 
 			if (aconnector->fake_enable)
@@ -2846,7 +2846,7 @@ static void handle_hpd_rx_irq(void *param)
 			dm_restore_drm_connector_state(dev, connector);
 			drm_modeset_unlock_all(dev);
 
-			drm_kms_helper_hotplug_event(dev);
+			drm_kms_helper_connector_hotplug_event(connector);
 		}
 	}
 #ifdef CONFIG_DRM_AMD_DC_HDCP
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c
index 87daa78a32b8..23e789855d17 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c
@@ -1241,7 +1241,7 @@ static ssize_t trigger_hotplug(struct file *f, const char __user *buf,
 		dm_restore_drm_connector_state(dev, connector);
 		drm_modeset_unlock_all(dev);
 
-		drm_kms_helper_hotplug_event(dev);
+		drm_kms_helper_connector_hotplug_event(connector);
 	} else if (param[0] == 0) {
 		if (!aconnector->dc_link)
 			goto unlock;
@@ -1263,7 +1263,7 @@ static ssize_t trigger_hotplug(struct file *f, const char __user *buf,
 		dm_restore_drm_connector_state(dev, connector);
 		drm_modeset_unlock_all(dev);
 
-		drm_kms_helper_hotplug_event(dev);
+		drm_kms_helper_connector_hotplug_event(connector);
 	}
 
 unlock:
-- 
2.33.1

[PATCH v3 5/6] drm/probe-helper: use drm_kms_helper_connector_hotplug_event

[Simon Ser]

If an hotplug event only updates a single connector, use
drm_kms_helper_connector_hotplug_event instead of
drm_kms_helper_hotplug_event.

Signed-off-by: Simon Ser <contact@emersion.fr>
---
 drivers/gpu/drm/drm_probe_helper.c | 20 ++++++++++++++++----
 1 file changed, 16 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/drm_probe_helper.c b/drivers/gpu/drm/drm_probe_helper.c
index 3aef3b188c99..6049dc92324b 100644
--- a/drivers/gpu/drm/drm_probe_helper.c
+++ b/drivers/gpu/drm/drm_probe_helper.c
@@ -927,7 +927,7 @@ EXPORT_SYMBOL(drm_connector_helper_hpd_irq_event);
  */
 bool drm_helper_hpd_irq_event(struct drm_device *dev)
 {
-	struct drm_connector *connector;
+	struct drm_connector *connector, *changed_connector = NULL;
 	struct drm_connector_list_iter conn_iter;
 	bool changed = false;
 
@@ -941,15 +941,27 @@ bool drm_helper_hpd_irq_event(struct drm_device *dev)
 		if (!(connector->polled & DRM_CONNECTOR_POLL_HPD))
 			continue;
 
-		if (check_connector_changed(connector))
+		if (check_connector_changed(connector)) {
+			if (changed) {
+				if (changed_connector)
+					drm_connector_put(changed_connector);
+				changed_connector = NULL;
+			} else {
+				drm_connector_get(connector);
+				changed_connector = connector;
+			}
+
 			changed = true;
+		}
 	}
 	drm_connector_list_iter_end(&conn_iter);
 	mutex_unlock(&dev->mode_config.mutex);
 
-	if (changed) {
+	if (changed_connector) {
+		drm_kms_helper_connector_hotplug_event(changed_connector);
+		drm_connector_put(changed_connector);
+	} else if (changed) {
 		drm_kms_helper_hotplug_event(dev);
-		DRM_DEBUG_KMS("Sent hotplug event\n");
 	}
 
 	return changed;
-- 
2.33.1

[PATCH v3 6/6] i915/display/dp: send a more fine-grained link-status uevent

[Simon Ser]

When link-status changes, send a hotplug uevent which contains the
connector and property ID. That way, user-space can more easily
figure out that only the link-status property of this connector has
been updated.

Signed-off-by: Simon Ser <contact@emersion.fr>
---
 drivers/gpu/drm/i915/display/intel_dp.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index 04175f359fd6..afbe34b6e5be 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -5264,6 +5264,8 @@ static void intel_dp_modeset_retry_work_fn(struct work_struct *work)
 	mutex_unlock(&connector->dev->mode_config.mutex);
 	/* Send Hotplug uevent so userspace can reprobe */
 	drm_kms_helper_hotplug_event(connector->dev);
+	drm_sysfs_connector_status_event(connector,
+					 connector->dev->mode_config.link_status_property);
 }
 
 bool
-- 
2.33.1

Re: [PATCH v3 4/6] amdgpu: use drm_kms_helper_connector_hotplug_event

[Harry Wentland]

On 2021-10-15 12:33, Simon Ser wrote:
> When updating a single connector, use
> drm_kms_helper_connector_hotplug_event instead of
> drm_kms_helper_hotplug_event.
> 
> Signed-off-by: Simon Ser <contact@emersion.fr>

Reviewed-by: Harry Wentland <harry.wentland@amd.com>

Patches 1-3 are also
Acked-by: Harry Wentland <harry.wentland@amd.com>

Harry

> ---
>  drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c         | 8 ++++----
>  drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c | 4 ++--
>  2 files changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> index 9b1fc54555ee..c261e57d9a22 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -2652,7 +2652,7 @@ static void handle_hpd_irq(void *param)
>  		drm_modeset_unlock_all(dev);
>  
>  		if (aconnector->base.force == DRM_FORCE_UNSPECIFIED)
> -			drm_kms_helper_hotplug_event(dev);
> +			drm_kms_helper_connector_hotplug_event(connector);
>  
>  	} else if (dc_link_detect(aconnector->dc_link, DETECT_REASON_HPD)) {
>  		if (new_connection_type == dc_connection_none &&
> @@ -2666,7 +2666,7 @@ static void handle_hpd_irq(void *param)
>  		drm_modeset_unlock_all(dev);
>  
>  		if (aconnector->base.force == DRM_FORCE_UNSPECIFIED)
> -			drm_kms_helper_hotplug_event(dev);
> +			drm_kms_helper_connector_hotplug_event(connector);
>  	}
>  	mutex_unlock(&aconnector->hpd_lock);
>  
> @@ -2833,7 +2833,7 @@ static void handle_hpd_rx_irq(void *param)
>  			dm_restore_drm_connector_state(dev, connector);
>  			drm_modeset_unlock_all(dev);
>  
> -			drm_kms_helper_hotplug_event(dev);
> +			drm_kms_helper_connector_hotplug_event(connector);
>  		} else if (dc_link_detect(dc_link, DETECT_REASON_HPDRX)) {
>  
>  			if (aconnector->fake_enable)
> @@ -2846,7 +2846,7 @@ static void handle_hpd_rx_irq(void *param)
>  			dm_restore_drm_connector_state(dev, connector);
>  			drm_modeset_unlock_all(dev);
>  
> -			drm_kms_helper_hotplug_event(dev);
> +			drm_kms_helper_connector_hotplug_event(connector);
>  		}
>  	}
>  #ifdef CONFIG_DRM_AMD_DC_HDCP
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c
> index 87daa78a32b8..23e789855d17 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_debugfs.c
> @@ -1241,7 +1241,7 @@ static ssize_t trigger_hotplug(struct file *f, const char __user *buf,
>  		dm_restore_drm_connector_state(dev, connector);
>  		drm_modeset_unlock_all(dev);
>  
> -		drm_kms_helper_hotplug_event(dev);
> +		drm_kms_helper_connector_hotplug_event(connector);
>  	} else if (param[0] == 0) {
>  		if (!aconnector->dc_link)
>  			goto unlock;
> @@ -1263,7 +1263,7 @@ static ssize_t trigger_hotplug(struct file *f, const char __user *buf,
>  		dm_restore_drm_connector_state(dev, connector);
>  		drm_modeset_unlock_all(dev);
>  
> -		drm_kms_helper_hotplug_event(dev);
> +		drm_kms_helper_connector_hotplug_event(connector);
>  	}
>  
>  unlock:
> 

Re: [PATCH v3 1/6] drm/sysfs: introduce drm_sysfs_connector_hotplug_event

[Sam Ravnborg]

Hi Simon,

On Fri, Oct 15, 2021 at 04:33:41PM +0000, Simon Ser wrote:
> This function sends a hotplug uevent with a CONNECTOR property.

A little late feedback.

	Sam

> 
> Signed-off-by: Simon Ser <contact@emersion.fr>
> ---
>  drivers/gpu/drm/drm_sysfs.c | 25 +++++++++++++++++++++++++
>  include/drm/drm_sysfs.h     |  1 +
>  2 files changed, 26 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_sysfs.c b/drivers/gpu/drm/drm_sysfs.c
> index 76ff6ec3421b..430e00b16eec 100644
> --- a/drivers/gpu/drm/drm_sysfs.c
> +++ b/drivers/gpu/drm/drm_sysfs.c
> @@ -409,6 +409,31 @@ void drm_sysfs_hotplug_event(struct drm_device *dev)
>  }
>  EXPORT_SYMBOL(drm_sysfs_hotplug_event);
>  
> +/**
> + * drm_sysfs_connector_hotplug_event - generate a DRM uevent for any connector
> + * change
> + * @connector: connector which has changed
> + *
> + * Send a uevent for the DRM connector specified by @connector. This will send
> + * a uevent with the properties HOTPLUG=1 and CONNECTOR.
> + */
> +void drm_sysfs_connector_hotplug_event(struct drm_connector *connector)
> +{
> +	struct drm_device *dev = connector->dev;
> +	char hotplug_str[] = "HOTPLUG=1", conn_id[21];
> +	char *envp[] = { hotplug_str, conn_id, NULL };
> +
> +	snprintf(conn_id, sizeof(conn_id),
> +		 "CONNECTOR=%u", connector->base.id);
We have add_uevent_var() that seems a better choice than handrolling
snprintf here.

	Sam

> +
> +	drm_dbg_kms(connector->dev,
> +		    "[CONNECTOR:%d:%s] generating connector hotplug event\n",
> +		    connector->base.id, connector->name);
> +
> +	kobject_uevent_env(&dev->primary->kdev->kobj, KOBJ_CHANGE, envp);
> +}
> +EXPORT_SYMBOL(drm_sysfs_connector_hotplug_event);
> +
>  /**
>   * drm_sysfs_connector_status_event - generate a DRM uevent for connector
>   * property status change
> diff --git a/include/drm/drm_sysfs.h b/include/drm/drm_sysfs.h
> index d454ef617b2c..6273cac44e47 100644
> --- a/include/drm/drm_sysfs.h
> +++ b/include/drm/drm_sysfs.h
> @@ -11,6 +11,7 @@ int drm_class_device_register(struct device *dev);
>  void drm_class_device_unregister(struct device *dev);
>  
>  void drm_sysfs_hotplug_event(struct drm_device *dev);
> +void drm_sysfs_connector_hotplug_event(struct drm_connector *connector);
>  void drm_sysfs_connector_status_event(struct drm_connector *connector,
>  				      struct drm_property *property);
>  #endif
> -- 
> 2.33.1
> 

Re: [PATCH v3 5/6] drm/probe-helper: use drm_kms_helper_connector_hotplug_event

[Ville Syrjälä]

On Fri, Oct 15, 2021 at 04:33:45PM +0000, Simon Ser wrote:
> If an hotplug event only updates a single connector, use
> drm_kms_helper_connector_hotplug_event instead of
> drm_kms_helper_hotplug_event.
> 
> Signed-off-by: Simon Ser <contact@emersion.fr>
> ---
>  drivers/gpu/drm/drm_probe_helper.c | 20 ++++++++++++++++----
>  1 file changed, 16 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_probe_helper.c b/drivers/gpu/drm/drm_probe_helper.c
> index 3aef3b188c99..6049dc92324b 100644
> --- a/drivers/gpu/drm/drm_probe_helper.c
> +++ b/drivers/gpu/drm/drm_probe_helper.c
> @@ -927,7 +927,7 @@ EXPORT_SYMBOL(drm_connector_helper_hpd_irq_event);
>   */
>  bool drm_helper_hpd_irq_event(struct drm_device *dev)
>  {
> -	struct drm_connector *connector;
> +	struct drm_connector *connector, *changed_connector = NULL;
>  	struct drm_connector_list_iter conn_iter;
>  	bool changed = false;
>  
> @@ -941,15 +941,27 @@ bool drm_helper_hpd_irq_event(struct drm_device *dev)
>  		if (!(connector->polled & DRM_CONNECTOR_POLL_HPD))
>  			continue;
>  
> -		if (check_connector_changed(connector))
> +		if (check_connector_changed(connector)) {
> +			if (changed) {
> +				if (changed_connector)
> +					drm_connector_put(changed_connector);
> +				changed_connector = NULL;
> +			} else {
> +				drm_connector_get(connector);
> +				changed_connector = connector;
> +			}
> +
>  			changed = true;

So many things that "changed". Would it not be simpler to just grab the
first changed connector always, and count how many there were in total?

> +		}
>  	}
>  	drm_connector_list_iter_end(&conn_iter);
>  	mutex_unlock(&dev->mode_config.mutex);
>  
> -	if (changed) {
> +	if (changed_connector) {
> +		drm_kms_helper_connector_hotplug_event(changed_connector);
> +		drm_connector_put(changed_connector);
> +	} else if (changed) {
>  		drm_kms_helper_hotplug_event(dev);
> -		DRM_DEBUG_KMS("Sent hotplug event\n");
>  	}
>  
>  	return changed;
> -- 
> 2.33.1
> 

-- 
Ville Syrjälä
Intel

Re: [PATCH v3 1/6] drm/sysfs: introduce drm_sysfs_connector_hotplug_event

[Sam Ravnborg]

On Fri, Oct 15, 2021 at 09:37:04PM +0200, Sam Ravnborg wrote:
> Hi Simon,
> 
> On Fri, Oct 15, 2021 at 04:33:41PM +0000, Simon Ser wrote:
> > This function sends a hotplug uevent with a CONNECTOR property.
> 
> A little late feedback.
> 
> 	Sam

If you keep current code then patch is:
Reviewed-by: Sam Ravnborg <sam@ravnborg.org>

As current code is perfectly fine and a similar pattern is used in the
same file elsewhere.

	Sam

Re: [PATCH v3 2/6] drm/probe-helper: add drm_kms_helper_connector_hotplug_event

[Sam Ravnborg]

On Fri, Oct 15, 2021 at 04:33:42PM +0000, Simon Ser wrote:
> This function is the same as drm_kms_helper_hotplug_event, but takes
> a connector instead of a device.
> 
> Signed-off-by: Simon Ser <contact@emersion.fr>
Reviewed-by: Sam Ravnborg <sam@ravnborg.org>

	Sam

Re: [PATCH v3 3/6] drm/connector: use drm_sysfs_connector_hotplug_event

[Sam Ravnborg]

On Fri, Oct 15, 2021 at 04:33:43PM +0000, Simon Ser wrote:
> In drm_connector_register, use drm_sysfs_connector_hotplug_event
> instead of drm_sysfs_hotplug_event, because the hotplug event
> only updates a single connector.
> 
> Signed-off-by: Simon Ser <contact@emersion.fr>
Reviewed-by: Sam Ravnborg <sam@ravnborg.org>

Re: [PATCH v3 6/6] i915/display/dp: send a more fine-grained link-status uevent

[Ville Syrjälä]

On Fri, Oct 15, 2021 at 04:33:46PM +0000, Simon Ser wrote:
> When link-status changes, send a hotplug uevent which contains the
> connector and property ID. That way, user-space can more easily
> figure out that only the link-status property of this connector has
> been updated.
> 
> Signed-off-by: Simon Ser <contact@emersion.fr>
> ---
>  drivers/gpu/drm/i915/display/intel_dp.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index 04175f359fd6..afbe34b6e5be 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -5264,6 +5264,8 @@ static void intel_dp_modeset_retry_work_fn(struct work_struct *work)
>  	mutex_unlock(&connector->dev->mode_config.mutex);
>  	/* Send Hotplug uevent so userspace can reprobe */
>  	drm_kms_helper_hotplug_event(connector->dev);
> +	drm_sysfs_connector_status_event(connector,
> +					 connector->dev->mode_config.link_status_property);

So we're now doing two uevents back to back? Is one not enough?

>  }
>  
>  bool
> -- 
> 2.33.1
> 

-- 
Ville Syrjälä
Intel

Re: [PATCH v3 5/6] drm/probe-helper: use drm_kms_helper_connector_hotplug_event

[Sam Ravnborg]

Hi Simon,

On Fri, Oct 15, 2021 at 04:33:45PM +0000, Simon Ser wrote:
> If an hotplug event only updates a single connector, use
> drm_kms_helper_connector_hotplug_event instead of
> drm_kms_helper_hotplug_event.
> 
> Signed-off-by: Simon Ser <contact@emersion.fr>
> ---
>  drivers/gpu/drm/drm_probe_helper.c | 20 ++++++++++++++++----
>  1 file changed, 16 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_probe_helper.c b/drivers/gpu/drm/drm_probe_helper.c
> index 3aef3b188c99..6049dc92324b 100644
> --- a/drivers/gpu/drm/drm_probe_helper.c
> +++ b/drivers/gpu/drm/drm_probe_helper.c
> @@ -927,7 +927,7 @@ EXPORT_SYMBOL(drm_connector_helper_hpd_irq_event);
>   */
>  bool drm_helper_hpd_irq_event(struct drm_device *dev)
>  {
> -	struct drm_connector *connector;
> +	struct drm_connector *connector, *changed_connector = NULL;
>  	struct drm_connector_list_iter conn_iter;
>  	bool changed = false;
>  
> @@ -941,15 +941,27 @@ bool drm_helper_hpd_irq_event(struct drm_device *dev)
>  		if (!(connector->polled & DRM_CONNECTOR_POLL_HPD))
>  			continue;
>  
> -		if (check_connector_changed(connector))
> +		if (check_connector_changed(connector)) {
> +			if (changed) {
> +				if (changed_connector)
> +					drm_connector_put(changed_connector);
> +				changed_connector = NULL;
> +			} else {
> +				drm_connector_get(connector);
> +				changed_connector = connector;
> +			}
This code is a little confusing to read.

In case we have only one connector with a change we hit the else part.
What we really want to find out is if we have one or more connectors
with a change.
We could do something like:

	struct drm_connector *changed_connector = NULL;
	int changes = 0;


	...

	/* Find if we have one or more changed connectors */
	drm_for_each_connector_iter(connector, &conn_iter) {
		if (!(connector->polled & DRM_CONNECTOR_POLL_HPD))
			continue;

		if (check_connector_changed(connector)) {
			if (!changes) {
				changed_connector = connector;
				drm_connector_get(changed_connector);
			}

			changes++;
		}
	}
	drm_connector_list_iter_end(&conn_iter);
	mutex_unlock(&dev->mode_config.mutex);

	if (changes == 1)
		drm_kms_helper_connector_hotplug_event(changed_connector);
	else if (changes > 1)
		drm_kms_helper_hotplug_event(dev);

	if (changed_connector)
		drm_connector_put(changed_connector);


Maybe the only reason why I think this is better is bc I wrote it?!?

	Sam

Re: [PATCH v3 6/6] i915/display/dp: send a more fine-grained link-status uevent

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 3107 bytes --]

Hi Simon,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on f6632721cd6231e1bf28b5317dcc7543e43359f7]

url:    https://github.com/0day-ci/linux/commits/Simon-Ser/drm-add-per-connector-hotplug-events/20211016-003611
base:   f6632721cd6231e1bf28b5317dcc7543e43359f7
config: x86_64-randconfig-a006-20211015 (attached as .config)
compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project a49f5386ce6b091da66ea7c3a1d9a588d53becf7)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/fb2b373f5b3b0f1e37e56270bf7aa0e6332f880d
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Simon-Ser/drm-add-per-connector-hotplug-events/20211016-003611
        git checkout fb2b373f5b3b0f1e37e56270bf7aa0e6332f880d
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 ARCH=x86_64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

>> drivers/gpu/drm/i915/display/intel_dp.c:5267:2: error: implicit declaration of function 'drm_sysfs_connector_status_event' [-Werror,-Wimplicit-function-declaration]
           drm_sysfs_connector_status_event(connector,
           ^
   drivers/gpu/drm/i915/display/intel_dp.c:5267:2: note: did you mean 'drm_get_connector_status_name'?
   include/drm/drm_connector.h:1729:13: note: 'drm_get_connector_status_name' declared here
   const char *drm_get_connector_status_name(enum drm_connector_status status);
               ^
   1 error generated.


vim +/drm_sysfs_connector_status_event +5267 drivers/gpu/drm/i915/display/intel_dp.c

  5245	
  5246	static void intel_dp_modeset_retry_work_fn(struct work_struct *work)
  5247	{
  5248		struct intel_connector *intel_connector;
  5249		struct drm_connector *connector;
  5250	
  5251		intel_connector = container_of(work, typeof(*intel_connector),
  5252					       modeset_retry_work);
  5253		connector = &intel_connector->base;
  5254		DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n", connector->base.id,
  5255			      connector->name);
  5256	
  5257		/* Grab the locks before changing connector property*/
  5258		mutex_lock(&connector->dev->mode_config.mutex);
  5259		/* Set connector link status to BAD and send a Uevent to notify
  5260		 * userspace to do a modeset.
  5261		 */
  5262		drm_connector_set_link_status_property(connector,
  5263						       DRM_MODE_LINK_STATUS_BAD);
  5264		mutex_unlock(&connector->dev->mode_config.mutex);
  5265		/* Send Hotplug uevent so userspace can reprobe */
  5266		drm_kms_helper_hotplug_event(connector->dev);
> 5267		drm_sysfs_connector_status_event(connector,
  5268						 connector->dev->mode_config.link_status_property);
  5269	}
  5270	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 33784 bytes --]

Re: [PATCH v3 6/6] i915/display/dp: send a more fine-grained link-status uevent

[kernel test robot]

[-- Attachment #1: Type: text/plain, Size: 3176 bytes --]

Hi Simon,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on f6632721cd6231e1bf28b5317dcc7543e43359f7]

url:    https://github.com/0day-ci/linux/commits/Simon-Ser/drm-add-per-connector-hotplug-events/20211016-003611
base:   f6632721cd6231e1bf28b5317dcc7543e43359f7
config: x86_64-randconfig-a006-20211015 (attached as .config)
compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project a49f5386ce6b091da66ea7c3a1d9a588d53becf7)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/0day-ci/linux/commit/fb2b373f5b3b0f1e37e56270bf7aa0e6332f880d
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Simon-Ser/drm-add-per-connector-hotplug-events/20211016-003611
        git checkout fb2b373f5b3b0f1e37e56270bf7aa0e6332f880d
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 ARCH=x86_64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

>> drivers/gpu/drm/i915/display/intel_dp.c:5267:2: error: implicit declaration of function 'drm_sysfs_connector_status_event' [-Werror,-Wimplicit-function-declaration]
           drm_sysfs_connector_status_event(connector,
           ^
   drivers/gpu/drm/i915/display/intel_dp.c:5267:2: note: did you mean 'drm_get_connector_status_name'?
   include/drm/drm_connector.h:1729:13: note: 'drm_get_connector_status_name' declared here
   const char *drm_get_connector_status_name(enum drm_connector_status status);
               ^
   1 error generated.


vim +/drm_sysfs_connector_status_event +5267 drivers/gpu/drm/i915/display/intel_dp.c

  5245	
  5246	static void intel_dp_modeset_retry_work_fn(struct work_struct *work)
  5247	{
  5248		struct intel_connector *intel_connector;
  5249		struct drm_connector *connector;
  5250	
  5251		intel_connector = container_of(work, typeof(*intel_connector),
  5252					       modeset_retry_work);
  5253		connector = &intel_connector->base;
  5254		DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n", connector->base.id,
  5255			      connector->name);
  5256	
  5257		/* Grab the locks before changing connector property*/
  5258		mutex_lock(&connector->dev->mode_config.mutex);
  5259		/* Set connector link status to BAD and send a Uevent to notify
  5260		 * userspace to do a modeset.
  5261		 */
  5262		drm_connector_set_link_status_property(connector,
  5263						       DRM_MODE_LINK_STATUS_BAD);
  5264		mutex_unlock(&connector->dev->mode_config.mutex);
  5265		/* Send Hotplug uevent so userspace can reprobe */
  5266		drm_kms_helper_hotplug_event(connector->dev);
> 5267		drm_sysfs_connector_status_event(connector,
  5268						 connector->dev->mode_config.link_status_property);
  5269	}
  5270	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 33784 bytes --]

Re: [PATCH v3 5/6] drm/probe-helper: use drm_kms_helper_connector_hotplug_event

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 356 bytes --]

Hi Simon,

On Fri, Oct 15, 2021 at 04:33:45PM +0000, Simon Ser wrote:
> If an hotplug event only updates a single connector, use
> drm_kms_helper_connector_hotplug_event instead of
> drm_kms_helper_hotplug_event.
> 
> Signed-off-by: Simon Ser <contact@emersion.fr>

I guess we'd also need to update drm_connector_helper_hpd_irq_event ?

Maxime

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v3 6/6] i915/display/dp: send a more fine-grained link-status uevent

[Simon Ser]

On Friday, October 15th, 2021 at 21:44, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:

> >  	/* Send Hotplug uevent so userspace can reprobe */
> >  	drm_kms_helper_hotplug_event(connector->dev);
> > +	drm_sysfs_connector_status_event(connector,
> > +					 connector->dev->mode_config.link_status_property);
>
> So we're now doing two uevents back to back? Is one not enough?

Oops, I think I missed a step in my rebase. Will fix!

Re: [PATCH v3 5/6] drm/probe-helper: use drm_kms_helper_connector_hotplug_event

[Simon Ser]

On Friday, October 15th, 2021 at 21:41, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:

> So many things that "changed". Would it not be simpler to just grab the
> first changed connector always, and count how many there were in total?

Indeed, sounds much better. Will do that in the next version.

Re: [PATCH v3 5/6] drm/probe-helper: use drm_kms_helper_connector_hotplug_event

[Simon Ser]

On Monday, October 18th, 2021 at 10:15, Maxime Ripard <maxime@cerno.tech> wrote:

> I guess we'd also need to update drm_connector_helper_hpd_irq_event ?

Good catch! IIRC this function didn't exist when I first wrote the
patchset.

Re: [PATCH v3 5/6] drm/probe-helper: use drm_kms_helper_connector_hotplug_event

[Simon Ser]

On Friday, October 15th, 2021 at 22:03, Sam Ravnborg <sam@ravnborg.org> wrote:

> This code is a little confusing to read.
>
> In case we have only one connector with a change we hit the else part.
> What we really want to find out is if we have one or more connectors
> with a change.
> We could do something like:
>
> 	struct drm_connector *changed_connector = NULL;
> 	int changes = 0;
>
>
> 	...
>
> 	/* Find if we have one or more changed connectors */
> 	drm_for_each_connector_iter(connector, &conn_iter) {
> 		if (!(connector->polled & DRM_CONNECTOR_POLL_HPD))
> 			continue;
>
> 		if (check_connector_changed(connector)) {
> 			if (!changes) {
> 				changed_connector = connector;
> 				drm_connector_get(changed_connector);
> 			}
>
> 			changes++;
> 		}
> 	}
> 	drm_connector_list_iter_end(&conn_iter);
> 	mutex_unlock(&dev->mode_config.mutex);
>
> 	if (changes == 1)
> 		drm_kms_helper_connector_hotplug_event(changed_connector);
> 	else if (changes > 1)
> 		drm_kms_helper_hotplug_event(dev);
>
> 	if (changed_connector)
> 		drm_connector_put(changed_connector);
>
>
> Maybe the only reason why I think this is better is bc I wrote it?!?

Ah, it's not just you, this version is much better. Thanks for the suggestion,
will do that in the next version!

Re: [PATCH v3 3/6] drm/connector: use drm_sysfs_connector_hotplug_event

[Pekka Paalanen]

[-- Attachment #1: Type: text/plain, Size: 1893 bytes --]

On Fri, 15 Oct 2021 16:33:43 +0000
Simon Ser <contact@emersion.fr> wrote:

> In drm_connector_register, use drm_sysfs_connector_hotplug_event
> instead of drm_sysfs_hotplug_event, because the hotplug event
> only updates a single connector.
> 
> Signed-off-by: Simon Ser <contact@emersion.fr>
> ---
>  drivers/gpu/drm/drm_connector.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> index ec3973e8963c..a50c82bc2b2f 100644
> --- a/drivers/gpu/drm/drm_connector.c
> +++ b/drivers/gpu/drm/drm_connector.c
> @@ -547,7 +547,7 @@ int drm_connector_register(struct drm_connector *connector)
>  	connector->registration_state = DRM_CONNECTOR_REGISTERED;
>  
>  	/* Let userspace know we have a new connector */
> -	drm_sysfs_hotplug_event(connector->dev);
> +	drm_sysfs_connector_hotplug_event(connector);
>  
>  	if (connector->privacy_screen)
>  		drm_privacy_screen_register_notifier(connector->privacy_screen,

Hi Simon,

this might not work for Weston if I understand this right. Kernel is
adding a new connector, which means userspace does not recognise the
connector id in the uevent. Weston as it is right now would ignore the
event rather than add the connector.

The missing piece is for Weston to revert to the old fashioned "recheck
everything" behaviour when hotplug uevent carries anything
unrecognised. Grep for drm_backend_update_conn_props if you want to see
for yourself.

However, I wouldn't NAK this patch just for Weston, but I wonder if
other software would ignore events because of this as well.

A whole another question is, would anyone notice. I guess this can only
be an issue with MST.

All the other changes in this series look fine to me, so them I can give
Acked-by: Pekka Paalanen <pekka.paalanen@collabora.com>


Thanks,
pq

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v3 3/6] drm/connector: use drm_sysfs_connector_hotplug_event

[Simon Ser]

On Wednesday, October 27th, 2021 at 15:15, Pekka Paalanen <ppaalanen@gmail.com> wrote:

> On Fri, 15 Oct 2021 16:33:43 +0000
> Simon Ser <contact@emersion.fr> wrote:
>
> > In drm_connector_register, use drm_sysfs_connector_hotplug_event
> > instead of drm_sysfs_hotplug_event, because the hotplug event
> > only updates a single connector.
> >
> > Signed-off-by: Simon Ser <contact@emersion.fr>
> > ---
> >  drivers/gpu/drm/drm_connector.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> > index ec3973e8963c..a50c82bc2b2f 100644
> > --- a/drivers/gpu/drm/drm_connector.c
> > +++ b/drivers/gpu/drm/drm_connector.c
> > @@ -547,7 +547,7 @@ int drm_connector_register(struct drm_connector *connector)
> >  	connector->registration_state = DRM_CONNECTOR_REGISTERED;
> >
> >  	/* Let userspace know we have a new connector */
> > -	drm_sysfs_hotplug_event(connector->dev);
> > +	drm_sysfs_connector_hotplug_event(connector);
> >
> >  	if (connector->privacy_screen)
> >  		drm_privacy_screen_register_notifier(connector->privacy_screen,
>
> Hi Simon,
>
> this might not work for Weston if I understand this right. Kernel is
> adding a new connector, which means userspace does not recognise the
> connector id in the uevent. Weston as it is right now would ignore the
> event rather than add the connector.
>
> The missing piece is for Weston to revert to the old fashioned "recheck
> everything" behaviour when hotplug uevent carries anything
> unrecognised. Grep for drm_backend_update_conn_props if you want to see
> for yourself.
>
> However, I wouldn't NAK this patch just for Weston, but I wonder if
> other software would ignore events because of this as well.
>
> A whole another question is, would anyone notice. I guess this can only
> be an issue with MST.

I think Weston should be fine: udev_event_is_conn_prop_change returns false
if there's no PROPERTY in the uevent. An uevent with just a CONNECTOR and no
PROPERTY is something new. Weston already falls back to the old "reprobe the
world" approach in this case.

So far the CONNECTOR+PROPERTY uevent fields have only been used for content
protection stuff. I'm not aware of other user-space using it (checked Kodi
just in case, it doesn't do content protection nor handles uevents at all).

> All the other changes in this series look fine to me, so them I can give
> Acked-by: Pekka Paalanen <pekka.paalanen@collabora.com>

Thanks!

Re: [PATCH v3 3/6] drm/connector: use drm_sysfs_connector_hotplug_event

[Pekka Paalanen]

[-- Attachment #1: Type: text/plain, Size: 2833 bytes --]

On Wed, 27 Oct 2021 13:26:45 +0000
Simon Ser <contact@emersion.fr> wrote:

> On Wednesday, October 27th, 2021 at 15:15, Pekka Paalanen <ppaalanen@gmail.com> wrote:
> 
> > On Fri, 15 Oct 2021 16:33:43 +0000
> > Simon Ser <contact@emersion.fr> wrote:
> >  
> > > In drm_connector_register, use drm_sysfs_connector_hotplug_event
> > > instead of drm_sysfs_hotplug_event, because the hotplug event
> > > only updates a single connector.
> > >
> > > Signed-off-by: Simon Ser <contact@emersion.fr>
> > > ---
> > >  drivers/gpu/drm/drm_connector.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> > > index ec3973e8963c..a50c82bc2b2f 100644
> > > --- a/drivers/gpu/drm/drm_connector.c
> > > +++ b/drivers/gpu/drm/drm_connector.c
> > > @@ -547,7 +547,7 @@ int drm_connector_register(struct drm_connector *connector)
> > >  	connector->registration_state = DRM_CONNECTOR_REGISTERED;
> > >
> > >  	/* Let userspace know we have a new connector */
> > > -	drm_sysfs_hotplug_event(connector->dev);
> > > +	drm_sysfs_connector_hotplug_event(connector);
> > >
> > >  	if (connector->privacy_screen)
> > >  		drm_privacy_screen_register_notifier(connector->privacy_screen,  
> >
> > Hi Simon,
> >
> > this might not work for Weston if I understand this right. Kernel is
> > adding a new connector, which means userspace does not recognise the
> > connector id in the uevent. Weston as it is right now would ignore the
> > event rather than add the connector.
> >
> > The missing piece is for Weston to revert to the old fashioned "recheck
> > everything" behaviour when hotplug uevent carries anything
> > unrecognised. Grep for drm_backend_update_conn_props if you want to see
> > for yourself.
> >
> > However, I wouldn't NAK this patch just for Weston, but I wonder if
> > other software would ignore events because of this as well.
> >
> > A whole another question is, would anyone notice. I guess this can only
> > be an issue with MST.  
> 
> I think Weston should be fine: udev_event_is_conn_prop_change returns false
> if there's no PROPERTY in the uevent. An uevent with just a CONNECTOR and no
> PROPERTY is something new. Weston already falls back to the old "reprobe the
> world" approach in this case.
> 
> So far the CONNECTOR+PROPERTY uevent fields have only been used for content
> protection stuff. I'm not aware of other user-space using it (checked Kodi
> just in case, it doesn't do content protection nor handles uevents at all).
> 
> > All the other changes in this series look fine to me, so them I can give
> > Acked-by: Pekka Paalanen <pekka.paalanen@collabora.com>  

Hi Simon,

you're right! Therefore my Ack applies to this patch too.


Thanks,
pq

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]