Updated DP fixes

Updated DP fixes

[Jesse Barnes]

Addresses review comments, in particular:
  - pull out the retry code into a separate function
  - fix hotplug handling differently (just try to poke the sink on every
    hot plug if it was ever configured, i.e. has an assigned crtc)
  - add sink power management

Thanks,
Jesse

[PATCH 1/7] drm/i915/dp: retry link status read 3 times on failure

[Jesse Barnes]

Especially after a hotplug or power status change, the sink may not
reply immediately to a link status query.  So retry 3 times per the spec
to really make sure nothing is there.

See section 9.1 of the 1.1a DisplayPort spec.

Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
---
 drivers/gpu/drm/i915/intel_dp.c |   20 +++++++++++++-------
 1 files changed, 13 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 391b55f..86912bd 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1019,14 +1019,20 @@ intel_dp_dpms(struct drm_encoder *encoder, int mode)
 static bool
 intel_dp_get_link_status(struct intel_dp *intel_dp)
 {
-	int ret;
+	int ret, i;
+
+	/* Must try AUX reads for this at least 3 times */
+	for (i = 0; i < 3; i++) {
+		ret = intel_dp_aux_native_read(intel_dp,
+					       DP_LANE0_1_STATUS,
+					       intel_dp->link_status,
+					       DP_LINK_STATUS_SIZE);
+		if (ret == DP_LINK_STATUS_SIZE)
+			return true;
+		msleep(1);
+	}
 
-	ret = intel_dp_aux_native_read(intel_dp,
-				       DP_LANE0_1_STATUS,
-				       intel_dp->link_status, DP_LINK_STATUS_SIZE);
-	if (ret != DP_LINK_STATUS_SIZE)
-		return false;
-	return true;
+	return false;
 }
 
 static uint8_t
-- 
1.7.4.1

[PATCH 2/7] drm/i915/dp: use DP DPCD defines when looking at DPCD values

[Jesse Barnes]

Makes it easier to search for DP related constants.

Reviewed-by: Keith Packard <keithp@keithp.com>
Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
---
 drivers/gpu/drm/i915/intel_dp.c |   18 ++++++++++--------
 1 files changed, 10 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 86912bd..fc51730 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -138,8 +138,8 @@ intel_dp_max_lane_count(struct intel_dp *intel_dp)
 {
 	int max_lane_count = 4;
 
-	if (intel_dp->dpcd[0] >= 0x11) {
-		max_lane_count = intel_dp->dpcd[2] & 0x1f;
+	if (intel_dp->dpcd[DP_DPCD_REV] >= 0x11) {
+		max_lane_count = intel_dp->dpcd[DP_MAX_LANE_COUNT] & 0x1f;
 		switch (max_lane_count) {
 		case 1: case 2: case 4:
 			break;
@@ -153,7 +153,7 @@ intel_dp_max_lane_count(struct intel_dp *intel_dp)
 static int
 intel_dp_max_link_bw(struct intel_dp *intel_dp)
 {
-	int max_link_bw = intel_dp->dpcd[1];
+	int max_link_bw = intel_dp->dpcd[DP_MAX_LINK_RATE];
 
 	switch (max_link_bw) {
 	case DP_LINK_BW_1_62:
@@ -774,7 +774,8 @@ intel_dp_mode_set(struct drm_encoder *encoder, struct drm_display_mode *mode,
 	/*
 	 * Check for DPCD version > 1.1 and enhanced framing support
 	 */
-	if (intel_dp->dpcd[0] >= 0x11 && (intel_dp->dpcd[2] & DP_ENHANCED_FRAME_CAP)) {
+	if (intel_dp->dpcd[DP_DPCD_REV] >= 0x11 &&
+	    (intel_dp->dpcd[DP_MAX_LANE_COUNT] & DP_ENHANCED_FRAME_CAP)) {
 		intel_dp->link_configuration[1] |= DP_LANE_COUNT_ENHANCED_FRAME_EN;
 		intel_dp->DP |= DP_ENHANCED_FRAMING;
 	}
@@ -1553,7 +1554,7 @@ ironlake_dp_detect(struct intel_dp *intel_dp)
 				     0x000, intel_dp->dpcd,
 				     sizeof (intel_dp->dpcd))
 	    == sizeof(intel_dp->dpcd)) {
-		if (intel_dp->dpcd[0] != 0)
+		if (intel_dp->dpcd[DP_DPCD_REV] != 0)
 			status = connector_status_connected;
 	}
 	DRM_DEBUG_KMS("DPCD: %hx%hx%hx%hx\n", intel_dp->dpcd[0],
@@ -1592,7 +1593,7 @@ g4x_dp_detect(struct intel_dp *intel_dp)
 	if (intel_dp_aux_native_read(intel_dp, 0x000, intel_dp->dpcd,
 				     sizeof (intel_dp->dpcd)) == sizeof (intel_dp->dpcd))
 	{
-		if (intel_dp->dpcd[0] != 0)
+		if (intel_dp->dpcd[DP_DPCD_REV] != 0)
 			status = connector_status_connected;
 	}
 
@@ -1960,8 +1961,9 @@ intel_dp_init(struct drm_device *dev, int output_reg)
 					       sizeof(intel_dp->dpcd));
 		ironlake_edp_panel_vdd_off(intel_dp);
 		if (ret == sizeof(intel_dp->dpcd)) {
-			if (intel_dp->dpcd[0] >= 0x11)
-				dev_priv->no_aux_handshake = intel_dp->dpcd[3] &
+			if (intel_dp->dpcd[DP_DPCD_REV] >= 0x11)
+				dev_priv->no_aux_handshake =
+					intel_dp->dpcd[DP_MAX_DOWNSPREAD] &
 					DP_NO_AUX_HANDSHAKE_LINK_TRAINING;
 		} else {
 			/* if this fails, presume the device is a ghost */
-- 
1.7.4.1

[PATCH 3/7] drm/i915/dp: read more receiver capability bits on hotplug

[Jesse Barnes]

When a hotplug event is received, we need to check the receiver cap bits
in case they've changed (as they might with a hub or chain config).

Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
---
 drivers/gpu/drm/i915/intel_dp.c |   11 +++++++++++
 1 files changed, 11 insertions(+), 0 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index fc51730..a9a5051 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1522,6 +1522,8 @@ intel_dp_link_down(struct intel_dp *intel_dp)
 static void
 intel_dp_check_link_status(struct intel_dp *intel_dp)
 {
+	int ret;
+
 	if (!intel_dp->base.base.crtc)
 		return;
 
@@ -1530,6 +1532,15 @@ intel_dp_check_link_status(struct intel_dp *intel_dp)
 		return;
 	}
 
+	/* Try to read receiver status if the link appears to be up */
+	ret = intel_dp_aux_native_read(intel_dp,
+				       0x000, intel_dp->dpcd,
+				       sizeof (intel_dp->dpcd));
+	if (ret != sizeof(intel_dp->dpcd)) {
+		intel_dp_link_down(intel_dp);
+		return;
+	}
+
 	if (!intel_channel_eq_ok(intel_dp)) {
 		intel_dp_start_link_train(intel_dp);
 		intel_dp_complete_link_train(intel_dp);
-- 
1.7.4.1

[PATCH 4/7] drm/i915/dp: try to read receiver capabilities 3 times when detecting

[Jesse Barnes]

If ->detect is called too soon after a hot plug event, the sink may not
be ready yet.  So try up to 3 times with 1ms sleeps in between tries to
get the data (spec dictates that receivers must be ready to respond within
1ms and that sources should try 3 times).

See section 9.1 of the 1.1a DisplayPort spec.

Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
---
 drivers/gpu/drm/i915/intel_dp.c |   15 ++++++++++-----
 1 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index a9a5051..70075d9 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1551,6 +1551,7 @@ static enum drm_connector_status
 ironlake_dp_detect(struct intel_dp *intel_dp)
 {
 	enum drm_connector_status status;
+	int ret, i;
 
 	/* Can't disconnect eDP, but you can close the lid... */
 	if (is_edp(intel_dp)) {
@@ -1561,12 +1562,16 @@ ironlake_dp_detect(struct intel_dp *intel_dp)
 	}
 
 	status = connector_status_disconnected;
-	if (intel_dp_aux_native_read(intel_dp,
-				     0x000, intel_dp->dpcd,
-				     sizeof (intel_dp->dpcd))
-	    == sizeof(intel_dp->dpcd)) {
-		if (intel_dp->dpcd[DP_DPCD_REV] != 0)
+	for (i = 0; i < 3; i++) {
+		ret = intel_dp_aux_native_read(intel_dp,
+					       0x000, intel_dp->dpcd,
+					       sizeof (intel_dp->dpcd));
+		if (ret == sizeof(intel_dp->dpcd) &&
+		    intel_dp->dpcd[DP_DPCD_REV] != 0) {
 			status = connector_status_connected;
+			break;
+		}
+		msleep(1);
 	}
 	DRM_DEBUG_KMS("DPCD: %hx%hx%hx%hx\n", intel_dp->dpcd[0],
 		      intel_dp->dpcd[1], intel_dp->dpcd[2], intel_dp->dpcd[3]);
-- 
1.7.4.1

[PATCH 5/7] drm/i915/dp: remove DPMS mode tracking from DP

[Jesse Barnes]

We currently use this when a hot plug event is received, only checking
the link status and re-training if we had previously configured a link.
However if we want to preserve the DP configuration across both hot plug
and DPMS events (which we do for userspace apps that don't respond to
hot plug uevents), we need to unconditionally check the link and try to
bring it up on hot plug.

Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
---
 drivers/gpu/drm/i915/intel_dp.c |    6 +-----
 1 files changed, 1 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 70075d9..5f97c17 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -50,7 +50,6 @@ struct intel_dp {
 	bool has_audio;
 	int force_audio;
 	uint32_t color_range;
-	int dpms_mode;
 	uint8_t link_bw;
 	uint8_t lane_count;
 	uint8_t dpcd[4];
@@ -1010,7 +1009,6 @@ intel_dp_dpms(struct drm_encoder *encoder, int mode)
 		if (is_edp(intel_dp))
 			ironlake_edp_backlight_on(dev);
 	}
-	intel_dp->dpms_mode = mode;
 }
 
 /*
@@ -1813,8 +1811,7 @@ intel_dp_hot_plug(struct intel_encoder *intel_encoder)
 {
 	struct intel_dp *intel_dp = container_of(intel_encoder, struct intel_dp, base);
 
-	if (intel_dp->dpms_mode == DRM_MODE_DPMS_ON)
-		intel_dp_check_link_status(intel_dp);
+	intel_dp_check_link_status(intel_dp);
 }
 
 /* Return which DP Port should be selected for Transcoder DP control */
@@ -1882,7 +1879,6 @@ intel_dp_init(struct drm_device *dev, int output_reg)
 		return;
 
 	intel_dp->output_reg = output_reg;
-	intel_dp->dpms_mode = -1;
 
 	intel_connector = kzalloc(sizeof(struct intel_connector), GFP_KERNEL);
 	if (!intel_connector) {
-- 
1.7.4.1

[PATCH 6/7] drm/i915/dp: consolidate AUX retry code

[Jesse Barnes]

When checking link status during a hot plug event or detecting sink
presence, we need to retry 3 times per the spec (section 9.1 of the 1.1a
DisplayPort spec).  Consolidate the retry code into a
native_aux_read_retry function for use by get_link_status and _detect.

Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
---
 drivers/gpu/drm/i915/intel_dp.c |   51 +++++++++++++++++++++++----------------
 1 files changed, 30 insertions(+), 21 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 5f97c17..bca48b0 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1012,21 +1012,23 @@ intel_dp_dpms(struct drm_encoder *encoder, int mode)
 }
 
 /*
- * Fetch AUX CH registers 0x202 - 0x207 which contain
- * link status information
+ * Native read with retry for link status and receiver capability reads for
+ * cases where the sink may still be asleep.
  */
 static bool
-intel_dp_get_link_status(struct intel_dp *intel_dp)
+intel_dp_aux_native_read_retry(struct intel_dp *intel_dp, uint16_t address,
+			       uint8_t *recv, int recv_bytes)
 {
 	int ret, i;
 
-	/* Must try AUX reads for this at least 3 times */
+	/*
+	 * Sinks are *supposed* to come up within 1ms from an off state,
+	 * but we're also supposed to retry 3 times per the spec.
+	 */
 	for (i = 0; i < 3; i++) {
-		ret = intel_dp_aux_native_read(intel_dp,
-					       DP_LANE0_1_STATUS,
-					       intel_dp->link_status,
-					       DP_LINK_STATUS_SIZE);
-		if (ret == DP_LINK_STATUS_SIZE)
+		ret = intel_dp_aux_native_read(intel_dp, address, recv,
+					       recv_bytes);
+		if (ret == recv_bytes)
 			return true;
 		msleep(1);
 	}
@@ -1034,6 +1036,19 @@ intel_dp_get_link_status(struct intel_dp *intel_dp)
 	return false;
 }
 
+/*
+ * Fetch AUX CH registers 0x202 - 0x207 which contain
+ * link status information
+ */
+static bool
+intel_dp_get_link_status(struct intel_dp *intel_dp)
+{
+	return intel_dp_aux_native_read_retry(intel_dp,
+					      DP_LANE0_1_STATUS,
+					      intel_dp->link_status,
+					      DP_LINK_STATUS_SIZE);
+}
+
 static uint8_t
 intel_dp_link_status(uint8_t link_status[DP_LINK_STATUS_SIZE],
 		     int r)
@@ -1549,7 +1564,7 @@ static enum drm_connector_status
 ironlake_dp_detect(struct intel_dp *intel_dp)
 {
 	enum drm_connector_status status;
-	int ret, i;
+	bool ret;
 
 	/* Can't disconnect eDP, but you can close the lid... */
 	if (is_edp(intel_dp)) {
@@ -1560,17 +1575,11 @@ ironlake_dp_detect(struct intel_dp *intel_dp)
 	}
 
 	status = connector_status_disconnected;
-	for (i = 0; i < 3; i++) {
-		ret = intel_dp_aux_native_read(intel_dp,
-					       0x000, intel_dp->dpcd,
-					       sizeof (intel_dp->dpcd));
-		if (ret == sizeof(intel_dp->dpcd) &&
-		    intel_dp->dpcd[DP_DPCD_REV] != 0) {
-			status = connector_status_connected;
-			break;
-		}
-		msleep(1);
-	}
+	ret = intel_dp_aux_native_read_retry(intel_dp,
+					     0x000, intel_dp->dpcd,
+					     sizeof (intel_dp->dpcd));
+	if (ret && intel_dp->dpcd[DP_DPCD_REV] != 0)
+		status = connector_status_connected;
 	DRM_DEBUG_KMS("DPCD: %hx%hx%hx%hx\n", intel_dp->dpcd[0],
 		      intel_dp->dpcd[1], intel_dp->dpcd[2], intel_dp->dpcd[3]);
 	return status;
-- 
1.7.4.1

[PATCH 7/7] drm/i915/dp: manage sink power state if possible

[Jesse Barnes]

On sinks with a DPCD rev of 1.1 or greater, we can send sink power
management commands to address 0x600 per section 5.1.5 of the
DisplayPort 1.1a spec.

Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
---
 drivers/gpu/drm/i915/intel_dp.c |   35 +++++++++++++++++++++++++++++++++++
 1 files changed, 35 insertions(+), 0 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index bca48b0..e2aced6 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -942,11 +942,44 @@ static void ironlake_edp_pll_off(struct drm_encoder *encoder)
 	udelay(200);
 }
 
+/* If the sink supports it, try to set the power state appropriately */
+static void intel_dp_sink_dpms(struct intel_dp *intel_dp, int mode)
+{
+	int ret, i;
+
+	/* Should have a valid DPCD by this point */
+	if (intel_dp->dpcd[DP_DPCD_REV] < 0x11)
+		return;
+
+	if (mode != DRM_MODE_DPMS_ON) {
+		ret = intel_dp_aux_native_write_1(intel_dp, DP_SET_POWER,
+						  DP_SET_POWER_D3);
+		if (ret != 1)
+			DRM_DEBUG_DRIVER("failed to write sink power state\n");
+	} else {
+		/*
+		 * When turning on, we need to retry for 1ms to give the sink
+		 * time to wake up.
+		 */
+		for (i = 0; i < 3; i++) {
+			ret = intel_dp_aux_native_write_1(intel_dp,
+							  DP_SET_POWER,
+							  DP_SET_POWER_D0);
+			if (ret == 1)
+				break;
+			msleep(1);
+		}
+	}
+}
+
 static void intel_dp_prepare(struct drm_encoder *encoder)
 {
 	struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
 	struct drm_device *dev = encoder->dev;
 
+	/* Wake up the sink first */
+	intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_ON);
+
 	if (is_edp(intel_dp)) {
 		ironlake_edp_backlight_off(dev);
 		ironlake_edp_panel_off(dev);
@@ -990,6 +1023,7 @@ intel_dp_dpms(struct drm_encoder *encoder, int mode)
 	if (mode != DRM_MODE_DPMS_ON) {
 		if (is_edp(intel_dp))
 			ironlake_edp_backlight_off(dev);
+		intel_dp_sink_dpms(intel_dp, mode);
 		intel_dp_link_down(intel_dp);
 		if (is_edp(intel_dp))
 			ironlake_edp_panel_off(dev);
@@ -998,6 +1032,7 @@ intel_dp_dpms(struct drm_encoder *encoder, int mode)
 	} else {
 		if (is_edp(intel_dp))
 			ironlake_edp_panel_vdd_on(intel_dp);
+		intel_dp_sink_dpms(intel_dp, mode);
 		if (!(dp_reg & DP_PORT_EN)) {
 			intel_dp_start_link_train(intel_dp);
 			if (is_edp(intel_dp)) {
-- 
1.7.4.1

Re: [PATCH 4/7] drm/i915/dp: try to read receiver capabilities 3 times when detecting

[Keith Packard]

[-- Attachment #1.1: Type: text/plain, Size: 542 bytes --]

On Thu,  7 Jul 2011 11:11:00 -0700, Jesse Barnes <jbarnes@virtuousgeek.org> wrote:

> +	for (i = 0; i < 3; i++) {
> +		ret = intel_dp_aux_native_read(intel_dp,
> +					       0x000, intel_dp->dpcd,
> +					       sizeof (intel_dp->dpcd));
> +		if (ret == sizeof(intel_dp->dpcd) &&
> +		    intel_dp->dpcd[DP_DPCD_REV] != 0) {
>  			status = connector_status_connected;
> +			break;
> +		}
> +		msleep(1);
>  	}

Should this use the intel_dp_aux_native_read_retry function from the
later patch?

-- 
keith.packard@intel.com

[-- Attachment #1.2: Type: application/pgp-signature, Size: 189 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

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