All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm: Pass along the hotplug connector to the uevent
@ 2016-10-21  8:25 Chris Wilson
  2016-10-21  8:58 ` ✗ Fi.CI.BAT: failure for " Patchwork
                   ` (3 more replies)
  0 siblings, 4 replies; 14+ messages in thread
From: Chris Wilson @ 2016-10-21  8:25 UTC (permalink / raw)
  To: dri-devel; +Cc: intel-gfx

If we know which connector was plugged/unplugged or
connected/disconnected, we can pass that information along to userspace
inside the uevent to reduce the amount of work userspace has to perform
after the event (i.e. instead of looking over all connectors, it can
just reprobe the affected one).

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Villle Syrjälä <ville.syrjala@linux.intel.com>
Cc: Manasi Navare <manasi.d.navare@intel.com>
---
 drivers/gpu/drm/drm_probe_helper.c     | 10 ++++++----
 drivers/gpu/drm/drm_sysfs.c            | 19 +++++++++++++++----
 drivers/gpu/drm/i2c/tda998x_drv.c      |  2 +-
 drivers/gpu/drm/i915/intel_dp.c        |  3 ++-
 drivers/gpu/drm/i915/intel_dp_mst.c    |  2 +-
 drivers/gpu/drm/i915/intel_hotplug.c   |  6 +-----
 drivers/gpu/drm/qxl/qxl_display.c      |  2 +-
 drivers/gpu/drm/radeon/radeon_dp_mst.c |  2 +-
 drivers/gpu/drm/virtio/virtgpu_vq.c    |  2 +-
 drivers/gpu/drm/vmwgfx/vmwgfx_drv.c    |  2 +-
 drivers/gpu/drm/vmwgfx/vmwgfx_kms.c    |  2 +-
 include/drm/drmP.h                     |  3 ++-
 include/drm/drm_crtc_helper.h          |  3 ++-
 13 files changed, 35 insertions(+), 23 deletions(-)

diff --git a/drivers/gpu/drm/drm_probe_helper.c b/drivers/gpu/drm/drm_probe_helper.c
index f6b64d7d3528..8790ee35a7cd 100644
--- a/drivers/gpu/drm/drm_probe_helper.c
+++ b/drivers/gpu/drm/drm_probe_helper.c
@@ -347,6 +347,7 @@ EXPORT_SYMBOL(drm_helper_probe_single_connector_modes);
 /**
  * drm_kms_helper_hotplug_event - fire off KMS hotplug events
  * @dev: drm_device whose connector state changed
+ * @connector: connector on which the status changed, if any
  *
  * This function fires off the uevent for userspace and also calls the
  * output_poll_changed function, which is most commonly used to inform the fbdev
@@ -360,10 +361,11 @@ EXPORT_SYMBOL(drm_helper_probe_single_connector_modes);
  * This function must be called from process context with no mode
  * setting locks held.
  */
-void drm_kms_helper_hotplug_event(struct drm_device *dev)
+void drm_kms_helper_hotplug_event(struct drm_device *dev,
+				  struct drm_connector *connector)
 {
 	/* send a uevent + call fbdev */
-	drm_sysfs_hotplug_event(dev);
+	drm_sysfs_hotplug_event(dev, connector);
 	if (dev->mode_config.funcs->output_poll_changed)
 		dev->mode_config.funcs->output_poll_changed(dev);
 }
@@ -444,7 +446,7 @@ static void output_poll_execute(struct work_struct *work)
 
 out:
 	if (changed)
-		drm_kms_helper_hotplug_event(dev);
+		drm_kms_helper_hotplug_event(dev, NULL);
 
 	if (repoll)
 		schedule_delayed_work(delayed_work, DRM_OUTPUT_POLL_PERIOD);
@@ -578,7 +580,7 @@ bool drm_helper_hpd_irq_event(struct drm_device *dev)
 	mutex_unlock(&dev->mode_config.mutex);
 
 	if (changed)
-		drm_kms_helper_hotplug_event(dev);
+		drm_kms_helper_hotplug_event(dev, NULL);
 
 	return changed;
 }
diff --git a/drivers/gpu/drm/drm_sysfs.c b/drivers/gpu/drm/drm_sysfs.c
index 9a37196c1bf1..c8f46055e2d0 100644
--- a/drivers/gpu/drm/drm_sysfs.c
+++ b/drivers/gpu/drm/drm_sysfs.c
@@ -280,7 +280,7 @@ int drm_sysfs_connector_add(struct drm_connector *connector)
 	}
 
 	/* Let userspace know we have a new connector */
-	drm_sysfs_hotplug_event(dev);
+	drm_sysfs_hotplug_event(dev, connector);
 
 	return 0;
 }
@@ -312,19 +312,30 @@ void drm_sysfs_connector_remove(struct drm_connector *connector)
 /**
  * drm_sysfs_hotplug_event - generate a DRM uevent
  * @dev: DRM device
+ * @connector: the DRM connector, if any
  *
  * Send a uevent for the DRM device specified by @dev.  Currently we only
  * set HOTPLUG=1 in the uevent environment, but this could be expanded to
  * deal with other types of events.
  */
-void drm_sysfs_hotplug_event(struct drm_device *dev)
+void drm_sysfs_hotplug_event(struct drm_device *dev,
+			     struct drm_connector *connector)
 {
-	char *event_string = "HOTPLUG=1";
-	char *envp[] = { event_string, NULL };
+	char *envp[3] = { "HOTPLUG=1" };
+	char *connector_event = NULL;
+
+	if (connector)
+		connector_event = kasprintf(GFP_KERNEL,
+					    "CONNECTOR=%d",
+					    connector->base.id);
+	if (connector_event)
+		envp[1] = connector_event;
 
 	DRM_DEBUG("generating hotplug event\n");
 
 	kobject_uevent_env(&dev->primary->kdev->kobj, KOBJ_CHANGE, envp);
+
+	kfree(connector_event);
 }
 EXPORT_SYMBOL(drm_sysfs_hotplug_event);
 
diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c
index 9798d400d817..9fd873c87686 100644
--- a/drivers/gpu/drm/i2c/tda998x_drv.c
+++ b/drivers/gpu/drm/i2c/tda998x_drv.c
@@ -617,7 +617,7 @@ static void tda998x_detect_work(struct work_struct *work)
 	struct drm_device *dev = priv->encoder.dev;
 
 	if (dev)
-		drm_kms_helper_hotplug_event(dev);
+		drm_kms_helper_hotplug_event(dev, NULL);
 }
 
 /*
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 88f3b745a326..59cd185689c0 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -3981,7 +3981,8 @@ intel_dp_check_mst_status(struct intel_dp *intel_dp)
 			intel_dp->is_mst = false;
 			drm_dp_mst_topology_mgr_set_mst(&intel_dp->mst_mgr, intel_dp->is_mst);
 			/* send a hotplug event */
-			drm_kms_helper_hotplug_event(intel_dig_port->base.base.dev);
+			drm_kms_helper_hotplug_event(intel_dig_port->base.base.dev,
+						     &intel_dp->attached_connector->base);
 		}
 	}
 	return -EINVAL;
diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c b/drivers/gpu/drm/i915/intel_dp_mst.c
index 3ffbd69e4551..2bd48a987934 100644
--- a/drivers/gpu/drm/i915/intel_dp_mst.c
+++ b/drivers/gpu/drm/i915/intel_dp_mst.c
@@ -493,7 +493,7 @@ static void intel_dp_mst_hotplug(struct drm_dp_mst_topology_mgr *mgr)
 	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
 	struct drm_device *dev = intel_dig_port->base.base.dev;
 
-	drm_kms_helper_hotplug_event(dev);
+	drm_kms_helper_hotplug_event(dev, &intel_dp->attached_connector->base);
 }
 
 static const struct drm_dp_mst_topology_cbs mst_cbs = {
diff --git a/drivers/gpu/drm/i915/intel_hotplug.c b/drivers/gpu/drm/i915/intel_hotplug.c
index 334d47b5811a..72cd8622670c 100644
--- a/drivers/gpu/drm/i915/intel_hotplug.c
+++ b/drivers/gpu/drm/i915/intel_hotplug.c
@@ -307,7 +307,6 @@ static void i915_hotplug_work_func(struct work_struct *work)
 	struct intel_connector *intel_connector;
 	struct intel_encoder *intel_encoder;
 	struct drm_connector *connector;
-	bool changed = false;
 	u32 hpd_event_bits;
 
 	mutex_lock(&mode_config->mutex);
@@ -334,13 +333,10 @@ static void i915_hotplug_work_func(struct work_struct *work)
 			if (intel_encoder->hot_plug)
 				intel_encoder->hot_plug(intel_encoder);
 			if (intel_hpd_irq_event(dev, connector))
-				changed = true;
+				drm_kms_helper_hotplug_event(dev, connector);
 		}
 	}
 	mutex_unlock(&mode_config->mutex);
-
-	if (changed)
-		drm_kms_helper_hotplug_event(dev);
 }
 
 
diff --git a/drivers/gpu/drm/qxl/qxl_display.c b/drivers/gpu/drm/qxl/qxl_display.c
index 2e01c6e5d905..3c196a096c2d 100644
--- a/drivers/gpu/drm/qxl/qxl_display.c
+++ b/drivers/gpu/drm/qxl/qxl_display.c
@@ -135,7 +135,7 @@ void qxl_display_read_client_monitors_config(struct qxl_device *qdev)
 	if (!drm_helper_hpd_irq_event(qdev->ddev)) {
 		/* notify that the monitor configuration changed, to
 		   adjust at the arbitrary resolution */
-		drm_kms_helper_hotplug_event(qdev->ddev);
+		drm_kms_helper_hotplug_event(qdev->ddev, NULL);
 	}
 }
 
diff --git a/drivers/gpu/drm/radeon/radeon_dp_mst.c b/drivers/gpu/drm/radeon/radeon_dp_mst.c
index de504ea29c06..e80b1c365a2d 100644
--- a/drivers/gpu/drm/radeon/radeon_dp_mst.c
+++ b/drivers/gpu/drm/radeon/radeon_dp_mst.c
@@ -331,7 +331,7 @@ static void radeon_dp_mst_hotplug(struct drm_dp_mst_topology_mgr *mgr)
 	struct radeon_connector *master = container_of(mgr, struct radeon_connector, mst_mgr);
 	struct drm_device *dev = master->base.dev;
 
-	drm_kms_helper_hotplug_event(dev);
+	drm_kms_helper_hotplug_event(dev, NULL);
 }
 
 const struct drm_dp_mst_topology_cbs mst_cbs = {
diff --git a/drivers/gpu/drm/virtio/virtgpu_vq.c b/drivers/gpu/drm/virtio/virtgpu_vq.c
index 5a0f8a745b9d..72be43c9e008 100644
--- a/drivers/gpu/drm/virtio/virtgpu_vq.c
+++ b/drivers/gpu/drm/virtio/virtgpu_vq.c
@@ -583,7 +583,7 @@ static void virtio_gpu_cmd_get_display_info_cb(struct virtio_gpu_device *vgdev,
 	wake_up(&vgdev->resp_wq);
 
 	if (!drm_helper_hpd_irq_event(vgdev->ddev))
-		drm_kms_helper_hotplug_event(vgdev->ddev);
+		drm_kms_helper_hotplug_event(vgdev->ddev, NULL);
 }
 
 static void virtio_gpu_cmd_get_capset_info_cb(struct virtio_gpu_device *vgdev,
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
index e8ae3dc476d1..36aa1a0440c3 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
@@ -1233,7 +1233,7 @@ static int vmw_master_set(struct drm_device *dev,
 	}
 
 	dev_priv->active_master = vmaster;
-	drm_sysfs_hotplug_event(dev);
+	drm_sysfs_hotplug_event(dev, NULL);
 
 	return 0;
 }
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
index c965514b82be..26262763aa5f 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
@@ -1407,7 +1407,7 @@ static int vmw_du_update_layout(struct vmw_private *dev_priv, unsigned num,
 	}
 
 	mutex_unlock(&dev->mode_config.mutex);
-	drm_sysfs_hotplug_event(dev);
+	drm_sysfs_hotplug_event(dev, con);
 
 	return 0;
 }
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index 672644031bd5..0a4a4aa61fd3 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -1038,7 +1038,8 @@ extern struct drm_dma_handle *drm_pci_alloc(struct drm_device *dev, size_t size,
 extern void drm_pci_free(struct drm_device *dev, struct drm_dma_handle * dmah);
 
 			       /* sysfs support (drm_sysfs.c) */
-extern void drm_sysfs_hotplug_event(struct drm_device *dev);
+extern void drm_sysfs_hotplug_event(struct drm_device *dev,
+				    struct drm_connector *connector);
 
 
 struct drm_device *drm_dev_alloc(struct drm_driver *driver,
diff --git a/include/drm/drm_crtc_helper.h b/include/drm/drm_crtc_helper.h
index 982c299e435a..a8cd620e8a5c 100644
--- a/include/drm/drm_crtc_helper.h
+++ b/include/drm/drm_crtc_helper.h
@@ -69,7 +69,8 @@ extern int drm_helper_probe_single_connector_modes(struct drm_connector
 extern void drm_kms_helper_poll_init(struct drm_device *dev);
 extern void drm_kms_helper_poll_fini(struct drm_device *dev);
 extern bool drm_helper_hpd_irq_event(struct drm_device *dev);
-extern void drm_kms_helper_hotplug_event(struct drm_device *dev);
+extern void drm_kms_helper_hotplug_event(struct drm_device *dev,
+					 struct drm_connector *connector);
 
 extern void drm_kms_helper_poll_disable(struct drm_device *dev);
 extern void drm_kms_helper_poll_enable(struct drm_device *dev);
-- 
2.9.3

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* ✗ Fi.CI.BAT: failure for drm: Pass along the hotplug connector to the uevent
  2016-10-21  8:25 [PATCH] drm: Pass along the hotplug connector to the uevent Chris Wilson
@ 2016-10-21  8:58 ` Patchwork
  2016-10-21  9:01 ` Patchwork
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 14+ messages in thread
From: Patchwork @ 2016-10-21  8:58 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx

== Series Details ==

Series: drm: Pass along the hotplug connector to the uevent
URL   : https://patchwork.freedesktop.org/series/14152/
State : failure

== Summary ==

Series 14152v1 drm: Pass along the hotplug connector to the uevent
https://patchwork.freedesktop.org/api/1.0/series/14152/revisions/1/mbox/

Test kms_flip:
        Subgroup basic-flip-vs-dpms:
                pass       -> INCOMPLETE (fi-snb-2600)
                pass       -> INCOMPLETE (fi-ivb-3770)
        Subgroup basic-flip-vs-modeset:
                pass       -> INCOMPLETE (fi-ivb-3520m)

fi-bdw-5557u     total:246  pass:231  dwarn:0   dfail:0   fail:0   skip:15 
fi-bsw-n3050     total:246  pass:204  dwarn:0   dfail:0   fail:0   skip:42 
fi-bxt-t5700     total:246  pass:216  dwarn:0   dfail:0   fail:0   skip:30 
fi-byt-j1900     total:246  pass:215  dwarn:0   dfail:0   fail:0   skip:31 
fi-byt-n2820     total:246  pass:211  dwarn:0   dfail:0   fail:0   skip:35 
fi-hsw-4770      total:246  pass:224  dwarn:0   dfail:0   fail:0   skip:22 
fi-hsw-4770r     total:246  pass:224  dwarn:0   dfail:0   fail:0   skip:22 
fi-ilk-650       total:246  pass:185  dwarn:0   dfail:0   fail:1   skip:60 
fi-ivb-3520m     total:181  pass:159  dwarn:0   dfail:0   fail:0   skip:21 
fi-ivb-3770      total:180  pass:158  dwarn:0   dfail:0   fail:0   skip:21 
fi-kbl-7200u     total:246  pass:222  dwarn:0   dfail:0   fail:0   skip:24 
fi-skl-6260u     total:246  pass:232  dwarn:0   dfail:0   fail:0   skip:14 
fi-skl-6700hq    total:246  pass:222  dwarn:1   dfail:0   fail:0   skip:23 
fi-skl-6700k     total:246  pass:221  dwarn:1   dfail:0   fail:0   skip:24 
fi-skl-6770hq    total:246  pass:232  dwarn:0   dfail:0   fail:0   skip:14 
fi-snb-2520m     total:246  pass:210  dwarn:0   dfail:0   fail:0   skip:36 
fi-snb-2600      total:180  pass:152  dwarn:0   dfail:0   fail:0   skip:27 

Results at /archive/results/CI_IGT_test/Patchwork_2781/

8d34fff02efad9abfc12cace4c347eaa1c3804f7 drm-intel-nightly: 2016y-10m-20d-21h-52m-44s UTC integration manifest
6e7e5c1 drm: Pass along the hotplug connector to the uevent

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 14+ messages in thread

* ✗ Fi.CI.BAT: failure for drm: Pass along the hotplug connector to the uevent
  2016-10-21  8:25 [PATCH] drm: Pass along the hotplug connector to the uevent Chris Wilson
  2016-10-21  8:58 ` ✗ Fi.CI.BAT: failure for " Patchwork
@ 2016-10-21  9:01 ` Patchwork
  2016-10-21  9:13   ` Chris Wilson
  2016-10-21  9:14 ` [PATCH v2] " Chris Wilson
  2016-10-21 10:18 ` ✗ Fi.CI.BAT: warning for drm: Pass along the hotplug connector to the uevent (rev2) Patchwork
  3 siblings, 1 reply; 14+ messages in thread
From: Patchwork @ 2016-10-21  9:01 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx

== Series Details ==

Series: drm: Pass along the hotplug connector to the uevent
URL   : https://patchwork.freedesktop.org/series/14152/
State : failure

== Summary ==

Series 14152v1 drm: Pass along the hotplug connector to the uevent
https://patchwork.freedesktop.org/api/1.0/series/14152/revisions/1/mbox/

Test kms_flip:
        Subgroup basic-flip-vs-dpms:
                pass       -> INCOMPLETE (fi-snb-2600)
                pass       -> INCOMPLETE (fi-ivb-3770)
        Subgroup basic-flip-vs-modeset:
                pass       -> INCOMPLETE (fi-ivb-3520m)

fi-bdw-5557u     total:246  pass:231  dwarn:0   dfail:0   fail:0   skip:15 
fi-bsw-n3050     total:246  pass:204  dwarn:0   dfail:0   fail:0   skip:42 
fi-bxt-t5700     total:246  pass:216  dwarn:0   dfail:0   fail:0   skip:30 
fi-byt-j1900     total:246  pass:215  dwarn:0   dfail:0   fail:0   skip:31 
fi-byt-n2820     total:246  pass:211  dwarn:0   dfail:0   fail:0   skip:35 
fi-hsw-4770      total:246  pass:224  dwarn:0   dfail:0   fail:0   skip:22 
fi-hsw-4770r     total:246  pass:224  dwarn:0   dfail:0   fail:0   skip:22 
fi-ilk-650       total:246  pass:185  dwarn:0   dfail:0   fail:1   skip:60 
fi-ivb-3520m     total:181  pass:159  dwarn:0   dfail:0   fail:0   skip:21 
fi-ivb-3770      total:180  pass:158  dwarn:0   dfail:0   fail:0   skip:21 
fi-kbl-7200u     total:246  pass:222  dwarn:0   dfail:0   fail:0   skip:24 
fi-skl-6260u     total:246  pass:232  dwarn:0   dfail:0   fail:0   skip:14 
fi-skl-6700hq    total:246  pass:222  dwarn:1   dfail:0   fail:0   skip:23 
fi-skl-6700k     total:246  pass:221  dwarn:1   dfail:0   fail:0   skip:24 
fi-skl-6770hq    total:246  pass:232  dwarn:0   dfail:0   fail:0   skip:14 
fi-snb-2520m     total:246  pass:210  dwarn:0   dfail:0   fail:0   skip:36 
fi-snb-2600      total:180  pass:152  dwarn:0   dfail:0   fail:0   skip:27 

Results at /archive/results/CI_IGT_test/Patchwork_2781/

8d34fff02efad9abfc12cace4c347eaa1c3804f7 drm-intel-nightly: 2016y-10m-20d-21h-52m-44s UTC integration manifest
6e7e5c1 drm: Pass along the hotplug connector to the uevent

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: ✗ Fi.CI.BAT: failure for drm: Pass along the hotplug connector to the uevent
  2016-10-21  9:01 ` Patchwork
@ 2016-10-21  9:13   ` Chris Wilson
  0 siblings, 0 replies; 14+ messages in thread
From: Chris Wilson @ 2016-10-21  9:13 UTC (permalink / raw)
  To: intel-gfx

On Fri, Oct 21, 2016 at 09:01:59AM -0000, Patchwork wrote:
> == Series Details ==
> 
> Series: drm: Pass along the hotplug connector to the uevent
> URL   : https://patchwork.freedesktop.org/series/14152/
> State : failure
> 
> == Summary ==
> 
> Series 14152v1 drm: Pass along the hotplug connector to the uevent
> https://patchwork.freedesktop.org/api/1.0/series/14152/revisions/1/mbox/
> 
> Test kms_flip:
>         Subgroup basic-flip-vs-dpms:
>                 pass       -> INCOMPLETE (fi-snb-2600)
>                 pass       -> INCOMPLETE (fi-ivb-3770)
>         Subgroup basic-flip-vs-modeset:
>                 pass       -> INCOMPLETE (fi-ivb-3520m)

mutex deadlock at

[  259.382365] CPU: 0 PID: 71 Comm: kworker/0:1 Tainted: G     U          4.9.0-rc1-CI-Patchwork_2781+ #1
[  259.382369] Hardware name: Dell Inc. XPS 8300  /0Y2MRG, BIOS A06 10/17/2011
[  259.382409] Workqueue: events i915_hotplug_work_func [i915]
[  259.382414]  ffffc9000027bb40 ffffffff8142dce5 ffffffff827de530 ffffffff827de530
[  259.382423]  ffffc9000027bbf8 ffffffff810d8624 ffff88012a37af68 000000000027bb90
[  259.382432]  ffffffff810d6c3f ffff880100000003 ffffffff00012000 b57c70f93abc779a
[  259.382440] Call Trace:
[  259.382448]  [<ffffffff8142dce5>] dump_stack+0x67/0x92 
[  259.382455]  [<ffffffff810d8624>] __lock_acquire+0x1544/0x1930
[  259.382461]  [<ffffffff810d6c3f>] ? mark_held_locks+0x6f/0xa0
[  259.382468]  [<ffffffff810d6d92>] ? trace_hardirqs_on_caller+0x122/0x1b0
[  259.382474]  [<ffffffff810d6e2d>] ? trace_hardirqs_on+0xd/0x10
[  259.382480]  [<ffffffff810d8e32>] lock_acquire+0xb2/0x200
[  259.382486]  [<ffffffff8154ce5e>] ? drm_fb_helper_hotplug_event+0x2e/0x170
[  259.382493]  [<ffffffff8154ce5e>] ? drm_fb_helper_hotplug_event+0x2e/0x170
[  259.382500]  [<ffffffff81810862>] mutex_lock_nested+0x62/0x400
[  259.382506]  [<ffffffff8154ce5e>] ? drm_fb_helper_hotplug_event+0x2e/0x170
[  259.382512]  [<ffffffff81558938>] ? drm_sysfs_hotplug_event+0x88/0xa0
[  259.382518]  [<ffffffff8154ce5e>] drm_fb_helper_hotplug_event+0x2e/0x170
[  259.382558]  [<ffffffffa00dffcf>] intel_fbdev_output_poll_changed+0x1f/0x30 [i915]
[  259.382566]  [<ffffffff8153e2c2>] drm_kms_helper_hotplug_event+0x22/0x30
[  259.382604]  [<ffffffffa00d6750>] i915_hotplug_work_func+0x230/0x280 [i915]
[  259.382611]  [<ffffffff8109c22c>] process_one_work+0x1ec/0x6b0
[  259.382616]  [<ffffffff8109c1a6>] ? process_one_work+0x166/0x6b0 
[  259.382623]  [<ffffffff8109c739>] worker_thread+0x49/0x490
[  259.382628]  [<ffffffff8109c6f0>] ? process_one_work+0x6b0/0x6b0 
[  259.382634]  [<ffffffff8109c6f0>] ? process_one_work+0x6b0/0x6b0 
[  259.382639]  [<ffffffff810a2a5b>] kthread+0xeb/0x110
[  259.382645]  [<ffffffff810a2970>] ? kthread_park+0x60/0x60 
[  259.382651]  [<ffffffff818160a7>] ret_from_fork+0x27/0x40

due to fbdev callback from the helper. So that has to remain outside of the lock.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v2] drm: Pass along the hotplug connector to the uevent
  2016-10-21  8:25 [PATCH] drm: Pass along the hotplug connector to the uevent Chris Wilson
  2016-10-21  8:58 ` ✗ Fi.CI.BAT: failure for " Patchwork
  2016-10-21  9:01 ` Patchwork
@ 2016-10-21  9:14 ` Chris Wilson
  2016-10-21  9:27   ` Jani Nikula
                     ` (2 more replies)
  2016-10-21 10:18 ` ✗ Fi.CI.BAT: warning for drm: Pass along the hotplug connector to the uevent (rev2) Patchwork
  3 siblings, 3 replies; 14+ messages in thread
From: Chris Wilson @ 2016-10-21  9:14 UTC (permalink / raw)
  To: dri-devel; +Cc: Manasi Navare, intel-gfx

If we know which connector was plugged/unplugged or
connected/disconnected, we can pass that information along to userspace
inside the uevent to reduce the amount of work userspace has to perform
after the event (i.e. instead of looking over all connectors, it can
just reprobe the affected one).

v2: Don't convert intel_hotplug.c, it does a light probe and doesn't
need the force.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Villle Syrjälä <ville.syrjala@linux.intel.com>
Cc: Manasi Navare <manasi.d.navare@intel.com>
---
 drivers/gpu/drm/drm_probe_helper.c     | 10 ++++++----
 drivers/gpu/drm/drm_sysfs.c            | 19 +++++++++++++++----
 drivers/gpu/drm/i2c/tda998x_drv.c      |  2 +-
 drivers/gpu/drm/i915/intel_dp.c        |  3 ++-
 drivers/gpu/drm/i915/intel_dp_mst.c    |  2 +-
 drivers/gpu/drm/i915/intel_hotplug.c   |  2 +-
 drivers/gpu/drm/qxl/qxl_display.c      |  2 +-
 drivers/gpu/drm/radeon/radeon_dp_mst.c |  2 +-
 drivers/gpu/drm/virtio/virtgpu_vq.c    |  2 +-
 drivers/gpu/drm/vmwgfx/vmwgfx_drv.c    |  2 +-
 drivers/gpu/drm/vmwgfx/vmwgfx_kms.c    |  2 +-
 include/drm/drmP.h                     |  3 ++-
 include/drm/drm_crtc_helper.h          |  3 ++-
 13 files changed, 35 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/drm_probe_helper.c b/drivers/gpu/drm/drm_probe_helper.c
index f6b64d7d3528..8790ee35a7cd 100644
--- a/drivers/gpu/drm/drm_probe_helper.c
+++ b/drivers/gpu/drm/drm_probe_helper.c
@@ -347,6 +347,7 @@ EXPORT_SYMBOL(drm_helper_probe_single_connector_modes);
 /**
  * drm_kms_helper_hotplug_event - fire off KMS hotplug events
  * @dev: drm_device whose connector state changed
+ * @connector: connector on which the status changed, if any
  *
  * This function fires off the uevent for userspace and also calls the
  * output_poll_changed function, which is most commonly used to inform the fbdev
@@ -360,10 +361,11 @@ EXPORT_SYMBOL(drm_helper_probe_single_connector_modes);
  * This function must be called from process context with no mode
  * setting locks held.
  */
-void drm_kms_helper_hotplug_event(struct drm_device *dev)
+void drm_kms_helper_hotplug_event(struct drm_device *dev,
+				  struct drm_connector *connector)
 {
 	/* send a uevent + call fbdev */
-	drm_sysfs_hotplug_event(dev);
+	drm_sysfs_hotplug_event(dev, connector);
 	if (dev->mode_config.funcs->output_poll_changed)
 		dev->mode_config.funcs->output_poll_changed(dev);
 }
@@ -444,7 +446,7 @@ static void output_poll_execute(struct work_struct *work)
 
 out:
 	if (changed)
-		drm_kms_helper_hotplug_event(dev);
+		drm_kms_helper_hotplug_event(dev, NULL);
 
 	if (repoll)
 		schedule_delayed_work(delayed_work, DRM_OUTPUT_POLL_PERIOD);
@@ -578,7 +580,7 @@ bool drm_helper_hpd_irq_event(struct drm_device *dev)
 	mutex_unlock(&dev->mode_config.mutex);
 
 	if (changed)
-		drm_kms_helper_hotplug_event(dev);
+		drm_kms_helper_hotplug_event(dev, NULL);
 
 	return changed;
 }
diff --git a/drivers/gpu/drm/drm_sysfs.c b/drivers/gpu/drm/drm_sysfs.c
index 9a37196c1bf1..c8f46055e2d0 100644
--- a/drivers/gpu/drm/drm_sysfs.c
+++ b/drivers/gpu/drm/drm_sysfs.c
@@ -280,7 +280,7 @@ int drm_sysfs_connector_add(struct drm_connector *connector)
 	}
 
 	/* Let userspace know we have a new connector */
-	drm_sysfs_hotplug_event(dev);
+	drm_sysfs_hotplug_event(dev, connector);
 
 	return 0;
 }
@@ -312,19 +312,30 @@ void drm_sysfs_connector_remove(struct drm_connector *connector)
 /**
  * drm_sysfs_hotplug_event - generate a DRM uevent
  * @dev: DRM device
+ * @connector: the DRM connector, if any
  *
  * Send a uevent for the DRM device specified by @dev.  Currently we only
  * set HOTPLUG=1 in the uevent environment, but this could be expanded to
  * deal with other types of events.
  */
-void drm_sysfs_hotplug_event(struct drm_device *dev)
+void drm_sysfs_hotplug_event(struct drm_device *dev,
+			     struct drm_connector *connector)
 {
-	char *event_string = "HOTPLUG=1";
-	char *envp[] = { event_string, NULL };
+	char *envp[3] = { "HOTPLUG=1" };
+	char *connector_event = NULL;
+
+	if (connector)
+		connector_event = kasprintf(GFP_KERNEL,
+					    "CONNECTOR=%d",
+					    connector->base.id);
+	if (connector_event)
+		envp[1] = connector_event;
 
 	DRM_DEBUG("generating hotplug event\n");
 
 	kobject_uevent_env(&dev->primary->kdev->kobj, KOBJ_CHANGE, envp);
+
+	kfree(connector_event);
 }
 EXPORT_SYMBOL(drm_sysfs_hotplug_event);
 
diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c
index 9798d400d817..9fd873c87686 100644
--- a/drivers/gpu/drm/i2c/tda998x_drv.c
+++ b/drivers/gpu/drm/i2c/tda998x_drv.c
@@ -617,7 +617,7 @@ static void tda998x_detect_work(struct work_struct *work)
 	struct drm_device *dev = priv->encoder.dev;
 
 	if (dev)
-		drm_kms_helper_hotplug_event(dev);
+		drm_kms_helper_hotplug_event(dev, NULL);
 }
 
 /*
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 88f3b745a326..59cd185689c0 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -3981,7 +3981,8 @@ intel_dp_check_mst_status(struct intel_dp *intel_dp)
 			intel_dp->is_mst = false;
 			drm_dp_mst_topology_mgr_set_mst(&intel_dp->mst_mgr, intel_dp->is_mst);
 			/* send a hotplug event */
-			drm_kms_helper_hotplug_event(intel_dig_port->base.base.dev);
+			drm_kms_helper_hotplug_event(intel_dig_port->base.base.dev,
+						     &intel_dp->attached_connector->base);
 		}
 	}
 	return -EINVAL;
diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c b/drivers/gpu/drm/i915/intel_dp_mst.c
index 3ffbd69e4551..2bd48a987934 100644
--- a/drivers/gpu/drm/i915/intel_dp_mst.c
+++ b/drivers/gpu/drm/i915/intel_dp_mst.c
@@ -493,7 +493,7 @@ static void intel_dp_mst_hotplug(struct drm_dp_mst_topology_mgr *mgr)
 	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
 	struct drm_device *dev = intel_dig_port->base.base.dev;
 
-	drm_kms_helper_hotplug_event(dev);
+	drm_kms_helper_hotplug_event(dev, &intel_dp->attached_connector->base);
 }
 
 static const struct drm_dp_mst_topology_cbs mst_cbs = {
diff --git a/drivers/gpu/drm/i915/intel_hotplug.c b/drivers/gpu/drm/i915/intel_hotplug.c
index 334d47b5811a..da0649aff734 100644
--- a/drivers/gpu/drm/i915/intel_hotplug.c
+++ b/drivers/gpu/drm/i915/intel_hotplug.c
@@ -340,7 +340,7 @@ static void i915_hotplug_work_func(struct work_struct *work)
 	mutex_unlock(&mode_config->mutex);
 
 	if (changed)
-		drm_kms_helper_hotplug_event(dev);
+		drm_kms_helper_hotplug_event(dev, NULL);
 }
 
 
diff --git a/drivers/gpu/drm/qxl/qxl_display.c b/drivers/gpu/drm/qxl/qxl_display.c
index 2e01c6e5d905..3c196a096c2d 100644
--- a/drivers/gpu/drm/qxl/qxl_display.c
+++ b/drivers/gpu/drm/qxl/qxl_display.c
@@ -135,7 +135,7 @@ void qxl_display_read_client_monitors_config(struct qxl_device *qdev)
 	if (!drm_helper_hpd_irq_event(qdev->ddev)) {
 		/* notify that the monitor configuration changed, to
 		   adjust at the arbitrary resolution */
-		drm_kms_helper_hotplug_event(qdev->ddev);
+		drm_kms_helper_hotplug_event(qdev->ddev, NULL);
 	}
 }
 
diff --git a/drivers/gpu/drm/radeon/radeon_dp_mst.c b/drivers/gpu/drm/radeon/radeon_dp_mst.c
index de504ea29c06..e80b1c365a2d 100644
--- a/drivers/gpu/drm/radeon/radeon_dp_mst.c
+++ b/drivers/gpu/drm/radeon/radeon_dp_mst.c
@@ -331,7 +331,7 @@ static void radeon_dp_mst_hotplug(struct drm_dp_mst_topology_mgr *mgr)
 	struct radeon_connector *master = container_of(mgr, struct radeon_connector, mst_mgr);
 	struct drm_device *dev = master->base.dev;
 
-	drm_kms_helper_hotplug_event(dev);
+	drm_kms_helper_hotplug_event(dev, NULL);
 }
 
 const struct drm_dp_mst_topology_cbs mst_cbs = {
diff --git a/drivers/gpu/drm/virtio/virtgpu_vq.c b/drivers/gpu/drm/virtio/virtgpu_vq.c
index 5a0f8a745b9d..72be43c9e008 100644
--- a/drivers/gpu/drm/virtio/virtgpu_vq.c
+++ b/drivers/gpu/drm/virtio/virtgpu_vq.c
@@ -583,7 +583,7 @@ static void virtio_gpu_cmd_get_display_info_cb(struct virtio_gpu_device *vgdev,
 	wake_up(&vgdev->resp_wq);
 
 	if (!drm_helper_hpd_irq_event(vgdev->ddev))
-		drm_kms_helper_hotplug_event(vgdev->ddev);
+		drm_kms_helper_hotplug_event(vgdev->ddev, NULL);
 }
 
 static void virtio_gpu_cmd_get_capset_info_cb(struct virtio_gpu_device *vgdev,
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
index e8ae3dc476d1..36aa1a0440c3 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
@@ -1233,7 +1233,7 @@ static int vmw_master_set(struct drm_device *dev,
 	}
 
 	dev_priv->active_master = vmaster;
-	drm_sysfs_hotplug_event(dev);
+	drm_sysfs_hotplug_event(dev, NULL);
 
 	return 0;
 }
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
index c965514b82be..26262763aa5f 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
@@ -1407,7 +1407,7 @@ static int vmw_du_update_layout(struct vmw_private *dev_priv, unsigned num,
 	}
 
 	mutex_unlock(&dev->mode_config.mutex);
-	drm_sysfs_hotplug_event(dev);
+	drm_sysfs_hotplug_event(dev, con);
 
 	return 0;
 }
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index 672644031bd5..0a4a4aa61fd3 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -1038,7 +1038,8 @@ extern struct drm_dma_handle *drm_pci_alloc(struct drm_device *dev, size_t size,
 extern void drm_pci_free(struct drm_device *dev, struct drm_dma_handle * dmah);
 
 			       /* sysfs support (drm_sysfs.c) */
-extern void drm_sysfs_hotplug_event(struct drm_device *dev);
+extern void drm_sysfs_hotplug_event(struct drm_device *dev,
+				    struct drm_connector *connector);
 
 
 struct drm_device *drm_dev_alloc(struct drm_driver *driver,
diff --git a/include/drm/drm_crtc_helper.h b/include/drm/drm_crtc_helper.h
index 982c299e435a..a8cd620e8a5c 100644
--- a/include/drm/drm_crtc_helper.h
+++ b/include/drm/drm_crtc_helper.h
@@ -69,7 +69,8 @@ extern int drm_helper_probe_single_connector_modes(struct drm_connector
 extern void drm_kms_helper_poll_init(struct drm_device *dev);
 extern void drm_kms_helper_poll_fini(struct drm_device *dev);
 extern bool drm_helper_hpd_irq_event(struct drm_device *dev);
-extern void drm_kms_helper_hotplug_event(struct drm_device *dev);
+extern void drm_kms_helper_hotplug_event(struct drm_device *dev,
+					 struct drm_connector *connector);
 
 extern void drm_kms_helper_poll_disable(struct drm_device *dev);
 extern void drm_kms_helper_poll_enable(struct drm_device *dev);
-- 
2.9.3

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH v2] drm: Pass along the hotplug connector to the uevent
  2016-10-21  9:14 ` [PATCH v2] " Chris Wilson
@ 2016-10-21  9:27   ` Jani Nikula
  2016-10-21  9:46   ` Ville Syrjälä
  2016-10-21 12:45   ` [Intel-gfx] " Daniel Vetter
  2 siblings, 0 replies; 14+ messages in thread
From: Jani Nikula @ 2016-10-21  9:27 UTC (permalink / raw)
  To: Chris Wilson, dri-devel; +Cc: intel-gfx

On Fri, 21 Oct 2016, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> If we know which connector was plugged/unplugged or
> connected/disconnected, we can pass that information along to userspace
> inside the uevent to reduce the amount of work userspace has to perform
> after the event (i.e. instead of looking over all connectors, it can
> just reprobe the affected one).
>
> v2: Don't convert intel_hotplug.c, it does a light probe and doesn't
> need the force.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Villle Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Manasi Navare <manasi.d.navare@intel.com>
> ---
>  drivers/gpu/drm/drm_probe_helper.c     | 10 ++++++----
>  drivers/gpu/drm/drm_sysfs.c            | 19 +++++++++++++++----
>  drivers/gpu/drm/i2c/tda998x_drv.c      |  2 +-
>  drivers/gpu/drm/i915/intel_dp.c        |  3 ++-
>  drivers/gpu/drm/i915/intel_dp_mst.c    |  2 +-
>  drivers/gpu/drm/i915/intel_hotplug.c   |  2 +-
>  drivers/gpu/drm/qxl/qxl_display.c      |  2 +-
>  drivers/gpu/drm/radeon/radeon_dp_mst.c |  2 +-
>  drivers/gpu/drm/virtio/virtgpu_vq.c    |  2 +-
>  drivers/gpu/drm/vmwgfx/vmwgfx_drv.c    |  2 +-
>  drivers/gpu/drm/vmwgfx/vmwgfx_kms.c    |  2 +-
>  include/drm/drmP.h                     |  3 ++-
>  include/drm/drm_crtc_helper.h          |  3 ++-
>  13 files changed, 35 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_probe_helper.c b/drivers/gpu/drm/drm_probe_helper.c
> index f6b64d7d3528..8790ee35a7cd 100644
> --- a/drivers/gpu/drm/drm_probe_helper.c
> +++ b/drivers/gpu/drm/drm_probe_helper.c
> @@ -347,6 +347,7 @@ EXPORT_SYMBOL(drm_helper_probe_single_connector_modes);
>  /**
>   * drm_kms_helper_hotplug_event - fire off KMS hotplug events
>   * @dev: drm_device whose connector state changed
> + * @connector: connector on which the status changed, if any
>   *
>   * This function fires off the uevent for userspace and also calls the
>   * output_poll_changed function, which is most commonly used to inform the fbdev
> @@ -360,10 +361,11 @@ EXPORT_SYMBOL(drm_helper_probe_single_connector_modes);
>   * This function must be called from process context with no mode
>   * setting locks held.
>   */
> -void drm_kms_helper_hotplug_event(struct drm_device *dev)
> +void drm_kms_helper_hotplug_event(struct drm_device *dev,
> +				  struct drm_connector *connector)
>  {
>  	/* send a uevent + call fbdev */
> -	drm_sysfs_hotplug_event(dev);
> +	drm_sysfs_hotplug_event(dev, connector);
>  	if (dev->mode_config.funcs->output_poll_changed)
>  		dev->mode_config.funcs->output_poll_changed(dev);
>  }
> @@ -444,7 +446,7 @@ static void output_poll_execute(struct work_struct *work)
>  
>  out:
>  	if (changed)
> -		drm_kms_helper_hotplug_event(dev);
> +		drm_kms_helper_hotplug_event(dev, NULL);
>  
>  	if (repoll)
>  		schedule_delayed_work(delayed_work, DRM_OUTPUT_POLL_PERIOD);
> @@ -578,7 +580,7 @@ bool drm_helper_hpd_irq_event(struct drm_device *dev)
>  	mutex_unlock(&dev->mode_config.mutex);
>  
>  	if (changed)
> -		drm_kms_helper_hotplug_event(dev);
> +		drm_kms_helper_hotplug_event(dev, NULL);
>  
>  	return changed;
>  }
> diff --git a/drivers/gpu/drm/drm_sysfs.c b/drivers/gpu/drm/drm_sysfs.c
> index 9a37196c1bf1..c8f46055e2d0 100644
> --- a/drivers/gpu/drm/drm_sysfs.c
> +++ b/drivers/gpu/drm/drm_sysfs.c
> @@ -280,7 +280,7 @@ int drm_sysfs_connector_add(struct drm_connector *connector)
>  	}
>  
>  	/* Let userspace know we have a new connector */
> -	drm_sysfs_hotplug_event(dev);
> +	drm_sysfs_hotplug_event(dev, connector);
>  
>  	return 0;
>  }
> @@ -312,19 +312,30 @@ void drm_sysfs_connector_remove(struct drm_connector *connector)
>  /**
>   * drm_sysfs_hotplug_event - generate a DRM uevent
>   * @dev: DRM device
> + * @connector: the DRM connector, if any
>   *
>   * Send a uevent for the DRM device specified by @dev.  Currently we only
>   * set HOTPLUG=1 in the uevent environment, but this could be expanded to
>   * deal with other types of events.
>   */
> -void drm_sysfs_hotplug_event(struct drm_device *dev)
> +void drm_sysfs_hotplug_event(struct drm_device *dev,
> +			     struct drm_connector *connector)
>  {
> -	char *event_string = "HOTPLUG=1";
> -	char *envp[] = { event_string, NULL };
> +	char *envp[3] = { "HOTPLUG=1" };
> +	char *connector_event = NULL;
> +
> +	if (connector)
> +		connector_event = kasprintf(GFP_KERNEL,

GFP_TEMPORARY?

Otherwise, on a light reading of the patch, and assuming existing
userspace doesn't fall over because of this,

Reviewed-by: Jani Nikula <jani.nikula@intel.com>


> +					    "CONNECTOR=%d",
> +					    connector->base.id);
> +	if (connector_event)
> +		envp[1] = connector_event;
>  
>  	DRM_DEBUG("generating hotplug event\n");
>  
>  	kobject_uevent_env(&dev->primary->kdev->kobj, KOBJ_CHANGE, envp);
> +
> +	kfree(connector_event);
>  }
>  EXPORT_SYMBOL(drm_sysfs_hotplug_event);
>  
> diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c
> index 9798d400d817..9fd873c87686 100644
> --- a/drivers/gpu/drm/i2c/tda998x_drv.c
> +++ b/drivers/gpu/drm/i2c/tda998x_drv.c
> @@ -617,7 +617,7 @@ static void tda998x_detect_work(struct work_struct *work)
>  	struct drm_device *dev = priv->encoder.dev;
>  
>  	if (dev)
> -		drm_kms_helper_hotplug_event(dev);
> +		drm_kms_helper_hotplug_event(dev, NULL);
>  }
>  
>  /*
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 88f3b745a326..59cd185689c0 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -3981,7 +3981,8 @@ intel_dp_check_mst_status(struct intel_dp *intel_dp)
>  			intel_dp->is_mst = false;
>  			drm_dp_mst_topology_mgr_set_mst(&intel_dp->mst_mgr, intel_dp->is_mst);
>  			/* send a hotplug event */
> -			drm_kms_helper_hotplug_event(intel_dig_port->base.base.dev);
> +			drm_kms_helper_hotplug_event(intel_dig_port->base.base.dev,
> +						     &intel_dp->attached_connector->base);
>  		}
>  	}
>  	return -EINVAL;
> diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c b/drivers/gpu/drm/i915/intel_dp_mst.c
> index 3ffbd69e4551..2bd48a987934 100644
> --- a/drivers/gpu/drm/i915/intel_dp_mst.c
> +++ b/drivers/gpu/drm/i915/intel_dp_mst.c
> @@ -493,7 +493,7 @@ static void intel_dp_mst_hotplug(struct drm_dp_mst_topology_mgr *mgr)
>  	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
>  	struct drm_device *dev = intel_dig_port->base.base.dev;
>  
> -	drm_kms_helper_hotplug_event(dev);
> +	drm_kms_helper_hotplug_event(dev, &intel_dp->attached_connector->base);
>  }
>  
>  static const struct drm_dp_mst_topology_cbs mst_cbs = {
> diff --git a/drivers/gpu/drm/i915/intel_hotplug.c b/drivers/gpu/drm/i915/intel_hotplug.c
> index 334d47b5811a..da0649aff734 100644
> --- a/drivers/gpu/drm/i915/intel_hotplug.c
> +++ b/drivers/gpu/drm/i915/intel_hotplug.c
> @@ -340,7 +340,7 @@ static void i915_hotplug_work_func(struct work_struct *work)
>  	mutex_unlock(&mode_config->mutex);
>  
>  	if (changed)
> -		drm_kms_helper_hotplug_event(dev);
> +		drm_kms_helper_hotplug_event(dev, NULL);
>  }
>  
>  
> diff --git a/drivers/gpu/drm/qxl/qxl_display.c b/drivers/gpu/drm/qxl/qxl_display.c
> index 2e01c6e5d905..3c196a096c2d 100644
> --- a/drivers/gpu/drm/qxl/qxl_display.c
> +++ b/drivers/gpu/drm/qxl/qxl_display.c
> @@ -135,7 +135,7 @@ void qxl_display_read_client_monitors_config(struct qxl_device *qdev)
>  	if (!drm_helper_hpd_irq_event(qdev->ddev)) {
>  		/* notify that the monitor configuration changed, to
>  		   adjust at the arbitrary resolution */
> -		drm_kms_helper_hotplug_event(qdev->ddev);
> +		drm_kms_helper_hotplug_event(qdev->ddev, NULL);
>  	}
>  }
>  
> diff --git a/drivers/gpu/drm/radeon/radeon_dp_mst.c b/drivers/gpu/drm/radeon/radeon_dp_mst.c
> index de504ea29c06..e80b1c365a2d 100644
> --- a/drivers/gpu/drm/radeon/radeon_dp_mst.c
> +++ b/drivers/gpu/drm/radeon/radeon_dp_mst.c
> @@ -331,7 +331,7 @@ static void radeon_dp_mst_hotplug(struct drm_dp_mst_topology_mgr *mgr)
>  	struct radeon_connector *master = container_of(mgr, struct radeon_connector, mst_mgr);
>  	struct drm_device *dev = master->base.dev;
>  
> -	drm_kms_helper_hotplug_event(dev);
> +	drm_kms_helper_hotplug_event(dev, NULL);
>  }
>  
>  const struct drm_dp_mst_topology_cbs mst_cbs = {
> diff --git a/drivers/gpu/drm/virtio/virtgpu_vq.c b/drivers/gpu/drm/virtio/virtgpu_vq.c
> index 5a0f8a745b9d..72be43c9e008 100644
> --- a/drivers/gpu/drm/virtio/virtgpu_vq.c
> +++ b/drivers/gpu/drm/virtio/virtgpu_vq.c
> @@ -583,7 +583,7 @@ static void virtio_gpu_cmd_get_display_info_cb(struct virtio_gpu_device *vgdev,
>  	wake_up(&vgdev->resp_wq);
>  
>  	if (!drm_helper_hpd_irq_event(vgdev->ddev))
> -		drm_kms_helper_hotplug_event(vgdev->ddev);
> +		drm_kms_helper_hotplug_event(vgdev->ddev, NULL);
>  }
>  
>  static void virtio_gpu_cmd_get_capset_info_cb(struct virtio_gpu_device *vgdev,
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
> index e8ae3dc476d1..36aa1a0440c3 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
> @@ -1233,7 +1233,7 @@ static int vmw_master_set(struct drm_device *dev,
>  	}
>  
>  	dev_priv->active_master = vmaster;
> -	drm_sysfs_hotplug_event(dev);
> +	drm_sysfs_hotplug_event(dev, NULL);
>  
>  	return 0;
>  }
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> index c965514b82be..26262763aa5f 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> @@ -1407,7 +1407,7 @@ static int vmw_du_update_layout(struct vmw_private *dev_priv, unsigned num,
>  	}
>  
>  	mutex_unlock(&dev->mode_config.mutex);
> -	drm_sysfs_hotplug_event(dev);
> +	drm_sysfs_hotplug_event(dev, con);
>  
>  	return 0;
>  }
> diff --git a/include/drm/drmP.h b/include/drm/drmP.h
> index 672644031bd5..0a4a4aa61fd3 100644
> --- a/include/drm/drmP.h
> +++ b/include/drm/drmP.h
> @@ -1038,7 +1038,8 @@ extern struct drm_dma_handle *drm_pci_alloc(struct drm_device *dev, size_t size,
>  extern void drm_pci_free(struct drm_device *dev, struct drm_dma_handle * dmah);
>  
>  			       /* sysfs support (drm_sysfs.c) */
> -extern void drm_sysfs_hotplug_event(struct drm_device *dev);
> +extern void drm_sysfs_hotplug_event(struct drm_device *dev,
> +				    struct drm_connector *connector);
>  
>  
>  struct drm_device *drm_dev_alloc(struct drm_driver *driver,
> diff --git a/include/drm/drm_crtc_helper.h b/include/drm/drm_crtc_helper.h
> index 982c299e435a..a8cd620e8a5c 100644
> --- a/include/drm/drm_crtc_helper.h
> +++ b/include/drm/drm_crtc_helper.h
> @@ -69,7 +69,8 @@ extern int drm_helper_probe_single_connector_modes(struct drm_connector
>  extern void drm_kms_helper_poll_init(struct drm_device *dev);
>  extern void drm_kms_helper_poll_fini(struct drm_device *dev);
>  extern bool drm_helper_hpd_irq_event(struct drm_device *dev);
> -extern void drm_kms_helper_hotplug_event(struct drm_device *dev);
> +extern void drm_kms_helper_hotplug_event(struct drm_device *dev,
> +					 struct drm_connector *connector);
>  
>  extern void drm_kms_helper_poll_disable(struct drm_device *dev);
>  extern void drm_kms_helper_poll_enable(struct drm_device *dev);

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2] drm: Pass along the hotplug connector to the uevent
  2016-10-21  9:14 ` [PATCH v2] " Chris Wilson
  2016-10-21  9:27   ` Jani Nikula
@ 2016-10-21  9:46   ` Ville Syrjälä
  2016-10-21 10:05     ` Chris Wilson
  2016-10-21 12:45   ` [Intel-gfx] " Daniel Vetter
  2 siblings, 1 reply; 14+ messages in thread
From: Ville Syrjälä @ 2016-10-21  9:46 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx, dri-devel

On Fri, Oct 21, 2016 at 10:14:21AM +0100, Chris Wilson wrote:
> If we know which connector was plugged/unplugged or
> connected/disconnected, we can pass that information along to userspace
> inside the uevent to reduce the amount of work userspace has to perform
> after the event (i.e. instead of looking over all connectors, it can
> just reprobe the affected one).
> 
> v2: Don't convert intel_hotplug.c, it does a light probe and doesn't
> need the force.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Villle Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Manasi Navare <manasi.d.navare@intel.com>
> ---
>  drivers/gpu/drm/drm_probe_helper.c     | 10 ++++++----
>  drivers/gpu/drm/drm_sysfs.c            | 19 +++++++++++++++----
>  drivers/gpu/drm/i2c/tda998x_drv.c      |  2 +-
>  drivers/gpu/drm/i915/intel_dp.c        |  3 ++-
>  drivers/gpu/drm/i915/intel_dp_mst.c    |  2 +-
>  drivers/gpu/drm/i915/intel_hotplug.c   |  2 +-
>  drivers/gpu/drm/qxl/qxl_display.c      |  2 +-
>  drivers/gpu/drm/radeon/radeon_dp_mst.c |  2 +-
>  drivers/gpu/drm/virtio/virtgpu_vq.c    |  2 +-
>  drivers/gpu/drm/vmwgfx/vmwgfx_drv.c    |  2 +-
>  drivers/gpu/drm/vmwgfx/vmwgfx_kms.c    |  2 +-
>  include/drm/drmP.h                     |  3 ++-
>  include/drm/drm_crtc_helper.h          |  3 ++-
>  13 files changed, 35 insertions(+), 19 deletions(-)
> 
<snip>
> diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c b/drivers/gpu/drm/i915/intel_dp_mst.c
> index 3ffbd69e4551..2bd48a987934 100644
> --- a/drivers/gpu/drm/i915/intel_dp_mst.c
> +++ b/drivers/gpu/drm/i915/intel_dp_mst.c
> @@ -493,7 +493,7 @@ static void intel_dp_mst_hotplug(struct drm_dp_mst_topology_mgr *mgr)
>  	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
>  	struct drm_device *dev = intel_dig_port->base.base.dev;
>  
> -	drm_kms_helper_hotplug_event(dev);
> +	drm_kms_helper_hotplug_event(dev, &intel_dp->attached_connector->base);
>  }
>  
>  static const struct drm_dp_mst_topology_cbs mst_cbs = {
> diff --git a/drivers/gpu/drm/i915/intel_hotplug.c b/drivers/gpu/drm/i915/intel_hotplug.c
> index 334d47b5811a..da0649aff734 100644
> --- a/drivers/gpu/drm/i915/intel_hotplug.c
> +++ b/drivers/gpu/drm/i915/intel_hotplug.c
> @@ -340,7 +340,7 @@ static void i915_hotplug_work_func(struct work_struct *work)
>  	mutex_unlock(&mode_config->mutex);
>  
>  	if (changed)
> -		drm_kms_helper_hotplug_event(dev);
> +		drm_kms_helper_hotplug_event(dev, NULL);
>  }

So basically this patch deals with all the weird cases but doesn't do
anything for the normal case of a connector being connected/disconnected?

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2] drm: Pass along the hotplug connector to the uevent
  2016-10-21  9:46   ` Ville Syrjälä
@ 2016-10-21 10:05     ` Chris Wilson
  0 siblings, 0 replies; 14+ messages in thread
From: Chris Wilson @ 2016-10-21 10:05 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: Manasi Navare, intel-gfx, dri-devel

On Fri, Oct 21, 2016 at 12:46:53PM +0300, Ville Syrjälä wrote:
> On Fri, Oct 21, 2016 at 10:14:21AM +0100, Chris Wilson wrote:
> > If we know which connector was plugged/unplugged or
> > connected/disconnected, we can pass that information along to userspace
> > inside the uevent to reduce the amount of work userspace has to perform
> > after the event (i.e. instead of looking over all connectors, it can
> > just reprobe the affected one).
> > 
> > v2: Don't convert intel_hotplug.c, it does a light probe and doesn't
> > need the force.
> > 
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Villle Syrjälä <ville.syrjala@linux.intel.com>
> > Cc: Manasi Navare <manasi.d.navare@intel.com>
> > ---
> >  drivers/gpu/drm/drm_probe_helper.c     | 10 ++++++----
> >  drivers/gpu/drm/drm_sysfs.c            | 19 +++++++++++++++----
> >  drivers/gpu/drm/i2c/tda998x_drv.c      |  2 +-
> >  drivers/gpu/drm/i915/intel_dp.c        |  3 ++-
> >  drivers/gpu/drm/i915/intel_dp_mst.c    |  2 +-
> >  drivers/gpu/drm/i915/intel_hotplug.c   |  2 +-
> >  drivers/gpu/drm/qxl/qxl_display.c      |  2 +-
> >  drivers/gpu/drm/radeon/radeon_dp_mst.c |  2 +-
> >  drivers/gpu/drm/virtio/virtgpu_vq.c    |  2 +-
> >  drivers/gpu/drm/vmwgfx/vmwgfx_drv.c    |  2 +-
> >  drivers/gpu/drm/vmwgfx/vmwgfx_kms.c    |  2 +-
> >  include/drm/drmP.h                     |  3 ++-
> >  include/drm/drm_crtc_helper.h          |  3 ++-
> >  13 files changed, 35 insertions(+), 19 deletions(-)
> > 
> <snip>
> > diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c b/drivers/gpu/drm/i915/intel_dp_mst.c
> > index 3ffbd69e4551..2bd48a987934 100644
> > --- a/drivers/gpu/drm/i915/intel_dp_mst.c
> > +++ b/drivers/gpu/drm/i915/intel_dp_mst.c
> > @@ -493,7 +493,7 @@ static void intel_dp_mst_hotplug(struct drm_dp_mst_topology_mgr *mgr)
> >  	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> >  	struct drm_device *dev = intel_dig_port->base.base.dev;
> >  
> > -	drm_kms_helper_hotplug_event(dev);
> > +	drm_kms_helper_hotplug_event(dev, &intel_dp->attached_connector->base);
> >  }
> >  
> >  static const struct drm_dp_mst_topology_cbs mst_cbs = {
> > diff --git a/drivers/gpu/drm/i915/intel_hotplug.c b/drivers/gpu/drm/i915/intel_hotplug.c
> > index 334d47b5811a..da0649aff734 100644
> > --- a/drivers/gpu/drm/i915/intel_hotplug.c
> > +++ b/drivers/gpu/drm/i915/intel_hotplug.c
> > @@ -340,7 +340,7 @@ static void i915_hotplug_work_func(struct work_struct *work)
> >  	mutex_unlock(&mode_config->mutex);
> >  
> >  	if (changed)
> > -		drm_kms_helper_hotplug_event(dev);
> > +		drm_kms_helper_hotplug_event(dev, NULL);
> >  }
> 
> So basically this patch deals with all the weird cases but doesn't do
> anything for the normal case of a connector being connected/disconnected?

Yes. We can expand it to the lightweight hotplug events, but where it
was just checking the current connector->status I didn't feel it merited
the extra work in sending per-connector hotplug events.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 14+ messages in thread

* ✗ Fi.CI.BAT: warning for drm: Pass along the hotplug connector to the uevent (rev2)
  2016-10-21  8:25 [PATCH] drm: Pass along the hotplug connector to the uevent Chris Wilson
                   ` (2 preceding siblings ...)
  2016-10-21  9:14 ` [PATCH v2] " Chris Wilson
@ 2016-10-21 10:18 ` Patchwork
  3 siblings, 0 replies; 14+ messages in thread
From: Patchwork @ 2016-10-21 10:18 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx

== Series Details ==

Series: drm: Pass along the hotplug connector to the uevent (rev2)
URL   : https://patchwork.freedesktop.org/series/14152/
State : warning

== Summary ==

Series 14152v2 drm: Pass along the hotplug connector to the uevent
https://patchwork.freedesktop.org/api/1.0/series/14152/revisions/2/mbox/

Test drv_module_reload_basic:
                pass       -> SKIP       (fi-snb-2520m)

fi-bdw-5557u     total:246  pass:231  dwarn:0   dfail:0   fail:0   skip:15 
fi-bsw-n3050     total:246  pass:204  dwarn:0   dfail:0   fail:0   skip:42 
fi-bxt-t5700     total:246  pass:216  dwarn:0   dfail:0   fail:0   skip:30 
fi-byt-j1900     total:246  pass:215  dwarn:0   dfail:0   fail:0   skip:31 
fi-byt-n2820     total:246  pass:211  dwarn:0   dfail:0   fail:0   skip:35 
fi-hsw-4770      total:246  pass:224  dwarn:0   dfail:0   fail:0   skip:22 
fi-hsw-4770r     total:246  pass:224  dwarn:0   dfail:0   fail:0   skip:22 
fi-ilk-650       total:246  pass:185  dwarn:0   dfail:0   fail:1   skip:60 
fi-ivb-3520m     total:246  pass:221  dwarn:0   dfail:0   fail:0   skip:25 
fi-ivb-3770      total:246  pass:221  dwarn:0   dfail:0   fail:0   skip:25 
fi-kbl-7200u     total:246  pass:222  dwarn:0   dfail:0   fail:0   skip:24 
fi-skl-6260u     total:246  pass:232  dwarn:0   dfail:0   fail:0   skip:14 
fi-skl-6700hq    total:246  pass:222  dwarn:1   dfail:0   fail:0   skip:23 
fi-skl-6700k     total:246  pass:221  dwarn:1   dfail:0   fail:0   skip:24 
fi-skl-6770hq    total:246  pass:232  dwarn:0   dfail:0   fail:0   skip:14 
fi-snb-2520m     total:246  pass:209  dwarn:0   dfail:0   fail:0   skip:37 
fi-snb-2600 failed to connect after reboot

Results at /archive/results/CI_IGT_test/Patchwork_2783/

8d34fff02efad9abfc12cace4c347eaa1c3804f7 drm-intel-nightly: 2016y-10m-20d-21h-52m-44s UTC integration manifest
7fa3e21 drm: Pass along the hotplug connector to the uevent

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [Intel-gfx] [PATCH v2] drm: Pass along the hotplug connector to the uevent
  2016-10-21  9:14 ` [PATCH v2] " Chris Wilson
  2016-10-21  9:27   ` Jani Nikula
  2016-10-21  9:46   ` Ville Syrjälä
@ 2016-10-21 12:45   ` Daniel Vetter
  2016-10-21 13:17     ` Chris Wilson
  2 siblings, 1 reply; 14+ messages in thread
From: Daniel Vetter @ 2016-10-21 12:45 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx, dri-devel

On Fri, Oct 21, 2016 at 10:14:21AM +0100, Chris Wilson wrote:
> If we know which connector was plugged/unplugged or
> connected/disconnected, we can pass that information along to userspace
> inside the uevent to reduce the amount of work userspace has to perform
> after the event (i.e. instead of looking over all connectors, it can
> just reprobe the affected one).
> 
> v2: Don't convert intel_hotplug.c, it does a light probe and doesn't
> need the force.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Villle Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Manasi Navare <manasi.d.navare@intel.com>

Nothing is preventing multiple connectors to change state at the same
time. Or at least almost the same time, e.g. yank an entire dp mst chain.

I think we need something that works per-connector, so either send out
uevents for one that changed individually. Or go with the epoche counter
idea. The later has the upside that it disconnects probing from reporting:
Sometimes only deep down in the driver's probe function do we realize that
something changed (e.g. different edid, or different dpcd). With a helper
to increment the epoche there would be no need to wire the hotplug status
through the entire callchain.

To give us the same speed-up benefits like this here the only thing we'd
need to do is make sure reading a read-only (i.e. not userspace setable
prop) doesn't take any heavyweight locks. That should be easy to achieve.
Of course that leaves us with num_connector ioctl calls, but that should
be acceptable.

And it's an uabi change either way.
-Daniel

> ---
>  drivers/gpu/drm/drm_probe_helper.c     | 10 ++++++----
>  drivers/gpu/drm/drm_sysfs.c            | 19 +++++++++++++++----
>  drivers/gpu/drm/i2c/tda998x_drv.c      |  2 +-
>  drivers/gpu/drm/i915/intel_dp.c        |  3 ++-
>  drivers/gpu/drm/i915/intel_dp_mst.c    |  2 +-
>  drivers/gpu/drm/i915/intel_hotplug.c   |  2 +-
>  drivers/gpu/drm/qxl/qxl_display.c      |  2 +-
>  drivers/gpu/drm/radeon/radeon_dp_mst.c |  2 +-
>  drivers/gpu/drm/virtio/virtgpu_vq.c    |  2 +-
>  drivers/gpu/drm/vmwgfx/vmwgfx_drv.c    |  2 +-
>  drivers/gpu/drm/vmwgfx/vmwgfx_kms.c    |  2 +-
>  include/drm/drmP.h                     |  3 ++-
>  include/drm/drm_crtc_helper.h          |  3 ++-
>  13 files changed, 35 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_probe_helper.c b/drivers/gpu/drm/drm_probe_helper.c
> index f6b64d7d3528..8790ee35a7cd 100644
> --- a/drivers/gpu/drm/drm_probe_helper.c
> +++ b/drivers/gpu/drm/drm_probe_helper.c
> @@ -347,6 +347,7 @@ EXPORT_SYMBOL(drm_helper_probe_single_connector_modes);
>  /**
>   * drm_kms_helper_hotplug_event - fire off KMS hotplug events
>   * @dev: drm_device whose connector state changed
> + * @connector: connector on which the status changed, if any
>   *
>   * This function fires off the uevent for userspace and also calls the
>   * output_poll_changed function, which is most commonly used to inform the fbdev
> @@ -360,10 +361,11 @@ EXPORT_SYMBOL(drm_helper_probe_single_connector_modes);
>   * This function must be called from process context with no mode
>   * setting locks held.
>   */
> -void drm_kms_helper_hotplug_event(struct drm_device *dev)
> +void drm_kms_helper_hotplug_event(struct drm_device *dev,
> +				  struct drm_connector *connector)
>  {
>  	/* send a uevent + call fbdev */
> -	drm_sysfs_hotplug_event(dev);
> +	drm_sysfs_hotplug_event(dev, connector);
>  	if (dev->mode_config.funcs->output_poll_changed)
>  		dev->mode_config.funcs->output_poll_changed(dev);
>  }
> @@ -444,7 +446,7 @@ static void output_poll_execute(struct work_struct *work)
>  
>  out:
>  	if (changed)
> -		drm_kms_helper_hotplug_event(dev);
> +		drm_kms_helper_hotplug_event(dev, NULL);
>  
>  	if (repoll)
>  		schedule_delayed_work(delayed_work, DRM_OUTPUT_POLL_PERIOD);
> @@ -578,7 +580,7 @@ bool drm_helper_hpd_irq_event(struct drm_device *dev)
>  	mutex_unlock(&dev->mode_config.mutex);
>  
>  	if (changed)
> -		drm_kms_helper_hotplug_event(dev);
> +		drm_kms_helper_hotplug_event(dev, NULL);
>  
>  	return changed;
>  }
> diff --git a/drivers/gpu/drm/drm_sysfs.c b/drivers/gpu/drm/drm_sysfs.c
> index 9a37196c1bf1..c8f46055e2d0 100644
> --- a/drivers/gpu/drm/drm_sysfs.c
> +++ b/drivers/gpu/drm/drm_sysfs.c
> @@ -280,7 +280,7 @@ int drm_sysfs_connector_add(struct drm_connector *connector)
>  	}
>  
>  	/* Let userspace know we have a new connector */
> -	drm_sysfs_hotplug_event(dev);
> +	drm_sysfs_hotplug_event(dev, connector);
>  
>  	return 0;
>  }
> @@ -312,19 +312,30 @@ void drm_sysfs_connector_remove(struct drm_connector *connector)
>  /**
>   * drm_sysfs_hotplug_event - generate a DRM uevent
>   * @dev: DRM device
> + * @connector: the DRM connector, if any
>   *
>   * Send a uevent for the DRM device specified by @dev.  Currently we only
>   * set HOTPLUG=1 in the uevent environment, but this could be expanded to
>   * deal with other types of events.
>   */
> -void drm_sysfs_hotplug_event(struct drm_device *dev)
> +void drm_sysfs_hotplug_event(struct drm_device *dev,
> +			     struct drm_connector *connector)
>  {
> -	char *event_string = "HOTPLUG=1";
> -	char *envp[] = { event_string, NULL };
> +	char *envp[3] = { "HOTPLUG=1" };
> +	char *connector_event = NULL;
> +
> +	if (connector)
> +		connector_event = kasprintf(GFP_KERNEL,
> +					    "CONNECTOR=%d",
> +					    connector->base.id);
> +	if (connector_event)
> +		envp[1] = connector_event;
>  
>  	DRM_DEBUG("generating hotplug event\n");
>  
>  	kobject_uevent_env(&dev->primary->kdev->kobj, KOBJ_CHANGE, envp);
> +
> +	kfree(connector_event);
>  }
>  EXPORT_SYMBOL(drm_sysfs_hotplug_event);
>  
> diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c
> index 9798d400d817..9fd873c87686 100644
> --- a/drivers/gpu/drm/i2c/tda998x_drv.c
> +++ b/drivers/gpu/drm/i2c/tda998x_drv.c
> @@ -617,7 +617,7 @@ static void tda998x_detect_work(struct work_struct *work)
>  	struct drm_device *dev = priv->encoder.dev;
>  
>  	if (dev)
> -		drm_kms_helper_hotplug_event(dev);
> +		drm_kms_helper_hotplug_event(dev, NULL);
>  }
>  
>  /*
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 88f3b745a326..59cd185689c0 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -3981,7 +3981,8 @@ intel_dp_check_mst_status(struct intel_dp *intel_dp)
>  			intel_dp->is_mst = false;
>  			drm_dp_mst_topology_mgr_set_mst(&intel_dp->mst_mgr, intel_dp->is_mst);
>  			/* send a hotplug event */
> -			drm_kms_helper_hotplug_event(intel_dig_port->base.base.dev);
> +			drm_kms_helper_hotplug_event(intel_dig_port->base.base.dev,
> +						     &intel_dp->attached_connector->base);
>  		}
>  	}
>  	return -EINVAL;
> diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c b/drivers/gpu/drm/i915/intel_dp_mst.c
> index 3ffbd69e4551..2bd48a987934 100644
> --- a/drivers/gpu/drm/i915/intel_dp_mst.c
> +++ b/drivers/gpu/drm/i915/intel_dp_mst.c
> @@ -493,7 +493,7 @@ static void intel_dp_mst_hotplug(struct drm_dp_mst_topology_mgr *mgr)
>  	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
>  	struct drm_device *dev = intel_dig_port->base.base.dev;
>  
> -	drm_kms_helper_hotplug_event(dev);
> +	drm_kms_helper_hotplug_event(dev, &intel_dp->attached_connector->base);
>  }
>  
>  static const struct drm_dp_mst_topology_cbs mst_cbs = {
> diff --git a/drivers/gpu/drm/i915/intel_hotplug.c b/drivers/gpu/drm/i915/intel_hotplug.c
> index 334d47b5811a..da0649aff734 100644
> --- a/drivers/gpu/drm/i915/intel_hotplug.c
> +++ b/drivers/gpu/drm/i915/intel_hotplug.c
> @@ -340,7 +340,7 @@ static void i915_hotplug_work_func(struct work_struct *work)
>  	mutex_unlock(&mode_config->mutex);
>  
>  	if (changed)
> -		drm_kms_helper_hotplug_event(dev);
> +		drm_kms_helper_hotplug_event(dev, NULL);
>  }
>  
>  
> diff --git a/drivers/gpu/drm/qxl/qxl_display.c b/drivers/gpu/drm/qxl/qxl_display.c
> index 2e01c6e5d905..3c196a096c2d 100644
> --- a/drivers/gpu/drm/qxl/qxl_display.c
> +++ b/drivers/gpu/drm/qxl/qxl_display.c
> @@ -135,7 +135,7 @@ void qxl_display_read_client_monitors_config(struct qxl_device *qdev)
>  	if (!drm_helper_hpd_irq_event(qdev->ddev)) {
>  		/* notify that the monitor configuration changed, to
>  		   adjust at the arbitrary resolution */
> -		drm_kms_helper_hotplug_event(qdev->ddev);
> +		drm_kms_helper_hotplug_event(qdev->ddev, NULL);
>  	}
>  }
>  
> diff --git a/drivers/gpu/drm/radeon/radeon_dp_mst.c b/drivers/gpu/drm/radeon/radeon_dp_mst.c
> index de504ea29c06..e80b1c365a2d 100644
> --- a/drivers/gpu/drm/radeon/radeon_dp_mst.c
> +++ b/drivers/gpu/drm/radeon/radeon_dp_mst.c
> @@ -331,7 +331,7 @@ static void radeon_dp_mst_hotplug(struct drm_dp_mst_topology_mgr *mgr)
>  	struct radeon_connector *master = container_of(mgr, struct radeon_connector, mst_mgr);
>  	struct drm_device *dev = master->base.dev;
>  
> -	drm_kms_helper_hotplug_event(dev);
> +	drm_kms_helper_hotplug_event(dev, NULL);
>  }
>  
>  const struct drm_dp_mst_topology_cbs mst_cbs = {
> diff --git a/drivers/gpu/drm/virtio/virtgpu_vq.c b/drivers/gpu/drm/virtio/virtgpu_vq.c
> index 5a0f8a745b9d..72be43c9e008 100644
> --- a/drivers/gpu/drm/virtio/virtgpu_vq.c
> +++ b/drivers/gpu/drm/virtio/virtgpu_vq.c
> @@ -583,7 +583,7 @@ static void virtio_gpu_cmd_get_display_info_cb(struct virtio_gpu_device *vgdev,
>  	wake_up(&vgdev->resp_wq);
>  
>  	if (!drm_helper_hpd_irq_event(vgdev->ddev))
> -		drm_kms_helper_hotplug_event(vgdev->ddev);
> +		drm_kms_helper_hotplug_event(vgdev->ddev, NULL);
>  }
>  
>  static void virtio_gpu_cmd_get_capset_info_cb(struct virtio_gpu_device *vgdev,
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
> index e8ae3dc476d1..36aa1a0440c3 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
> @@ -1233,7 +1233,7 @@ static int vmw_master_set(struct drm_device *dev,
>  	}
>  
>  	dev_priv->active_master = vmaster;
> -	drm_sysfs_hotplug_event(dev);
> +	drm_sysfs_hotplug_event(dev, NULL);
>  
>  	return 0;
>  }
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> index c965514b82be..26262763aa5f 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> @@ -1407,7 +1407,7 @@ static int vmw_du_update_layout(struct vmw_private *dev_priv, unsigned num,
>  	}
>  
>  	mutex_unlock(&dev->mode_config.mutex);
> -	drm_sysfs_hotplug_event(dev);
> +	drm_sysfs_hotplug_event(dev, con);
>  
>  	return 0;
>  }
> diff --git a/include/drm/drmP.h b/include/drm/drmP.h
> index 672644031bd5..0a4a4aa61fd3 100644
> --- a/include/drm/drmP.h
> +++ b/include/drm/drmP.h
> @@ -1038,7 +1038,8 @@ extern struct drm_dma_handle *drm_pci_alloc(struct drm_device *dev, size_t size,
>  extern void drm_pci_free(struct drm_device *dev, struct drm_dma_handle * dmah);
>  
>  			       /* sysfs support (drm_sysfs.c) */
> -extern void drm_sysfs_hotplug_event(struct drm_device *dev);
> +extern void drm_sysfs_hotplug_event(struct drm_device *dev,
> +				    struct drm_connector *connector);
>  
>  
>  struct drm_device *drm_dev_alloc(struct drm_driver *driver,
> diff --git a/include/drm/drm_crtc_helper.h b/include/drm/drm_crtc_helper.h
> index 982c299e435a..a8cd620e8a5c 100644
> --- a/include/drm/drm_crtc_helper.h
> +++ b/include/drm/drm_crtc_helper.h
> @@ -69,7 +69,8 @@ extern int drm_helper_probe_single_connector_modes(struct drm_connector
>  extern void drm_kms_helper_poll_init(struct drm_device *dev);
>  extern void drm_kms_helper_poll_fini(struct drm_device *dev);
>  extern bool drm_helper_hpd_irq_event(struct drm_device *dev);
> -extern void drm_kms_helper_hotplug_event(struct drm_device *dev);
> +extern void drm_kms_helper_hotplug_event(struct drm_device *dev,
> +					 struct drm_connector *connector);
>  
>  extern void drm_kms_helper_poll_disable(struct drm_device *dev);
>  extern void drm_kms_helper_poll_enable(struct drm_device *dev);
> -- 
> 2.9.3
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2] drm: Pass along the hotplug connector to the uevent
  2016-10-21 12:45   ` [Intel-gfx] " Daniel Vetter
@ 2016-10-21 13:17     ` Chris Wilson
  2016-10-21 13:42       ` [PATCH] drm: Add timestamp of last modification to GETCONNECTOR ioctl Chris Wilson
  0 siblings, 1 reply; 14+ messages in thread
From: Chris Wilson @ 2016-10-21 13:17 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: intel-gfx, dri-devel

On Fri, Oct 21, 2016 at 02:45:41PM +0200, Daniel Vetter wrote:
> On Fri, Oct 21, 2016 at 10:14:21AM +0100, Chris Wilson wrote:
> > If we know which connector was plugged/unplugged or
> > connected/disconnected, we can pass that information along to userspace
> > inside the uevent to reduce the amount of work userspace has to perform
> > after the event (i.e. instead of looking over all connectors, it can
> > just reprobe the affected one).
> > 
> > v2: Don't convert intel_hotplug.c, it does a light probe and doesn't
> > need the force.
> > 
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Villle Syrjälä <ville.syrjala@linux.intel.com>
> > Cc: Manasi Navare <manasi.d.navare@intel.com>
> 
> Nothing is preventing multiple connectors to change state at the same
> time. Or at least almost the same time, e.g. yank an entire dp mst chain.

Or from sending multiple uevents in the time it takes userspace to respond.
 
> I think we need something that works per-connector, so either send out
> uevents for one that changed individually. Or go with the epoche counter
> idea. The later has the upside that it disconnects probing from reporting:
> Sometimes only deep down in the driver's probe function do we realize that
> something changed (e.g. different edid, or different dpcd). With a helper
> to increment the epoche there would be no need to wire the hotplug status
> through the entire callchain.
> 
> To give us the same speed-up benefits like this here the only thing we'd
> need to do is make sure reading a read-only (i.e. not userspace setable
> prop) doesn't take any heavyweight locks. That should be easy to achieve.
> Of course that leaves us with num_connector ioctl calls, but that should
> be acceptable.
> 
> And it's an uabi change either way.

This is a very simple extension to the current abi that provides a small
additional hint. (And it is purely an optional hint.) I agree it is not
as impactful as being able to categorically detect whether or not connector
state has changed using an epoch counter, but it can provide a meaningful
improvement right now. :)

The simplest way to add the epoch would either be an extension to
GETCONNECTOR or GETPROPBLOB, as there is no way to query a connector
property otherwise (at least not after a cursory double check).
getconnector->pad has never been set, so it would require
GETCONNECTORv2 (just to add a new 64bit field to the struct, the
simplest being a u64 nsec timestamp of last change). A simple
extension either way.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH] drm: Add timestamp of last modification to GETCONNECTOR ioctl
  2016-10-21 13:17     ` Chris Wilson
@ 2016-10-21 13:42       ` Chris Wilson
  2016-10-21 16:55         ` Daniel Vetter
  2016-10-21 17:17         ` Ville Syrjälä
  0 siblings, 2 replies; 14+ messages in thread
From: Chris Wilson @ 2016-10-21 13:42 UTC (permalink / raw)
  To: dri-devel

After hotplug notification, userspace has to reprobe all connectors to
detect any changes. This can be expensive (some outputs may require time
consuming load detection, some outputs may simply be slow to respond)
and not all outputs need to be checked everytime. The kernel is usually
in a very good position to know which outputs need to be reprobed (since
it has just responded to the hardware notification) and can convey this
information to userspace by tracking the timestamp of the last connector
change onto the GETCONNECTOR query.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
Adding an epoch/timestamp field would look something like this. There are
probably a few more places where the hardware indicates a real change
(more than just status).

That update_timestamp() should probably be
	atomic64_set(&timestamp, ktime_get_ns());
---
 drivers/gpu/drm/drm_connector.c             |  4 +++-
 drivers/gpu/drm/drm_crtc.c                  |  4 +++-
 drivers/gpu/drm/drm_probe_helper.c          |  6 +++++-
 drivers/gpu/drm/gma500/mdfld_dsi_dpi.c      |  1 +
 drivers/gpu/drm/gma500/mdfld_dsi_output.c   |  1 +
 drivers/gpu/drm/i915/intel_dp.c             |  4 ++++
 drivers/gpu/drm/i915/intel_dp_mst.c         |  2 ++
 drivers/gpu/drm/i915/intel_hotplug.c        |  5 ++++-
 drivers/gpu/drm/i915/intel_lvds.c           |  1 +
 drivers/gpu/drm/nouveau/nouveau_connector.c |  1 +
 drivers/gpu/drm/vmwgfx/vmwgfx_ldu.c         |  1 +
 drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c        |  1 +
 drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c        |  1 +
 include/drm/drm_connector.h                 |  8 ++++++++
 include/uapi/drm/drm.h                      |  2 +-
 include/uapi/drm/drm_mode.h                 | 26 ++++++++++++++++++++++++++
 16 files changed, 63 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index 2db7fb510b6c..cd3d85e94b91 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -227,6 +227,7 @@ int drm_connector_init(struct drm_device *dev,
 	INIT_LIST_HEAD(&connector->modes);
 	connector->edid_blob_ptr = NULL;
 	connector->status = connector_status_unknown;
+	drm_connector_update_timestamp(connector);
 
 	drm_connector_get_cmdline_mode(connector);
 
@@ -1010,7 +1011,7 @@ static bool drm_mode_expose_to_userspace(const struct drm_display_mode *mode,
 int drm_mode_getconnector(struct drm_device *dev, void *data,
 			  struct drm_file *file_priv)
 {
-	struct drm_mode_get_connector *out_resp = data;
+	struct drm_mode_get_connector_v2 *out_resp = data;
 	struct drm_connector *connector;
 	struct drm_encoder *encoder;
 	struct drm_display_mode *mode;
@@ -1058,6 +1059,7 @@ int drm_mode_getconnector(struct drm_device *dev, void *data,
 	out_resp->mm_height = connector->display_info.height_mm;
 	out_resp->subpixel = connector->display_info.subpixel_order;
 	out_resp->connection = connector->status;
+	out_resp->timestamp = ktime_to_ns(connector->timestamp);
 
 	drm_modeset_lock(&dev->mode_config.connection_mutex, NULL);
 	encoder = drm_connector_get_encoder(connector);
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index 788d6cdc49a0..8668e95bb5b1 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -949,9 +949,11 @@ void drm_mode_config_reset(struct drm_device *dev)
 			encoder->funcs->reset(encoder);
 
 	mutex_lock(&dev->mode_config.mutex);
-	drm_for_each_connector(connector, dev)
+	drm_for_each_connector(connector, dev) {
+		drm_connector_update_timestamp(connector);
 		if (connector->funcs->reset)
 			connector->funcs->reset(connector);
+	}
 	mutex_unlock(&dev->mode_config.mutex);
 }
 EXPORT_SYMBOL(drm_mode_config_reset);
diff --git a/drivers/gpu/drm/drm_probe_helper.c b/drivers/gpu/drm/drm_probe_helper.c
index 8790ee35a7cd..4b6882edc3fb 100644
--- a/drivers/gpu/drm/drm_probe_helper.c
+++ b/drivers/gpu/drm/drm_probe_helper.c
@@ -249,6 +249,7 @@ int drm_helper_probe_single_connector_modes(struct drm_connector *connector,
 	 * check here, and if anything changed start the hotplug code.
 	 */
 	if (old_status != connector->status) {
+		drm_connector_update_timestamp(connector);
 		DRM_DEBUG_KMS("[CONNECTOR:%d:%s] status updated from %s to %s\n",
 			      connector->base.id,
 			      connector->name,
@@ -438,6 +439,7 @@ static void output_poll_execute(struct work_struct *work)
 				      connector->name,
 				      old, new);
 
+			drm_connector_update_timestamp(connector);
 			changed = true;
 		}
 	}
@@ -573,8 +575,10 @@ bool drm_helper_hpd_irq_event(struct drm_device *dev)
 			      connector->name,
 			      drm_get_connector_status_name(old_status),
 			      drm_get_connector_status_name(connector->status));
-		if (old_status != connector->status)
+		if (old_status != connector->status) {
+			drm_connector_update_timestamp(connector);
 			changed = true;
+		}
 	}
 
 	mutex_unlock(&dev->mode_config.mutex);
diff --git a/drivers/gpu/drm/gma500/mdfld_dsi_dpi.c b/drivers/gpu/drm/gma500/mdfld_dsi_dpi.c
index a05c020602bd..8980d7a08c5d 100644
--- a/drivers/gpu/drm/gma500/mdfld_dsi_dpi.c
+++ b/drivers/gpu/drm/gma500/mdfld_dsi_dpi.c
@@ -971,6 +971,7 @@ struct mdfld_dsi_encoder *mdfld_dsi_dpi_init(struct drm_device *dev,
 			DRM_INFO("pipe %d power mode 0x%x\n", pipe, data);
 			dsi_connector->status = connector_status_connected;
 		}
+		drm_connector_update_timestamp(connector);
 	}
 
 	dpi_output = kzalloc(sizeof(struct mdfld_dsi_dpi_output), GFP_KERNEL);
diff --git a/drivers/gpu/drm/gma500/mdfld_dsi_output.c b/drivers/gpu/drm/gma500/mdfld_dsi_output.c
index acb3848ef1c9..f288d9ac9b12 100644
--- a/drivers/gpu/drm/gma500/mdfld_dsi_output.c
+++ b/drivers/gpu/drm/gma500/mdfld_dsi_output.c
@@ -238,6 +238,7 @@ mdfld_dsi_connector_detect(struct drm_connector *connector, bool force)
 		= mdfld_dsi_connector(connector);
 
 	dsi_connector->status = connector_status_connected;
+	drm_connector_update_timestamp(connector);
 
 	return dsi_connector->status;
 }
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 59cd185689c0..56fcc738edc1 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -3980,6 +3980,10 @@ intel_dp_check_mst_status(struct intel_dp *intel_dp)
 			DRM_DEBUG_KMS("failed to get ESI - device may have failed\n");
 			intel_dp->is_mst = false;
 			drm_dp_mst_topology_mgr_set_mst(&intel_dp->mst_mgr, intel_dp->is_mst);
+
+			if (intel_dp->attached_connector)
+				drm_connector_update_timestamp(&intel_dp->attached_connector->base);
+
 			/* send a hotplug event */
 			drm_kms_helper_hotplug_event(intel_dig_port->base.base.dev,
 						     &intel_dp->attached_connector->base);
diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c b/drivers/gpu/drm/i915/intel_dp_mst.c
index 2bd48a987934..d66e576eff83 100644
--- a/drivers/gpu/drm/i915/intel_dp_mst.c
+++ b/drivers/gpu/drm/i915/intel_dp_mst.c
@@ -493,6 +493,8 @@ static void intel_dp_mst_hotplug(struct drm_dp_mst_topology_mgr *mgr)
 	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
 	struct drm_device *dev = intel_dig_port->base.base.dev;
 
+	if (intel_dp->attached_connector)
+		drm_connector_update_timestamp(&intel_dp->attached_connector->base);
 	drm_kms_helper_hotplug_event(dev, &intel_dp->attached_connector->base);
 }
 
diff --git a/drivers/gpu/drm/i915/intel_hotplug.c b/drivers/gpu/drm/i915/intel_hotplug.c
index da0649aff734..f3c2747be183 100644
--- a/drivers/gpu/drm/i915/intel_hotplug.c
+++ b/drivers/gpu/drm/i915/intel_hotplug.c
@@ -238,6 +238,7 @@ static bool intel_hpd_irq_event(struct drm_device *dev,
 	if (old_status == connector->status)
 		return false;
 
+	drm_connector_update_timestamp(connector);
 	DRM_DEBUG_KMS("[CONNECTOR:%d:%s] status updated from %s to %s\n",
 		      connector->base.id,
 		      connector->name,
@@ -333,8 +334,10 @@ static void i915_hotplug_work_func(struct work_struct *work)
 				      connector->name, intel_encoder->hpd_pin);
 			if (intel_encoder->hot_plug)
 				intel_encoder->hot_plug(intel_encoder);
-			if (intel_hpd_irq_event(dev, connector))
+			if (intel_hpd_irq_event(dev, connector)) {
+				drm_connector_update_timestamp(connector);
 				changed = true;
+			}
 		}
 	}
 	mutex_unlock(&mode_config->mutex);
diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
index 199b90c7907a..27f7044300ff 100644
--- a/drivers/gpu/drm/i915/intel_lvds.c
+++ b/drivers/gpu/drm/i915/intel_lvds.c
@@ -545,6 +545,7 @@ static int intel_lid_notify(struct notifier_block *nb, unsigned long val,
 	 * the LID nofication event.
 	 */
 	connector->status = connector->funcs->detect(connector, false);
+	drm_connector_update_timestamp(connector);
 
 	/* Don't force modeset on machines where it causes a GPU lockup */
 	if (dmi_check_system(intel_no_modeset_on_lid))
diff --git a/drivers/gpu/drm/nouveau/nouveau_connector.c b/drivers/gpu/drm/nouveau/nouveau_connector.c
index c1084088f9e4..47ee48489cf3 100644
--- a/drivers/gpu/drm/nouveau/nouveau_connector.c
+++ b/drivers/gpu/drm/nouveau/nouveau_connector.c
@@ -455,6 +455,7 @@ nouveau_connector_force(struct drm_connector *connector)
 		NV_ERROR(drm, "can't find encoder to force %s on!\n",
 			 connector->name);
 		connector->status = connector_status_disconnected;
+		drm_connector_update_timestamp(connector);
 		return;
 	}
 
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_ldu.c b/drivers/gpu/drm/vmwgfx/vmwgfx_ldu.c
index 61a3cce08dfe..4500723cc856 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_ldu.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_ldu.c
@@ -363,6 +363,7 @@ static int vmw_ldu_init(struct vmw_private *dev_priv, unsigned unit)
 	drm_connector_init(dev, connector, &vmw_legacy_connector_funcs,
 			   DRM_MODE_CONNECTOR_VIRTUAL);
 	connector->status = vmw_du_connector_detect(connector, true);
+	drm_connector_update_timestamp(connector);
 
 	drm_encoder_init(dev, encoder, &vmw_legacy_encoder_funcs,
 			 DRM_MODE_ENCODER_VIRTUAL, NULL);
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c b/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c
index 573c7407c32c..acd79e0edf5a 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c
@@ -524,6 +524,7 @@ static int vmw_sou_init(struct vmw_private *dev_priv, unsigned unit)
 	drm_connector_init(dev, connector, &vmw_sou_connector_funcs,
 			   DRM_MODE_CONNECTOR_VIRTUAL);
 	connector->status = vmw_du_connector_detect(connector, true);
+	drm_connector_update_timestamp(connector);
 
 	drm_encoder_init(dev, encoder, &vmw_screen_object_encoder_funcs,
 			 DRM_MODE_ENCODER_VIRTUAL, NULL);
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c b/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
index 01c4d574fa78..3c2b71e605d6 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
@@ -1117,6 +1117,7 @@ static int vmw_stdu_init(struct vmw_private *dev_priv, unsigned unit)
 	drm_connector_init(dev, connector, &vmw_stdu_connector_funcs,
 			   DRM_MODE_CONNECTOR_VIRTUAL);
 	connector->status = vmw_du_connector_detect(connector, false);
+	drm_connector_update_timestamp(connector);
 
 	drm_encoder_init(dev, encoder, &vmw_stdu_encoder_funcs,
 			 DRM_MODE_ENCODER_VIRTUAL, NULL);
diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
index ac9d7d8e0e43..698d86a6af83 100644
--- a/include/drm/drm_connector.h
+++ b/include/drm/drm_connector.h
@@ -25,6 +25,7 @@
 
 #include <linux/list.h>
 #include <linux/ctype.h>
+#include <linux/ktime.h>
 #include <drm/drm_mode_object.h>
 
 #include <uapi/drm/drm_mode.h>
@@ -576,6 +577,7 @@ struct drm_connector {
 	struct list_head modes; /* list of modes on this connector */
 
 	enum drm_connector_status status;
+	ktime_t timestamp;
 
 	/* these are modes added by probing with DDC or the BIOS */
 	struct list_head probed_modes;
@@ -761,6 +763,12 @@ int drm_mode_connector_set_tile_property(struct drm_connector *connector);
 int drm_mode_connector_update_edid_property(struct drm_connector *connector,
 					    const struct edid *edid);
 
+static inline void
+drm_connector_update_timestamp(struct drm_connector *connector)
+{
+	connector->timestamp = ktime_get();
+}
+
 /**
  * drm_for_each_connector - iterate over all connectors
  * @connector: the loop cursor
diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h
index b2c52843bc70..5c5941aedd8c 100644
--- a/include/uapi/drm/drm.h
+++ b/include/uapi/drm/drm.h
@@ -787,7 +787,7 @@ extern "C" {
 #define DRM_IOCTL_MODE_GETGAMMA		DRM_IOWR(0xA4, struct drm_mode_crtc_lut)
 #define DRM_IOCTL_MODE_SETGAMMA		DRM_IOWR(0xA5, struct drm_mode_crtc_lut)
 #define DRM_IOCTL_MODE_GETENCODER	DRM_IOWR(0xA6, struct drm_mode_get_encoder)
-#define DRM_IOCTL_MODE_GETCONNECTOR	DRM_IOWR(0xA7, struct drm_mode_get_connector)
+#define DRM_IOCTL_MODE_GETCONNECTOR	DRM_IOWR(0xA7, struct drm_mode_get_connector_v2)
 #define DRM_IOCTL_MODE_ATTACHMODE	DRM_IOWR(0xA8, struct drm_mode_mode_cmd) /* deprecated (never worked) */
 #define DRM_IOCTL_MODE_DETACHMODE	DRM_IOWR(0xA9, struct drm_mode_mode_cmd) /* deprecated (never worked) */
 
diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
index 084b50a02dc5..d3ecb546918c 100644
--- a/include/uapi/drm/drm_mode.h
+++ b/include/uapi/drm/drm_mode.h
@@ -286,6 +286,32 @@ struct drm_mode_get_connector {
 	__u32 pad;
 };
 
+struct drm_mode_get_connector_v2 {
+
+	__u64 encoders_ptr;
+	__u64 modes_ptr;
+	__u64 props_ptr;
+	__u64 prop_values_ptr;
+
+	__u32 count_modes;
+	__u32 count_props;
+	__u32 count_encoders;
+
+	__u32 encoder_id; /**< Current Encoder */
+	__u32 connector_id; /**< Id */
+	__u32 connector_type;
+	__u32 connector_type_id;
+
+	__u32 connection;
+	__u32 mm_width;  /**< width in millimeters */
+	__u32 mm_height; /**< height in millimeters */
+	__u32 subpixel;
+
+	__u32 pad; /* align to u64 */
+
+	__u64 timestamp;
+};
+
 #define DRM_MODE_PROP_PENDING	(1<<0)
 #define DRM_MODE_PROP_RANGE	(1<<1)
 #define DRM_MODE_PROP_IMMUTABLE	(1<<2)
-- 
2.9.3

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH] drm: Add timestamp of last modification to GETCONNECTOR ioctl
  2016-10-21 13:42       ` [PATCH] drm: Add timestamp of last modification to GETCONNECTOR ioctl Chris Wilson
@ 2016-10-21 16:55         ` Daniel Vetter
  2016-10-21 17:17         ` Ville Syrjälä
  1 sibling, 0 replies; 14+ messages in thread
From: Daniel Vetter @ 2016-10-21 16:55 UTC (permalink / raw)
  To: Chris Wilson; +Cc: dri-devel

On Fri, Oct 21, 2016 at 02:42:12PM +0100, Chris Wilson wrote:
> After hotplug notification, userspace has to reprobe all connectors to
> detect any changes. This can be expensive (some outputs may require time
> consuming load detection, some outputs may simply be slow to respond)
> and not all outputs need to be checked everytime. The kernel is usually
> in a very good position to know which outputs need to be reprobed (since
> it has just responded to the hardware notification) and can convey this
> information to userspace by tracking the timestamp of the last connector
> change onto the GETCONNECTOR query.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
> Adding an epoch/timestamp field would look something like this. There are
> probably a few more places where the hardware indicates a real change
> (more than just status).
> 
> That update_timestamp() should probably be
> 	atomic64_set(&timestamp, ktime_get_ns());

Hm, I thought we'd just add a new epoch property, attach it to every
connector, and userspace would inquire about it using
DRM_IOCTL_MODE_OBJ_GETPROPERTIES.

The only nitpick/polish would be to audit
drm_mode_obj_get_properties_ioctl and figure out how much locking it
really needs (probably almost nothing at all).

> ---
>  drivers/gpu/drm/drm_connector.c             |  4 +++-
>  drivers/gpu/drm/drm_crtc.c                  |  4 +++-
>  drivers/gpu/drm/drm_probe_helper.c          |  6 +++++-
>  drivers/gpu/drm/gma500/mdfld_dsi_dpi.c      |  1 +
>  drivers/gpu/drm/gma500/mdfld_dsi_output.c   |  1 +
>  drivers/gpu/drm/i915/intel_dp.c             |  4 ++++
>  drivers/gpu/drm/i915/intel_dp_mst.c         |  2 ++
>  drivers/gpu/drm/i915/intel_hotplug.c        |  5 ++++-
>  drivers/gpu/drm/i915/intel_lvds.c           |  1 +
>  drivers/gpu/drm/nouveau/nouveau_connector.c |  1 +
>  drivers/gpu/drm/vmwgfx/vmwgfx_ldu.c         |  1 +
>  drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c        |  1 +
>  drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c        |  1 +
>  include/drm/drm_connector.h                 |  8 ++++++++
>  include/uapi/drm/drm.h                      |  2 +-
>  include/uapi/drm/drm_mode.h                 | 26 ++++++++++++++++++++++++++
>  16 files changed, 63 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> index 2db7fb510b6c..cd3d85e94b91 100644
> --- a/drivers/gpu/drm/drm_connector.c
> +++ b/drivers/gpu/drm/drm_connector.c
> @@ -227,6 +227,7 @@ int drm_connector_init(struct drm_device *dev,
>  	INIT_LIST_HEAD(&connector->modes);
>  	connector->edid_blob_ptr = NULL;
>  	connector->status = connector_status_unknown;
> +	drm_connector_update_timestamp(connector);
>  
>  	drm_connector_get_cmdline_mode(connector);
>  
> @@ -1010,7 +1011,7 @@ static bool drm_mode_expose_to_userspace(const struct drm_display_mode *mode,
>  int drm_mode_getconnector(struct drm_device *dev, void *data,
>  			  struct drm_file *file_priv)
>  {
> -	struct drm_mode_get_connector *out_resp = data;
> +	struct drm_mode_get_connector_v2 *out_resp = data;
>  	struct drm_connector *connector;
>  	struct drm_encoder *encoder;
>  	struct drm_display_mode *mode;
> @@ -1058,6 +1059,7 @@ int drm_mode_getconnector(struct drm_device *dev, void *data,
>  	out_resp->mm_height = connector->display_info.height_mm;
>  	out_resp->subpixel = connector->display_info.subpixel_order;
>  	out_resp->connection = connector->status;
> +	out_resp->timestamp = ktime_to_ns(connector->timestamp);
>  
>  	drm_modeset_lock(&dev->mode_config.connection_mutex, NULL);
>  	encoder = drm_connector_get_encoder(connector);
> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> index 788d6cdc49a0..8668e95bb5b1 100644
> --- a/drivers/gpu/drm/drm_crtc.c
> +++ b/drivers/gpu/drm/drm_crtc.c
> @@ -949,9 +949,11 @@ void drm_mode_config_reset(struct drm_device *dev)
>  			encoder->funcs->reset(encoder);
>  
>  	mutex_lock(&dev->mode_config.mutex);
> -	drm_for_each_connector(connector, dev)
> +	drm_for_each_connector(connector, dev) {
> +		drm_connector_update_timestamp(connector);
>  		if (connector->funcs->reset)
>  			connector->funcs->reset(connector);
> +	}
>  	mutex_unlock(&dev->mode_config.mutex);
>  }
>  EXPORT_SYMBOL(drm_mode_config_reset);
> diff --git a/drivers/gpu/drm/drm_probe_helper.c b/drivers/gpu/drm/drm_probe_helper.c
> index 8790ee35a7cd..4b6882edc3fb 100644
> --- a/drivers/gpu/drm/drm_probe_helper.c
> +++ b/drivers/gpu/drm/drm_probe_helper.c
> @@ -249,6 +249,7 @@ int drm_helper_probe_single_connector_modes(struct drm_connector *connector,
>  	 * check here, and if anything changed start the hotplug code.
>  	 */
>  	if (old_status != connector->status) {
> +		drm_connector_update_timestamp(connector);
>  		DRM_DEBUG_KMS("[CONNECTOR:%d:%s] status updated from %s to %s\n",
>  			      connector->base.id,
>  			      connector->name,
> @@ -438,6 +439,7 @@ static void output_poll_execute(struct work_struct *work)
>  				      connector->name,
>  				      old, new);
>  
> +			drm_connector_update_timestamp(connector);
>  			changed = true;
>  		}
>  	}
> @@ -573,8 +575,10 @@ bool drm_helper_hpd_irq_event(struct drm_device *dev)
>  			      connector->name,
>  			      drm_get_connector_status_name(old_status),
>  			      drm_get_connector_status_name(connector->status));
> -		if (old_status != connector->status)
> +		if (old_status != connector->status) {
> +			drm_connector_update_timestamp(connector);
>  			changed = true;
> +		}
>  	}
>  
>  	mutex_unlock(&dev->mode_config.mutex);

I think with the above updates in probe helpers and _init you don't really
need any of the below in either driver init or probe code. Anything in
driver load code isn't needed anyway since with proper load ordering
userspace can't see the connector before it's finalized.

One additional one which we probably want is in the edid update function,
to catch any changes in edid. Plus maybe the same thing in the eld
updater (although that's just derived, but you never know ...).

Of course we'll still need additional calls (in short pulse handlers or
wherever), but that should reduce the need for them a lot.

> diff --git a/drivers/gpu/drm/gma500/mdfld_dsi_dpi.c b/drivers/gpu/drm/gma500/mdfld_dsi_dpi.c
> index a05c020602bd..8980d7a08c5d 100644
> --- a/drivers/gpu/drm/gma500/mdfld_dsi_dpi.c
> +++ b/drivers/gpu/drm/gma500/mdfld_dsi_dpi.c
> @@ -971,6 +971,7 @@ struct mdfld_dsi_encoder *mdfld_dsi_dpi_init(struct drm_device *dev,
>  			DRM_INFO("pipe %d power mode 0x%x\n", pipe, data);
>  			dsi_connector->status = connector_status_connected;
>  		}
> +		drm_connector_update_timestamp(connector);
>  	}
>  
>  	dpi_output = kzalloc(sizeof(struct mdfld_dsi_dpi_output), GFP_KERNEL);
> diff --git a/drivers/gpu/drm/gma500/mdfld_dsi_output.c b/drivers/gpu/drm/gma500/mdfld_dsi_output.c
> index acb3848ef1c9..f288d9ac9b12 100644
> --- a/drivers/gpu/drm/gma500/mdfld_dsi_output.c
> +++ b/drivers/gpu/drm/gma500/mdfld_dsi_output.c
> @@ -238,6 +238,7 @@ mdfld_dsi_connector_detect(struct drm_connector *connector, bool force)
>  		= mdfld_dsi_connector(connector);
>  
>  	dsi_connector->status = connector_status_connected;
> +	drm_connector_update_timestamp(connector);
>  
>  	return dsi_connector->status;
>  }
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 59cd185689c0..56fcc738edc1 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -3980,6 +3980,10 @@ intel_dp_check_mst_status(struct intel_dp *intel_dp)
>  			DRM_DEBUG_KMS("failed to get ESI - device may have failed\n");
>  			intel_dp->is_mst = false;
>  			drm_dp_mst_topology_mgr_set_mst(&intel_dp->mst_mgr, intel_dp->is_mst);
> +
> +			if (intel_dp->attached_connector)
> +				drm_connector_update_timestamp(&intel_dp->attached_connector->base);
> +
>  			/* send a hotplug event */
>  			drm_kms_helper_hotplug_event(intel_dig_port->base.base.dev,
>  						     &intel_dp->attached_connector->base);
> diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c b/drivers/gpu/drm/i915/intel_dp_mst.c
> index 2bd48a987934..d66e576eff83 100644
> --- a/drivers/gpu/drm/i915/intel_dp_mst.c
> +++ b/drivers/gpu/drm/i915/intel_dp_mst.c
> @@ -493,6 +493,8 @@ static void intel_dp_mst_hotplug(struct drm_dp_mst_topology_mgr *mgr)
>  	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
>  	struct drm_device *dev = intel_dig_port->base.base.dev;
>  
> +	if (intel_dp->attached_connector)
> +		drm_connector_update_timestamp(&intel_dp->attached_connector->base);
>  	drm_kms_helper_hotplug_event(dev, &intel_dp->attached_connector->base);
>  }
>  
> diff --git a/drivers/gpu/drm/i915/intel_hotplug.c b/drivers/gpu/drm/i915/intel_hotplug.c
> index da0649aff734..f3c2747be183 100644
> --- a/drivers/gpu/drm/i915/intel_hotplug.c
> +++ b/drivers/gpu/drm/i915/intel_hotplug.c
> @@ -238,6 +238,7 @@ static bool intel_hpd_irq_event(struct drm_device *dev,
>  	if (old_status == connector->status)
>  		return false;
>  
> +	drm_connector_update_timestamp(connector);
>  	DRM_DEBUG_KMS("[CONNECTOR:%d:%s] status updated from %s to %s\n",
>  		      connector->base.id,
>  		      connector->name,
> @@ -333,8 +334,10 @@ static void i915_hotplug_work_func(struct work_struct *work)
>  				      connector->name, intel_encoder->hpd_pin);
>  			if (intel_encoder->hot_plug)
>  				intel_encoder->hot_plug(intel_encoder);
> -			if (intel_hpd_irq_event(dev, connector))
> +			if (intel_hpd_irq_event(dev, connector)) {
> +				drm_connector_update_timestamp(connector);
>  				changed = true;
> +			}
>  		}
>  	}
>  	mutex_unlock(&mode_config->mutex);
> diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
> index 199b90c7907a..27f7044300ff 100644
> --- a/drivers/gpu/drm/i915/intel_lvds.c
> +++ b/drivers/gpu/drm/i915/intel_lvds.c
> @@ -545,6 +545,7 @@ static int intel_lid_notify(struct notifier_block *nb, unsigned long val,
>  	 * the LID nofication event.
>  	 */
>  	connector->status = connector->funcs->detect(connector, false);
> +	drm_connector_update_timestamp(connector);
>  
>  	/* Don't force modeset on machines where it causes a GPU lockup */
>  	if (dmi_check_system(intel_no_modeset_on_lid))
> diff --git a/drivers/gpu/drm/nouveau/nouveau_connector.c b/drivers/gpu/drm/nouveau/nouveau_connector.c
> index c1084088f9e4..47ee48489cf3 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_connector.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_connector.c
> @@ -455,6 +455,7 @@ nouveau_connector_force(struct drm_connector *connector)
>  		NV_ERROR(drm, "can't find encoder to force %s on!\n",
>  			 connector->name);
>  		connector->status = connector_status_disconnected;
> +		drm_connector_update_timestamp(connector);
>  		return;
>  	}
>  
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_ldu.c b/drivers/gpu/drm/vmwgfx/vmwgfx_ldu.c
> index 61a3cce08dfe..4500723cc856 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_ldu.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_ldu.c
> @@ -363,6 +363,7 @@ static int vmw_ldu_init(struct vmw_private *dev_priv, unsigned unit)
>  	drm_connector_init(dev, connector, &vmw_legacy_connector_funcs,
>  			   DRM_MODE_CONNECTOR_VIRTUAL);
>  	connector->status = vmw_du_connector_detect(connector, true);
> +	drm_connector_update_timestamp(connector);
>  
>  	drm_encoder_init(dev, encoder, &vmw_legacy_encoder_funcs,
>  			 DRM_MODE_ENCODER_VIRTUAL, NULL);
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c b/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c
> index 573c7407c32c..acd79e0edf5a 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c
> @@ -524,6 +524,7 @@ static int vmw_sou_init(struct vmw_private *dev_priv, unsigned unit)
>  	drm_connector_init(dev, connector, &vmw_sou_connector_funcs,
>  			   DRM_MODE_CONNECTOR_VIRTUAL);
>  	connector->status = vmw_du_connector_detect(connector, true);
> +	drm_connector_update_timestamp(connector);
>  
>  	drm_encoder_init(dev, encoder, &vmw_screen_object_encoder_funcs,
>  			 DRM_MODE_ENCODER_VIRTUAL, NULL);
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c b/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
> index 01c4d574fa78..3c2b71e605d6 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
> @@ -1117,6 +1117,7 @@ static int vmw_stdu_init(struct vmw_private *dev_priv, unsigned unit)
>  	drm_connector_init(dev, connector, &vmw_stdu_connector_funcs,
>  			   DRM_MODE_CONNECTOR_VIRTUAL);
>  	connector->status = vmw_du_connector_detect(connector, false);
> +	drm_connector_update_timestamp(connector);
>  
>  	drm_encoder_init(dev, encoder, &vmw_stdu_encoder_funcs,
>  			 DRM_MODE_ENCODER_VIRTUAL, NULL);
> diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
> index ac9d7d8e0e43..698d86a6af83 100644
> --- a/include/drm/drm_connector.h
> +++ b/include/drm/drm_connector.h
> @@ -25,6 +25,7 @@
>  
>  #include <linux/list.h>
>  #include <linux/ctype.h>
> +#include <linux/ktime.h>
>  #include <drm/drm_mode_object.h>
>  
>  #include <uapi/drm/drm_mode.h>
> @@ -576,6 +577,7 @@ struct drm_connector {
>  	struct list_head modes; /* list of modes on this connector */
>  
>  	enum drm_connector_status status;
> +	ktime_t timestamp;
>  
>  	/* these are modes added by probing with DDC or the BIOS */
>  	struct list_head probed_modes;
> @@ -761,6 +763,12 @@ int drm_mode_connector_set_tile_property(struct drm_connector *connector);
>  int drm_mode_connector_update_edid_property(struct drm_connector *connector,
>  					    const struct edid *edid);
>  
> +static inline void
> +drm_connector_update_timestamp(struct drm_connector *connector)

Needs some kerneldoc. Otherwise I like.
> +{
> +	connector->timestamp = ktime_get();

Should we make sure that the new value is different from the current one,
and if not, just add 1? We want to guarantee that it's increasing on every
change.

Cheers, Daniel

> +}
> +
>  /**
>   * drm_for_each_connector - iterate over all connectors
>   * @connector: the loop cursor
> diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h
> index b2c52843bc70..5c5941aedd8c 100644
> --- a/include/uapi/drm/drm.h
> +++ b/include/uapi/drm/drm.h
> @@ -787,7 +787,7 @@ extern "C" {
>  #define DRM_IOCTL_MODE_GETGAMMA		DRM_IOWR(0xA4, struct drm_mode_crtc_lut)
>  #define DRM_IOCTL_MODE_SETGAMMA		DRM_IOWR(0xA5, struct drm_mode_crtc_lut)
>  #define DRM_IOCTL_MODE_GETENCODER	DRM_IOWR(0xA6, struct drm_mode_get_encoder)
> -#define DRM_IOCTL_MODE_GETCONNECTOR	DRM_IOWR(0xA7, struct drm_mode_get_connector)
> +#define DRM_IOCTL_MODE_GETCONNECTOR	DRM_IOWR(0xA7, struct drm_mode_get_connector_v2)
>  #define DRM_IOCTL_MODE_ATTACHMODE	DRM_IOWR(0xA8, struct drm_mode_mode_cmd) /* deprecated (never worked) */
>  #define DRM_IOCTL_MODE_DETACHMODE	DRM_IOWR(0xA9, struct drm_mode_mode_cmd) /* deprecated (never worked) */
>  
> diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
> index 084b50a02dc5..d3ecb546918c 100644
> --- a/include/uapi/drm/drm_mode.h
> +++ b/include/uapi/drm/drm_mode.h
> @@ -286,6 +286,32 @@ struct drm_mode_get_connector {
>  	__u32 pad;
>  };
>  
> +struct drm_mode_get_connector_v2 {
> +
> +	__u64 encoders_ptr;
> +	__u64 modes_ptr;
> +	__u64 props_ptr;
> +	__u64 prop_values_ptr;
> +
> +	__u32 count_modes;
> +	__u32 count_props;
> +	__u32 count_encoders;
> +
> +	__u32 encoder_id; /**< Current Encoder */
> +	__u32 connector_id; /**< Id */
> +	__u32 connector_type;
> +	__u32 connector_type_id;
> +
> +	__u32 connection;
> +	__u32 mm_width;  /**< width in millimeters */
> +	__u32 mm_height; /**< height in millimeters */
> +	__u32 subpixel;
> +
> +	__u32 pad; /* align to u64 */
> +
> +	__u64 timestamp;
> +};
> +
>  #define DRM_MODE_PROP_PENDING	(1<<0)
>  #define DRM_MODE_PROP_RANGE	(1<<1)
>  #define DRM_MODE_PROP_IMMUTABLE	(1<<2)
> -- 
> 2.9.3
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] drm: Add timestamp of last modification to GETCONNECTOR ioctl
  2016-10-21 13:42       ` [PATCH] drm: Add timestamp of last modification to GETCONNECTOR ioctl Chris Wilson
  2016-10-21 16:55         ` Daniel Vetter
@ 2016-10-21 17:17         ` Ville Syrjälä
  1 sibling, 0 replies; 14+ messages in thread
From: Ville Syrjälä @ 2016-10-21 17:17 UTC (permalink / raw)
  To: Chris Wilson; +Cc: dri-devel

On Fri, Oct 21, 2016 at 02:42:12PM +0100, Chris Wilson wrote:
> After hotplug notification, userspace has to reprobe all connectors to
> detect any changes. This can be expensive (some outputs may require time
> consuming load detection, some outputs may simply be slow to respond)
> and not all outputs need to be checked everytime. The kernel is usually
> in a very good position to know which outputs need to be reprobed (since
> it has just responded to the hardware notification) and can convey this
> information to userspace by tracking the timestamp of the last connector
> change onto the GETCONNECTOR query.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
> Adding an epoch/timestamp field would look something like this. There are
> probably a few more places where the hardware indicates a real change
> (more than just status).
> 
> That update_timestamp() should probably be
> 	atomic64_set(&timestamp, ktime_get_ns());
> ---
>  drivers/gpu/drm/drm_connector.c             |  4 +++-
>  drivers/gpu/drm/drm_crtc.c                  |  4 +++-
>  drivers/gpu/drm/drm_probe_helper.c          |  6 +++++-
>  drivers/gpu/drm/gma500/mdfld_dsi_dpi.c      |  1 +
>  drivers/gpu/drm/gma500/mdfld_dsi_output.c   |  1 +
>  drivers/gpu/drm/i915/intel_dp.c             |  4 ++++
>  drivers/gpu/drm/i915/intel_dp_mst.c         |  2 ++
>  drivers/gpu/drm/i915/intel_hotplug.c        |  5 ++++-
>  drivers/gpu/drm/i915/intel_lvds.c           |  1 +
>  drivers/gpu/drm/nouveau/nouveau_connector.c |  1 +
>  drivers/gpu/drm/vmwgfx/vmwgfx_ldu.c         |  1 +
>  drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c        |  1 +
>  drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c        |  1 +
>  include/drm/drm_connector.h                 |  8 ++++++++
>  include/uapi/drm/drm.h                      |  2 +-
>  include/uapi/drm/drm_mode.h                 | 26 ++++++++++++++++++++++++++
>  16 files changed, 63 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> index 2db7fb510b6c..cd3d85e94b91 100644
> --- a/drivers/gpu/drm/drm_connector.c
> +++ b/drivers/gpu/drm/drm_connector.c
> @@ -227,6 +227,7 @@ int drm_connector_init(struct drm_device *dev,
>  	INIT_LIST_HEAD(&connector->modes);
>  	connector->edid_blob_ptr = NULL;
>  	connector->status = connector_status_unknown;
> +	drm_connector_update_timestamp(connector);
>  
>  	drm_connector_get_cmdline_mode(connector);
>  
> @@ -1010,7 +1011,7 @@ static bool drm_mode_expose_to_userspace(const struct drm_display_mode *mode,
>  int drm_mode_getconnector(struct drm_device *dev, void *data,
>  			  struct drm_file *file_priv)
>  {
> -	struct drm_mode_get_connector *out_resp = data;
> +	struct drm_mode_get_connector_v2 *out_resp = data;
>  	struct drm_connector *connector;
>  	struct drm_encoder *encoder;
>  	struct drm_display_mode *mode;
> @@ -1058,6 +1059,7 @@ int drm_mode_getconnector(struct drm_device *dev, void *data,
>  	out_resp->mm_height = connector->display_info.height_mm;
>  	out_resp->subpixel = connector->display_info.subpixel_order;
>  	out_resp->connection = connector->status;
> +	out_resp->timestamp = ktime_to_ns(connector->timestamp);

Is there benefit to making it a timestamp as opposed to just an
incrementing counter?

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2016-10-21 17:24 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-10-21  8:25 [PATCH] drm: Pass along the hotplug connector to the uevent Chris Wilson
2016-10-21  8:58 ` ✗ Fi.CI.BAT: failure for " Patchwork
2016-10-21  9:01 ` Patchwork
2016-10-21  9:13   ` Chris Wilson
2016-10-21  9:14 ` [PATCH v2] " Chris Wilson
2016-10-21  9:27   ` Jani Nikula
2016-10-21  9:46   ` Ville Syrjälä
2016-10-21 10:05     ` Chris Wilson
2016-10-21 12:45   ` [Intel-gfx] " Daniel Vetter
2016-10-21 13:17     ` Chris Wilson
2016-10-21 13:42       ` [PATCH] drm: Add timestamp of last modification to GETCONNECTOR ioctl Chris Wilson
2016-10-21 16:55         ` Daniel Vetter
2016-10-21 17:17         ` Ville Syrjälä
2016-10-21 10:18 ` ✗ Fi.CI.BAT: warning for drm: Pass along the hotplug connector to the uevent (rev2) Patchwork

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.