public inbox for intel-gfx@lists.freedesktop.org
 help / color / mirror / Atom feed
* [PATCH 0/4] edid caching
@ 2012-06-11  7:03 Daniel Vetter
  2012-06-11  7:03 ` [PATCH 1/4] drm/i915: cache crt edid Daniel Vetter
                   ` (4 more replies)
  0 siblings, 5 replies; 6+ messages in thread
From: Daniel Vetter @ 2012-06-11  7:03 UTC (permalink / raw)
  To: Intel Graphics Development; +Cc: Daniel Vetter

Hi all,

This are a few patches from my hpd series to implement edid caching. It's not
nirvana, but at least it cuts down a bit on our massive edid re-re-reading ...

As usual, comments&review highly welcom.

Yours, Daniel

Daniel Vetter (4):
  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

 drivers/gpu/drm/i915/intel_crt.c   |   25 ++++++++++++++---
 drivers/gpu/drm/i915/intel_dp.c    |   47 ++++++++++++++++------------------
 drivers/gpu/drm/i915/intel_drv.h   |    2 +
 drivers/gpu/drm/i915/intel_hdmi.c  |   49 +++++++++++++++++++++--------------
 drivers/gpu/drm/i915/intel_modes.c |   18 ++++++++++---
 drivers/gpu/drm/i915/intel_sdvo.c  |   41 ++++++++++++++++++-----------
 6 files changed, 112 insertions(+), 70 deletions(-)

-- 
1.7.7.6

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

* [PATCH 1/4] drm/i915: cache crt edid
  2012-06-11  7:03 [PATCH 0/4] edid caching Daniel Vetter
@ 2012-06-11  7:03 ` Daniel Vetter
  2012-06-11  7:03 ` [PATCH 2/4] drm/i915: cache dp edid Daniel Vetter
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: Daniel Vetter @ 2012-06-11  7:03 UTC (permalink / raw)
  To: Intel Graphics Development; +Cc: 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.

Note that without the HPD rework the benefits are a bit less stellar, but things
still help. Also, ->detect is still the right place conceptually to drop the
cached edid.

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 c35edd7..d073623 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -337,6 +337,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

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

* [PATCH 2/4] drm/i915: cache dp edid
  2012-06-11  7:03 [PATCH 0/4] edid caching Daniel Vetter
  2012-06-11  7:03 ` [PATCH 1/4] drm/i915: cache crt edid Daniel Vetter
@ 2012-06-11  7:03 ` Daniel Vetter
  2012-06-11  7:03 ` [PATCH 3/4] drm/i915: cache hdmi edid Daniel Vetter
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 6+ messages in thread
From: Daniel Vetter @ 2012-06-11  7:03 UTC (permalink / raw)
  To: Intel Graphics Development; +Cc: 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 6538c46..cafbc37 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;
 };
 
 /**
@@ -2117,30 +2118,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.
  *
@@ -2155,6 +2145,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))
@@ -2175,11 +2169,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);
 		}
 	}
 
@@ -2191,12 +2184,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;
@@ -2236,16 +2233,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;
@@ -2309,6 +2304,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))
@@ -2316,6 +2312,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

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

* [PATCH 3/4] drm/i915: cache hdmi edid
  2012-06-11  7:03 [PATCH 0/4] edid caching Daniel Vetter
  2012-06-11  7:03 ` [PATCH 1/4] drm/i915: cache crt edid Daniel Vetter
  2012-06-11  7:03 ` [PATCH 2/4] drm/i915: cache dp edid Daniel Vetter
@ 2012-06-11  7:03 ` Daniel Vetter
  2012-06-11  7:03 ` [PATCH 4/4] drm/i915/sdvo: implement correct return value for ->get_modes Daniel Vetter
  2012-06-11 10:15 ` [PATCH 0/4] edid caching Chris Wilson
  4 siblings, 0 replies; 6+ messages in thread
From: Daniel Vetter @ 2012-06-11  7:03 UTC (permalink / raw)
  To: Intel Graphics Development; +Cc: 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.

v2: Fixup use-after-free in edid caching because I've missed a kfree
somewhere. Dunno how that one escape, because I clearly remember
fixing this while testing the patch :(

v3: Really fix this up.

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

diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index d073623..ddb60bc 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -303,6 +303,7 @@ struct intel_hdmi {
 				struct dip_infoframe *frame);
 	void (*set_infoframes)(struct drm_encoder *encoder,
 			       struct drm_display_mode *adjusted_mode);
+	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 69637db..d3dbc32 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -689,22 +689,40 @@ static bool g4x_hdmi_connected(struct intel_hdmi *intel_hdmi)
 	return I915_READ(PORT_HOTPLUG_STAT) & bit;
 }
 
+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;
 
 	if (IS_G4X(connector->dev) && !g4x_hdmi_connected(intel_hdmi))
 		return status;
 
+	/* 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) {
@@ -715,7 +733,6 @@ intel_hdmi_detect(struct drm_connector *connector, bool force)
 			intel_hdmi->has_audio = drm_detect_monitor_audio(edid);
 		}
 		connector->display_info.raw_edid = NULL;
-		kfree(edid);
 	}
 
 	if (status == connector_status_connected) {
@@ -729,35 +746,24 @@ 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);
 
 		connector->display_info.raw_edid = NULL;
-		kfree(edid);
 	}
 
 	return has_audio;
@@ -820,8 +826,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

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

* [PATCH 4/4] drm/i915/sdvo: implement correct return value for ->get_modes
  2012-06-11  7:03 [PATCH 0/4] edid caching Daniel Vetter
                   ` (2 preceding siblings ...)
  2012-06-11  7:03 ` [PATCH 3/4] drm/i915: cache hdmi edid Daniel Vetter
@ 2012-06-11  7:03 ` Daniel Vetter
  2012-06-11 10:15 ` [PATCH 0/4] edid caching Chris Wilson
  4 siblings, 0 replies; 6+ messages in thread
From: Daniel Vetter @ 2012-06-11  7:03 UTC (permalink / raw)
  To: Intel Graphics Development; +Cc: 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 10ae2d3..0d6116d 100644
--- a/drivers/gpu/drm/i915/intel_sdvo.c
+++ b/drivers/gpu/drm/i915/intel_sdvo.c
@@ -1439,8 +1439,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 */
@@ -1459,12 +1460,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;
 }
 
 /*
@@ -1532,12 +1535,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.
@@ -1547,39 +1550,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 */
@@ -1591,6 +1599,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++;
 		}
 	}
 
@@ -1605,6 +1615,7 @@ end:
 		}
 	}
 
+	return ret;
 }
 
 static int intel_sdvo_get_modes(struct drm_connector *connector)
@@ -1612,13 +1623,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

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

* Re: [PATCH 0/4] edid caching
  2012-06-11  7:03 [PATCH 0/4] edid caching Daniel Vetter
                   ` (3 preceding siblings ...)
  2012-06-11  7:03 ` [PATCH 4/4] drm/i915/sdvo: implement correct return value for ->get_modes Daniel Vetter
@ 2012-06-11 10:15 ` Chris Wilson
  4 siblings, 0 replies; 6+ messages in thread
From: Chris Wilson @ 2012-06-11 10:15 UTC (permalink / raw)
  To: Intel Graphics Development; +Cc: Daniel Vetter

On Mon, 11 Jun 2012 09:03:12 +0200, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> Hi all,
> 
> This are a few patches from my hpd series to implement edid caching. It's not
> nirvana, but at least it cuts down a bit on our massive edid re-re-reading ...

Afterwards lvds/dp/hdmi/crt, leaving sdvo/tv behind, all implement edid
caching. It would seem like we could reduce the code slightly by moving
that to intel_connector, then at least the destructor is common and we
won't have repeated members across the different connectors.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

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

end of thread, other threads:[~2012-06-11 10:15 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-06-11  7:03 [PATCH 0/4] edid caching Daniel Vetter
2012-06-11  7:03 ` [PATCH 1/4] drm/i915: cache crt edid Daniel Vetter
2012-06-11  7:03 ` [PATCH 2/4] drm/i915: cache dp edid Daniel Vetter
2012-06-11  7:03 ` [PATCH 3/4] drm/i915: cache hdmi edid Daniel Vetter
2012-06-11  7:03 ` [PATCH 4/4] drm/i915/sdvo: implement correct return value for ->get_modes Daniel Vetter
2012-06-11 10:15 ` [PATCH 0/4] edid caching Chris Wilson

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox