[RFC] [PATCH 00/14] HPD/connector-polling rework

[RFC] [PATCH 00/14] HPD/connector-polling rework

[Daniel Vetter]

Hi all,

I've got fed up with our sorry state of connector detection and rampant edid re
and rere-reading.

This patch series lays the groundwork in the drm helpers so that drivers can
avoid all this madness (at least on working hw) and properly cache the edid.

With the additional changes for drm/i915, the edid is now read _once_ per plug
event (or at boot-up/resume time). A further step would be to integrate the
hotplug handling into the driver itself and only call ->detect on the connectors
for which the irq handler received a hotplug event.

By adding POLL_FORCE drivers can get back the old behaviour of calling ->detect
every time probe_single_connector is called from userspace. I've splattered that
over all drivers where I've thought it might be required.

Note though that setting this doesn't avoid all regressions - the regular output
poll work will still ignore any connectors with POLL_HPD set. If a driver/hw
with broken hpd got away due to this, this will break stuff. But that should be
easy to fix by admitting the defeat and setting POLL_CONNECT|DISCONNECT
directly.

If other people want to convert over their drivers, the following steps are
required:
- Ensure that the connector status is unknown every time the driver could have
  missed a hpd event (e.g. after resume). drm_mode_config_reset will do that for
  you.
- Drop the POLL_FORCE flag for connectors where hdp is fully reliable.
- Implement edid caching - that's a nice way to figure out whether hpd is
  actually reliable, because if it isn't, this step will ensure that you get bug
  reports because the the edid won't ever get updated ;-)
- Optionally teach the driver some smarts about which specific connectors
  actually got a hotplug event. Mostly useful on cheap hw (like intel's) that
  can't distinguish between hdmi and dp without trying some aux channel
  transfers.

As you can guess from the patch series, I've discovered the hard way that i915
sdvo support is totally broken. Tested on most of the intel machines I have and
also quickly on my radeon hd5000.

Comments, flames, ideas and test reports highly welcome.

Cheers, Daniel

Daniel Vetter (14):
  drm: extract drm_kms_helper_hotplug_event
  drm: handle HDP and polled connectors separately
  drm: introduce DRM_CONNECTOR_POLL_FORCE
  drm/i915: set POLL_FORCE for sdvo outputs
  drm: properly init/reset connector status
  drm: kill unnecessary calls to connector->detect
  drm: don't start the poll engine in probe_single_connector
  drm: don't unnecessarily enable the polling work
  drm: don't poll forced connectors
  drm/i915: cache crt edid
  drm/i915: cache dp edid
  drm/i915: cache hdmi edid
  drm/i915/sdvo: implement correct return value for ->get_modes
  drm/i915: s/mdelay/msleep/ in the sdvo detect function

 drivers/gpu/drm/drm_crtc.c                    |    6 ++-
 drivers/gpu/drm/drm_crtc_helper.c             |   76 ++++++++++++++++++-------
 drivers/gpu/drm/exynos/exynos_drm_connector.c |    2 +
 drivers/gpu/drm/gma500/cdv_intel_crt.c        |    2 +
 drivers/gpu/drm/gma500/cdv_intel_hdmi.c       |    2 +
 drivers/gpu/drm/gma500/mdfld_dsi_output.c     |    1 +
 drivers/gpu/drm/gma500/oaktrail_hdmi.c        |    2 +
 drivers/gpu/drm/gma500/psb_intel_sdvo.c       |    2 +
 drivers/gpu/drm/i915/intel_crt.c              |   28 +++++++--
 drivers/gpu/drm/i915/intel_dp.c               |   47 +++++++--------
 drivers/gpu/drm/i915/intel_drv.h              |    2 +
 drivers/gpu/drm/i915/intel_hdmi.c             |   48 ++++++++++------
 drivers/gpu/drm/i915/intel_modes.c            |   18 +++++-
 drivers/gpu/drm/i915/intel_sdvo.c             |   45 +++++++++------
 drivers/gpu/drm/i915/intel_tv.c               |    3 +-
 drivers/gpu/drm/nouveau/nouveau_connector.c   |    1 +
 drivers/gpu/drm/radeon/radeon_connectors.c    |    5 ++
 drivers/gpu/drm/udl/udl_connector.c           |    2 +
 drivers/staging/omapdrm/omap_connector.c      |    2 +
 include/drm/drm_crtc.h                        |    5 ++
 include/drm/drm_crtc_helper.h                 |    1 +
 21 files changed, 208 insertions(+), 92 deletions(-)

-- 
1.7.7.6

[PATCH 01/14] drm: extract drm_kms_helper_hotplug_event

[Daniel Vetter]

Useful if drivers want to be slightly more clever about hotplug
handling.

Signed-Off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/drm_crtc_helper.c |   17 +++++++++++------
 include/drm/drm_crtc_helper.h     |    1 +
 2 files changed, 12 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/drm_crtc_helper.c b/drivers/gpu/drm/drm_crtc_helper.c
index 974196a..909a85c 100644
--- a/drivers/gpu/drm/drm_crtc_helper.c
+++ b/drivers/gpu/drm/drm_crtc_helper.c
@@ -918,6 +918,15 @@ int drm_helper_resume_force_mode(struct drm_device *dev)
 }
 EXPORT_SYMBOL(drm_helper_resume_force_mode);
 
