diff mbox series

drm/i915: Fix S0ix/S3 suspend stress issue

Message ID 1568786033-1434-1-git-send-email-gaurav.k.singh@intel.com (mailing list archive)
State New, archived
Headers show
Series drm/i915: Fix S0ix/S3 suspend stress issue | expand

Commit Message

Gaurav K Singh Sept. 18, 2019, 5:53 a.m. UTC
During S0ix/S3 suspend stress test on Cometlake chromebook,
after few iterations we are seeing failure wrt PSR link CRC
Error and stress test stops. This S0ix test is failing only
when there is a CRC mismatch. In case of CRC mismatch, panel
generates IRQ_HD and whenever there is CRC mismatch, we are
disabling PSR2 in driver.

By not disabling PSR2 in driver and only by writing 1 to clear
sticky bit 0 in DPCD 0x2006 in panel,issue goes away.
Completed 2500 S0ix/S3 test cycles on multiple CML chromebooks.

As per EDP spec for CRC mismatch, nothing has been mentioned
explicitly for Source device, only by writing 1 to clear
sticky bit 0 in DPCD 0x2006 in sink is mentioned.

Signed-off-by: Gaurav K Singh <gaurav.k.singh@intel.com>
---
 drivers/gpu/drm/i915/display/intel_psr.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

Comments

Souza, Jose Sept. 18, 2019, 5:43 p.m. UTC | #1
On Wed, 2019-09-18 at 11:23 +0530, Gaurav K Singh wrote:
> During S0ix/S3 suspend stress test on Cometlake chromebook,
> after few iterations we are seeing failure wrt PSR link CRC
> Error and stress test stops. This S0ix test is failing only
> when there is a CRC mismatch. In case of CRC mismatch, panel
> generates IRQ_HD and whenever there is CRC mismatch, we are
> disabling PSR2 in driver.
> 
> By not disabling PSR2 in driver and only by writing 1 to clear
> sticky bit 0 in DPCD 0x2006 in panel,issue goes away.
> Completed 2500 S0ix/S3 test cycles on multiple CML chromebooks.
> 
> As per EDP spec for CRC mismatch, nothing has been mentioned
> explicitly for Source device, only by writing 1 to clear
> sticky bit 0 in DPCD 0x2006 in sink is mentioned.
> 
> Signed-off-by: Gaurav K Singh <gaurav.k.singh@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_psr.c | 6 +-----
>  1 file changed, 1 insertion(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_psr.c
> b/drivers/gpu/drm/i915/display/intel_psr.c
> index b3c7eef53bf3..502e29dbbea9 100644
> --- a/drivers/gpu/drm/i915/display/intel_psr.c
> +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> @@ -1325,15 +1325,11 @@ void intel_psr_short_pulse(struct intel_dp
> *intel_dp)
>  	if (val & DP_PSR_VSC_SDP_UNCORRECTABLE_ERROR)
>  		DRM_DEBUG_KMS("PSR VSC SDP uncorrectable error,
> disabling PSR\n");
>  	if (val & DP_PSR_LINK_CRC_ERROR)
> -		DRM_ERROR("PSR Link CRC error, disabling PSR\n");
> +		DRM_DEBUG_KMS("PSR Link CRC error, clearing PSR error
> status DPCD\n");
>  
>  	if (val & ~errors)
>  		DRM_ERROR("PSR_ERROR_STATUS unhandled errors %x\n",
>  			  val & ~errors);
> -	if (val & errors) {
> -		intel_psr_disable_locked(intel_dp);
> -		psr->sink_not_reliable = true;
> -	}

With this change you are breaking CRC and all the other error handling.

If CRC did not match, it means the image that was received by sink do
not match the expected, it could happen because of problems on sink,
source or flat cable.

It is better have perfect frames than save power, so this is not
acceptable.

>  	/* clear status register */
>  	drm_dp_dpcd_writeb(&intel_dp->aux, DP_PSR_ERROR_STATUS, val);
>  exit:
Gaurav K Singh Sept. 19, 2019, 7:20 a.m. UTC | #2
-----Original Message-----
From: Souza, Jose 
Sent: Wednesday, September 18, 2019 11:14 PM
To: Singh, Gaurav K <gaurav.k.singh@intel.com>; intel-gfx@lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH] drm/i915: Fix S0ix/S3 suspend stress issue

On Wed, 2019-09-18 at 11:23 +0530, Gaurav K Singh wrote:
> During S0ix/S3 suspend stress test on Cometlake chromebook, after few 
> iterations we are seeing failure wrt PSR link CRC Error and stress 
> test stops. This S0ix test is failing only when there is a CRC 
> mismatch. In case of CRC mismatch, panel generates IRQ_HD and whenever 
> there is CRC mismatch, we are disabling PSR2 in driver.
> 
> By not disabling PSR2 in driver and only by writing 1 to clear sticky 
> bit 0 in DPCD 0x2006 in panel,issue goes away.
> Completed 2500 S0ix/S3 test cycles on multiple CML chromebooks.
> 
> As per EDP spec for CRC mismatch, nothing has been mentioned 
> explicitly for Source device, only by writing 1 to clear sticky bit 0 
> in DPCD 0x2006 in sink is mentioned.
> 
> Signed-off-by: Gaurav K Singh <gaurav.k.singh@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_psr.c | 6 +-----
>  1 file changed, 1 insertion(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_psr.c
> b/drivers/gpu/drm/i915/display/intel_psr.c
> index b3c7eef53bf3..502e29dbbea9 100644
> --- a/drivers/gpu/drm/i915/display/intel_psr.c
> +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> @@ -1325,15 +1325,11 @@ void intel_psr_short_pulse(struct intel_dp
> *intel_dp)
>  	if (val & DP_PSR_VSC_SDP_UNCORRECTABLE_ERROR)
>  		DRM_DEBUG_KMS("PSR VSC SDP uncorrectable error, disabling PSR\n");
>  	if (val & DP_PSR_LINK_CRC_ERROR)
> -		DRM_ERROR("PSR Link CRC error, disabling PSR\n");
> +		DRM_DEBUG_KMS("PSR Link CRC error, clearing PSR error
> status DPCD\n");
>  
>  	if (val & ~errors)
>  		DRM_ERROR("PSR_ERROR_STATUS unhandled errors %x\n",
>  			  val & ~errors);
> -	if (val & errors) {
> -		intel_psr_disable_locked(intel_dp);
> -		psr->sink_not_reliable = true;
> -	}

With this change you are breaking CRC and all the other error handling.

If CRC did not match, it means the image that was received by sink do not match the expected, it could happen because of problems on sink, source or flat cable.

It is better have perfect frames than save power, so this is not acceptable.


Thanks Jose for your comments. There is issue with my thunderbird, using outlook for the reply.

We do not see any such CRC issue on Innolux PSR2 panel but seeing only on AUO PSR2 panel. Now since this issue can happen because of problems on sink, source or flat cable,  I see this as expected behavior from driver side. Please comment.


With regards,
Gaurav


>  	/* clear status register */
>  	drm_dp_dpcd_writeb(&intel_dp->aux, DP_PSR_ERROR_STATUS, val);
>  exit:
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c
index b3c7eef53bf3..502e29dbbea9 100644
--- a/drivers/gpu/drm/i915/display/intel_psr.c
+++ b/drivers/gpu/drm/i915/display/intel_psr.c
@@ -1325,15 +1325,11 @@  void intel_psr_short_pulse(struct intel_dp *intel_dp)
 	if (val & DP_PSR_VSC_SDP_UNCORRECTABLE_ERROR)
 		DRM_DEBUG_KMS("PSR VSC SDP uncorrectable error, disabling PSR\n");
 	if (val & DP_PSR_LINK_CRC_ERROR)
-		DRM_ERROR("PSR Link CRC error, disabling PSR\n");
+		DRM_DEBUG_KMS("PSR Link CRC error, clearing PSR error status DPCD\n");
 
 	if (val & ~errors)
 		DRM_ERROR("PSR_ERROR_STATUS unhandled errors %x\n",
 			  val & ~errors);
-	if (val & errors) {
-		intel_psr_disable_locked(intel_dp);
-		psr->sink_not_reliable = true;
-	}
 	/* clear status register */
 	drm_dp_dpcd_writeb(&intel_dp->aux, DP_PSR_ERROR_STATUS, val);
 exit: