[PATCH] drm/i915/dp: Read link status more times when EQ not done

[PATCH] drm/i915/dp: Read link status more times when EQ not done

[Lee, Shawn C]

From: "Lee, Shawn C" <shawn.c.lee@intel.com>

When user space link status, display driver read DPCD register
0x202, 0x203 and 0x204 to identify sink status. When PSR exit
is ongoing before EQ done. Panel will report EQ & symbol lock
not done. Both of them are under progressing at the same time
to cause this issue.

This WA try to read link status more times if EQ not done.
Panel spec request at least 1000us for fast link train when
PSR exit. So driver will wait 1000us~1500us then retrieve
sink link status again.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=99639
TEST=Reboot DUT and no flicking on local display at login screen

Cc: Cooper Chiou <cooper.chiou@intel.com>
Cc: Wei Shun Chen <wei.shun.chang@intel.com>
Cc: Gary C Wang <gary.c.wang@intel.com>
Cc: Jani Nikula <jani.nikula@intel.com>

Signed-off-by: Lee, Shawn C <shawn.c.lee@intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c |   33 ++++++++++++++++++++++++++++-----
 1 file changed, 28 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index e80d620846c8..a9a6ce476438 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -4136,13 +4136,36 @@ static void intel_dp_handle_test_request(struct intel_dp *intel_dp)
 		return;
 
 	/* if link training is requested we should perform it always */
-	if ((intel_dp->compliance.test_type == DP_TEST_LINK_TRAINING) ||
-	    (!drm_dp_channel_eq_ok(link_status, intel_dp->lane_count))) {
-		DRM_DEBUG_KMS("%s: channel EQ not ok, retraining\n",
-			      intel_encoder->base.name);
+	if (intel_dp->compliance.test_type == DP_TEST_LINK_TRAINING) {
+		DRM_DEBUG_KMS("%s: compliance test mode, retraining\n",
+			intel_encoder->base.name);
+	} else {
+		u8 retry;
+
+		for (retry = 0; retry < 3; retry++) {
+			if (!drm_dp_channel_eq_ok(link_status, intel_dp->lane_count)) {
+				/*
+				 * EQ not ok may caused by fast link train while exit PSR active,
+				 * wait at least 1000 us then read it again.
+				 */
+				DRM_DEBUG_KMS("%s: channel EQ not ok, retry = %d, DPCD 0x202 = 0x%x, 0x203 = 0x%x, 0x204 = 0x%x\n",
+					intel_encoder->base.name, retry, link_status[0], link_status[1], link_status[2]);
+				usleep_range(1000, 1500);
+				if (!intel_dp_get_link_status(intel_dp, link_status)) {
+					DRM_ERROR("Failed to get link status\n");
+					return;
+				}
+			} else {
+				/* channel EQ is fine */
+				return;
+			}
+		}
 
-		intel_dp_retrain_link(intel_dp);
+		DRM_DEBUG_KMS("%s: channel EQ not ok, retraining\n",
+			intel_encoder->base.name);
 	}
+
+	intel_dp_retrain_link(intel_dp);
 }
 
 /*
-- 
1.7.9.5

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

Re: [PATCH] drm/i915/dp: Read link status more times when EQ not done

[Jani Nikula]

On Thu, 02 Feb 2017, "Lee, Shawn C" <shawn.c.lee@intel.com> wrote:
> From: "Lee, Shawn C" <shawn.c.lee@intel.com>
>
> When user space link status, display driver read DPCD register
> 0x202, 0x203 and 0x204 to identify sink status. When PSR exit
> is ongoing before EQ done. Panel will report EQ & symbol lock
> not done. Both of them are under progressing at the same time
> to cause this issue.
>
> This WA try to read link status more times if EQ not done.
> Panel spec request at least 1000us for fast link train when
> PSR exit. So driver will wait 1000us~1500us then retrieve
> sink link status again.

I'm not sure about the patch, but no matter what it needs to be rebased
on top of current drm-tip to even be considered.

If you're working around a specific eDP PSR issue, why do this on *all*
DP?

BR,
Jani.


>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=99639
> TEST=Reboot DUT and no flicking on local display at login screen
>
> Cc: Cooper Chiou <cooper.chiou@intel.com>
> Cc: Wei Shun Chen <wei.shun.chang@intel.com>
> Cc: Gary C Wang <gary.c.wang@intel.com>
> Cc: Jani Nikula <jani.nikula@intel.com>
>
> Signed-off-by: Lee, Shawn C <shawn.c.lee@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_dp.c |   33 ++++++++++++++++++++++++++++-----
>  1 file changed, 28 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index e80d620846c8..a9a6ce476438 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -4136,13 +4136,36 @@ static void intel_dp_handle_test_request(struct intel_dp *intel_dp)
>  		return;
>  
>  	/* if link training is requested we should perform it always */
> -	if ((intel_dp->compliance.test_type == DP_TEST_LINK_TRAINING) ||
> -	    (!drm_dp_channel_eq_ok(link_status, intel_dp->lane_count))) {
> -		DRM_DEBUG_KMS("%s: channel EQ not ok, retraining\n",
> -			      intel_encoder->base.name);
> +	if (intel_dp->compliance.test_type == DP_TEST_LINK_TRAINING) {
> +		DRM_DEBUG_KMS("%s: compliance test mode, retraining\n",
> +			intel_encoder->base.name);
> +	} else {
> +		u8 retry;
> +
> +		for (retry = 0; retry < 3; retry++) {
> +			if (!drm_dp_channel_eq_ok(link_status, intel_dp->lane_count)) {
> +				/*
> +				 * EQ not ok may caused by fast link train while exit PSR active,
> +				 * wait at least 1000 us then read it again.
> +				 */
> +				DRM_DEBUG_KMS("%s: channel EQ not ok, retry = %d, DPCD 0x202 = 0x%x, 0x203 = 0x%x, 0x204 = 0x%x\n",
> +					intel_encoder->base.name, retry, link_status[0], link_status[1], link_status[2]);
> +				usleep_range(1000, 1500);
> +				if (!intel_dp_get_link_status(intel_dp, link_status)) {
> +					DRM_ERROR("Failed to get link status\n");
> +					return;
> +				}
> +			} else {
> +				/* channel EQ is fine */
> +				return;
> +			}
> +		}
>  
> -		intel_dp_retrain_link(intel_dp);
> +		DRM_DEBUG_KMS("%s: channel EQ not ok, retraining\n",
> +			intel_encoder->base.name);
>  	}
> +
> +	intel_dp_retrain_link(intel_dp);
>  }
>  
>  /*

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

[PATCH] drm/i915/dp: Read link status more times when EQ not done

[Lee, Shawn C]

From: "Lee, Shawn C" <shawn.c.lee@intel.com>

Display driver read DPCD register 0x202, 0x203 and 0x204 to identify
eDP sink status.If PSR exit is ongoing at eDP sink, and eDP source
read these registers at the same time. Panel will report EQ & symbol
lock not done. It will cause panel display flicking.

Try to read link status more times if eDP EQ not done. Panel side
request at least 1000us for fast link train while doing PSR exit.
So wait more than 1000us then retrieve sink's status again.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=99639
TEST=Reboot DUT and no flicking on local display at login screen

Cc: Cooper Chiou <cooper.chiou@intel.com>
Cc: Wei Shun Chen <wei.shun.chang@intel.com>
Cc: Gary C Wang <gary.c.wang@intel.com>
Cc: Jani Nikula <jani.nikula@intel.com>

Signed-off-by: Lee, Shawn C <shawn.c.lee@intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c |   22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 024798a9c016..f6229d616971 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -4225,6 +4225,7 @@ static void intel_dp_handle_test_request(struct intel_dp *intel_dp)
 {
 	struct intel_encoder *intel_encoder = &dp_to_dig_port(intel_dp)->base;
 	struct drm_device *dev = intel_dp_to_dev(intel_dp);
+	struct drm_i915_private *dev_priv = dev->dev_private;
 	u8 link_status[DP_LINK_STATUS_SIZE];
 
 	WARN_ON(!drm_modeset_is_locked(&dev->mode_config.connection_mutex));
@@ -4247,6 +4248,27 @@ static void intel_dp_handle_test_request(struct intel_dp *intel_dp)
 
 	/* Retrain if Channel EQ or CR not ok */
 	if (!drm_dp_channel_eq_ok(link_status, intel_dp->lane_count)) {
+		if (is_edp(intel_dp) && dev_priv->psr.enabled) {
+			u8 retry;
+
+			for (retry = 0; retry < 3; retry++) {
+				/*
+				 * EQ not ok may caused by fast link train while exit PSR active,
+				 * wait at least 1000 us then read it again.
+				 */
+				DRM_DEBUG_KMS("%s: channel EQ not ok, retry = %d, DPCD 0x202 = 0x%x, 0x203 = 0x%x, 0x204 = 0x%x\n",
+					intel_encoder->base.name, retry, link_status[0], link_status[1], link_status[2]);
+				usleep_range(1000, 1500);
+				if (!intel_dp_get_link_status(intel_dp, link_status)) {
+					DRM_ERROR("Failed to get link status\n");
+					return;
+				}
+
+				if (drm_dp_channel_eq_ok(link_status, intel_dp->lane_count))
+					return;
+			}
+		}
+
 		DRM_DEBUG_KMS("%s: channel EQ not ok, retraining\n",
 			      intel_encoder->base.name);
 
-- 
1.7.9.5

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

[PATCH v2] drm/i915/dp: Read link status more times when EQ not done

[Lee, Shawn C]

From: "Lee, Shawn C" <shawn.c.lee@intel.com>

Display driver read DPCD register 0x202, 0x203 and 0x204 to identify
eDP sink status.If PSR exit is ongoing at eDP sink, and eDP source
read these registers at the same time. Panel will report EQ & symbol
lock not done. It will cause panel display flicking.

Try to read link status more times if eDP EQ not done. Panel side
request at least 1000us for fast link train while doing PSR exit.
So wait more than 1000us then retrieve sink's status again.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=99639
TEST=Reboot DUT and no flicking on local display at login screen

Cc: Cooper Chiou <cooper.chiou@intel.com>
Cc: Wei Shun Chen <wei.shun.chang@intel.com>
Cc: Gary C Wang <gary.c.wang@intel.com>
Cc: Jani Nikula <jani.nikula@intel.com>

Signed-off-by: Lee, Shawn C <shawn.c.lee@intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c |   22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 024798a9c016..f6229d616971 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -4225,6 +4225,7 @@ static void intel_dp_handle_test_request(struct intel_dp *intel_dp)
 {
 	struct intel_encoder *intel_encoder = &dp_to_dig_port(intel_dp)->base;
 	struct drm_device *dev = intel_dp_to_dev(intel_dp);
+	struct drm_i915_private *dev_priv = dev->dev_private;
 	u8 link_status[DP_LINK_STATUS_SIZE];
 
 	WARN_ON(!drm_modeset_is_locked(&dev->mode_config.connection_mutex));
@@ -4247,6 +4248,27 @@ static void intel_dp_handle_test_request(struct intel_dp *intel_dp)
 
 	/* Retrain if Channel EQ or CR not ok */
 	if (!drm_dp_channel_eq_ok(link_status, intel_dp->lane_count)) {
+		if (is_edp(intel_dp) && dev_priv->psr.enabled) {
+			u8 retry;
+
+			for (retry = 0; retry < 3; retry++) {
+				/*
+				 * EQ not ok may caused by fast link train while exit PSR active,
+				 * wait at least 1000 us then read it again.
+				 */
+				DRM_DEBUG_KMS("%s: channel EQ not ok, retry = %d, DPCD 0x202 = 0x%x, 0x203 = 0x%x, 0x204 = 0x%x\n",
+					intel_encoder->base.name, retry, link_status[0], link_status[1], link_status[2]);
+				usleep_range(1000, 1500);
+				if (!intel_dp_get_link_status(intel_dp, link_status)) {
+					DRM_ERROR("Failed to get link status\n");
+					return;
+				}
+
+				if (drm_dp_channel_eq_ok(link_status, intel_dp->lane_count))
+					return;
+			}
+		}
+
 		DRM_DEBUG_KMS("%s: channel EQ not ok, retraining\n",
 			      intel_encoder->base.name);
 
-- 
1.7.9.5

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

Re: [PATCH v2] drm/i915/dp: Read link status more times when EQ not done

[Jani Nikula]

On Thu, 02 Mar 2017, "Lee, Shawn C" <shawn.c.lee@intel.com> wrote:
> From: "Lee, Shawn C" <shawn.c.lee@intel.com>
>
> Display driver read DPCD register 0x202, 0x203 and 0x204 to identify
> eDP sink status.If PSR exit is ongoing at eDP sink, and eDP source
> read these registers at the same time. Panel will report EQ & symbol
> lock not done. It will cause panel display flicking.
>
> Try to read link status more times if eDP EQ not done. Panel side
> request at least 1000us for fast link train while doing PSR exit.
> So wait more than 1000us then retrieve sink's status again.
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=99639
> TEST=Reboot DUT and no flicking on local display at login screen
>
> Cc: Cooper Chiou <cooper.chiou@intel.com>
> Cc: Wei Shun Chen <wei.shun.chang@intel.com>
> Cc: Gary C Wang <gary.c.wang@intel.com>
> Cc: Jani Nikula <jani.nikula@intel.com>
>
> Signed-off-by: Lee, Shawn C <shawn.c.lee@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_dp.c |   22 ++++++++++++++++++++++
>  1 file changed, 22 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 024798a9c016..f6229d616971 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -4225,6 +4225,7 @@ static void intel_dp_handle_test_request(struct intel_dp *intel_dp)
>  {
>  	struct intel_encoder *intel_encoder = &dp_to_dig_port(intel_dp)->base;
>  	struct drm_device *dev = intel_dp_to_dev(intel_dp);
> +	struct drm_i915_private *dev_priv = dev->dev_private;
>  	u8 link_status[DP_LINK_STATUS_SIZE];
>  
>  	WARN_ON(!drm_modeset_is_locked(&dev->mode_config.connection_mutex));
> @@ -4247,6 +4248,27 @@ static void intel_dp_handle_test_request(struct intel_dp *intel_dp)
>  
>  	/* Retrain if Channel EQ or CR not ok */
>  	if (!drm_dp_channel_eq_ok(link_status, intel_dp->lane_count)) {
> +		if (is_edp(intel_dp) && dev_priv->psr.enabled) {
> +			u8 retry;
> +
> +			for (retry = 0; retry < 3; retry++) {
> +				/*
> +				 * EQ not ok may caused by fast link train while exit PSR active,
> +				 * wait at least 1000 us then read it again.
> +				 */
> +				DRM_DEBUG_KMS("%s: channel EQ not ok, retry = %d, DPCD 0x202 = 0x%x, 0x203 = 0x%x, 0x204 = 0x%x\n",
> +					intel_encoder->base.name, retry, link_status[0], link_status[1], link_status[2]);
> +				usleep_range(1000, 1500);
> +				if (!intel_dp_get_link_status(intel_dp, link_status)) {
> +					DRM_ERROR("Failed to get link status\n");
> +					return;
> +				}
> +
> +				if (drm_dp_channel_eq_ok(link_status, intel_dp->lane_count))
> +					return;
> +			}
> +		}

Sorry, I'm really not fond of how this basically duplicates the entire
intel_dp_check_link_status() function for the retry. Looks like you
should move the intel_dp_get_link_status() call closer to the
drm_dp_channel_eq_ok() call in the function, and abstract them into a
separate helper. Then call the helper in a loop, but only really repeat
for eDP and PSR active, settle for one try otherwise.

But before you do, please check with Rodrigo (Cc'd) if he thinks the
idea is sane to begin with.

BR,
Jani.



> +
>  		DRM_DEBUG_KMS("%s: channel EQ not ok, retraining\n",
>  			      intel_encoder->base.name);

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

[PATCH v3] drm/i915/dp: Read link status more times when EQ not done

[Lee, Shawn C]

From: "Lee, Shawn C" <shawn.c.lee@intel.com>

Display driver read DPCD register 0x202, 0x203 and 0x204 to identify
eDP sink status.If PSR exit is ongoing at eDP sink, and eDP source
read these registers at the same time. Panel will report EQ & symbol
lock not done. It will cause panel display flicking.

Try to read link status more times if eDP EQ not done. Panel side
request at least 1000us for fast link train while doing PSR exit.
So wait more than 1000us then retrieve sink's status again.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=99639
TEST=Reboot DUT and no flicking on local display at login screen

Cc: Cooper Chiou <cooper.chiou@intel.com>
Cc: Wei Shun Chen <wei.shun.chang@intel.com>
Cc: Gary C Wang <gary.c.wang@intel.com>
Cc: Jani Nikula <jani.nikula@intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>

Signed-off-by: Lee, Shawn C <shawn.c.lee@intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c |   34 ++++++++++++++++++++++++----------
 1 file changed, 24 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 024798a9c016..d50827a92aa2 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -4225,15 +4225,11 @@ static void intel_dp_handle_test_request(struct intel_dp *intel_dp)
 {
 	struct intel_encoder *intel_encoder = &dp_to_dig_port(intel_dp)->base;
 	struct drm_device *dev = intel_dp_to_dev(intel_dp);
-	u8 link_status[DP_LINK_STATUS_SIZE];
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	u8 link_status[DP_LINK_STATUS_SIZE], retry = 1;
 
 	WARN_ON(!drm_modeset_is_locked(&dev->mode_config.connection_mutex));
 
-	if (!intel_dp_get_link_status(intel_dp, link_status)) {
-		DRM_ERROR("Failed to get link status\n");
-		return;
-	}
-
 	if (!intel_encoder->base.crtc)
 		return;
 
@@ -4245,13 +4241,31 @@ static void intel_dp_handle_test_request(struct intel_dp *intel_dp)
 	if (!intel_dp->lane_count)
 		return;
 
+	if (is_edp(intel_dp) && dev_priv->psr.enabled) 
+		retry = 3;
+
 	/* Retrain if Channel EQ or CR not ok */
-	if (!drm_dp_channel_eq_ok(link_status, intel_dp->lane_count)) {
-		DRM_DEBUG_KMS("%s: channel EQ not ok, retraining\n",
-			      intel_encoder->base.name);
+	while ( retry-- ) {
+		if (!intel_dp_get_link_status(intel_dp, link_status)) {
+			DRM_ERROR("Failed to get link status\n");
+			return;
+		}
 
-		intel_dp_retrain_link(intel_dp);
+		if (drm_dp_channel_eq_ok(link_status, intel_dp->lane_count))
+			return;
+
+		/*
+		 * EQ not ok may caused by fast link train while exit PSR active,
+		 * wait at least 1000 us then read it again.
+		 */
+		if (retry)
+			usleep_range(1000, 1500);
 	}
+
+	DRM_DEBUG_KMS("%s: channel EQ not ok, retraining\n",
+		intel_encoder->base.name);
+
+	intel_dp_retrain_link(intel_dp);
 }
 
 /*
-- 
1.7.9.5

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

Re: [PATCH v3] drm/i915/dp: Read link status more times when EQ not done

[Rodrigo Vivi]

On Mon, Mar 13, 2017 at 1:12 AM, Lee, Shawn C <shawn.c.lee@intel.com> wrote:
> From: "Lee, Shawn C" <shawn.c.lee@intel.com>
>
> Display driver read DPCD register 0x202, 0x203 and 0x204 to identify
> eDP sink status.If PSR exit is ongoing at eDP sink, and eDP source
> read these registers at the same time. Panel will report EQ & symbol
> lock not done. It will cause panel display flicking.
>
> Try to read link status more times if eDP EQ not done. Panel side
> request at least 1000us for fast link train while doing PSR exit.
> So wait more than 1000us then retrieve sink's status again.

it is missing a v2 and v3 here with explanations on the changes.
It was hard to follow the changes.

>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=99639
> TEST=Reboot DUT and no flicking on local display at login screen
>
> Cc: Cooper Chiou <cooper.chiou@intel.com>
> Cc: Wei Shun Chen <wei.shun.chang@intel.com>
> Cc: Gary C Wang <gary.c.wang@intel.com>
> Cc: Jani Nikula <jani.nikula@intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
>
> Signed-off-by: Lee, Shawn C <shawn.c.lee@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_dp.c |   34 ++++++++++++++++++++++++----------
>  1 file changed, 24 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 024798a9c016..d50827a92aa2 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -4225,15 +4225,11 @@ static void intel_dp_handle_test_request(struct intel_dp *intel_dp)
>  {
>         struct intel_encoder *intel_encoder = &dp_to_dig_port(intel_dp)->base;
>         struct drm_device *dev = intel_dp_to_dev(intel_dp);
> -       u8 link_status[DP_LINK_STATUS_SIZE];
> +       struct drm_i915_private *dev_priv = dev->dev_private;
> +       u8 link_status[DP_LINK_STATUS_SIZE], retry = 1;
>
>         WARN_ON(!drm_modeset_is_locked(&dev->mode_config.connection_mutex));
>
> -       if (!intel_dp_get_link_status(intel_dp, link_status)) {
> -               DRM_ERROR("Failed to get link status\n");
> -               return;
> -       }
> -
>         if (!intel_encoder->base.crtc)
>                 return;
>
> @@ -4245,13 +4241,31 @@ static void intel_dp_handle_test_request(struct intel_dp *intel_dp)
>         if (!intel_dp->lane_count)
>                 return;
>
> +       if (is_edp(intel_dp) && dev_priv->psr.enabled)
> +               retry = 3;
> +
>         /* Retrain if Channel EQ or CR not ok */
> -       if (!drm_dp_channel_eq_ok(link_status, intel_dp->lane_count)) {
> -               DRM_DEBUG_KMS("%s: channel EQ not ok, retraining\n",
> -                             intel_encoder->base.name);
> +       while ( retry-- ) {
> +               if (!intel_dp_get_link_status(intel_dp, link_status)) {
> +                       DRM_ERROR("Failed to get link status\n");
> +                       return;

Well, if link status is not ok you return without retrying, so, why is
this here?

> +               }
>
> -               intel_dp_retrain_link(intel_dp);
> +               if (drm_dp_channel_eq_ok(link_status, intel_dp->lane_count))
> +                       return;
> +
> +               /*
> +                * EQ not ok may caused by fast link train while exit PSR active,
> +                * wait at least 1000 us then read it again.
> +                */
> +               if (retry)
> +                       usleep_range(1000, 1500);

maybe this retry is randomly just masking the real issue.
Jim recently found out that on psr enable we are clearing a bit that
we should never touch by spec.
I'd try Jim's patch(es) first to see if they solve the issue for you.

>         }
> +
> +       DRM_DEBUG_KMS("%s: channel EQ not ok, retraining\n",
> +               intel_encoder->base.name);
> +
> +       intel_dp_retrain_link(intel_dp);
>  }
>
>  /*
> --
> 1.7.9.5
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Rodrigo Vivi
Blog: http://blog.vivi.eng.br
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v3] drm/i915/dp: Read link status more times when EQ not done

[Jim Bride]

On Fri, Mar 31, 2017 at 04:25:31PM -0700, Rodrigo Vivi wrote:
> On Mon, Mar 13, 2017 at 1:12 AM, Lee, Shawn C <shawn.c.lee@intel.com> wrote:
> > From: "Lee, Shawn C" <shawn.c.lee@intel.com>
> >
> > Display driver read DPCD register 0x202, 0x203 and 0x204 to identify
> > eDP sink status.If PSR exit is ongoing at eDP sink, and eDP source
> > read these registers at the same time. Panel will report EQ & symbol
> > lock not done. It will cause panel display flicking.
> >
> > Try to read link status more times if eDP EQ not done. Panel side
> > request at least 1000us for fast link train while doing PSR exit.
> > So wait more than 1000us then retrieve sink's status again.
> 
> it is missing a v2 and v3 here with explanations on the changes.
> It was hard to follow the changes.
> 
> >
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=99639
> > TEST=Reboot DUT and no flicking on local display at login screen
> >
> > Cc: Cooper Chiou <cooper.chiou@intel.com>
> > Cc: Wei Shun Chen <wei.shun.chang@intel.com>
> > Cc: Gary C Wang <gary.c.wang@intel.com>
> > Cc: Jani Nikula <jani.nikula@intel.com>
> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> >
> > Signed-off-by: Lee, Shawn C <shawn.c.lee@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_dp.c |   34 ++++++++++++++++++++++++----------
> >  1 file changed, 24 insertions(+), 10 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> > index 024798a9c016..d50827a92aa2 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -4225,15 +4225,11 @@ static void intel_dp_handle_test_request(struct intel_dp *intel_dp)
> >  {
> >         struct intel_encoder *intel_encoder = &dp_to_dig_port(intel_dp)->base;
> >         struct drm_device *dev = intel_dp_to_dev(intel_dp);
> > -       u8 link_status[DP_LINK_STATUS_SIZE];
> > +       struct drm_i915_private *dev_priv = dev->dev_private;
> > +       u8 link_status[DP_LINK_STATUS_SIZE], retry = 1;
> >
> >         WARN_ON(!drm_modeset_is_locked(&dev->mode_config.connection_mutex));
> >
> > -       if (!intel_dp_get_link_status(intel_dp, link_status)) {
> > -               DRM_ERROR("Failed to get link status\n");
> > -               return;
> > -       }
> > -
> >         if (!intel_encoder->base.crtc)
> >                 return;
> >
> > @@ -4245,13 +4241,31 @@ static void intel_dp_handle_test_request(struct intel_dp *intel_dp)
> >         if (!intel_dp->lane_count)
> >                 return;
> >
> > +       if (is_edp(intel_dp) && dev_priv->psr.enabled)
> > +               retry = 3;
> > +
> >         /* Retrain if Channel EQ or CR not ok */
> > -       if (!drm_dp_channel_eq_ok(link_status, intel_dp->lane_count)) {
> > -               DRM_DEBUG_KMS("%s: channel EQ not ok, retraining\n",
> > -                             intel_encoder->base.name);
> > +       while ( retry-- ) {
> > +               if (!intel_dp_get_link_status(intel_dp, link_status)) {
> > +                       DRM_ERROR("Failed to get link status\n");
> > +                       return;
> 
> Well, if link status is not ok you return without retrying, so, why is
> this here?
> 
> > +               }
> >
> > -               intel_dp_retrain_link(intel_dp);
> > +               if (drm_dp_channel_eq_ok(link_status, intel_dp->lane_count))
> > +                       return;
> > +
> > +               /*
> > +                * EQ not ok may caused by fast link train while exit PSR active,
> > +                * wait at least 1000 us then read it again.
> > +                */
> > +               if (retry)
> > +                       usleep_range(1000, 1500);
> 
> maybe this retry is randomly just masking the real issue.
> Jim recently found out that on psr enable we are clearing a bit that
> we should never touch by spec.
> I'd try Jim's patch(es) first to see if they solve the issue for you.

Actually, I'd go one better.  If you look at the code, we should
never get to running the actual link status checks due to the
following snippet a few lines up.

       if (!to_intel_crtc(intel_encoder->base.crtc)->active)
                       return;
		       
Basically, we only check the link status if the pipe is not active.
If the pipe isn't active, then PSR should be disabled.  If it's not,
then that's a scarier problem.  I've been tempted to throw a WARN_ON
in intel_dp_start_link_training() to ensure that PSR is off before
we start modifying the link state, because it really messes
with the re-syncing done at PSR exit time (which this patch was
trying to compensate for.)  In any event, assuming the pipe is
disabled, then PSR shouldn't be an issue.

The patch that Rodrigo is referring to is
https://patchwork.freedesktop.org/patch/147942/ and it could
certainly help if on SKL+.

Jim


> 
> >         }
> > +
> > +       DRM_DEBUG_KMS("%s: channel EQ not ok, retraining\n",
> > +               intel_encoder->base.name);
> > +
> > +       intel_dp_retrain_link(intel_dp);
> >  }
> >
> >  /*
> > --
> > 1.7.9.5
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> 
> 
> -- 
> Rodrigo Vivi
> Blog: http://blog.vivi.eng.br
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v4] drm/i915/edp: Read link status more times when EQ not done

[Lee, Shawn C]

From: "Lee, Shawn C" <shawn.c.lee@intel.com>

Display driver read DPCD register 0x202, 0x203 and 0x204 to identify
eDP sink status.If PSR exit is ongoing at eDP sink, and eDP source
read these registers at the same time. Panel will report EQ & symbol
lock not done. It will cause panel display flicking.
Try to read link status more times if eDP EQ not done. Panel side
request at least 1000us for fast link train while doing PSR exit.
So wait more than 1000us then retrieve sink's status again.
Change log:
v2:
- modify this mechanism just used for eDP + PSR enabled.
v3:
- move the intel_dp_get_link_status() call closer to the
  drm_dp_channel_eq_ok()
v4:
- Rebase / minor typo fix.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=99639
TEST=Reboot DUT and no flicking on local display at login screen

Cc: Cooper Chiou <cooper.chiou@intel.com>
Cc: Gary C Wang <gary.c.wang@intel.com>
Cc: Jani Nikula <jani.nikula@intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: Jim Bride <jim.bride@intel.com>

Signed-off-by: Shawn Lee <shawn.c.lee@intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c |   36 +++++++++++++++++++++++++-----------
 1 file changed, 25 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 08834f74d396..7d2b68c538cf 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -4256,15 +4256,11 @@ static void intel_dp_handle_test_request(struct intel_dp *intel_dp)
 {
 	struct intel_encoder *intel_encoder = &dp_to_dig_port(intel_dp)->base;
 	struct drm_device *dev = intel_dp_to_dev(intel_dp);
-	u8 link_status[DP_LINK_STATUS_SIZE];
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	u8 link_status[DP_LINK_STATUS_SIZE], retry = 1;
 
 	WARN_ON(!drm_modeset_is_locked(&dev->mode_config.connection_mutex));
 
-	if (!intel_dp_get_link_status(intel_dp, link_status)) {
-		DRM_ERROR("Failed to get link status\n");
-		return;
-	}
-
 	if (!intel_encoder->base.crtc)
 		return;
 
@@ -4278,13 +4274,31 @@ static void intel_dp_handle_test_request(struct intel_dp *intel_dp)
 	if (!intel_dp_link_params_valid(intel_dp))
 		return;
 
-	/* Retrain if Channel EQ or CR not ok */
-	if (!drm_dp_channel_eq_ok(link_status, intel_dp->lane_count)) {
-		DRM_DEBUG_KMS("%s: channel EQ not ok, retraining\n",
-			      intel_encoder->base.name);
+	if (is_edp(intel_dp) && dev_priv->psr.enabled)
+		retry = 3;
+
+	while (retry--) {
+		if (!intel_dp_get_link_status(intel_dp, link_status)) {
+			DRM_ERROR("Failed to get link status\n");
+			return;
+		}
 
-		intel_dp_retrain_link(intel_dp);
+		if (drm_dp_channel_eq_ok(link_status, intel_dp->lane_count))
+			return;
+
+		/*
+		 * EQ not ok may caused by fast link train while exit PSR active,
+		 * wait at least 1000 us then read it again.
+		 */
+		if (retry)
+			usleep_range(1000, 1500);
 	}
+
+	/* Retrain if Channel EQ or CR not ok */
+	DRM_DEBUG_KMS("%s: channel EQ not ok, retraining\n",
+	      intel_encoder->base.name);
+
+	intel_dp_retrain_link(intel_dp);
 }
 
 /*
-- 
1.7.9.5

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

✗ Fi.CI.BAT: warning for drm/i915/dp: Read link status more times when EQ not done (rev3)

[Patchwork]

== Series Details ==

Series: drm/i915/dp: Read link status more times when EQ not done (rev3)
URL   : https://patchwork.freedesktop.org/series/18982/
State : warning

== Summary ==

Series 18982v3 drm/i915/dp: Read link status more times when EQ not done
https://patchwork.freedesktop.org/api/1.0/series/18982/revisions/3/mbox/

Test kms_pipe_crc_basic:
        Subgroup nonblocking-crc-pipe-a:
                pass       -> DMESG-WARN (fi-ilk-650)

fi-bdw-5557u     total:278  pass:267  dwarn:0   dfail:0   fail:0   skip:11 
fi-bsw-n3050     total:278  pass:239  dwarn:0   dfail:0   fail:0   skip:39 
fi-bxt-j4205     total:278  pass:259  dwarn:0   dfail:0   fail:0   skip:19 
fi-bxt-t5700     total:278  pass:258  dwarn:0   dfail:0   fail:0   skip:20 
fi-byt-j1900     total:278  pass:251  dwarn:0   dfail:0   fail:0   skip:27 
fi-byt-n2820     total:278  pass:247  dwarn:0   dfail:0   fail:0   skip:31 
fi-hsw-4770      total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16 
fi-hsw-4770r     total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16 
fi-ilk-650       total:278  pass:227  dwarn:1   dfail:0   fail:0   skip:50 
fi-ivb-3520m     total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18 
fi-ivb-3770      total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18 
fi-kbl-7500u     total:278  pass:259  dwarn:1   dfail:0   fail:0   skip:18 
fi-skl-6260u     total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10 
fi-skl-6700hq    total:278  pass:261  dwarn:0   dfail:0   fail:0   skip:17 
fi-skl-6700k     total:278  pass:256  dwarn:4   dfail:0   fail:0   skip:18 
fi-skl-6770hq    total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10 
fi-snb-2520m     total:278  pass:250  dwarn:0   dfail:0   fail:0   skip:28 
fi-snb-2600      total:278  pass:248  dwarn:0   dfail:0   fail:1   skip:29 

a8edd566a8e63f24bde1c074578b7a018fe8cdc9 drm-tip: 2017y-03m-02d-08h-19m-33s UTC integration manifest
5e6e2a6 drm/i915/dp: Read link status more times when EQ not done

== Logs ==

For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_4027/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.BAT: success for drm/i915/dp: Read link status more times when EQ not done (rev4)

[Patchwork]

== Series Details ==

Series: drm/i915/dp: Read link status more times when EQ not done (rev4)
URL   : https://patchwork.freedesktop.org/series/18982/
State : success

== Summary ==

Series 18982v4 drm/i915/dp: Read link status more times when EQ not done
https://patchwork.freedesktop.org/api/1.0/series/18982/revisions/4/mbox/

Test gem_exec_flush:
        Subgroup basic-batch-kernel-default-uc:
                pass       -> FAIL       (fi-snb-2600) fdo#100007
Test kms_force_connector_basic:
        Subgroup prune-stale-modes:
                skip       -> PASS       (fi-snb-2520m)
Test kms_pipe_crc_basic:
        Subgroup suspend-read-crc-pipe-a:
                pass       -> DMESG-WARN (fi-byt-n2820) fdo#100126

fdo#100007 https://bugs.freedesktop.org/show_bug.cgi?id=100007
fdo#100126 https://bugs.freedesktop.org/show_bug.cgi?id=100126

fi-bdw-5557u     total:278  pass:267  dwarn:0   dfail:0   fail:0   skip:11  time: 457s
fi-bsw-n3050     total:278  pass:239  dwarn:0   dfail:0   fail:0   skip:39  time: 604s
fi-bxt-j4205     total:278  pass:259  dwarn:0   dfail:0   fail:0   skip:19  time: 531s
fi-bxt-t5700     total:278  pass:258  dwarn:0   dfail:0   fail:0   skip:20  time: 572s
fi-byt-j1900     total:278  pass:251  dwarn:0   dfail:0   fail:0   skip:27  time: 505s
fi-byt-n2820     total:278  pass:246  dwarn:1   dfail:0   fail:0   skip:31  time: 510s
fi-hsw-4770      total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  time: 436s
fi-hsw-4770r     total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  time: 432s
fi-ilk-650       total:278  pass:228  dwarn:0   dfail:0   fail:0   skip:50  time: 442s
fi-ivb-3520m     total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time: 508s
fi-ivb-3770      total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time: 494s
fi-kbl-7500u     total:278  pass:259  dwarn:1   dfail:0   fail:0   skip:18  time: 476s
fi-skl-6260u     total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time: 500s
fi-skl-6700hq    total:278  pass:261  dwarn:0   dfail:0   fail:0   skip:17  time: 590s
fi-skl-6700k     total:278  pass:256  dwarn:4   dfail:0   fail:0   skip:18  time: 495s
fi-skl-6770hq    total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time: 533s
fi-snb-2520m     total:278  pass:250  dwarn:0   dfail:0   fail:0   skip:28  time: 552s
fi-snb-2600      total:278  pass:248  dwarn:0   dfail:0   fail:1   skip:29  time: 419s

2cb12884655eab41d4992b33ccb36c609c4537d3 drm-tip: 2017y-03m-12d-13h-00m-05s UTC integration manifest
e91cd55 drm/i915/dp: Read link status more times when EQ not done

== Logs ==

For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_4148/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.BAT: success for drm/i915/dp: Read link status more times when EQ not done (rev5)

[Patchwork]

== Series Details ==

Series: drm/i915/dp: Read link status more times when EQ not done (rev5)
URL   : https://patchwork.freedesktop.org/series/18982/
State : success

== Summary ==

Series 18982v5 drm/i915/dp: Read link status more times when EQ not done
https://patchwork.freedesktop.org/api/1.0/series/18982/revisions/5/mbox/

Test gem_exec_suspend:
        Subgroup basic-s4-devices:
                pass       -> DMESG-WARN (fi-kbl-7560u) fdo#100125

fdo#100125 https://bugs.freedesktop.org/show_bug.cgi?id=100125

fi-bdw-5557u     total:278  pass:267  dwarn:0   dfail:0   fail:0   skip:11  time:436s
fi-bdw-gvtdvm    total:278  pass:256  dwarn:8   dfail:0   fail:0   skip:14  time:427s
fi-bsw-n3050     total:278  pass:242  dwarn:0   dfail:0   fail:0   skip:36  time:571s
fi-bxt-j4205     total:278  pass:259  dwarn:0   dfail:0   fail:0   skip:19  time:512s
fi-bxt-t5700     total:278  pass:258  dwarn:0   dfail:0   fail:0   skip:20  time:556s
fi-byt-j1900     total:278  pass:254  dwarn:0   dfail:0   fail:0   skip:24  time:479s
fi-byt-n2820     total:278  pass:250  dwarn:0   dfail:0   fail:0   skip:28  time:482s
fi-hsw-4770      total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  time:415s
fi-hsw-4770r     total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  time:403s
fi-ilk-650       total:278  pass:228  dwarn:0   dfail:0   fail:0   skip:50  time:427s
fi-ivb-3520m     total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time:486s
fi-ivb-3770      total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time:464s
fi-kbl-7500u     total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time:455s
fi-kbl-7560u     total:278  pass:267  dwarn:1   dfail:0   fail:0   skip:10  time:570s
fi-skl-6260u     total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time:456s
fi-skl-6700hq    total:278  pass:261  dwarn:0   dfail:0   fail:0   skip:17  time:577s
fi-skl-6700k     total:278  pass:256  dwarn:4   dfail:0   fail:0   skip:18  time:455s
fi-skl-6770hq    total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time:490s
fi-skl-gvtdvm    total:278  pass:265  dwarn:0   dfail:0   fail:0   skip:13  time:426s
fi-snb-2520m     total:278  pass:250  dwarn:0   dfail:0   fail:0   skip:28  time:531s
fi-snb-2600      total:278  pass:249  dwarn:0   dfail:0   fail:0   skip:29  time:401s

2c92b47c7d4fb987555114dcbef02faa42100fb5 drm-tip: 2017y-04m-24d-14h-55m-24s UTC integration manifest
f205889 drm/i915/edp: Read link status more times when EQ not done

== Logs ==

For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_4540/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx