All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/i915/crt: Keep the EDID for the whole detect cycle
@ 2016-09-29 10:52 Chris Wilson
  2016-09-29 11:20 ` ✗ Fi.CI.BAT: warning for " Patchwork
  2016-09-29 12:49 ` [PATCH] " Imre Deak
  0 siblings, 2 replies; 4+ messages in thread
From: Chris Wilson @ 2016-09-29 10:52 UTC (permalink / raw)
  To: intel-gfx

Other than reducing the number of EDID reads required for probing the
modes on a connector, this refactor has the importance of centralising
the logic for deciding when we need to probe for an analogue output via
DVI-I and then consistently using that information between detection and
mode computation.

References: https://bugs.freedesktop.org/show_bug.cgi?id=97971
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Imre Deak <imre.deak@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_crt.c | 98 +++++++++++++++-------------------------
 1 file changed, 36 insertions(+), 62 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c
index 88ebbdde185a..b9b88b0b73e7 100644
--- a/drivers/gpu/drm/i915/intel_crt.c
+++ b/drivers/gpu/drm/i915/intel_crt.c
@@ -459,8 +459,8 @@ static bool intel_crt_detect_hotplug(struct drm_connector *connector)
 	return ret;
 }
 
-static struct edid *intel_crt_get_edid(struct drm_connector *connector,
-				struct i2c_adapter *i2c)
+static struct edid *__intel_crt_get_edid(struct drm_connector *connector,
+					 struct i2c_adapter *i2c)
 {
 	struct edid *edid;
 
@@ -473,59 +473,50 @@ static struct edid *intel_crt_get_edid(struct drm_connector *connector,
 		intel_gmbus_force_bit(i2c, false);
 	}
 
-	return edid;
-}
-
-/* local version of intel_ddc_get_modes() to use intel_crt_get_edid() */
-static int intel_crt_ddc_get_modes(struct drm_connector *connector,
-				struct i2c_adapter *adapter)
-{
-	struct edid *edid;
-	int ret;
-
-	edid = intel_crt_get_edid(connector, adapter);
-	if (!edid)
-		return 0;
-
-	ret = intel_connector_update_modes(connector, edid);
-	kfree(edid);
+	if (edid && edid->input & DRM_EDID_INPUT_DIGITAL) {
+		DRM_DEBUG_KMS("EDID reports a digital output, ignoring for this VGA connector\n");
+		kfree(edid);
+		edid = NULL;
+	}
 
-	return ret;
+	return edid;
 }
 
-static bool intel_crt_detect_ddc(struct drm_connector *connector)
+static struct edid *intel_crt_get_edid(struct drm_connector *connector)
 {
-	struct intel_crt *crt = intel_attached_crt(connector);
-	struct drm_i915_private *dev_priv = to_i915(crt->base.base.dev);
-	struct edid *edid;
+#define HAS_DVI_I(p) 1 /* XXX anything with HDMI, i.e. g33+ */
+	struct drm_i915_private *dev_priv = to_i915(connector->dev);
 	struct i2c_adapter *i2c;
-
-	BUG_ON(crt->base.type != INTEL_OUTPUT_ANALOG);
+	struct edid *edid;
 
 	i2c = intel_gmbus_get_adapter(dev_priv, dev_priv->vbt.crt_ddc_pin);
-	edid = intel_crt_get_edid(connector, i2c);
-
-	if (edid) {
-		bool is_digital = edid->input & DRM_EDID_INPUT_DIGITAL;
-
+	edid = __intel_crt_get_edid(connector, i2c);
+	if (!edid && HAS_DVI_I(dev_priv)) {
 		/*
 		 * This may be a DVI-I connector with a shared DDC
 		 * link between analog and digital outputs, so we
 		 * have to check the EDID input spec of the attached device.
 		 */
-		if (!is_digital) {
-			DRM_DEBUG_KMS("CRT detected via DDC:0x50 [EDID]\n");
-			return true;
-		}
-
-		DRM_DEBUG_KMS("CRT not detected via DDC:0x50 [EDID reports a digital panel]\n");
-	} else {
-		DRM_DEBUG_KMS("CRT not detected via DDC:0x50 [no valid EDID found]\n");
+		i2c = intel_gmbus_get_adapter(dev_priv, GMBUS_PIN_DPB);
+		edid = __intel_crt_get_edid(connector, i2c);
 	}
 
-	kfree(edid);
+	return edid;
+}
+
+static bool intel_crt_detect_ddc(struct drm_connector *connector)
+{
+	struct edid *edid;
+
+	kfree(to_intel_connector(connector)->detect_edid);
+	to_intel_connector(connector)->detect_edid = NULL;
 
-	return false;
+	edid = intel_crt_get_edid(connector);
+	if (!edid)
+		return false;
+
+	to_intel_connector(connector)->detect_edid = edid;
+	return true;
 }
 
 static enum drm_connector_status
@@ -727,30 +718,13 @@ static void intel_crt_destroy(struct drm_connector *connector)
 
 static int intel_crt_get_modes(struct drm_connector *connector)
 {
-	struct drm_device *dev = connector->dev;
-	struct drm_i915_private *dev_priv = to_i915(dev);
-	struct intel_crt *crt = intel_attached_crt(connector);
-	struct intel_encoder *intel_encoder = &crt->base;
-	enum intel_display_power_domain power_domain;
-	int ret;
-	struct i2c_adapter *i2c;
-
-	power_domain = intel_display_port_power_domain(intel_encoder);
-	intel_display_power_get(dev_priv, power_domain);
-
-	i2c = intel_gmbus_get_adapter(dev_priv, dev_priv->vbt.crt_ddc_pin);
-	ret = intel_crt_ddc_get_modes(connector, i2c);
-	if (ret || !IS_G4X(dev))
-		goto out;
-
-	/* Try to probe digital port for output in DVI-I -> VGA mode. */
-	i2c = intel_gmbus_get_adapter(dev_priv, GMBUS_PIN_DPB);
-	ret = intel_crt_ddc_get_modes(connector, i2c);
+	struct edid *edid;
 
-out:
-	intel_display_power_put(dev_priv, power_domain);
+	edid = to_intel_connector(connector)->detect_edid;
+	if (!edid)
+		return 0;
 
-	return ret;
+	return intel_connector_update_modes(connector, edid);
 }
 
 static int intel_crt_set_property(struct drm_connector *connector,
-- 
2.9.3

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

^ permalink raw reply related	[flat|nested] 4+ messages in thread
* [PATCH 1/4] drm/i915: Don't leak edid in intel_crt_detect_ddc()
@ 2017-01-20 14:28 Ander Conselvan de Oliveira
  2017-01-20 14:51 ` [PATCH] drm/i915/crt: Keep the EDID for the whole detect cycle Chris Wilson
  0 siblings, 1 reply; 4+ messages in thread
From: Ander Conselvan de Oliveira @ 2017-01-20 14:28 UTC (permalink / raw)
  To: intel-gfx
  Cc: Ander Conselvan de Oliveira, Chris Wilson, Daniel Vetter,
	Daniel Vetter, Jani Nikula, stable

In the path where intel_crt_detect_ddc() detects a CRT, if would return
true without freeing the edid.

Found by coverity.

Fixes: a2bd1f541f19 ("drm/i915: check whether we actually received an
edid in detect_ddc")
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Daniel Vetter <daniel.vetter@intel.com>
Cc: Jani Nikula <jani.nikula@linux.intel.com>
Cc: intel-gfx@lists.freedesktop.org
Cc: <stable@vger.kernel.org> # v3.6+
Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
---
 drivers/gpu/drm/i915/intel_crt.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c
index 385e29a..2bf5aca 100644
--- a/drivers/gpu/drm/i915/intel_crt.c
+++ b/drivers/gpu/drm/i915/intel_crt.c
@@ -499,6 +499,7 @@ static bool intel_crt_detect_ddc(struct drm_connector *connector)
 	struct drm_i915_private *dev_priv = to_i915(crt->base.base.dev);
 	struct edid *edid;
 	struct i2c_adapter *i2c;
+	bool ret = false;
 
 	BUG_ON(crt->base.type != INTEL_OUTPUT_ANALOG);
 
@@ -515,17 +516,17 @@ static bool intel_crt_detect_ddc(struct drm_connector *connector)
 		 */
 		if (!is_digital) {
 			DRM_DEBUG_KMS("CRT detected via DDC:0x50 [EDID]\n");
-			return true;
+			ret = true;
+		} else {
+			DRM_DEBUG_KMS("CRT not detected via DDC:0x50 [EDID reports a digital panel]\n");
 		}
-
-		DRM_DEBUG_KMS("CRT not detected via DDC:0x50 [EDID reports a digital panel]\n");
 	} else {
 		DRM_DEBUG_KMS("CRT not detected via DDC:0x50 [no valid EDID found]\n");
 	}
 
 	kfree(edid);
 
-	return false;
+	return ret;
 }
 
 static enum drm_connector_status
-- 
2.5.5


^ permalink raw reply related	[flat|nested] 4+ messages in thread
end of thread, other threads:[~2017-01-20 14:51 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-09-29 10:52 [PATCH] drm/i915/crt: Keep the EDID for the whole detect cycle Chris Wilson
2016-09-29 11:20 ` ✗ Fi.CI.BAT: warning for " Patchwork
2016-09-29 12:49 ` [PATCH] " Imre Deak
2017-01-20 14:28 [PATCH 1/4] drm/i915: Don't leak edid in intel_crt_detect_ddc() Ander Conselvan de Oliveira
2017-01-20 14:51 ` [PATCH] drm/i915/crt: Keep the EDID for the whole detect cycle Chris Wilson

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.