All of lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH] drm/i915/edp: Read link status after exit PSR
  2017-04-27 14:35 [PATCH] drm/i915/edp: Read link status after exit PSR Lee, Shawn C
@ 2017-04-27 14:16 ` Chris Wilson
  2017-04-27 14:38 ` Ville Syrjälä
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 13+ messages in thread
From: Chris Wilson @ 2017-04-27 14:16 UTC (permalink / raw)
  To: Lee, Shawn C
  Cc: Cooper Chiou, Jim Bride, Jani Nikula, intel-gfx, Rodrigo Vivi, Ryan Lin

On Thu, Apr 27, 2017 at 10:35:22PM +0800, Lee, Shawn C 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.
> So driver have to make sure PSR already exit before read link status.
> 
> 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: Jani Nikula <jani.nikula@intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Cc: Jim Bride <jim.bride@intel.com>
> Cc: Ryan Lin <ryan.lin@intel.com>
> 
> Signed-off-by: Shawn Lee <shawn.c.lee@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_dp.c |   34 +++++++++++++++++++++++++++++-----
>  1 file changed, 29 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 08834f74d396..cc431337b2dc 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -4252,19 +4252,35 @@ static void intel_dp_handle_test_request(struct intel_dp *intel_dp)
>  }
>  
>  static void
> +intel_edp_wait_PSR_exit(struct intel_dp *intel_dp)
> +{
> +	struct drm_device *dev = intel_dp_to_dev(intel_dp);
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	u32 srd_status, count = 100;
> +
> +	while (count--) {
> +		srd_status = I915_READ(EDP_PSR_STATUS_CTL);
> +
> +		if ((srd_status & EDP_PSR_STATUS_SENDING_TP1) ||
> +		    (srd_status & EDP_PSR_STATUS_SENDING_TP2_TP3) ||
> +		    (srd_status & EDP_PSR_STATUS_SENDING_IDLE) ||
> +		    (srd_status & EDP_PSR_STATUS_AUX_SENDING)) {
> +			usleep_range(100, 150);
> +		} else
> +			return;
> +	}

See intel_wait_for_register(i915,
			    EDP_PSR_STATUS_CTL,
			    (EDP_PSR_STATUS_SENDING_TP1 |
			     EDP_PSR_STATUS_SENDING_TP2_TP3 |
			     EDP_PSR_STATUS_SENDING_IDLE |
			     EDP_PSR_STATUS_AUX_SENDING),
			     0,
			     15);
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* [PATCH] drm/i915/edp: Read link status after exit PSR
@ 2017-04-27 14:35 Lee, Shawn C
  2017-04-27 14:16 ` Chris Wilson
                   ` (6 more replies)
  0 siblings, 7 replies; 13+ messages in thread
From: Lee, Shawn C @ 2017-04-27 14:35 UTC (permalink / raw)
  To: intel-gfx; +Cc: Cooper Chiou, Jim Bride, Jani Nikula, Rodrigo Vivi, Ryan Lin

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.
So driver have to make sure PSR already exit before read link status.

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: Jani Nikula <jani.nikula@intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: Jim Bride <jim.bride@intel.com>
Cc: Ryan Lin <ryan.lin@intel.com>

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

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 08834f74d396..cc431337b2dc 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -4252,19 +4252,35 @@ static void intel_dp_handle_test_request(struct intel_dp *intel_dp)
 }
 
 static void
+intel_edp_wait_PSR_exit(struct intel_dp *intel_dp)
+{
+	struct drm_device *dev = intel_dp_to_dev(intel_dp);
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	u32 srd_status, count = 100;
+
+	while (count--) {
+		srd_status = I915_READ(EDP_PSR_STATUS_CTL);
+
+		if ((srd_status & EDP_PSR_STATUS_SENDING_TP1) ||
+		    (srd_status & EDP_PSR_STATUS_SENDING_TP2_TP3) ||
+		    (srd_status & EDP_PSR_STATUS_SENDING_IDLE) ||
+		    (srd_status & EDP_PSR_STATUS_AUX_SENDING)) {
+			usleep_range(100, 150);
+		} else
+			return;
+	}
+}
+
+static void
 intel_dp_check_link_status(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));
 
-	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,6 +4294,14 @@ static void intel_dp_handle_test_request(struct intel_dp *intel_dp)
 	if (!intel_dp_link_params_valid(intel_dp))
 		return;
 
+	if (is_edp(intel_dp) && dev_priv->psr.enabled)
+		intel_edp_wait_PSR_exit(intel_dp);
+
+	if (!intel_dp_get_link_status(intel_dp, link_status)) {
+		DRM_ERROR("Failed to get link status\n");
+		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",
-- 
1.7.9.5

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

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

* Re: [PATCH] drm/i915/edp: Read link status after exit PSR
  2017-04-27 14:35 [PATCH] drm/i915/edp: Read link status after exit PSR Lee, Shawn C
  2017-04-27 14:16 ` Chris Wilson
@ 2017-04-27 14:38 ` Ville Syrjälä
  2017-04-28  3:08   ` Lee, Shawn C
  2017-04-27 14:46 ` ✓ Fi.CI.BAT: success for " Patchwork
                   ` (4 subsequent siblings)
  6 siblings, 1 reply; 13+ messages in thread
From: Ville Syrjälä @ 2017-04-27 14:38 UTC (permalink / raw)
  To: Lee, Shawn C
  Cc: Cooper Chiou, Jim Bride, Jani Nikula, intel-gfx, Rodrigo Vivi, Ryan Lin

On Thu, Apr 27, 2017 at 10:35:22PM +0800, Lee, Shawn C 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.
> So driver have to make sure PSR already exit before read link status.

And what exactly guarantees that it will exit PSR eventually?

> 
> 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: Jani Nikula <jani.nikula@intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Cc: Jim Bride <jim.bride@intel.com>
> Cc: Ryan Lin <ryan.lin@intel.com>
> 
> Signed-off-by: Shawn Lee <shawn.c.lee@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_dp.c |   34 +++++++++++++++++++++++++++++-----
>  1 file changed, 29 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 08834f74d396..cc431337b2dc 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -4252,19 +4252,35 @@ static void intel_dp_handle_test_request(struct intel_dp *intel_dp)
>  }
>  
>  static void
> +intel_edp_wait_PSR_exit(struct intel_dp *intel_dp)
> +{
> +	struct drm_device *dev = intel_dp_to_dev(intel_dp);
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	u32 srd_status, count = 100;
> +
> +	while (count--) {
> +		srd_status = I915_READ(EDP_PSR_STATUS_CTL);
> +
> +		if ((srd_status & EDP_PSR_STATUS_SENDING_TP1) ||
> +		    (srd_status & EDP_PSR_STATUS_SENDING_TP2_TP3) ||
> +		    (srd_status & EDP_PSR_STATUS_SENDING_IDLE) ||
> +		    (srd_status & EDP_PSR_STATUS_AUX_SENDING)) {
> +			usleep_range(100, 150);
> +		} else
> +			return;
> +	}
> +}
> +
> +static void
>  intel_dp_check_link_status(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));
>  
> -	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,6 +4294,14 @@ static void intel_dp_handle_test_request(struct intel_dp *intel_dp)
>  	if (!intel_dp_link_params_valid(intel_dp))
>  		return;
>  
> +	if (is_edp(intel_dp) && dev_priv->psr.enabled)
> +		intel_edp_wait_PSR_exit(intel_dp);
> +
> +	if (!intel_dp_get_link_status(intel_dp, link_status)) {
> +		DRM_ERROR("Failed to get link status\n");
> +		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",
> -- 
> 1.7.9.5
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* ✓ Fi.CI.BAT: success for drm/i915/edp: Read link status after exit PSR
  2017-04-27 14:35 [PATCH] drm/i915/edp: Read link status after exit PSR Lee, Shawn C
  2017-04-27 14:16 ` Chris Wilson
  2017-04-27 14:38 ` Ville Syrjälä
@ 2017-04-27 14:46 ` Patchwork
  2017-04-28  9:58 ` [PATCH v2] drm/i915/edp: Read link status after exit link training Lee, Shawn C
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 13+ messages in thread
From: Patchwork @ 2017-04-27 14:46 UTC (permalink / raw)
  To: Lee, Shawn C; +Cc: intel-gfx

== Series Details ==

Series: drm/i915/edp: Read link status after exit PSR
URL   : https://patchwork.freedesktop.org/series/23631/
State : success

== Summary ==

Series 23631v1 drm/i915/edp: Read link status after exit PSR
https://patchwork.freedesktop.org/api/1.0/series/23631/revisions/1/mbox/

fi-bdw-5557u     total:278  pass:267  dwarn:0   dfail:0   fail:0   skip:11  time:432s
fi-bdw-gvtdvm    total:278  pass:256  dwarn:8   dfail:0   fail:0   skip:14  time:424s
fi-bsw-n3050     total:278  pass:242  dwarn:0   dfail:0   fail:0   skip:36  time:581s
fi-bxt-j4205     total:278  pass:259  dwarn:0   dfail:0   fail:0   skip:19  time:505s
fi-bxt-t5700     total:278  pass:258  dwarn:0   dfail:0   fail:0   skip:20  time:550s
fi-byt-j1900     total:278  pass:254  dwarn:0   dfail:0   fail:0   skip:24  time:485s
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:405s
fi-hsw-4770r     total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  time:405s
fi-ilk-650       total:278  pass:228  dwarn:0   dfail:0   fail:0   skip:50  time:411s
fi-ivb-3520m     total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time:482s
fi-ivb-3770      total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time:472s
fi-kbl-7500u     total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time:459s
fi-kbl-7560u     total:278  pass:267  dwarn:1   dfail:0   fail:0   skip:10  time:564s
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:578s
fi-skl-6700k     total:278  pass:256  dwarn:4   dfail:0   fail:0   skip:18  time:458s
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:431s
fi-snb-2520m     total:278  pass:250  dwarn:0   dfail:0   fail:0   skip:28  time:535s
fi-snb-2600      total:278  pass:249  dwarn:0   dfail:0   fail:0   skip:29  time:395s

8b5a41bbd270c3a8db6d48bc1d6d6bafb59e6753 drm-tip: 2017y-04m-27d-13h-10m-59s UTC integration manifest
578c26d drm/i915/edp: Read link status after exit PSR

== Logs ==

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

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

* Re: [PATCH] drm/i915/edp: Read link status after exit PSR
  2017-04-27 14:38 ` Ville Syrjälä
@ 2017-04-28  3:08   ` Lee, Shawn C
  2017-04-28 11:11     ` Ville Syrjälä
  0 siblings, 1 reply; 13+ messages in thread
From: Lee, Shawn C @ 2017-04-28  3:08 UTC (permalink / raw)
  To: Ville Syrjälä
  Cc: Chiou, Cooper, Bride, Jim, Nikula, Jani, intel-gfx, Vivi,
	Rodrigo, Lin, Ryan



-----Original Message-----
From: Ville Syrjälä [mailto:ville.syrjala@linux.intel.com] 
Sent: Thursday, April 27, 2017 10:39 PM
To: Lee, Shawn C <shawn.c.lee@intel.com>
Cc: intel-gfx@lists.freedesktop.org; Chiou, Cooper <cooper.chiou@intel.com>; Bride, Jim <jim.bride@intel.com>; Nikula, Jani <jani.nikula@intel.com>; Vivi, Rodrigo <rodrigo.vivi@intel.com>; Lin, Ryan <ryan.lin@intel.com>
Subject: Re: [Intel-gfx] [PATCH] drm/i915/edp: Read link status after exit PSR

On Thu, Apr 27, 2017 at 10:35:22PM +0800, Lee, Shawn C 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.
> So driver have to make sure PSR already exit before read link status.

And what exactly guarantees that it will exit PSR eventually?

When read DPCD link status register, eDP can't at link train stage (after PSR exit). 
TCON report EQ not done then will cause link re-training and display flicker. 
So we read SRD_STATUS to make sure source did not send TP1, TP2, TP3 or idle
pattern to sink side for link training.

> 
> 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: Jani Nikula <jani.nikula@intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Cc: Jim Bride <jim.bride@intel.com>
> Cc: Ryan Lin <ryan.lin@intel.com>
> 
> Signed-off-by: Shawn Lee <shawn.c.lee@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_dp.c |   34 +++++++++++++++++++++++++++++-----
>  1 file changed, 29 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c 
> b/drivers/gpu/drm/i915/intel_dp.c index 08834f74d396..cc431337b2dc 
> 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -4252,19 +4252,35 @@ static void 
> intel_dp_handle_test_request(struct intel_dp *intel_dp)  }
>  
>  static void
> +intel_edp_wait_PSR_exit(struct intel_dp *intel_dp) {
> +	struct drm_device *dev = intel_dp_to_dev(intel_dp);
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	u32 srd_status, count = 100;
> +
> +	while (count--) {
> +		srd_status = I915_READ(EDP_PSR_STATUS_CTL);
> +
> +		if ((srd_status & EDP_PSR_STATUS_SENDING_TP1) ||
> +		    (srd_status & EDP_PSR_STATUS_SENDING_TP2_TP3) ||
> +		    (srd_status & EDP_PSR_STATUS_SENDING_IDLE) ||
> +		    (srd_status & EDP_PSR_STATUS_AUX_SENDING)) {
> +			usleep_range(100, 150);
> +		} else
> +			return;
> +	}
> +}
> +
> +static void
>  intel_dp_check_link_status(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));
>  
> -	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,6 +4294,14 @@ static void intel_dp_handle_test_request(struct intel_dp *intel_dp)
>  	if (!intel_dp_link_params_valid(intel_dp))
>  		return;
>  
> +	if (is_edp(intel_dp) && dev_priv->psr.enabled)
> +		intel_edp_wait_PSR_exit(intel_dp);
> +
> +	if (!intel_dp_get_link_status(intel_dp, link_status)) {
> +		DRM_ERROR("Failed to get link status\n");
> +		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",
> --
> 1.7.9.5
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* [PATCH v2] drm/i915/edp: Read link status after exit link training
  2017-04-27 14:35 [PATCH] drm/i915/edp: Read link status after exit PSR Lee, Shawn C
                   ` (2 preceding siblings ...)
  2017-04-27 14:46 ` ✓ Fi.CI.BAT: success for " Patchwork
@ 2017-04-28  9:58 ` Lee, Shawn C
  2017-04-28 11:50   ` Tvrtko Ursulin
  2017-04-28 11:19 ` ✓ Fi.CI.BAT: success for drm/i915/edp: Read link status after exit PSR (rev2) Patchwork
                   ` (2 subsequent siblings)
  6 siblings, 1 reply; 13+ messages in thread
From: Lee, Shawn C @ 2017-04-28  9:58 UTC (permalink / raw)
  To: intel-gfx; +Cc: Cooper Chiou, Jim Bride, Jani Nikula, Rodrigo Vivi, Ryan Lin

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.
So driver have to make sure PSR already exit before read link status.

Change log:
v2:
- Use intel_wait_for_register() to replace I915_READ().

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>
Cc: Ryan Lin <ryan.lin@intel.com>

Signed-off-by: Shawn Lee <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 08834f74d396..099b6c0605a7 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -4252,19 +4252,34 @@ static void intel_dp_handle_test_request(struct intel_dp *intel_dp)
 }
 
 static void
+intel_edp_wait_PSR_exit(struct intel_dp *intel_dp)
+{
+	struct drm_device *dev = intel_dp_to_dev(intel_dp);
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	u16 count = 100;
+
+	while (count--)
+		if (!intel_wait_for_register(dev_priv,
+			    EDP_PSR_STATUS_CTL,
+			    (EDP_PSR_STATUS_SENDING_TP1 |
+			     EDP_PSR_STATUS_SENDING_TP2_TP3 |
+			     EDP_PSR_STATUS_SENDING_IDLE |
+			     EDP_PSR_STATUS_AUX_SENDING),
+			     0,
+			     1))
+			return;
+}
+
+static void
 intel_dp_check_link_status(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));
 
-	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,6 +4293,14 @@ static void intel_dp_handle_test_request(struct intel_dp *intel_dp)
 	if (!intel_dp_link_params_valid(intel_dp))
 		return;
 
+	if (is_edp(intel_dp) && dev_priv->psr.enabled)
+		intel_edp_wait_PSR_exit(intel_dp);
+
+	if (!intel_dp_get_link_status(intel_dp, link_status)) {
+		DRM_ERROR("Failed to get link status\n");
+		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",
-- 
1.7.9.5

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

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

* Re: [PATCH] drm/i915/edp: Read link status after exit PSR
  2017-04-28  3:08   ` Lee, Shawn C
@ 2017-04-28 11:11     ` Ville Syrjälä
  2017-05-03  7:06       ` Lee, Shawn C
  0 siblings, 1 reply; 13+ messages in thread
From: Ville Syrjälä @ 2017-04-28 11:11 UTC (permalink / raw)
  To: Lee, Shawn C
  Cc: Chiou, Cooper, Bride, Jim, Nikula, Jani, intel-gfx, Vivi,
	Rodrigo, Lin, Ryan

On Fri, Apr 28, 2017 at 03:08:53AM +0000, Lee, Shawn C wrote:
> 
> 
> -----Original Message-----
> From: Ville Syrjälä [mailto:ville.syrjala@linux.intel.com] 
> Sent: Thursday, April 27, 2017 10:39 PM
> To: Lee, Shawn C <shawn.c.lee@intel.com>
> Cc: intel-gfx@lists.freedesktop.org; Chiou, Cooper <cooper.chiou@intel.com>; Bride, Jim <jim.bride@intel.com>; Nikula, Jani <jani.nikula@intel.com>; Vivi, Rodrigo <rodrigo.vivi@intel.com>; Lin, Ryan <ryan.lin@intel.com>
> Subject: Re: [Intel-gfx] [PATCH] drm/i915/edp: Read link status after exit PSR
> 
> On Thu, Apr 27, 2017 at 10:35:22PM +0800, Lee, Shawn C 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.
> > So driver have to make sure PSR already exit before read link status.
> 
> And what exactly guarantees that it will exit PSR eventually?
> 
> When read DPCD link status register, eDP can't at link train stage (after PSR exit). 

There seem to be some important word(s) missing from that sentence.
As is I don't really know what you're trying to say here.

> TCON report EQ not done then will cause link re-training and display flicker. 
> So we read SRD_STATUS to make sure source did not send TP1, TP2, TP3 or idle
> pattern to sink side for link training.

That doesn't answer my question.

> 
> > 
> > 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: Jani Nikula <jani.nikula@intel.com>
> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > Cc: Jim Bride <jim.bride@intel.com>
> > Cc: Ryan Lin <ryan.lin@intel.com>
> > 
> > Signed-off-by: Shawn Lee <shawn.c.lee@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_dp.c |   34 +++++++++++++++++++++++++++++-----
> >  1 file changed, 29 insertions(+), 5 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c 
> > b/drivers/gpu/drm/i915/intel_dp.c index 08834f74d396..cc431337b2dc 
> > 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -4252,19 +4252,35 @@ static void 
> > intel_dp_handle_test_request(struct intel_dp *intel_dp)  }
> >  
> >  static void
> > +intel_edp_wait_PSR_exit(struct intel_dp *intel_dp) {
> > +	struct drm_device *dev = intel_dp_to_dev(intel_dp);
> > +	struct drm_i915_private *dev_priv = dev->dev_private;
> > +	u32 srd_status, count = 100;
> > +
> > +	while (count--) {
> > +		srd_status = I915_READ(EDP_PSR_STATUS_CTL);
> > +
> > +		if ((srd_status & EDP_PSR_STATUS_SENDING_TP1) ||
> > +		    (srd_status & EDP_PSR_STATUS_SENDING_TP2_TP3) ||
> > +		    (srd_status & EDP_PSR_STATUS_SENDING_IDLE) ||
> > +		    (srd_status & EDP_PSR_STATUS_AUX_SENDING)) {
> > +			usleep_range(100, 150);
> > +		} else
> > +			return;
> > +	}
> > +}
> > +
> > +static void
> >  intel_dp_check_link_status(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));
> >  
> > -	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,6 +4294,14 @@ static void intel_dp_handle_test_request(struct intel_dp *intel_dp)
> >  	if (!intel_dp_link_params_valid(intel_dp))
> >  		return;
> >  
> > +	if (is_edp(intel_dp) && dev_priv->psr.enabled)
> > +		intel_edp_wait_PSR_exit(intel_dp);
> > +
> > +	if (!intel_dp_get_link_status(intel_dp, link_status)) {
> > +		DRM_ERROR("Failed to get link status\n");
> > +		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",
> > --
> > 1.7.9.5
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> --
> Ville Syrjälä
> Intel OTC

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* ✓ Fi.CI.BAT: success for drm/i915/edp: Read link status after exit PSR (rev2)
  2017-04-27 14:35 [PATCH] drm/i915/edp: Read link status after exit PSR Lee, Shawn C
                   ` (3 preceding siblings ...)
  2017-04-28  9:58 ` [PATCH v2] drm/i915/edp: Read link status after exit link training Lee, Shawn C
@ 2017-04-28 11:19 ` Patchwork
  2017-05-05  6:38 ` ✓ Fi.CI.BAT: success for drm/i915/edp: Read link status after exit PSR (rev3) Patchwork
  2017-05-05  6:50 ` [PATCH v3] drm/i915/edp: Read link status after exit link training Lee, Shawn C
  6 siblings, 0 replies; 13+ messages in thread
From: Patchwork @ 2017-04-28 11:19 UTC (permalink / raw)
  To: Lee, Shawn C; +Cc: intel-gfx

== Series Details ==

Series: drm/i915/edp: Read link status after exit PSR (rev2)
URL   : https://patchwork.freedesktop.org/series/23631/
State : success

== Summary ==

Series 23631v2 drm/i915/edp: Read link status after exit PSR
https://patchwork.freedesktop.org/api/1.0/series/23631/revisions/2/mbox/

fi-bdw-5557u     total:278  pass:267  dwarn:0   dfail:0   fail:0   skip:11  time:433s
fi-bdw-gvtdvm    total:278  pass:256  dwarn:8   dfail:0   fail:0   skip:14  time:431s
fi-bsw-n3050     total:278  pass:242  dwarn:0   dfail:0   fail:0   skip:36  time:575s
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:555s
fi-byt-j1900     total:278  pass:254  dwarn:0   dfail:0   fail:0   skip:24  time:496s
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:409s
fi-hsw-4770r     total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  time:401s
fi-ilk-650       total:278  pass:228  dwarn:0   dfail:0   fail:0   skip:50  time:418s
fi-ivb-3520m     total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time:494s
fi-ivb-3770      total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time:484s
fi-kbl-7500u     total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time:463s
fi-kbl-7560u     total:278  pass:267  dwarn:1   dfail:0   fail:0   skip:10  time:661s
fi-skl-6260u     total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time:460s
fi-skl-6700hq    total:278  pass:261  dwarn:0   dfail:0   fail:0   skip:17  time:574s
fi-skl-6700k     total:278  pass:256  dwarn:4   dfail:0   fail:0   skip:18  time:461s
fi-skl-6770hq    total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time:494s
fi-skl-gvtdvm    total:278  pass:265  dwarn:0   dfail:0   fail:0   skip:13  time:428s
fi-snb-2520m     total:278  pass:250  dwarn:0   dfail:0   fail:0   skip:28  time:527s
fi-snb-2600      total:278  pass:249  dwarn:0   dfail:0   fail:0   skip:29  time:414s

86cc4197d2fa4c45b75bf54026765d27d86b84c8 drm-tip: 2017y-04m-28d-09h-14m-47s UTC integration manifest
4d009e1 drm/i915/edp: Read link status after exit link training

== Logs ==

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

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

* Re: [PATCH v2] drm/i915/edp: Read link status after exit link training
  2017-04-28  9:58 ` [PATCH v2] drm/i915/edp: Read link status after exit link training Lee, Shawn C
@ 2017-04-28 11:50   ` Tvrtko Ursulin
  2017-05-03  7:42     ` Lee, Shawn C
  0 siblings, 1 reply; 13+ messages in thread
From: Tvrtko Ursulin @ 2017-04-28 11:50 UTC (permalink / raw)
  To: Lee, Shawn C, intel-gfx
  Cc: Jani Nikula, Cooper Chiou, Jim Bride, Ryan Lin, Rodrigo Vivi


On 28/04/2017 10:58, Lee, Shawn C 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.
> So driver have to make sure PSR already exit before read link status.
>
> Change log:
> v2:
> - Use intel_wait_for_register() to replace I915_READ().
>
> 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>
> Cc: Ryan Lin <ryan.lin@intel.com>
>
> Signed-off-by: Shawn Lee <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 08834f74d396..099b6c0605a7 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -4252,19 +4252,34 @@ static void intel_dp_handle_test_request(struct intel_dp *intel_dp)
>  }
>
>  static void
> +intel_edp_wait_PSR_exit(struct intel_dp *intel_dp)
> +{
> +	struct drm_device *dev = intel_dp_to_dev(intel_dp);
> +	struct drm_i915_private *dev_priv = dev->dev_private;

... dev_priv = to_i915(dev);

> +	u16 count = 100;
> +
> +	while (count--)
> +		if (!intel_wait_for_register(dev_priv,
> +			    EDP_PSR_STATUS_CTL,
> +			    (EDP_PSR_STATUS_SENDING_TP1 |
> +			     EDP_PSR_STATUS_SENDING_TP2_TP3 |
> +			     EDP_PSR_STATUS_SENDING_IDLE |
> +			     EDP_PSR_STATUS_AUX_SENDING),
> +			     0,
> +			     1))
> +			return;
> +}

You don't need your own retry loop - intel_wait_for_register will do 
that for you. But it will only check every millisecond, if the inital 
busy wait for 2us wasn't successful.

I've  noticed that your initial patch had a 10ms timeout with a 100us 
period. If you had a good reason for that, then with this conversion you 
only have a 1ms timeout with a 1ms period, so if you need a shorter 
period, this solution is not ideal for you.

I've complicated the explanation a bit - but a bottom line is, v1 and v2 
are a lot different. Depending on the typical response time one of the 
two will have a much worse latency.

> +
> +static void
>  intel_dp_check_link_status(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;

to_i915 also.

>  	u8 link_status[DP_LINK_STATUS_SIZE];
>
>  	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,6 +4293,14 @@ static void intel_dp_handle_test_request(struct intel_dp *intel_dp)
>  	if (!intel_dp_link_params_valid(intel_dp))
>  		return;
>
> +	if (is_edp(intel_dp) && dev_priv->psr.enabled)
> +		intel_edp_wait_PSR_exit(intel_dp);
> +
> +	if (!intel_dp_get_link_status(intel_dp, link_status)) {
> +		DRM_ERROR("Failed to get link status\n");
> +		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",
>

Regards,

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

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

* Re: [PATCH] drm/i915/edp: Read link status after exit PSR
  2017-04-28 11:11     ` Ville Syrjälä
@ 2017-05-03  7:06       ` Lee, Shawn C
  0 siblings, 0 replies; 13+ messages in thread
From: Lee, Shawn C @ 2017-05-03  7:06 UTC (permalink / raw)
  To: Ville Syrjälä
  Cc: Chiou, Cooper, Bride, Jim, Nikula, Jani, intel-gfx, Vivi,
	Rodrigo, Lin, Ryan

On Fri, Apr 28, 2017 at 03:08:53AM +0000, Lee, Shawn C 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.
> > So driver have to make sure PSR already exit before read link status.
> 
> And what exactly guarantees that it will exit PSR eventually?
> 
> When read DPCD link status register, eDP can't at link train stage (after PSR exit). 

There seem to be some important word(s) missing from that sentence.
As is I don't really know what you're trying to say here.

My idea is checking SRD_STATUS before read link status registers.
If SRD_STATUS did not sent TP1, TP2, TP3 or idle pattern to sink. 
That means PSR did not at "training" stage after PSR exit. 
So host can read link status from sink and avoid unexpected result.

> TCON report EQ not done then will cause link re-training and display flicker. 
> So we read SRD_STATUS to make sure source did not send TP1, TP2, TP3 
> or idle pattern to sink side for link training.

That doesn't answer my question.

> 
> > 
> > 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: Jani Nikula <jani.nikula@intel.com>
> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > Cc: Jim Bride <jim.bride@intel.com>
> > Cc: Ryan Lin <ryan.lin@intel.com>
> > 
> > Signed-off-by: Shawn Lee <shawn.c.lee@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_dp.c |   34 +++++++++++++++++++++++++++++-----
> >  1 file changed, 29 insertions(+), 5 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c 
> > b/drivers/gpu/drm/i915/intel_dp.c index 08834f74d396..cc431337b2dc
> > 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -4252,19 +4252,35 @@ static void 
> > intel_dp_handle_test_request(struct intel_dp *intel_dp)  }
> >  
> >  static void
> > +intel_edp_wait_PSR_exit(struct intel_dp *intel_dp) {
> > +	struct drm_device *dev = intel_dp_to_dev(intel_dp);
> > +	struct drm_i915_private *dev_priv = dev->dev_private;
> > +	u32 srd_status, count = 100;
> > +
> > +	while (count--) {
> > +		srd_status = I915_READ(EDP_PSR_STATUS_CTL);
> > +
> > +		if ((srd_status & EDP_PSR_STATUS_SENDING_TP1) ||
> > +		    (srd_status & EDP_PSR_STATUS_SENDING_TP2_TP3) ||
> > +		    (srd_status & EDP_PSR_STATUS_SENDING_IDLE) ||
> > +		    (srd_status & EDP_PSR_STATUS_AUX_SENDING)) {
> > +			usleep_range(100, 150);
> > +		} else
> > +			return;
> > +	}
> > +}
> > +
> > +static void
> >  intel_dp_check_link_status(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));
> >  
> > -	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,6 +4294,14 @@ static void intel_dp_handle_test_request(struct intel_dp *intel_dp)
> >  	if (!intel_dp_link_params_valid(intel_dp))
> >  		return;
> >  
> > +	if (is_edp(intel_dp) && dev_priv->psr.enabled)
> > +		intel_edp_wait_PSR_exit(intel_dp);
> > +
> > +	if (!intel_dp_get_link_status(intel_dp, link_status)) {
> > +		DRM_ERROR("Failed to get link status\n");
> > +		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",
> > --
> > 1.7.9.5
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> --
> Ville Syrjälä
> Intel OTC

--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH v2] drm/i915/edp: Read link status after exit link training
  2017-04-28 11:50   ` Tvrtko Ursulin
@ 2017-05-03  7:42     ` Lee, Shawn C
  0 siblings, 0 replies; 13+ messages in thread
From: Lee, Shawn C @ 2017-05-03  7:42 UTC (permalink / raw)
  To: Tvrtko Ursulin, intel-gfx
  Cc: Nikula, Jani, Chiou, Cooper, Bride, Jim, Lin, Ryan, Vivi, Rodrigo


On 28/04/2017 10:58, Lee, Shawn C 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.
>> So driver have to make sure PSR already exit before read link status.
>>
>> Change log:
>> v2:
>> - Use intel_wait_for_register() to replace I915_READ().
>>
>> 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>
>> Cc: Ryan Lin <ryan.lin@intel.com>
>>
>> Signed-off-by: Shawn Lee <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 08834f74d396..099b6c0605a7 
>> 100644
>> --- a/drivers/gpu/drm/i915/intel_dp.c
>> +++ b/drivers/gpu/drm/i915/intel_dp.c
>> @@ -4252,19 +4252,34 @@ static void 
>> intel_dp_handle_test_request(struct intel_dp *intel_dp)  }
>>
>>  static void
>> +intel_edp_wait_PSR_exit(struct intel_dp *intel_dp) {
>> +	struct drm_device *dev = intel_dp_to_dev(intel_dp);
>> +	struct drm_i915_private *dev_priv = dev->dev_private;
> 
>.... dev_priv = to_i915(dev);
>

I will use to_i915() to replace it at patch v3.

>> +	u16 count = 100;
>> +
>> +	while (count--)
>> +		if (!intel_wait_for_register(dev_priv,
>> +			    EDP_PSR_STATUS_CTL,
>> +			    (EDP_PSR_STATUS_SENDING_TP1 |
>> +			     EDP_PSR_STATUS_SENDING_TP2_TP3 |
>> +			     EDP_PSR_STATUS_SENDING_IDLE |
>> +			     EDP_PSR_STATUS_AUX_SENDING),
>> +			     0,
>> +			     1))
>> +			return;
>> +}
>
>You don't need your own retry loop - intel_wait_for_register will do that for you. But it will only check every millisecond, if the inital busy wait for 2us wasn't successful.
>
>I've  noticed that your initial patch had a 10ms timeout with a 100us period. If you had a good reason for that, then with this conversion you only have a 1ms timeout with a 1ms period, so if you need a shorter period, this solution is not ideal for you.
>
>I've complicated the explanation a bit - but a bottom line is, v1 and v2 are a lot different. Depending on the typical response time one of the two will have a much worse latency.
>

Here is a little tricky to know when sink's PSR state transit to "PSR inactive" and timing re-sync is complete after PSR exit. 
So if SRD_STATUS shows TP1, TP2, TP3 or idle pattern was not sent to sink side. That means Main-Link Training is not ongoing.
But the time for link training is hard to predict. So use a loop to monitor it every 1ms until link training was done.

>> +
>> +static void
>>  intel_dp_check_link_status(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;
>
>to_i915 also.
>

I will use to_i915() to replace it at patch v3.

>>  	u8 link_status[DP_LINK_STATUS_SIZE];
>>
>>  	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,6 +4293,14 @@ static void intel_dp_handle_test_request(struct intel_dp *intel_dp)
>>  	if (!intel_dp_link_params_valid(intel_dp))
>>  		return;
>>
>> +	if (is_edp(intel_dp) && dev_priv->psr.enabled)
>> +		intel_edp_wait_PSR_exit(intel_dp);
>> +
>> +	if (!intel_dp_get_link_status(intel_dp, link_status)) {
>> +		DRM_ERROR("Failed to get link status\n");
>> +		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",
>
>
>Regards,
>
>Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* ✓ Fi.CI.BAT: success for drm/i915/edp: Read link status after exit PSR (rev3)
  2017-04-27 14:35 [PATCH] drm/i915/edp: Read link status after exit PSR Lee, Shawn C
                   ` (4 preceding siblings ...)
  2017-04-28 11:19 ` ✓ Fi.CI.BAT: success for drm/i915/edp: Read link status after exit PSR (rev2) Patchwork
@ 2017-05-05  6:38 ` Patchwork
  2017-05-05  6:50 ` [PATCH v3] drm/i915/edp: Read link status after exit link training Lee, Shawn C
  6 siblings, 0 replies; 13+ messages in thread
From: Patchwork @ 2017-05-05  6:38 UTC (permalink / raw)
  To: Lee, Shawn C; +Cc: intel-gfx

== Series Details ==

Series: drm/i915/edp: Read link status after exit PSR (rev3)
URL   : https://patchwork.freedesktop.org/series/23631/
State : success

== Summary ==

Series 23631v3 drm/i915/edp: Read link status after exit PSR
https://patchwork.freedesktop.org/api/1.0/series/23631/revisions/3/mbox/

fi-bdw-5557u     total:278  pass:267  dwarn:0   dfail:0   fail:0   skip:11  time:433s
fi-bdw-gvtdvm    total:278  pass:256  dwarn:8   dfail:0   fail:0   skip:14  time:431s
fi-bsw-n3050     total:278  pass:242  dwarn:0   dfail:0   fail:0   skip:36  time:579s
fi-bxt-j4205     total:278  pass:259  dwarn:0   dfail:0   fail:0   skip:19  time:504s
fi-bxt-t5700     total:278  pass:258  dwarn:0   dfail:0   fail:0   skip:20  time:548s
fi-byt-j1900     total:278  pass:254  dwarn:0   dfail:0   fail:0   skip:24  time:481s
fi-byt-n2820     total:278  pass:250  dwarn:0   dfail:0   fail:0   skip:28  time:486s
fi-hsw-4770      total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  time:408s
fi-hsw-4770r     total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  time:402s
fi-ilk-650       total:278  pass:228  dwarn:0   dfail:0   fail:0   skip:50  time:419s
fi-ivb-3520m     total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time:489s
fi-ivb-3770      total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time:469s
fi-kbl-7500u     total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time:457s
fi-kbl-7560u     total:278  pass:267  dwarn:1   dfail:0   fail:0   skip:10  time:578s
fi-skl-6260u     total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time:452s
fi-skl-6700hq    total:278  pass:261  dwarn:0   dfail:0   fail:0   skip:17  time:567s
fi-skl-6700k     total:278  pass:256  dwarn:4   dfail:0   fail:0   skip:18  time:464s
fi-skl-6770hq    total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time:494s
fi-skl-gvtdvm    total:278  pass:265  dwarn:0   dfail:0   fail:0   skip:13  time:430s
fi-snb-2520m     total:278  pass:250  dwarn:0   dfail:0   fail:0   skip:28  time:535s
fi-snb-2600      total:278  pass:249  dwarn:0   dfail:0   fail:0   skip:29  time:404s

369880c1680bf9bde467a40d2a03d3ad32341281 drm-tip: 2017y-05m-04d-15h-00m-33s UTC integration manifest
7ae2b6b drm/i915/edp: Read link status after exit link training

== Logs ==

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

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

* [PATCH v3] drm/i915/edp: Read link status after exit link training
  2017-04-27 14:35 [PATCH] drm/i915/edp: Read link status after exit PSR Lee, Shawn C
                   ` (5 preceding siblings ...)
  2017-05-05  6:38 ` ✓ Fi.CI.BAT: success for drm/i915/edp: Read link status after exit PSR (rev3) Patchwork
@ 2017-05-05  6:50 ` Lee, Shawn C
  6 siblings, 0 replies; 13+ messages in thread
From: Lee, Shawn C @ 2017-05-05  6:50 UTC (permalink / raw)
  To: intel-gfx; +Cc: Cooper Chiou, Jim Bride, Jani Nikula, Rodrigo Vivi, Ryan Lin

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 and link trainign are ongoing at eDP
sink. And eDP source read these registers at the same time.
eDP sink will report EQ & symbol lock not done. Then caused eDP
display flicking.

So driver have to make sure PSR already at inactive state before
read link status.

Change log:
v2:
- Use intel_wait_for_register() to replace I915_READ().
v3:
- Use to_i915() to retrieve drm_i915_private.
- Remove loop and extend wait_for_register timeout value to 100ms.

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>
Cc: Ryan Lin <ryan.lin@intel.com>
Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
Cc: Ville Syrjala <ville.syrjala@linux.intel.com>

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

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 08834f74d396..8be9fd2ef9e0 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -4252,19 +4252,32 @@ static void intel_dp_handle_test_request(struct intel_dp *intel_dp)
 }
 
 static void
+intel_edp_wait_link_train_complete(struct intel_dp *intel_dp)
+{
+	struct drm_device *dev = intel_dp_to_dev(intel_dp);
+	struct drm_i915_private *dev_priv = to_i915(dev);
+
+	if (!intel_wait_for_register(dev_priv,
+		    EDP_PSR_STATUS_CTL,
+		    (EDP_PSR_STATUS_SENDING_TP1 |
+		     EDP_PSR_STATUS_SENDING_TP2_TP3 |
+		     EDP_PSR_STATUS_SENDING_IDLE |
+		     EDP_PSR_STATUS_AUX_SENDING),
+		     0,
+		     100))
+		return;
+}
+
+static void
 intel_dp_check_link_status(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 = to_i915(dev);
 	u8 link_status[DP_LINK_STATUS_SIZE];
 
 	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,6 +4291,14 @@ static void intel_dp_handle_test_request(struct intel_dp *intel_dp)
 	if (!intel_dp_link_params_valid(intel_dp))
 		return;
 
+	if (is_edp(intel_dp) && dev_priv->psr.enabled)
+		intel_edp_wait_link_train_complete(intel_dp);
+
+	if (!intel_dp_get_link_status(intel_dp, link_status)) {
+		DRM_ERROR("Failed to get link status\n");
+		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",
-- 
1.7.9.5

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

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

end of thread, other threads:[~2017-05-05  6:38 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-04-27 14:35 [PATCH] drm/i915/edp: Read link status after exit PSR Lee, Shawn C
2017-04-27 14:16 ` Chris Wilson
2017-04-27 14:38 ` Ville Syrjälä
2017-04-28  3:08   ` Lee, Shawn C
2017-04-28 11:11     ` Ville Syrjälä
2017-05-03  7:06       ` Lee, Shawn C
2017-04-27 14:46 ` ✓ Fi.CI.BAT: success for " Patchwork
2017-04-28  9:58 ` [PATCH v2] drm/i915/edp: Read link status after exit link training Lee, Shawn C
2017-04-28 11:50   ` Tvrtko Ursulin
2017-05-03  7:42     ` Lee, Shawn C
2017-04-28 11:19 ` ✓ Fi.CI.BAT: success for drm/i915/edp: Read link status after exit PSR (rev2) Patchwork
2017-05-05  6:38 ` ✓ Fi.CI.BAT: success for drm/i915/edp: Read link status after exit PSR (rev3) Patchwork
2017-05-05  6:50 ` [PATCH v3] drm/i915/edp: Read link status after exit link training Lee, Shawn C

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.