+void drm_kms_helper_hotplug_event(struct drm_device *dev)
+{
+	/* send a uevent + call fbdev */
+	drm_sysfs_hotplug_event(dev);
+	if (dev->mode_config.funcs->output_poll_changed)
+		dev->mode_config.funcs->output_poll_changed(dev);
+}
+EXPORT_SYMBOL(drm_kms_helper_hotplug_event);
+
 #define DRM_OUTPUT_POLL_PERIOD (10*HZ)
 static void output_poll_execute(struct work_struct *work)
 {
@@ -960,12 +969,8 @@ static void output_poll_execute(struct work_struct *work)
 
 	mutex_unlock(&dev->mode_config.mutex);
 
-	if (changed) {
-		/* send a uevent + call fbdev */
-		drm_sysfs_hotplug_event(dev);
-		if (dev->mode_config.funcs->output_poll_changed)
-			dev->mode_config.funcs->output_poll_changed(dev);
-	}
+	if (changed)
+		drm_kms_helper_hotplug_event(dev);
 
 	if (repoll)
 		queue_delayed_work(system_nrt_wq, delayed_work, DRM_OUTPUT_POLL_PERIOD);
diff --git a/include/drm/drm_crtc_helper.h b/include/drm/drm_crtc_helper.h
index 3add00e..9a9288d 100644
--- a/include/drm/drm_crtc_helper.h
+++ b/include/drm/drm_crtc_helper.h
@@ -141,6 +141,7 @@ extern int drm_helper_resume_force_mode(struct drm_device *dev);
 extern void drm_kms_helper_poll_init(struct drm_device *dev);
 extern void drm_kms_helper_poll_fini(struct drm_device *dev);
 extern void 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_poll_disable(struct drm_device *dev);
 extern void drm_kms_helper_poll_enable(struct drm_device *dev);
-- 
1.7.7.6

[PATCH 02/14] drm: handle HDP and polled connectors separately

[Daniel Vetter]

Instead of reusing the polling code for hdp handling, split them up.
This has a few consequences:
- Don't touch HDP capable connectors in the poll loop.
- Only touch HDP capable connectors in drm_helper_hpd_irq_event.
- Run the HDP handling directly instead of going through a work item -
  all callers already call drm_helper_hpd_irq_event from process
  context without holding the mode_config mutex.

The ultimate goal is that drivers grow some smarts about which
connectors have received a hotplug event and only call the detect code
of that connector. But that's a second step.

Signed-Off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/drm_crtc_helper.c |   39 ++++++++++++++++++++++++++++--------
 1 files changed, 30 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/drm_crtc_helper.c b/drivers/gpu/drm/drm_crtc_helper.c
index 909a85c..b1d643d 100644
--- a/drivers/gpu/drm/drm_crtc_helper.c
+++ b/drivers/gpu/drm/drm_crtc_helper.c
@@ -942,9 +942,9 @@ static void output_poll_execute(struct work_struct *work)
 	mutex_lock(&dev->mode_config.mutex);
 	list_for_each_entry(connector, &dev->mode_config.connector_list, head) {
 
-		/* if this is HPD or polled don't check it -
-		   TV out for instance */
-		if (!connector->polled)
+		/* Ignore HDP capable connectors and connectors where we don't
+		 * want any hotplug detection at all for polling. */
+		if (!connector->polled || connector->polled == DRM_CONNECTOR_POLL_HPD)
 			continue;
 
 		else if (connector->polled & (DRM_CONNECTOR_POLL_CONNECT | DRM_CONNECTOR_POLL_DISCONNECT))
@@ -954,8 +954,7 @@ static void output_poll_execute(struct work_struct *work)
 		/* if we are connected and don't want to poll for disconnect
 		   skip it */
 		if (old_status == connector_status_connected &&
-		    !(connector->polled & DRM_CONNECTOR_POLL_DISCONNECT) &&
-		    !(connector->polled & DRM_CONNECTOR_POLL_HPD))
+		    !(connector->polled & DRM_CONNECTOR_POLL_DISCONNECT))
 			continue;
 
 		connector->status = connector->funcs->detect(connector, false);
@@ -1019,12 +1018,34 @@ EXPORT_SYMBOL(drm_kms_helper_poll_fini);
 
 void drm_helper_hpd_irq_event(struct drm_device *dev)
 {
+	struct drm_connector *connector;
+	enum drm_connector_status old_status;
+	bool changed = false;
+
 	if (!dev->mode_config.poll_enabled)
 		return;
 
-	/* kill timer and schedule immediate execution, this doesn't block */
-	cancel_delayed_work(&dev->mode_config.output_poll_work);
-	if (drm_kms_helper_poll)
-		queue_delayed_work(system_nrt_wq, &dev->mode_config.output_poll_work, 0);
+	mutex_lock(&dev->mode_config.mutex);
+	list_for_each_entry(connector, &dev->mode_config.connector_list, head) {
+
+		/* Only handle HPD capable connectors. */
+		if (connector->polled != DRM_CONNECTOR_POLL_HPD)
+			continue;
+
+		old_status = connector->status;
+
+		connector->status = connector->funcs->detect(connector, false);
+		DRM_DEBUG_KMS("[CONNECTOR:%d:%s] status updated from %d to %d\n",
+			      connector->base.id,
+			      drm_get_connector_name(connector),
+			      old_status, connector->status);
+		if (old_status != connector->status)
+			changed = true;
+	}
+
+	mutex_unlock(&dev->mode_config.mutex);
+
+	if (changed)
+		drm_kms_helper_hotplug_event(dev);
 }
 EXPORT_SYMBOL(drm_helper_hpd_irq_event);
-- 
1.7.7.6

[PATCH 03/14] drm: introduce DRM_CONNECTOR_POLL_FORCE

[Daniel Vetter]

Useful for ->detect functions that have different behaviour if force
is set. This way probe_single_connector can avoid to do the expensive
edid dance on connectors where this is not needed.

I've checked through all drivers and set this flag everywhere where
the connector->detect function has different behaviour if force is
set. For nouveau and radeon I've got lost in the code traces, so I've
set this flag unconditionally.

Note that we also need to update the poll_execute function to now
also ignore connectors which have only this new flag set.

v2: Change POLL_HDP checks so that they ignore POLL_FORCE for both the
poll and the hpd handling code.

v3: Sprinkle POLL_FORCE more liberally over drivers. It should be now
everywhere where a non-intel driver can return anything else than
connector_status_connected in its detect callback.

Signed-Off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/drm_crtc_helper.c             |    6 ++++--
 drivers/gpu/drm/exynos/exynos_drm_connector.c |    2 ++
 drivers/gpu/drm/gma500/cdv_intel_crt.c        |    2 ++
 drivers/gpu/drm/gma500/cdv_intel_hdmi.c       |    2 ++
 drivers/gpu/drm/gma500/mdfld_dsi_output.c     |    1 +
 drivers/gpu/drm/gma500/oaktrail_hdmi.c        |    2 ++
 drivers/gpu/drm/gma500/psb_intel_sdvo.c       |    2 ++
 drivers/gpu/drm/i915/intel_crt.c              |    3 ++-
 drivers/gpu/drm/i915/intel_tv.c               |    3 ++-
 drivers/gpu/drm/nouveau/nouveau_connector.c   |    1 +
 drivers/gpu/drm/radeon/radeon_connectors.c    |    5 +++++
 drivers/gpu/drm/udl/udl_connector.c           |    2 ++
 drivers/staging/omapdrm/omap_connector.c      |    2 ++
 include/drm/drm_crtc.h                        |    4 ++++
 14 files changed, 33 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/drm_crtc_helper.c b/drivers/gpu/drm/drm_crtc_helper.c
index b1d643d..8ea1c1e 100644
--- a/drivers/gpu/drm/drm_crtc_helper.c
+++ b/drivers/gpu/drm/drm_crtc_helper.c
@@ -944,7 +944,9 @@ static void output_poll_execute(struct work_struct *work)
 
 		/* Ignore HDP capable connectors and connectors where we don't
 		 * want any hotplug detection at all for polling. */
-		if (!connector->polled || connector->polled == DRM_CONNECTOR_POLL_HPD)
+		if (!connector->polled ||
+		    connector->polled == DRM_CONNECTOR_POLL_FORCE ||
+		    (connector->polled & DRM_CONNECTOR_POLL_HPD))
 			continue;
 
 		else if (connector->polled & (DRM_CONNECTOR_POLL_CONNECT | DRM_CONNECTOR_POLL_DISCONNECT))
@@ -1029,7 +1031,7 @@ void drm_helper_hpd_irq_event(struct drm_device *dev)
 	list_for_each_entry(connector, &dev->mode_config.connector_list, head) {
 
 		/* Only handle HPD capable connectors. */
-		if (connector->polled != DRM_CONNECTOR_POLL_HPD)
+		if (!(connector->polled & DRM_CONNECTOR_POLL_HPD))
 			continue;
 
 		old_status = connector->status;
diff --git a/drivers/gpu/drm/exynos/exynos_drm_connector.c b/drivers/gpu/drm/exynos/exynos_drm_connector.c
index bf791fa..e5a8a27 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_connector.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_connector.c
@@ -327,6 +327,8 @@ struct drm_connector *exynos_drm_connector_create(struct drm_device *dev,
 	drm_connector_init(dev, connector, &exynos_connector_funcs, type);
 	drm_connector_helper_add(connector, &exynos_connector_helper_funcs);
 
+	connector->polled |= DRM_CONNECTOR_POLL_FORCE;
+
 	err = drm_sysfs_connector_add(connector);
 	if (err)
 		goto err_connector;
diff --git a/drivers/gpu/drm/gma500/cdv_intel_crt.c b/drivers/gpu/drm/gma500/cdv_intel_crt.c
index 1874220..e6b2e49 100644
--- a/drivers/gpu/drm/gma500/cdv_intel_crt.c
+++ b/drivers/gpu/drm/gma500/cdv_intel_crt.c
@@ -313,6 +313,8 @@ void cdv_intel_crt_init(struct drm_device *dev,
 	drm_connector_helper_add(connector,
 					&cdv_intel_crt_connector_helper_funcs);
 
+	connector->polled |= DRM_CONNECTOR_POLL_FORCE;
+
 	drm_sysfs_connector_add(connector);
 
 	return;
diff --git a/drivers/gpu/drm/gma500/cdv_intel_hdmi.c b/drivers/gpu/drm/gma500/cdv_intel_hdmi.c
index 88b59d4..766aec8 100644
--- a/drivers/gpu/drm/gma500/cdv_intel_hdmi.c
+++ b/drivers/gpu/drm/gma500/cdv_intel_hdmi.c
@@ -373,6 +373,8 @@ void cdv_hdmi_init(struct drm_device *dev,
 	hdmi_priv->hdmi_i2c_adapter =
 				&(psb_intel_encoder->i2c_bus->adapter);
 	hdmi_priv->dev = dev;
+	connector->polled |= DRM_CONNECTOR_POLL_FORCE;
+
 	drm_sysfs_connector_add(connector);
 	return;
 
diff --git a/drivers/gpu/drm/gma500/mdfld_dsi_output.c b/drivers/gpu/drm/gma500/mdfld_dsi_output.c
index 5675d93..0e97a91 100644
--- a/drivers/gpu/drm/gma500/mdfld_dsi_output.c
+++ b/drivers/gpu/drm/gma500/mdfld_dsi_output.c
@@ -604,6 +604,7 @@ void mdfld_dsi_output_init(struct drm_device *dev,
 	dsi_config->encoder = encoder;
 	encoder->base.type = (pipe == 0) ? INTEL_OUTPUT_MIPI :
 		INTEL_OUTPUT_MIPI2;
+	connector->polled |= DRM_CONNECTOR_POLL_FORCE;
 	drm_sysfs_connector_add(connector);
 	return;
 
diff --git a/drivers/gpu/drm/gma500/oaktrail_hdmi.c b/drivers/gpu/drm/gma500/oaktrail_hdmi.c
index c10899c..23b8d52 100644
--- a/drivers/gpu/drm/gma500/oaktrail_hdmi.c
+++ b/drivers/gpu/drm/gma500/oaktrail_hdmi.c
@@ -349,6 +349,8 @@ void oaktrail_hdmi_init(struct drm_device *dev,
 	connector->display_info.subpixel_order = SubPixelHorizontalRGB;
 	connector->interlace_allowed = false;
 	connector->doublescan_allowed = false;
+	connector->polled |= DRM_CONNECTOR_POLL_FORCE;
+
 	drm_sysfs_connector_add(connector);
 
 	return;
diff --git a/drivers/gpu/drm/gma500/psb_intel_sdvo.c b/drivers/gpu/drm/gma500/psb_intel_sdvo.c
index d39b15b..e86d8ba 100644
--- a/drivers/gpu/drm/gma500/psb_intel_sdvo.c
+++ b/drivers/gpu/drm/gma500/psb_intel_sdvo.c
@@ -2028,6 +2028,8 @@ psb_intel_sdvo_connector_init(struct psb_intel_sdvo_connector *connector,
 	connector->base.base.display_info.subpixel_order = SubPixelHorizontalRGB;
 
 	psb_intel_connector_attach_encoder(&connector->base, &encoder->base);
+	connector->polled |= DRM_CONNECTOR_POLL_FORCE;
+
 	drm_sysfs_connector_add(&connector->base.base);
 }
 
diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c
index 75a70c4..a60d131 100644
--- a/drivers/gpu/drm/i915/intel_crt.c
+++ b/drivers/gpu/drm/i915/intel_crt.c
@@ -639,7 +639,8 @@ void intel_crt_init(struct drm_device *dev)
 	if (I915_HAS_HOTPLUG(dev))
 		connector->polled = DRM_CONNECTOR_POLL_HPD;
 	else
-		connector->polled = DRM_CONNECTOR_POLL_CONNECT;
+		connector->polled = DRM_CONNECTOR_POLL_CONNECT |
+				    DRM_CONNECTOR_POLL_FORCE;
 
 	/*
 	 * Configure the automatic hotplug detection stuff
diff --git a/drivers/gpu/drm/i915/intel_tv.c b/drivers/gpu/drm/i915/intel_tv.c
index a233a51..dad7d8e 100644
--- a/drivers/gpu/drm/i915/intel_tv.c
+++ b/drivers/gpu/drm/i915/intel_tv.c
@@ -1614,7 +1614,8 @@ intel_tv_init(struct drm_device *dev)
 	 *
 	 * More recent chipsets favour HDMI rather than integrated S-Video.
 	 */
-	connector->polled = DRM_CONNECTOR_POLL_CONNECT;
+	connector->polled = DRM_CONNECTOR_POLL_CONNECT |
+			    DRM_CONNECTOR_POLL_FORCE;
 
 	drm_connector_init(dev, connector, &intel_tv_connector_funcs,
 			   DRM_MODE_CONNECTOR_SVIDEO);
diff --git a/drivers/gpu/drm/nouveau/nouveau_connector.c b/drivers/gpu/drm/nouveau/nouveau_connector.c
index fa86035..780d608 100644
--- a/drivers/gpu/drm/nouveau/nouveau_connector.c
+++ b/drivers/gpu/drm/nouveau/nouveau_connector.c
@@ -1085,6 +1085,7 @@ nouveau_connector_create(struct drm_device *dev, int index)
 		if (ret == 0)
 			connector->polled = DRM_CONNECTOR_POLL_HPD;
 	}
+	connector->polled |= DRM_CONNECTOR_POLL_FORCE;
 
 	drm_sysfs_connector_add(connector);
 	return connector;
diff --git a/drivers/gpu/drm/radeon/radeon_connectors.c b/drivers/gpu/drm/radeon/radeon_connectors.c
index 2914c57..c64ecc9 100644
--- a/drivers/gpu/drm/radeon/radeon_connectors.c
+++ b/drivers/gpu/drm/radeon/radeon_connectors.c
@@ -1830,6 +1830,8 @@ radeon_add_atom_connector(struct drm_device *dev,
 	} else
 		connector->polled = DRM_CONNECTOR_POLL_HPD;
 
+	connector->polled |= DRM_CONNECTOR_POLL_FORCE;
+
 	connector->display_info.subpixel_order = subpixel_order;
 	drm_sysfs_connector_add(connector);
 	return;
@@ -1987,6 +1989,9 @@ radeon_add_legacy_connector(struct drm_device *dev,
 			connector->polled = DRM_CONNECTOR_POLL_CONNECT;
 	} else
 		connector->polled = DRM_CONNECTOR_POLL_HPD;
+
+	connector->polled |= DRM_CONNECTOR_POLL_FORCE;
+
 	connector->display_info.subpixel_order = subpixel_order;
 	drm_sysfs_connector_add(connector);
 	if (connector_type == DRM_MODE_CONNECTOR_LVDS) {
diff --git a/drivers/gpu/drm/udl/udl_connector.c b/drivers/gpu/drm/udl/udl_connector.c
index ba055e9..838dc71 100644
--- a/drivers/gpu/drm/udl/udl_connector.c
+++ b/drivers/gpu/drm/udl/udl_connector.c
@@ -131,6 +131,8 @@ int udl_connector_init(struct drm_device *dev, struct drm_encoder *encoder)
 	drm_connector_init(dev, connector, &udl_connector_funcs, DRM_MODE_CONNECTOR_DVII);
 	drm_connector_helper_add(connector, &udl_connector_helper_funcs);
 
+	connector->polled |= DRM_CONNECTOR_POLL_FORCE;
+
 	drm_sysfs_connector_add(connector);
 	drm_mode_connector_attach_encoder(connector, encoder);
 
diff --git a/drivers/staging/omapdrm/omap_connector.c b/drivers/staging/omapdrm/omap_connector.c
index 5e2856c..bb8b134 100644
--- a/drivers/staging/omapdrm/omap_connector.c
+++ b/drivers/staging/omapdrm/omap_connector.c
@@ -358,6 +358,8 @@ struct drm_connector *omap_connector_init(struct drm_device *dev,
 	connector->interlace_allowed = 1;
 	connector->doublescan_allowed = 0;
 
+	connector->polled |= DRM_CONNECTOR_POLL_FORCE;
+
 	drm_sysfs_connector_add(connector);
 
 	return connector;
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index d59bb7d..4417da2 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -514,6 +514,10 @@ enum drm_connector_force {
 /* can cleanly poll for disconnections without flickering the screen */
 /* DACs should rarely do this without a lot of testing */
 #define DRM_CONNECTOR_POLL_DISCONNECT (1 << 2)
+/* connector's ->detect function needs to be called when userspace forces
+ * detection. Should only be used if the detect function has special handling
+ * when force is set to avoid spurious re-reading of the edid. */
+#define DRM_CONNECTOR_POLL_FORCE (1 << 3)
 
 #define MAX_ELD_BYTES	128
 
-- 
1.7.7.6

[PATCH 04/14] drm/i915: set POLL_FORCE for sdvo outputs

[Daniel Vetter]

The detection function is simply too unreliable - it doesn't
properly pick up changes right away.

For now, just set the compat flag and ignore this, because on a
quick look fixing this properly is a very big fish to fry.

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/intel_sdvo.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
index a658207..fdc0574 100644
--- a/drivers/gpu/drm/i915/intel_sdvo.c
+++ b/drivers/gpu/drm/i915/intel_sdvo.c
@@ -2012,6 +2012,8 @@ intel_sdvo_connector_init(struct intel_sdvo_connector *connector,
 	connector->base.base.doublescan_allowed = 0;
 	connector->base.base.display_info.subpixel_order = SubPixelHorizontalRGB;
 
+	connector->base.base.polled |= DRM_CONNECTOR_POLL_FORCE;
+
 	intel_connector_attach_encoder(&connector->base, &encoder->base);
 	drm_sysfs_connector_add(&connector->base.base);
 }
-- 
1.7.7.6

[PATCH 05/14] drm: properly init/reset connector status

[Daniel Vetter]

We need this because otherwise the improved connector code has no idea
when it needs to run the ->detect callback after boot/resume on all
connectors.

Because drm/i915 is the only driver that properly calls
mode_config_reset at resume time, this will horribly blow up
everywhere else.

Signed-Off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/drm_crtc.c |    6 +++++-
 1 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index a177d0a..82eaff0 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -495,6 +495,7 @@ int drm_connector_init(struct drm_device *dev,
 	INIT_LIST_HEAD(&connector->probed_modes);
 	INIT_LIST_HEAD(&connector->modes);
 	connector->edid_blob_ptr = NULL;
+	connector->status = connector_status_unknown;
 
 	list_add_tail(&connector->head, &dev->mode_config.connector_list);
 	dev->mode_config.num_connector++;
@@ -3521,9 +3522,12 @@ void drm_mode_config_reset(struct drm_device *dev)
 		if (encoder->funcs->reset)
 			encoder->funcs->reset(encoder);
 
-	list_for_each_entry(connector, &dev->mode_config.connector_list, head)
+	list_for_each_entry(connector, &dev->mode_config.connector_list, head) {
+		connector->status = connector_status_unknown;
+
 		if (connector->funcs->reset)
 			connector->funcs->reset(connector);
+	}
 }
 EXPORT_SYMBOL(drm_mode_config_reset);
 
-- 
1.7.7.6

[PATCH 06/14] drm: kill unnecessary calls to connector->detect

[Daniel Vetter]

Only call that function if something has actually changed (i.e. in the
output polling or hdp handling functions) or when userspace asks for
the information and DRM_CONNECTOR_POLL_FORCE is set.

Let's see how many bugs this uncovers.

v2: Run ->detect if the current connector status is 'unknown' -
otherwise we won't ever detect the boot-up/resume state correctly.

Signed-Off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/drm_crtc_helper.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/drivers/gpu/drm/drm_crtc_helper.c b/drivers/gpu/drm/drm_crtc_helper.c
index 8ea1c1e..db93e4d 100644
--- a/drivers/gpu/drm/drm_crtc_helper.c
+++ b/drivers/gpu/drm/drm_crtc_helper.c
@@ -107,7 +107,8 @@ int drm_helper_probe_single_connector_modes(struct drm_connector *connector,
 			connector->status = connector_status_disconnected;
 		if (connector->funcs->force)
 			connector->funcs->force(connector);
-	} else {
+	} else if (connector->polled & DRM_CONNECTOR_POLL_FORCE ||
+		   connector->status == connector_status_unknown) {
 		connector->status = connector->funcs->detect(connector, true);
 		drm_kms_helper_poll_enable(dev);
 	}
-- 
1.7.7.6

[PATCH 07/14] drm: don't start the poll engine in probe_single_connector

[Daniel Vetter]

Actually there's a reason this stuff is there, and it's called

commit e58f637bb96d5a0ae0919b9998b891d1ba7e47c9
Author: Chris Wilson <chris@chris-wilson.co.uk>
Date:   Fri Aug 20 09:13:36 2010 +0100

    drm/kms: Add a module parameter to disable polling

The idea has been that users can enable/disable polling at runtime. So
the quick hack has been to just re-enable the output polling if xrandr
asks for the latest state of the connectors.

The problem with that hack is that when we force connectors to another
state than what would be detected, we nicely ping-pong:
- Userspace calls probe, gets the forced state, but polling starts
  again.
- Polling notices that the state is actually different, wakes up
  userspace.
- Repeat.

As that commit already explains, the right fix would be to make the
locking more fine-grained, so that hotplug detection on one output
does not interfere with cursor updates on another crtc.

But that is way too much work. So let's just safe this gross hack by
caching the last-seen state of drm_kms_helper_poll for that driver,
and only fire up the poll engine again if it changed from off to on.

v2: Fixup the edge detection of drm_kms_helper_poll.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=49907
Tested-by: Tvrtko Ursulin <tvrtko.ursulin@onelan.co.uk>
Signed-Off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/drm_crtc_helper.c |    7 ++++++-
 include/drm/drm_crtc.h            |    1 +
 2 files changed, 7 insertions(+), 1 deletions(-)

diff --git a/drivers/gpu/drm/drm_crtc_helper.c b/drivers/gpu/drm/drm_crtc_helper.c
index db93e4d..f17953e 100644
--- a/drivers/gpu/drm/drm_crtc_helper.c
+++ b/drivers/gpu/drm/drm_crtc_helper.c
@@ -110,9 +110,14 @@ int drm_helper_probe_single_connector_modes(struct drm_connector *connector,
 	} else if (connector->polled & DRM_CONNECTOR_POLL_FORCE ||
 		   connector->status == connector_status_unknown) {
 		connector->status = connector->funcs->detect(connector, true);
-		drm_kms_helper_poll_enable(dev);
 	}
 
+	/* Re-enable polling in case the global poll config changed. */
+	if (drm_kms_helper_poll != dev->mode_config.poll_running)
+		drm_kms_helper_poll_enable(dev);
+
+	dev->mode_config.poll_running = drm_kms_helper_poll;
+
 	if (connector->status == connector_status_disconnected) {
 		DRM_DEBUG_KMS("[CONNECTOR:%d:%s] disconnected\n",
 			connector->base.id, drm_get_connector_name(connector));
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index 4417da2..fb21121 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -783,6 +783,7 @@ struct drm_mode_config {
 
 	/* output poll support */
 	bool poll_enabled;
+	bool poll_running;
 	struct delayed_work output_poll_work;
 
 	/* pointers to standard properties */
-- 
1.7.7.6

[PATCH 08/14] drm: don't unnecessarily enable the polling work

[Daniel Vetter]

... by properly checking connector->polled. This doesn't matter too
much because the polling work itself gets this slightly more right and
doesn't set repoll if there's nothing to do. But we can do better.

Signed-Off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/drm_crtc_helper.c |    6 ++----
 1 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/drm_crtc_helper.c b/drivers/gpu/drm/drm_crtc_helper.c
index f17953e..87a45de 100644
--- a/drivers/gpu/drm/drm_crtc_helper.c
+++ b/drivers/gpu/drm/drm_crtc_helper.c
@@ -955,9 +955,6 @@ static void output_poll_execute(struct work_struct *work)
 		    (connector->polled & DRM_CONNECTOR_POLL_HPD))
 			continue;
 
-		else if (connector->polled & (DRM_CONNECTOR_POLL_CONNECT | DRM_CONNECTOR_POLL_DISCONNECT))
-			repoll = true;
-
 		old_status = connector->status;
 		/* if we are connected and don't want to poll for disconnect
 		   skip it */
@@ -1000,7 +997,8 @@ void drm_kms_helper_poll_enable(struct drm_device *dev)
 		return;
 
 	list_for_each_entry(connector, &dev->mode_config.connector_list, head) {
-		if (connector->polled)
+		if (connector->polled & (DRM_CONNECTOR_POLL_CONNECT |
+					 DRM_CONNECTOR_POLL_DISCONNECT))
 			poll = true;
 	}
 
-- 
1.7.7.6

[PATCH 09/14] drm: don't poll forced connectors

[Daniel Vetter]

Otherwise if the detect callback reports a different state than what
the user forced (rather likely), we continously annoy userspace about
a hotplug uevent.

Signed-Off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/drm_crtc_helper.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/drivers/gpu/drm/drm_crtc_helper.c b/drivers/gpu/drm/drm_crtc_helper.c
index 87a45de..9214612 100644
--- a/drivers/gpu/drm/drm_crtc_helper.c
+++ b/drivers/gpu/drm/drm_crtc_helper.c
@@ -948,6 +948,10 @@ static void output_poll_execute(struct work_struct *work)
 	mutex_lock(&dev->mode_config.mutex);
 	list_for_each_entry(connector, &dev->mode_config.connector_list, head) {
 
+		/* Ignore forced connectors. */
+		if (connector->force)
+			continue;
+
 		/* Ignore HDP capable connectors and connectors where we don't
 		 * want any hotplug detection at all for polling. */
 		if (!connector->polled ||
-- 
1.7.7.6

[PATCH 10/14] drm/i915: cache crt edid

[Daniel Vetter]

Let's put all this new&neat output detection infrastructure and rework
to some good use and cache the crt edid. Given that the drm helpers
now only call ->detect when actually required, we only need to reset
the edid there and can keep it otherwise.

Slashes xrandr time on systems that are hotplug capable if there's
something connected to the VGA connector.

v2: Remember to clean up the cached edid on destroy.

Signed-Off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/intel_crt.c   |   25 ++++++++++++++++++++-----
 drivers/gpu/drm/i915/intel_drv.h   |    1 +
 drivers/gpu/drm/i915/intel_modes.c |   18 ++++++++++++++----
 3 files changed, 35 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c
index a60d131..46a0716 100644
--- a/drivers/gpu/drm/i915/intel_crt.c
+++ b/drivers/gpu/drm/i915/intel_crt.c
@@ -47,6 +47,7 @@
 struct intel_crt {
 	struct intel_encoder base;
 	bool force_hotplug_required;
+	struct edid *cached_edid;
 };
 
 static struct intel_crt *intel_attached_crt(struct drm_connector *connector)
@@ -310,7 +311,7 @@ static bool intel_crt_detect_ddc(struct drm_connector *connector)
 		if (edid != NULL) {
 			is_digital = edid->input & DRM_EDID_INPUT_DIGITAL;
 			connector->display_info.raw_edid = NULL;
-			kfree(edid);
+			crt->cached_edid = edid;
 		}
 
 		if (!is_digital) {
@@ -452,6 +453,10 @@ intel_crt_detect(struct drm_connector *connector, bool force)
 	enum drm_connector_status status;
 	struct intel_load_detect_pipe tmp;
 
+	/* Clean the edid cache. */
+	kfree(crt->cached_edid);
+	crt->cached_edid = NULL;
+
 	if (I915_HAS_HOTPLUG(dev)) {
 		if (intel_crt_detect_hotplug(connector)) {
 			DRM_DEBUG_KMS("CRT detected via hotplug\n");
@@ -485,8 +490,11 @@ intel_crt_detect(struct drm_connector *connector, bool force)
 
 static void intel_crt_destroy(struct drm_connector *connector)
 {
+	struct intel_crt *crt = intel_attached_crt(connector);
+
 	drm_sysfs_connector_remove(connector);
 	drm_connector_cleanup(connector);
+	kfree(crt->cached_edid);
 	kfree(connector);
 }
 
@@ -494,17 +502,24 @@ static int intel_crt_get_modes(struct drm_connector *connector)
 {
 	struct drm_device *dev = connector->dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
-	int ret;
+	struct intel_crt *crt = intel_attached_crt(connector);
+	int ret = 0;
 	struct i2c_adapter *i2c;
 
-	i2c = intel_gmbus_get_adapter(dev_priv, dev_priv->crt_ddc_pin);
-	ret = intel_ddc_get_modes(connector, i2c);
+	if (!crt->cached_edid) {
+		i2c = intel_gmbus_get_adapter(dev_priv, dev_priv->crt_ddc_pin);
+		crt->cached_edid = drm_get_edid(connector, i2c);
+	}
+
+	ret = intel_edid_get_modes(connector, crt->cached_edid);
 	if (ret || !IS_G4X(dev))
 		return ret;
 
 	/* Try to probe digital port for output in DVI-I -> VGA mode. */
 	i2c = intel_gmbus_get_adapter(dev_priv, GMBUS_PORT_DPB);
-	return intel_ddc_get_modes(connector, i2c);
+	kfree(crt->cached_edid);
+	crt->cached_edid = drm_get_edid(connector, i2c);
+	return intel_edid_get_modes(connector, crt->cached_edid);
 }
 
 static int intel_crt_set_property(struct drm_connector *connector,
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 3e09188..3b6f716 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -335,6 +335,7 @@ struct intel_fbc_work {
 };
 
 int intel_ddc_get_modes(struct drm_connector *c, struct i2c_adapter *adapter);
+int intel_edid_get_modes(struct drm_connector *connector, struct edid *edid);
 extern bool intel_ddc_probe(struct intel_encoder *intel_encoder, int ddc_bus);
 
 extern void intel_attach_force_audio_property(struct drm_connector *connector);
diff --git a/drivers/gpu/drm/i915/intel_modes.c b/drivers/gpu/drm/i915/intel_modes.c
index d67ec3a..6c723d9 100644
--- a/drivers/gpu/drm/i915/intel_modes.c
+++ b/drivers/gpu/drm/i915/intel_modes.c
@@ -60,6 +60,19 @@ bool intel_ddc_probe(struct intel_encoder *intel_encoder, int ddc_bus)
 			    msgs, 2) == 2;
 }
 
+int intel_edid_get_modes(struct drm_connector *connector,
+			 struct edid *edid)
+{
+	int ret;
+
+	drm_mode_connector_update_edid_property(connector, edid);
+	ret = drm_add_edid_modes(connector, edid);
+	drm_edid_to_eld(connector, edid);
+	connector->display_info.raw_edid = NULL;
+
+	return ret;
+}
+
 /**
  * intel_ddc_get_modes - get modelist from monitor
  * @connector: DRM connector device to use
@@ -75,10 +88,7 @@ int intel_ddc_get_modes(struct drm_connector *connector,
 
 	edid = drm_get_edid(connector, adapter);
 	if (edid) {
-		drm_mode_connector_update_edid_property(connector, edid);
-		ret = drm_add_edid_modes(connector, edid);
-		drm_edid_to_eld(connector, edid);
-		connector->display_info.raw_edid = NULL;
+		intel_edid_get_modes(connector, edid);
 		kfree(edid);
 	}
 
-- 
1.7.7.6

[PATCH 11/14] drm/i915: cache dp edid

[Daniel Vetter]

Again, let's be slightly more clever here.

Signed-Off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/intel_dp.c |   47 ++++++++++++++++++--------------------
 1 files changed, 22 insertions(+), 25 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 3bbd754..1c84a97 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -67,6 +67,7 @@ struct intel_dp {
 	struct drm_display_mode *panel_fixed_mode;  /* for eDP */
 	struct delayed_work panel_vdd_work;
 	bool want_panel_vdd;
+	struct edid *cached_edid;
 };
 
 /**
@@ -2089,30 +2090,19 @@ g4x_dp_detect(struct intel_dp *intel_dp)
 }
 
 static struct edid *
-intel_dp_get_edid(struct drm_connector *connector, struct i2c_adapter *adapter)
+intel_dp_get_edid(struct drm_connector *connector)
 {
 	struct intel_dp *intel_dp = intel_attached_dp(connector);
-	struct edid	*edid;
-
-	ironlake_edp_panel_vdd_on(intel_dp);
-	edid = drm_get_edid(connector, adapter);
-	ironlake_edp_panel_vdd_off(intel_dp, false);
-	return edid;
-}
 
-static int
-intel_dp_get_edid_modes(struct drm_connector *connector, struct i2c_adapter *adapter)
-{
-	struct intel_dp *intel_dp = intel_attached_dp(connector);
-	int	ret;
+	if (!intel_dp->cached_edid) {
+		ironlake_edp_panel_vdd_on(intel_dp);
+		intel_dp->cached_edid = drm_get_edid(connector, &intel_dp->adapter);
+		ironlake_edp_panel_vdd_off(intel_dp, false);
+	}
 
-	ironlake_edp_panel_vdd_on(intel_dp);
-	ret = intel_ddc_get_modes(connector, adapter);
-	ironlake_edp_panel_vdd_off(intel_dp, false);
-	return ret;
+	return intel_dp->cached_edid;
 }
 
-
 /**
  * Uses CRT_HOTPLUG_EN and CRT_HOTPLUG_STAT to detect DP connection.
  *
@@ -2127,6 +2117,10 @@ intel_dp_detect(struct drm_connector *connector, bool force)
 	enum drm_connector_status status;
 	struct edid *edid = NULL;
 
+	/* Clean the edid cache. */
+	kfree(intel_dp->cached_edid);
+	intel_dp->cached_edid = NULL;
+
 	intel_dp->has_audio = false;
 
 	if (HAS_PCH_SPLIT(dev))
@@ -2145,11 +2139,10 @@ intel_dp_detect(struct drm_connector *connector, bool force)
 	if (intel_dp->force_audio != HDMI_AUDIO_AUTO) {
 		intel_dp->has_audio = (intel_dp->force_audio == HDMI_AUDIO_ON);
 	} else {
-		edid = intel_dp_get_edid(connector, &intel_dp->adapter);
+		edid = intel_dp_get_edid(connector);
 		if (edid) {
 			intel_dp->has_audio = drm_detect_monitor_audio(edid);
 			connector->display_info.raw_edid = NULL;
-			kfree(edid);
 		}
 	}
 
@@ -2161,12 +2154,16 @@ static int intel_dp_get_modes(struct drm_connector *connector)
 	struct intel_dp *intel_dp = intel_attached_dp(connector);
 	struct drm_device *dev = intel_dp->base.base.dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
-	int ret;
+	struct edid	*edid;
+	int ret = 0;
 
 	/* We should parse the EDID data and find out if it has an audio sink
 	 */
 
-	ret = intel_dp_get_edid_modes(connector, &intel_dp->adapter);
+	edid = intel_dp_get_edid(connector);
+	if (edid)
+		ret = intel_edid_get_modes(connector, edid);
+
 	if (ret) {
 		if (is_edp(intel_dp) && !intel_dp->panel_fixed_mode) {
 			struct drm_display_mode *newmode;
@@ -2206,16 +2203,14 @@ static int intel_dp_get_modes(struct drm_connector *connector)
 static bool
 intel_dp_detect_audio(struct drm_connector *connector)
 {
-	struct intel_dp *intel_dp = intel_attached_dp(connector);
 	struct edid *edid;
 	bool has_audio = false;
 
-	edid = intel_dp_get_edid(connector, &intel_dp->adapter);
+	edid = intel_dp_get_edid(connector);
 	if (edid) {
 		has_audio = drm_detect_monitor_audio(edid);
 
 		connector->display_info.raw_edid = NULL;
-		kfree(edid);
 	}
 
 	return has_audio;
@@ -2279,6 +2274,7 @@ done:
 static void
 intel_dp_destroy(struct drm_connector *connector)
 {
+	struct intel_dp *intel_dp = intel_attached_dp(connector);
 	struct drm_device *dev = connector->dev;
 
 	if (intel_dpd_is_edp(dev))
@@ -2286,6 +2282,7 @@ intel_dp_destroy(struct drm_connector *connector)
 
 	drm_sysfs_connector_remove(connector);
 	drm_connector_cleanup(connector);
+	kfree(intel_dp->cached_edid);
 	kfree(connector);
 }
 
-- 
1.7.7.6

[PATCH 12/14] drm/i915: cache hdmi edid

[Daniel Vetter]

Like the previous patches.

While at it also kill a stale comment - we've moved hdmi audio
detection from ->get_modes to ->detect and the audio property handling
functions.

Signed-Off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/intel_drv.h  |    1 +
 drivers/gpu/drm/i915/intel_hdmi.c |   48 +++++++++++++++++++++++--------------
 2 files changed, 31 insertions(+), 18 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 3b6f716..8693551 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -301,6 +301,7 @@ struct intel_hdmi {
 	enum hdmi_force_audio force_audio;
 	void (*write_infoframe)(struct drm_encoder *encoder,
 				struct dip_infoframe *frame);
+	struct edid *cached_edid;
 };
 
 static inline struct drm_crtc *
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index 2ead3bf..373d252 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -452,19 +452,37 @@ static bool intel_hdmi_mode_fixup(struct drm_encoder *encoder,
 	return true;
 }
 
+struct edid *
+intel_hdmi_get_edid(struct drm_connector *connector)
+{
+	struct intel_hdmi *intel_hdmi = intel_attached_hdmi(connector);
+	struct drm_i915_private *dev_priv = connector->dev->dev_private;
+
+	if (!intel_hdmi->cached_edid) {
+		struct i2c_adapter *adapter;
+
+		adapter = intel_gmbus_get_adapter(dev_priv,
+						  intel_hdmi->ddc_bus);
+		intel_hdmi->cached_edid = drm_get_edid(connector, adapter);
+	}
+
+	return intel_hdmi->cached_edid;
+}
+
 static enum drm_connector_status
 intel_hdmi_detect(struct drm_connector *connector, bool force)
 {
 	struct intel_hdmi *intel_hdmi = intel_attached_hdmi(connector);
-	struct drm_i915_private *dev_priv = connector->dev->dev_private;
 	struct edid *edid;
 	enum drm_connector_status status = connector_status_disconnected;
 
+	/* Clean the edid cache. */
+	kfree(intel_hdmi->cached_edid);
+	intel_hdmi->cached_edid = NULL;
+
 	intel_hdmi->has_hdmi_sink = false;
 	intel_hdmi->has_audio = false;
-	edid = drm_get_edid(connector,
-			    intel_gmbus_get_adapter(dev_priv,
-						    intel_hdmi->ddc_bus));
+	edid = intel_hdmi_get_edid(connector);
 
 	if (edid) {
 		if (edid->input & DRM_EDID_INPUT_DIGITAL) {
@@ -477,6 +495,7 @@ intel_hdmi_detect(struct drm_connector *connector, bool force)
 		connector->display_info.raw_edid = NULL;
 		kfree(edid);
 	}
+	intel_hdmi->cached_edid = edid;
 
 	if (status == connector_status_connected) {
 		if (intel_hdmi->force_audio != HDMI_AUDIO_AUTO)
@@ -489,29 +508,19 @@ intel_hdmi_detect(struct drm_connector *connector, bool force)
 
 static int intel_hdmi_get_modes(struct drm_connector *connector)
 {
-	struct intel_hdmi *intel_hdmi = intel_attached_hdmi(connector);
-	struct drm_i915_private *dev_priv = connector->dev->dev_private;
-
-	/* We should parse the EDID data and find out if it's an HDMI sink so
-	 * we can send audio to it.
-	 */
+	struct edid *edid;
 
-	return intel_ddc_get_modes(connector,
-				   intel_gmbus_get_adapter(dev_priv,
-							   intel_hdmi->ddc_bus));
+	edid = intel_hdmi_get_edid(connector);
+	return intel_edid_get_modes(connector, edid);
 }
 
 static bool
 intel_hdmi_detect_audio(struct drm_connector *connector)
 {
-	struct intel_hdmi *intel_hdmi = intel_attached_hdmi(connector);
-	struct drm_i915_private *dev_priv = connector->dev->dev_private;
 	struct edid *edid;
 	bool has_audio = false;
 
-	edid = drm_get_edid(connector,
-			    intel_gmbus_get_adapter(dev_priv,
-						    intel_hdmi->ddc_bus));
+	edid = intel_hdmi_get_edid(connector);
 	if (edid) {
 		if (edid->input & DRM_EDID_INPUT_DIGITAL)
 			has_audio = drm_detect_monitor_audio(edid);
@@ -580,8 +589,11 @@ done:
 
 static void intel_hdmi_destroy(struct drm_connector *connector)
 {
+	struct intel_hdmi *intel_hdmi = intel_attached_hdmi(connector);
+
 	drm_sysfs_connector_remove(connector);
 	drm_connector_cleanup(connector);
+	kfree(intel_hdmi->cached_edid);
 	kfree(connector);
 }
 
-- 
1.7.7.6

[PATCH 13/14] drm/i915/sdvo: implement correct return value for ->get_modes

[Daniel Vetter]

We should return the number of added modes. Luckily no one really
cares, but it kinda sticked out compared to the other ->get_modes
functions I've looked at recently.

Signed-Off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/intel_sdvo.c |   41 ++++++++++++++++++++++--------------
 1 files changed, 25 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
index fdc0574..6056603 100644
--- a/drivers/gpu/drm/i915/intel_sdvo.c
+++ b/drivers/gpu/drm/i915/intel_sdvo.c
@@ -1428,8 +1428,9 @@ intel_sdvo_detect(struct drm_connector *connector, bool force)
 	return ret;
 }
 
-static void intel_sdvo_get_ddc_modes(struct drm_connector *connector)
+static int intel_sdvo_get_ddc_modes(struct drm_connector *connector)
 {
+	int ret = 0;
 	struct edid *edid;
 
 	/* set the bus switch and get the modes */
@@ -1448,12 +1449,14 @@ static void intel_sdvo_get_ddc_modes(struct drm_connector *connector)
 		if (intel_sdvo_connector_matches_edid(to_intel_sdvo_connector(connector),
 						      edid)) {
 			drm_mode_connector_update_edid_property(connector, edid);
-			drm_add_edid_modes(connector, edid);
+			ret = drm_add_edid_modes(connector, edid);
 		}
 
 		connector->display_info.raw_edid = NULL;
 		kfree(edid);
 	}
+
+	return ret;
 }
 
 /*
@@ -1521,12 +1524,12 @@ static const struct drm_display_mode sdvo_tv_modes[] = {
 		   DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC) },
 };
 
-static void intel_sdvo_get_tv_modes(struct drm_connector *connector)
+static int intel_sdvo_get_tv_modes(struct drm_connector *connector)
 {
 	struct intel_sdvo *intel_sdvo = intel_attached_sdvo(connector);
 	struct intel_sdvo_sdtv_resolution_request tv_res;
 	uint32_t reply = 0, format_map = 0;
-	int i;
+	int i, ret = 0;
 
 	/* Read the list of supported input resolutions for the selected TV
 	 * format.
@@ -1536,39 +1539,44 @@ static void intel_sdvo_get_tv_modes(struct drm_connector *connector)
 	       min(sizeof(format_map), sizeof(struct intel_sdvo_sdtv_resolution_request)));
 
 	if (!intel_sdvo_set_target_output(intel_sdvo, intel_sdvo->attached_output))
-		return;
+		return 0;
 
 	BUILD_BUG_ON(sizeof(tv_res) != 3);
 	if (!intel_sdvo_write_cmd(intel_sdvo,
 				  SDVO_CMD_GET_SDTV_RESOLUTION_SUPPORT,
 				  &tv_res, sizeof(tv_res)))
-		return;
+		return 0;
 	if (!intel_sdvo_read_response(intel_sdvo, &reply, 3))
-		return;
+		return 0;
 
 	for (i = 0; i < ARRAY_SIZE(sdvo_tv_modes); i++)
 		if (reply & (1 << i)) {
 			struct drm_display_mode *nmode;
 			nmode = drm_mode_duplicate(connector->dev,
 						   &sdvo_tv_modes[i]);
-			if (nmode)
+			if (nmode) {
 				drm_mode_probed_add(connector, nmode);
+				ret++;
+			}
 		}
+
+	return ret;
 }
 
-static void intel_sdvo_get_lvds_modes(struct drm_connector *connector)
+static int intel_sdvo_get_lvds_modes(struct drm_connector *connector)
 {
 	struct intel_sdvo *intel_sdvo = intel_attached_sdvo(connector);
 	struct drm_i915_private *dev_priv = connector->dev->dev_private;
 	struct drm_display_mode *newmode;
+	int ret;
 
 	/*
 	 * Attempt to get the mode list from DDC.
 	 * Assume that the preferred modes are
 	 * arranged in priority order.
 	 */
-	intel_ddc_get_modes(connector, intel_sdvo->i2c);
-	if (list_empty(&connector->probed_modes) == false)
+	ret = intel_ddc_get_modes(connector, intel_sdvo->i2c);
+	if (ret)
 		goto end;
 
 	/* Fetch modes from VBT */
@@ -1580,6 +1588,8 @@ static void intel_sdvo_get_lvds_modes(struct drm_connector *connector)
 			newmode->type = (DRM_MODE_TYPE_PREFERRED |
 					 DRM_MODE_TYPE_DRIVER);
 			drm_mode_probed_add(connector, newmode);
+
+			ret++;
 		}
 	}
 
@@ -1594,6 +1604,7 @@ end:
 		}
 	}
 
+	return ret;
 }
 
 static int intel_sdvo_get_modes(struct drm_connector *connector)
@@ -1601,13 +1612,11 @@ static int intel_sdvo_get_modes(struct drm_connector *connector)
 	struct intel_sdvo_connector *intel_sdvo_connector = to_intel_sdvo_connector(connector);
 
 	if (IS_TV(intel_sdvo_connector))
-		intel_sdvo_get_tv_modes(connector);
+		return intel_sdvo_get_tv_modes(connector);
 	else if (IS_LVDS(intel_sdvo_connector))
-		intel_sdvo_get_lvds_modes(connector);
+		return intel_sdvo_get_lvds_modes(connector);
 	else
-		intel_sdvo_get_ddc_modes(connector);
-
-	return !list_empty(&connector->probed_modes);
+		return intel_sdvo_get_ddc_modes(connector);
 }
 
 static void
-- 
1.7.7.6

[PATCH 14/14] drm/i915: s/mdelay/msleep/ in the sdvo detect function

[Daniel Vetter]

A 30 ms delay is simply way too big to waste cpu cycles on.

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/intel_sdvo.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
index 6056603..efa0d17 100644
--- a/drivers/gpu/drm/i915/intel_sdvo.c
+++ b/drivers/gpu/drm/i915/intel_sdvo.c
@@ -1370,7 +1370,7 @@ intel_sdvo_detect(struct drm_connector *connector, bool force)
 
 	/* add 30ms delay when the output type might be TV */
 	if (intel_sdvo->caps.output_flags & SDVO_TV_MASK)
-		mdelay(30);
+		msleep(30);
 
 	if (!intel_sdvo_read_response(intel_sdvo, &response, 2))
 		return connector_status_unknown;
-- 
1.7.7.6

[PATCH] drm: introduce DRM_CONNECTOR_POLL_FORCE

[Daniel Vetter]

Useful for ->detect functions that have different behaviour if force
is set. This way probe_single_connector can avoid to do the expensive
edid dance on connectors where this is not needed.

I've checked through all drivers and set this flag everywhere where
the connector->detect function has different behaviour if force is
set. For nouveau and radeon I've got lost in the code traces, so I've
set this flag unconditionally.

Note that we also need to update the poll_execute function to now
also ignore connectors which have only this new flag set.

v2: Change POLL_HDP checks so that they ignore POLL_FORCE for both the
poll and the hpd handling code.

v3: Sprinkle POLL_FORCE more liberally over drivers. It should be now
everywhere where a non-intel driver can return anything else than
connector_status_connected in its detect callback.

v4: Fixup psb compile fail.

Signed-Off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/drm_crtc_helper.c             |    6 ++++--
 drivers/gpu/drm/exynos/exynos_drm_connector.c |    2 ++
 drivers/gpu/drm/gma500/cdv_intel_crt.c        |    2 ++
 drivers/gpu/drm/gma500/cdv_intel_hdmi.c       |    2 ++
 drivers/gpu/drm/gma500/mdfld_dsi_output.c     |    1 +
 drivers/gpu/drm/gma500/oaktrail_hdmi.c        |    2 ++
 drivers/gpu/drm/gma500/psb_intel_sdvo.c       |    2 ++
 drivers/gpu/drm/i915/intel_crt.c              |    3 ++-
 drivers/gpu/drm/i915/intel_tv.c               |    3 ++-
 drivers/gpu/drm/nouveau/nouveau_connector.c   |    1 +
 drivers/gpu/drm/radeon/radeon_connectors.c    |    5 +++++
 drivers/gpu/drm/udl/udl_connector.c           |    2 ++
 drivers/staging/omapdrm/omap_connector.c      |    2 ++
 include/drm/drm_crtc.h                        |    4 ++++
 14 files changed, 33 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/drm_crtc_helper.c b/drivers/gpu/drm/drm_crtc_helper.c
index b1d643d..8ea1c1e 100644
--- a/drivers/gpu/drm/drm_crtc_helper.c
+++ b/drivers/gpu/drm/drm_crtc_helper.c
@@ -944,7 +944,9 @@ static void output_poll_execute(struct work_struct *work)
 
 		/* Ignore HDP capable connectors and connectors where we don't
 		 * want any hotplug detection at all for polling. */
-		if (!connector->polled || connector->polled == DRM_CONNECTOR_POLL_HPD)
+		if (!connector->polled ||
+		    connector->polled == DRM_CONNECTOR_POLL_FORCE ||
+		    (connector->polled & DRM_CONNECTOR_POLL_HPD))
 			continue;
 
 		else if (connector->polled & (DRM_CONNECTOR_POLL_CONNECT | DRM_CONNECTOR_POLL_DISCONNECT))
@@ -1029,7 +1031,7 @@ void drm_helper_hpd_irq_event(struct drm_device *dev)
 	list_for_each_entry(connector, &dev->mode_config.connector_list, head) {
 
 		/* Only handle HPD capable connectors. */
-		if (connector->polled != DRM_CONNECTOR_POLL_HPD)
+		if (!(connector->polled & DRM_CONNECTOR_POLL_HPD))
 			continue;
 
 		old_status = connector->status;
diff --git a/drivers/gpu/drm/exynos/exynos_drm_connector.c b/drivers/gpu/drm/exynos/exynos_drm_connector.c
index bf791fa..e5a8a27 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_connector.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_connector.c
@@ -327,6 +327,8 @@ struct drm_connector *exynos_drm_connector_create(struct drm_device *dev,
 	drm_connector_init(dev, connector, &exynos_connector_funcs, type);
 	drm_connector_helper_add(connector, &exynos_connector_helper_funcs);
 
+	connector->polled |= DRM_CONNECTOR_POLL_FORCE;
+
 	err = drm_sysfs_connector_add(connector);
 	if (err)
 		goto err_connector;
diff --git a/drivers/gpu/drm/gma500/cdv_intel_crt.c b/drivers/gpu/drm/gma500/cdv_intel_crt.c
index 1874220..e6b2e49 100644
--- a/drivers/gpu/drm/gma500/cdv_intel_crt.c
+++ b/drivers/gpu/drm/gma500/cdv_intel_crt.c
@@ -313,6 +313,8 @@ void cdv_intel_crt_init(struct drm_device *dev,
 	drm_connector_helper_add(connector,
 					&cdv_intel_crt_connector_helper_funcs);
 
+	connector->polled |= DRM_CONNECTOR_POLL_FORCE;
+
 	drm_sysfs_connector_add(connector);
 
 	return;
diff --git a/drivers/gpu/drm/gma500/cdv_intel_hdmi.c b/drivers/gpu/drm/gma500/cdv_intel_hdmi.c
index 88b59d4..766aec8 100644
--- a/drivers/gpu/drm/gma500/cdv_intel_hdmi.c
+++ b/drivers/gpu/drm/gma500/cdv_intel_hdmi.c
@@ -373,6 +373,8 @@ void cdv_hdmi_init(struct drm_device *dev,
 	hdmi_priv->hdmi_i2c_adapter =
 				&(psb_intel_encoder->i2c_bus->adapter);
 	hdmi_priv->dev = dev;
+	connector->polled |= DRM_CONNECTOR_POLL_FORCE;
+
 	drm_sysfs_connector_add(connector);
 	return;
 
diff --git a/drivers/gpu/drm/gma500/mdfld_dsi_output.c b/drivers/gpu/drm/gma500/mdfld_dsi_output.c
index 5675d93..0e97a91 100644
--- a/drivers/gpu/drm/gma500/mdfld_dsi_output.c
+++ b/drivers/gpu/drm/gma500/mdfld_dsi_output.c
@@ -604,6 +604,7 @@ void mdfld_dsi_output_init(struct drm_device *dev,
 	dsi_config->encoder = encoder;
 	encoder->base.type = (pipe == 0) ? INTEL_OUTPUT_MIPI :
 		INTEL_OUTPUT_MIPI2;
+	connector->polled |= DRM_CONNECTOR_POLL_FORCE;
 	drm_sysfs_connector_add(connector);
 	return;
 
diff --git a/drivers/gpu/drm/gma500/oaktrail_hdmi.c b/drivers/gpu/drm/gma500/oaktrail_hdmi.c
index c10899c..23b8d52 100644
--- a/drivers/gpu/drm/gma500/oaktrail_hdmi.c
+++ b/drivers/gpu/drm/gma500/oaktrail_hdmi.c
@@ -349,6 +349,8 @@ void oaktrail_hdmi_init(struct drm_device *dev,
 	connector->display_info.subpixel_order = SubPixelHorizontalRGB;
 	connector->interlace_allowed = false;
 	connector->doublescan_allowed = false;
+	connector->polled |= DRM_CONNECTOR_POLL_FORCE;
+
 	drm_sysfs_connector_add(connector);
 
 	return;
diff --git a/drivers/gpu/drm/gma500/psb_intel_sdvo.c b/drivers/gpu/drm/gma500/psb_intel_sdvo.c
index d39b15b..fdfc4a0 100644
--- a/drivers/gpu/drm/gma500/psb_intel_sdvo.c
+++ b/drivers/gpu/drm/gma500/psb_intel_sdvo.c
@@ -2028,6 +2028,8 @@ psb_intel_sdvo_connector_init(struct psb_intel_sdvo_connector *connector,
 	connector->base.base.display_info.subpixel_order = SubPixelHorizontalRGB;
 
 	psb_intel_connector_attach_encoder(&connector->base, &encoder->base);
+	connector->base.base.polled |= DRM_CONNECTOR_POLL_FORCE;
+
 	drm_sysfs_connector_add(&connector->base.base);
 }
 
diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c
index 75a70c4..a60d131 100644
--- a/drivers/gpu/drm/i915/intel_crt.c
+++ b/drivers/gpu/drm/i915/intel_crt.c
@@ -639,7 +639,8 @@ void intel_crt_init(struct drm_device *dev)
 	if (I915_HAS_HOTPLUG(dev))
 		connector->polled = DRM_CONNECTOR_POLL_HPD;
 	else
-		connector->polled = DRM_CONNECTOR_POLL_CONNECT;
+		connector->polled = DRM_CONNECTOR_POLL_CONNECT |
+				    DRM_CONNECTOR_POLL_FORCE;
 
 	/*
 	 * Configure the automatic hotplug detection stuff
diff --git a/drivers/gpu/drm/i915/intel_tv.c b/drivers/gpu/drm/i915/intel_tv.c
index a233a51..dad7d8e 100644
--- a/drivers/gpu/drm/i915/intel_tv.c
+++ b/drivers/gpu/drm/i915/intel_tv.c
@@ -1614,7 +1614,8 @@ intel_tv_init(struct drm_device *dev)
 	 *
 	 * More recent chipsets favour HDMI rather than integrated S-Video.
 	 */
-	connector->polled = DRM_CONNECTOR_POLL_CONNECT;
+	connector->polled = DRM_CONNECTOR_POLL_CONNECT |
+			    DRM_CONNECTOR_POLL_FORCE;
 
 	drm_connector_init(dev, connector, &intel_tv_connector_funcs,
 			   DRM_MODE_CONNECTOR_SVIDEO);
diff --git a/drivers/gpu/drm/nouveau/nouveau_connector.c b/drivers/gpu/drm/nouveau/nouveau_connector.c
index fa86035..780d608 100644
--- a/drivers/gpu/drm/nouveau/nouveau_connector.c
+++ b/drivers/gpu/drm/nouveau/nouveau_connector.c
@@ -1085,6 +1085,7 @@ nouveau_connector_create(struct drm_device *dev, int index)
 		if (ret == 0)
 			connector->polled = DRM_CONNECTOR_POLL_HPD;
 	}
+	connector->polled |= DRM_CONNECTOR_POLL_FORCE;
 
 	drm_sysfs_connector_add(connector);
 	return connector;
diff --git a/drivers/gpu/drm/radeon/radeon_connectors.c b/drivers/gpu/drm/radeon/radeon_connectors.c
index 2914c57..c64ecc9 100644
--- a/drivers/gpu/drm/radeon/radeon_connectors.c
+++ b/drivers/gpu/drm/radeon/radeon_connectors.c
@@ -1830,6 +1830,8 @@ radeon_add_atom_connector(struct drm_device *dev,
 	} else
 		connector->polled = DRM_CONNECTOR_POLL_HPD;
 
+	connector->polled |= DRM_CONNECTOR_POLL_FORCE;
+
 	connector->display_info.subpixel_order = subpixel_order;
 	drm_sysfs_connector_add(connector);
 	return;
@@ -1987,6 +1989,9 @@ radeon_add_legacy_connector(struct drm_device *dev,
 			connector->polled = DRM_CONNECTOR_POLL_CONNECT;
 	} else
 		connector->polled = DRM_CONNECTOR_POLL_HPD;
+
+	connector->polled |= DRM_CONNECTOR_POLL_FORCE;
+
 	connector->display_info.subpixel_order = subpixel_order;
 	drm_sysfs_connector_add(connector);
 	if (connector_type == DRM_MODE_CONNECTOR_LVDS) {
diff --git a/drivers/gpu/drm/udl/udl_connector.c b/drivers/gpu/drm/udl/udl_connector.c
index ba055e9..838dc71 100644
--- a/drivers/gpu/drm/udl/udl_connector.c
+++ b/drivers/gpu/drm/udl/udl_connector.c
@@ -131,6 +131,8 @@ int udl_connector_init(struct drm_device *dev, struct drm_encoder *encoder)
 	drm_connector_init(dev, connector, &udl_connector_funcs, DRM_MODE_CONNECTOR_DVII);
 	drm_connector_helper_add(connector, &udl_connector_helper_funcs);
 
+	connector->polled |= DRM_CONNECTOR_POLL_FORCE;
+
 	drm_sysfs_connector_add(connector);
 	drm_mode_connector_attach_encoder(connector, encoder);
 
diff --git a/drivers/staging/omapdrm/omap_connector.c b/drivers/staging/omapdrm/omap_connector.c
index 5e2856c..bb8b134 100644
--- a/drivers/staging/omapdrm/omap_connector.c
+++ b/drivers/staging/omapdrm/omap_connector.c
@@ -358,6 +358,8 @@ struct drm_connector *omap_connector_init(struct drm_device *dev,
 	connector->interlace_allowed = 1;
 	connector->doublescan_allowed = 0;
 
+	connector->polled |= DRM_CONNECTOR_POLL_FORCE;
+
 	drm_sysfs_connector_add(connector);
 
 	return connector;
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index d59bb7d..4417da2 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -514,6 +514,10 @@ enum drm_connector_force {
 /* can cleanly poll for disconnections without flickering the screen */
 /* DACs should rarely do this without a lot of testing */
 #define DRM_CONNECTOR_POLL_DISCONNECT (1 << 2)
+/* connector's ->detect function needs to be called when userspace forces
+ * detection. Should only be used if the detect function has special handling
+ * when force is set to avoid spurious re-reading of the edid. */
+#define DRM_CONNECTOR_POLL_FORCE (1 << 3)
 
 #define MAX_ELD_BYTES	128
 
-- 
1.7.7.6

Re: [Intel-gfx] [PATCH 02/14] drm: handle HDP and polled connectors separately

[Adam Jackson]

On 5/24/12 3:26 PM, Daniel Vetter wrote:
> Instead of reusing the polling code for hdp handling, split them up.
> This has a few consequences:
> - Don't touch HDP capable connectors in the poll loop.
> - Only touch HDP capable connectors in drm_helper_hpd_irq_event.
> - Run the HDP handling directly instead of going through a work item -
>    all callers already call drm_helper_hpd_irq_event from process
>    context without holding the mode_config mutex.

"HPD".

> -		/* if this is HPD or polled don't check it -
> -		   TV out for instance */
> -		if (!connector->polled)
> +		/* Ignore HDP capable connectors and connectors where we don't
> +		 * want any hotplug detection at all for polling. */
> +		if (!connector->polled || connector->polled == DRM_CONNECTOR_POLL_HPD)

Here too.  Also in the subject.

- ajax

[PATCH] drm: handle HPD and polled connectors separately

[Daniel Vetter]

Instead of reusing the polling code for hpd handling, split them up.
This has a few consequences:
- Don't touch HPD capable connectors in the poll loop.
- Only touch HPD capable connectors in drm_helper_hpd_irq_event.
- Run the HPD handling directly instead of going through a work item -
  all callers already call drm_helper_hpd_irq_event from process
  context without holding the mode_config mutex.

The ultimate goal is that drivers grow some smarts about which
connectors have received a hotplug event and only call the detect code
of that connector. But that's a second step.

v2: s/hdp/hpd/, noticed by Adam Jackson. I can't type.

Signed-Off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/drm_crtc_helper.c |   39 ++++++++++++++++++++++++++++--------
 1 files changed, 30 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/drm_crtc_helper.c b/drivers/gpu/drm/drm_crtc_helper.c
index 909a85c..5461ee6 100644
--- a/drivers/gpu/drm/drm_crtc_helper.c
+++ b/drivers/gpu/drm/drm_crtc_helper.c
@@ -942,9 +942,9 @@ static void output_poll_execute(struct work_struct *work)
 	mutex_lock(&dev->mode_config.mutex);
 	list_for_each_entry(connector, &dev->mode_config.connector_list, head) {
 
-		/* if this is HPD or polled don't check it -
-		   TV out for instance */
-		if (!connector->polled)
+		/* Ignore HPD capable connectors and connectors where we don't
+		 * want any hotplug detection at all for polling. */
+		if (!connector->polled || connector->polled == DRM_CONNECTOR_POLL_HPD)
 			continue;
 
 		else if (connector->polled & (DRM_CONNECTOR_POLL_CONNECT | DRM_CONNECTOR_POLL_DISCONNECT))
@@ -954,8 +954,7 @@ static void output_poll_execute(struct work_struct *work)
 		/* if we are connected and don't want to poll for disconnect
 		   skip it */
 		if (old_status == connector_status_connected &&
-		    !(connector->polled & DRM_CONNECTOR_POLL_DISCONNECT) &&
-		    !(connector->polled & DRM_CONNECTOR_POLL_HPD))
+		    !(connector->polled & DRM_CONNECTOR_POLL_DISCONNECT))
 			continue;
 
 		connector->status = connector->funcs->detect(connector, false);
@@ -1019,12 +1018,34 @@ EXPORT_SYMBOL(drm_kms_helper_poll_fini);
 
 void drm_helper_hpd_irq_event(struct drm_device *dev)
 {
+	struct drm_connector *connector;
+	enum drm_connector_status old_status;
+	bool changed = false;
+
 	if (!dev->mode_config.poll_enabled)
 		return;
 
-	/* kill timer and schedule immediate execution, this doesn't block */
-	cancel_delayed_work(&dev->mode_config.output_poll_work);
-	if (drm_kms_helper_poll)
-		queue_delayed_work(system_nrt_wq, &dev->mode_config.output_poll_work, 0);
+	mutex_lock(&dev->mode_config.mutex);
+	list_for_each_entry(connector, &dev->mode_config.connector_list, head) {
+
+		/* Only handle HPD capable connectors. */
+		if (connector->polled != DRM_CONNECTOR_POLL_HPD)
+			continue;
+
+		old_status = connector->status;
+
+		connector->status = connector->funcs->detect(connector, false);
+		DRM_DEBUG_KMS("[CONNECTOR:%d:%s] status updated from %d to %d\n",
+			      connector->base.id,
+			      drm_get_connector_name(connector),
+			      old_status, connector->status);
+		if (old_status != connector->status)
+			changed = true;
+	}
+
+	mutex_unlock(&dev->mode_config.mutex);
+
+	if (changed)
+		drm_kms_helper_hotplug_event(dev);
 }
 EXPORT_SYMBOL(drm_helper_hpd_irq_event);
-- 
1.7.7.6

[PATCH 1/2] drm: handle HPD and polled connectors separately

[Daniel Vetter]

Instead of reusing the polling code for hpd handling, split them up.
This has a few consequences:
- Don't touch HPD capable connectors in the poll loop.
- Only touch HPD capable connectors in drm_helper_hpd_irq_event.
- We could run the HPD handling directly (because all callers already
  use their own work item), but for easier bisect that happens in it's
  own patch.

The ultimate goal is that drivers grow some smarts about which
connectors have received a hotplug event and only call the detect code
of that connector. But that's a second step.

v2: s/hdp/hpd/, noticed by Adam Jackson. I can't type.

v3: Split out the work item removal as requested by Dave Airlie. This
results in a temporary mode_config.hpd_irq_work item to keep things
the same.

Signed-Off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/drm_crtc_helper.c |   51 ++++++++++++++++++++++++++++++------
 include/drm/drm_crtc.h            |    1 +
 2 files changed, 43 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/drm_crtc_helper.c b/drivers/gpu/drm/drm_crtc_helper.c
index 909a85c..74da342 100644
--- a/drivers/gpu/drm/drm_crtc_helper.c
+++ b/drivers/gpu/drm/drm_crtc_helper.c
@@ -942,9 +942,9 @@ static void output_poll_execute(struct work_struct *work)
 	mutex_lock(&dev->mode_config.mutex);
 	list_for_each_entry(connector, &dev->mode_config.connector_list, head) {
 
-		/* if this is HPD or polled don't check it -
-		   TV out for instance */
-		if (!connector->polled)
+		/* Ignore HPD capable connectors and connectors where we don't
+		 * want any hotplug detection at all for polling. */
+		if (!connector->polled || connector->polled == DRM_CONNECTOR_POLL_HPD)
 			continue;
 
 		else if (connector->polled & (DRM_CONNECTOR_POLL_CONNECT | DRM_CONNECTOR_POLL_DISCONNECT))
@@ -954,8 +954,7 @@ static void output_poll_execute(struct work_struct *work)
 		/* if we are connected and don't want to poll for disconnect
 		   skip it */
 		if (old_status == connector_status_connected &&
-		    !(connector->polled & DRM_CONNECTOR_POLL_DISCONNECT) &&
-		    !(connector->polled & DRM_CONNECTOR_POLL_HPD))
+		    !(connector->polled & DRM_CONNECTOR_POLL_DISCONNECT))
 			continue;
 
 		connector->status = connector->funcs->detect(connector, false);
@@ -1002,9 +1001,12 @@ void drm_kms_helper_poll_enable(struct drm_device *dev)
 }
 EXPORT_SYMBOL(drm_kms_helper_poll_enable);
 
+static void hpd_irq_event_execute(struct work_struct *work);
+
 void drm_kms_helper_poll_init(struct drm_device *dev)
 {
 	INIT_DELAYED_WORK(&dev->mode_config.output_poll_work, output_poll_execute);
+	INIT_DELAYED_WORK(&dev->mode_config.hpd_irq_work, hpd_irq_event_execute);
 	dev->mode_config.poll_enabled = true;
 
 	drm_kms_helper_poll_enable(dev);
@@ -1017,14 +1019,45 @@ void drm_kms_helper_poll_fini(struct drm_device *dev)
 }
 EXPORT_SYMBOL(drm_kms_helper_poll_fini);
 
-void drm_helper_hpd_irq_event(struct drm_device *dev)
+static void hpd_irq_event_execute(struct work_struct *work)
 {
+	struct delayed_work *delayed_work = to_delayed_work(work);
+	struct drm_device *dev = container_of(delayed_work, struct drm_device, mode_config.hpd_irq_work);
+	struct drm_connector *connector;
+	enum drm_connector_status old_status;
+	bool changed = false;
+
 	if (!dev->mode_config.poll_enabled)
 		return;
 
-	/* kill timer and schedule immediate execution, this doesn't block */
-	cancel_delayed_work(&dev->mode_config.output_poll_work);
+	mutex_lock(&dev->mode_config.mutex);
+	list_for_each_entry(connector, &dev->mode_config.connector_list, head) {
+
+		/* Only handle HPD capable connectors. */
+		if (connector->polled != DRM_CONNECTOR_POLL_HPD)
+			continue;
+
+		old_status = connector->status;
+
+		connector->status = connector->funcs->detect(connector, false);
+		DRM_DEBUG_KMS("[CONNECTOR:%d:%s] status updated from %d to %d\n",
+			      connector->base.id,
+			      drm_get_connector_name(connector),
+			      old_status, connector->status);
+		if (old_status != connector->status)
+			changed = true;
+	}
+
+	mutex_unlock(&dev->mode_config.mutex);
+
+	if (changed)
+		drm_kms_helper_hotplug_event(dev);
+}
+
+void drm_helper_hpd_irq_event(struct drm_device *dev)
+{
+	cancel_delayed_work(&dev->mode_config.hpd_irq_work);
 	if (drm_kms_helper_poll)
-		queue_delayed_work(system_nrt_wq, &dev->mode_config.output_poll_work, 0);
+		queue_delayed_work(system_nrt_wq, &dev->mode_config.hpd_irq_work, 0);
 }
 EXPORT_SYMBOL(drm_helper_hpd_irq_event);
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index d59bb7d..03b08f7 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -780,6 +780,7 @@ struct drm_mode_config {
 	/* output poll support */
 	bool poll_enabled;
 	struct delayed_work output_poll_work;
+	struct delayed_work hpd_irq_work;
 
 	/* pointers to standard properties */
 	struct list_head property_blob_list;
-- 
1.7.7.6

[PATCH 2/2] drm: run the hpd irq event code directly

[Daniel Vetter]

All drivers already have a work item to run the hpd code, so we don't
need to launch a new one in the helper code. Dave Airlie mentioned
that the cancel+re-queue might paper over DP related hpd ping-pongs,
hence why this is split out.

Signed-Off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/drm_crtc_helper.c |   14 +-------------
 include/drm/drm_crtc.h            |    1 -
 2 files changed, 1 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/drm_crtc_helper.c b/drivers/gpu/drm/drm_crtc_helper.c
index 74da342..5461ee6 100644
--- a/drivers/gpu/drm/drm_crtc_helper.c
+++ b/drivers/gpu/drm/drm_crtc_helper.c
@@ -1001,12 +1001,9 @@ void drm_kms_helper_poll_enable(struct drm_device *dev)
 }
 EXPORT_SYMBOL(drm_kms_helper_poll_enable);
 
-static void hpd_irq_event_execute(struct work_struct *work);
-
 void drm_kms_helper_poll_init(struct drm_device *dev)
 {
 	INIT_DELAYED_WORK(&dev->mode_config.output_poll_work, output_poll_execute);
-	INIT_DELAYED_WORK(&dev->mode_config.hpd_irq_work, hpd_irq_event_execute);
 	dev->mode_config.poll_enabled = true;
 
 	drm_kms_helper_poll_enable(dev);
@@ -1019,10 +1016,8 @@ void drm_kms_helper_poll_fini(struct drm_device *dev)
 }
 EXPORT_SYMBOL(drm_kms_helper_poll_fini);
 
-static void hpd_irq_event_execute(struct work_struct *work)
+void drm_helper_hpd_irq_event(struct drm_device *dev)
 {
-	struct delayed_work *delayed_work = to_delayed_work(work);
-	struct drm_device *dev = container_of(delayed_work, struct drm_device, mode_config.hpd_irq_work);
 	struct drm_connector *connector;
 	enum drm_connector_status old_status;
 	bool changed = false;
@@ -1053,11 +1048,4 @@ static void hpd_irq_event_execute(struct work_struct *work)
 	if (changed)
 		drm_kms_helper_hotplug_event(dev);
 }
-
-void drm_helper_hpd_irq_event(struct drm_device *dev)
-{
-	cancel_delayed_work(&dev->mode_config.hpd_irq_work);
-	if (drm_kms_helper_poll)
-		queue_delayed_work(system_nrt_wq, &dev->mode_config.hpd_irq_work, 0);
-}
 EXPORT_SYMBOL(drm_helper_hpd_irq_event);
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index 03b08f7..d59bb7d 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -780,7 +780,6 @@ struct drm_mode_config {
 	/* output poll support */
 	bool poll_enabled;
 	struct delayed_work output_poll_work;
-	struct delayed_work hpd_irq_work;
 
 	/* pointers to standard properties */
 	struct list_head property_blob_list;
-- 
1.7.7.6

Re: [PATCH 14/14] drm/i915: s/mdelay/msleep/ in the sdvo detect function

[Daniel Vetter]

On Thu, May 24, 2012 at 09:26:49PM +0200, Daniel Vetter wrote:
> A 30 ms delay is simply way too big to waste cpu cycles on.
> 
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
I've queued this patch here for -next with Chris' irc ack added.
-Daniel
-- 
Daniel Vetter
Mail: daniel@ffwll.ch
Mobile: +41 (0)79 365 57 48

RE: [PATCH 14/14] drm/i915: s/mdelay/msleep/ in the sdvo detect function

[Singh, Satyeshwar]

Is there a reason for the 30 ms delay in the first place?
-Satyeshwar

-----Original Message-----
From: dri-devel-bounces+satyeshwar.singh=intel.com@lists.freedesktop.org [mailto:dri-devel-bounces+satyeshwar.singh=intel.com@lists.freedesktop.org] On Behalf Of Daniel Vetter
Sent: Thursday, May 31, 2012 1:29 AM
To: Intel Graphics Development; DRI Development
Cc: Daniel Vetter
Subject: Re: [PATCH 14/14] drm/i915: s/mdelay/msleep/ in the sdvo detect function

On Thu, May 24, 2012 at 09:26:49PM +0200, Daniel Vetter wrote:
> A 30 ms delay is simply way too big to waste cpu cycles on.
> 
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
I've queued this patch here for -next with Chris' irc ack added.
-Daniel
-- 
Daniel Vetter
Mail: daniel@ffwll.ch
Mobile: +41 (0)79 365 57 48
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 14/14] drm/i915: s/mdelay/msleep/ in the sdvo detect function

[Daniel Vetter]

On Thu, May 31, 2012 at 9:24 PM, Singh, Satyeshwar
<satyeshwar.singh@intel.com> wrote:
> Is there a reason for the 30 ms delay in the first place?

git blame says there is, it supposedely fixes a bug with tv detection.
-Daniel
-- 
Daniel Vetter
daniel.vetter@ffwll.ch - +41 (0) 79 364 57 48 - http://blog.ffwll.ch

Re: [RFC] [PATCH 00/14] HPD/connector-polling rework

[Alex Deucher]

On Thu, May 24, 2012 at 3:26 PM, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> Hi all,
>
> I've got fed up with our sorry state of connector detection and rampant edid re
> and rere-reading.
>
> This patch series lays the groundwork in the drm helpers so that drivers can
> avoid all this madness (at least on working hw) and properly cache the edid.
>
> With the additional changes for drm/i915, the edid is now read _once_ per plug
> event (or at boot-up/resume time). A further step would be to integrate the
> hotplug handling into the driver itself and only call ->detect on the connectors
> for which the irq handler received a hotplug event.
>
> By adding POLL_FORCE drivers can get back the old behaviour of calling ->detect
> every time probe_single_connector is called from userspace. I've splattered that
> over all drivers where I've thought it might be required.
>
> Note though that setting this doesn't avoid all regressions - the regular output
> poll work will still ignore any connectors with POLL_HPD set. If a driver/hw
> with broken hpd got away due to this, this will break stuff. But that should be
> easy to fix by admitting the defeat and setting POLL_CONNECT|DISCONNECT
> directly.
>
> If other people want to convert over their drivers, the following steps are
> required:
> - Ensure that the connector status is unknown every time the driver could have
>   missed a hpd event (e.g. after resume). drm_mode_config_reset will do that for
>   you.
> - Drop the POLL_FORCE flag for connectors where hdp is fully reliable.
> - Implement edid caching - that's a nice way to figure out whether hpd is
>   actually reliable, because if it isn't, this step will ensure that you get bug
>   reports because the the edid won't ever get updated ;-)
> - Optionally teach the driver some smarts about which specific connectors
>   actually got a hotplug event. Mostly useful on cheap hw (like intel's) that
>   can't distinguish between hdmi and dp without trying some aux channel
>   transfers.
>
> As you can guess from the patch series, I've discovered the hard way that i915
> sdvo support is totally broken. Tested on most of the intel machines I have and
> also quickly on my radeon hd5000.
>
> Comments, flames, ideas and test reports highly welcome.

This set looks good to me.  Do you have plans to revive this?  At
least the common drm ones are much better than what we currently have.

Alex


>
> Cheers, Daniel
>
> Daniel Vetter (14):
>   drm: extract drm_kms_helper_hotplug_event
>   drm: handle HDP and polled connectors separately
>   drm: introduce DRM_CONNECTOR_POLL_FORCE
>   drm/i915: set POLL_FORCE for sdvo outputs
>   drm: properly init/reset connector status
>   drm: kill unnecessary calls to connector->detect
>   drm: don't start the poll engine in probe_single_connector
>   drm: don't unnecessarily enable the polling work
>   drm: don't poll forced connectors
>   drm/i915: cache crt edid
>   drm/i915: cache dp edid
>   drm/i915: cache hdmi edid
>   drm/i915/sdvo: implement correct return value for ->get_modes
>   drm/i915: s/mdelay/msleep/ in the sdvo detect function
>
>  drivers/gpu/drm/drm_crtc.c                    |    6 ++-
>  drivers/gpu/drm/drm_crtc_helper.c             |   76 ++++++++++++++++++-------
>  drivers/gpu/drm/exynos/exynos_drm_connector.c |    2 +
>  drivers/gpu/drm/gma500/cdv_intel_crt.c        |    2 +
>  drivers/gpu/drm/gma500/cdv_intel_hdmi.c       |    2 +
>  drivers/gpu/drm/gma500/mdfld_dsi_output.c     |    1 +
>  drivers/gpu/drm/gma500/oaktrail_hdmi.c        |    2 +
>  drivers/gpu/drm/gma500/psb_intel_sdvo.c       |    2 +
>  drivers/gpu/drm/i915/intel_crt.c              |   28 +++++++--
>  drivers/gpu/drm/i915/intel_dp.c               |   47 +++++++--------
>  drivers/gpu/drm/i915/intel_drv.h              |    2 +
>  drivers/gpu/drm/i915/intel_hdmi.c             |   48 ++++++++++------
>  drivers/gpu/drm/i915/intel_modes.c            |   18 +++++-
>  drivers/gpu/drm/i915/intel_sdvo.c             |   45 +++++++++------
>  drivers/gpu/drm/i915/intel_tv.c               |    3 +-
>  drivers/gpu/drm/nouveau/nouveau_connector.c   |    1 +
>  drivers/gpu/drm/radeon/radeon_connectors.c    |    5 ++
>  drivers/gpu/drm/udl/udl_connector.c           |    2 +
>  drivers/staging/omapdrm/omap_connector.c      |    2 +
>  include/drm/drm_crtc.h                        |    5 ++
>  include/drm/drm_crtc_helper.h                 |    1 +
>  21 files changed, 208 insertions(+), 92 deletions(-)
>
> --
> 1.7.7.6
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [RFC] [PATCH 00/14] HPD/connector-polling rework

[Daniel Vetter]

On Thu, Oct 04, 2012 at 12:07:01PM -0400, Alex Deucher wrote:
> On Thu, May 24, 2012 at 3:26 PM, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> > Hi all,
> >
> > I've got fed up with our sorry state of connector detection and rampant edid re
> > and rere-reading.
> >
> > This patch series lays the groundwork in the drm helpers so that drivers can
> > avoid all this madness (at least on working hw) and properly cache the edid.
> >
> > With the additional changes for drm/i915, the edid is now read _once_ per plug
> > event (or at boot-up/resume time). A further step would be to integrate the
> > hotplug handling into the driver itself and only call ->detect on the connectors
> > for which the irq handler received a hotplug event.
> >
> > By adding POLL_FORCE drivers can get back the old behaviour of calling ->detect
> > every time probe_single_connector is called from userspace. I've splattered that
> > over all drivers where I've thought it might be required.
> >
> > Note though that setting this doesn't avoid all regressions - the regular output
> > poll work will still ignore any connectors with POLL_HPD set. If a driver/hw
> > with broken hpd got away due to this, this will break stuff. But that should be
> > easy to fix by admitting the defeat and setting POLL_CONNECT|DISCONNECT
> > directly.
> >
> > If other people want to convert over their drivers, the following steps are
> > required:
> > - Ensure that the connector status is unknown every time the driver could have
> >   missed a hpd event (e.g. after resume). drm_mode_config_reset will do that for
> >   you.
> > - Drop the POLL_FORCE flag for connectors where hdp is fully reliable.
> > - Implement edid caching - that's a nice way to figure out whether hpd is
> >   actually reliable, because if it isn't, this step will ensure that you get bug
> >   reports because the the edid won't ever get updated ;-)
> > - Optionally teach the driver some smarts about which specific connectors
> >   actually got a hotplug event. Mostly useful on cheap hw (like intel's) that
> >   can't distinguish between hdmi and dp without trying some aux channel
> >   transfers.
> >
> > As you can guess from the patch series, I've discovered the hard way that i915
> > sdvo support is totally broken. Tested on most of the intel machines I have and
> > also quickly on my radeon hd5000.
> >
> > Comments, flames, ideas and test reports highly welcome.
> 
> This set looks good to me.  Do you have plans to revive this?  At
> least the common drm ones are much better than what we currently have.

The issue I've meanwhile discovered with hpd handling is that we really
need the smarts in the driver to only do probing on native hdmi/dp outputs
if we have a hpd irq for that specific port: Since we have 2 encoders for
the same port, if e.g. dp is enabled and we do edid probing on the hdmi
encoder/connector the display might get unhappy, which can happen even
with these patches applied e.g. when pluggin in a 2nd display.

So the new plan (for 3.8 hopefully) is to revamp the i915-internal hpd
handling and keep on using the crtc helper stuff. For connectors with
reliable hpd we'd simply return the tracked stated in ->detect without
touching the hw at all. So I don't think we need any changes in the
hotplug/polling helper code. And looking again at these patches, they're a
rather decent kludge and it's probably better to do this all in the
drivers. Since the helper code sets the force parameter to false for all
the background polling, you can just return any driver-internal cached
state. And when force=true you could invalidate that cached state or do
some additional (more expensive) detection.

In short: No plans to revive this, but certainly plans to improve the
drm/i915 hpd handling.

Cheers, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: [RFC] [PATCH 00/14] HPD/connector-polling rework

[Alex Deucher]

On Thu, Oct 4, 2012 at 1:16 PM, Daniel Vetter <daniel@ffwll.ch> wrote:
> On Thu, Oct 04, 2012 at 12:07:01PM -0400, Alex Deucher wrote:
>> On Thu, May 24, 2012 at 3:26 PM, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
>> > Hi all,
>> >
>> > I've got fed up with our sorry state of connector detection and rampant edid re
>> > and rere-reading.
>> >
>> > This patch series lays the groundwork in the drm helpers so that drivers can
>> > avoid all this madness (at least on working hw) and properly cache the edid.
>> >
>> > With the additional changes for drm/i915, the edid is now read _once_ per plug
>> > event (or at boot-up/resume time). A further step would be to integrate the
>> > hotplug handling into the driver itself and only call ->detect on the connectors
>> > for which the irq handler received a hotplug event.
>> >
>> > By adding POLL_FORCE drivers can get back the old behaviour of calling ->detect
>> > every time probe_single_connector is called from userspace. I've splattered that
>> > over all drivers where I've thought it might be required.
>> >
>> > Note though that setting this doesn't avoid all regressions - the regular output
>> > poll work will still ignore any connectors with POLL_HPD set. If a driver/hw
>> > with broken hpd got away due to this, this will break stuff. But that should be
>> > easy to fix by admitting the defeat and setting POLL_CONNECT|DISCONNECT
>> > directly.
>> >
>> > If other people want to convert over their drivers, the following steps are
>> > required:
>> > - Ensure that the connector status is unknown every time the driver could have
>> >   missed a hpd event (e.g. after resume). drm_mode_config_reset will do that for
>> >   you.
>> > - Drop the POLL_FORCE flag for connectors where hdp is fully reliable.
>> > - Implement edid caching - that's a nice way to figure out whether hpd is
>> >   actually reliable, because if it isn't, this step will ensure that you get bug
>> >   reports because the the edid won't ever get updated ;-)
>> > - Optionally teach the driver some smarts about which specific connectors
>> >   actually got a hotplug event. Mostly useful on cheap hw (like intel's) that
>> >   can't distinguish between hdmi and dp without trying some aux channel
>> >   transfers.
>> >
>> > As you can guess from the patch series, I've discovered the hard way that i915
>> > sdvo support is totally broken. Tested on most of the intel machines I have and
>> > also quickly on my radeon hd5000.
>> >
>> > Comments, flames, ideas and test reports highly welcome.
>>
>> This set looks good to me.  Do you have plans to revive this?  At
>> least the common drm ones are much better than what we currently have.
>
> The issue I've meanwhile discovered with hpd handling is that we really
> need the smarts in the driver to only do probing on native hdmi/dp outputs
> if we have a hpd irq for that specific port: Since we have 2 encoders for
> the same port, if e.g. dp is enabled and we do edid probing on the hdmi
> encoder/connector the display might get unhappy, which can happen even
> with these patches applied e.g. when pluggin in a 2nd display.
>
> So the new plan (for 3.8 hopefully) is to revamp the i915-internal hpd
> handling and keep on using the crtc helper stuff. For connectors with
> reliable hpd we'd simply return the tracked stated in ->detect without
> touching the hw at all. So I don't think we need any changes in the
> hotplug/polling helper code. And looking again at these patches, they're a
> rather decent kludge and it's probably better to do this all in the
> drivers. Since the helper code sets the force parameter to false for all
> the background polling, you can just return any driver-internal cached
> state. And when force=true you could invalidate that cached state or do
> some additional (more expensive) detection.

Well the current code skips all hotplug event propagation if the poll
option is disabled (even for hpd capable connectors) which is fail so
the first few patches still seem like a win.  I guess you are not
planning to use any of the current common hpd code?  If so, do you
mind if we pull at least the first few patches in in the interim for
other drivers?

Alex

>
> In short: No plans to revive this, but certainly plans to improve the
> drm/i915 hpd handling.
>
> Cheers, Daniel
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch