[PATCH 1/8] drm/i915/psr: Share PSR and PSR2 exit mask

[PATCH 1/8] drm/i915/psr: Share PSR and PSR2 exit mask

[José Roberto de Souza]

Now both PSR and PSR2 have the same exit mask, so let's share then
instead of have the same code 2 times.

Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
---
 drivers/gpu/drm/i915/intel_psr.c | 34 ++++++++++++--------------------
 1 file changed, 13 insertions(+), 21 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index b6838b525502..358bbcd3b5f3 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -575,28 +575,20 @@ static void intel_psr_enable_source(struct intel_dp *intel_dp,
 		else
 			chicken &= ~VSC_DATA_SEL_SOFTWARE_CONTROL;
 		I915_WRITE(CHICKEN_TRANS(cpu_transcoder), chicken);
-
-		I915_WRITE(EDP_PSR_DEBUG,
-			   EDP_PSR_DEBUG_MASK_MEMUP |
-			   EDP_PSR_DEBUG_MASK_HPD |
-			   EDP_PSR_DEBUG_MASK_LPSP |
-			   EDP_PSR_DEBUG_MASK_MAX_SLEEP |
-			   EDP_PSR_DEBUG_MASK_DISP_REG_WRITE);
-	} else {
-		/*
-		 * Per Spec: Avoid continuous PSR exit by masking MEMUP
-		 * and HPD. also mask LPSP to avoid dependency on other
-		 * drivers that might block runtime_pm besides
-		 * preventing  other hw tracking issues now we can rely
-		 * on frontbuffer tracking.
-		 */
-		I915_WRITE(EDP_PSR_DEBUG,
-			   EDP_PSR_DEBUG_MASK_MEMUP |
-			   EDP_PSR_DEBUG_MASK_HPD |
-			   EDP_PSR_DEBUG_MASK_LPSP |
-			   EDP_PSR_DEBUG_MASK_DISP_REG_WRITE |
-			   EDP_PSR_DEBUG_MASK_MAX_SLEEP);
 	}
+
+	/*
+	 * Per Spec: Avoid continuous PSR exit by masking MEMUP and HPD also
+	 * mask LPSP to avoid dependency on other drivers that might block
+	 * runtime_pm besides preventing  other hw tracking issues now we
+	 * can rely on frontbuffer tracking.
+	 */
+	I915_WRITE(EDP_PSR_DEBUG,
+		   EDP_PSR_DEBUG_MASK_MEMUP |
+		   EDP_PSR_DEBUG_MASK_HPD |
+		   EDP_PSR_DEBUG_MASK_LPSP |
+		   EDP_PSR_DEBUG_MASK_DISP_REG_WRITE |
+		   EDP_PSR_DEBUG_MASK_MAX_SLEEP);
 }
 
 static void intel_psr_enable_locked(struct drm_i915_private *dev_priv,
-- 
2.19.0

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

[PATCH 2/8] drm/i915/psr: Do not set MASK_DISP_REG_WRITE in ICL

[José Roberto de Souza]

ICL spec states that this bit is now reserved.

Spec: 7722

Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h  |  4 ++--
 drivers/gpu/drm/i915/intel_psr.c | 17 +++++++++++------
 2 files changed, 13 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 4948b352bf4c..4dd5290a3b95 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -4195,7 +4195,7 @@ enum {
 #define   EDP_PSR_DEBUG_MASK_LPSP              (1 << 27)
 #define   EDP_PSR_DEBUG_MASK_MEMUP             (1 << 26)
 #define   EDP_PSR_DEBUG_MASK_HPD               (1 << 25)
-#define   EDP_PSR_DEBUG_MASK_DISP_REG_WRITE    (1 << 16)
+#define   EDP_PSR_DEBUG_MASK_DISP_REG_WRITE    (1 << 16) /* Reserved in ICL+ */
 #define   EDP_PSR_DEBUG_EXIT_ON_PIXEL_UNDERRUN (1 << 15) /* SKL+ */
 
 #define EDP_PSR2_CTL			_MMIO(0x6f900)
@@ -4232,7 +4232,7 @@ enum {
 #define  PSR_EVENT_FRONT_BUFFER_MODIFY		(1 << 9)
 #define  PSR_EVENT_WD_TIMER_EXPIRE		(1 << 8)
 #define  PSR_EVENT_PIPE_REGISTERS_UPDATE	(1 << 6)
-#define  PSR_EVENT_REGISTER_UPDATE		(1 << 5)
+#define  PSR_EVENT_REGISTER_UPDATE		(1 << 5) /* Reserved in ICL+ */
 #define  PSR_EVENT_HDCP_ENABLE			(1 << 4)
 #define  PSR_EVENT_KVMR_SESSION_ENABLE		(1 << 3)
 #define  PSR_EVENT_VBI_ENABLE			(1 << 2)
diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index 358bbcd3b5f3..6f3c6f0c539f 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -558,6 +558,7 @@ static void intel_psr_enable_source(struct intel_dp *intel_dp,
 {
 	struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
 	enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
+	u32 mask;
 
 	/* Only HSW and BDW have PSR AUX registers that need to be setup. SKL+
 	 * use hardcoded values PSR AUX transactions
@@ -583,12 +584,16 @@ static void intel_psr_enable_source(struct intel_dp *intel_dp,
 	 * runtime_pm besides preventing  other hw tracking issues now we
 	 * can rely on frontbuffer tracking.
 	 */
-	I915_WRITE(EDP_PSR_DEBUG,
-		   EDP_PSR_DEBUG_MASK_MEMUP |
-		   EDP_PSR_DEBUG_MASK_HPD |
-		   EDP_PSR_DEBUG_MASK_LPSP |
-		   EDP_PSR_DEBUG_MASK_DISP_REG_WRITE |
-		   EDP_PSR_DEBUG_MASK_MAX_SLEEP);
+	mask = EDP_PSR_DEBUG_MASK_MEMUP |
+	       EDP_PSR_DEBUG_MASK_HPD |
+	       EDP_PSR_DEBUG_MASK_LPSP |
+	       EDP_PSR_DEBUG_MASK_DISP_REG_WRITE |
+	       EDP_PSR_DEBUG_MASK_MAX_SLEEP;
+
+	if (INTEL_GEN(dev_priv) >= 11)
+		mask &= ~EDP_PSR_DEBUG_MASK_DISP_REG_WRITE;
+
+	I915_WRITE(EDP_PSR_DEBUG, mask);
 }
 
 static void intel_psr_enable_locked(struct drm_i915_private *dev_priv,
-- 
2.19.0

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

Re: [PATCH 2/8] drm/i915/psr: Do not set MASK_DISP_REG_WRITE in ICL

[Dhinakaran Pandiyan]

On Thursday, September 20, 2018 1:43:21 PM PDT José Roberto de Souza wrote:
> ICL spec states that this bit is now reserved.

It reads better if you state the bit name and register in the commit message. 
With this nit addressed,
Reviewed-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> 
> Spec: 7722
Change this to  Bspec: 7722 to be clear? But, I don't know if there is a tag 
that we consistently use for citing bspec.

> 
> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_reg.h  |  4 ++--
>  drivers/gpu/drm/i915/intel_psr.c | 17 +++++++++++------
>  2 files changed, 13 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h
> b/drivers/gpu/drm/i915/i915_reg.h index 4948b352bf4c..4dd5290a3b95 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -4195,7 +4195,7 @@ enum {
>  #define   EDP_PSR_DEBUG_MASK_LPSP              (1 << 27)
>  #define   EDP_PSR_DEBUG_MASK_MEMUP             (1 << 26)
>  #define   EDP_PSR_DEBUG_MASK_HPD               (1 << 25)
> -#define   EDP_PSR_DEBUG_MASK_DISP_REG_WRITE    (1 << 16)
> +#define   EDP_PSR_DEBUG_MASK_DISP_REG_WRITE    (1 << 16) /* Reserved in
> ICL+ */ #define   EDP_PSR_DEBUG_EXIT_ON_PIXEL_UNDERRUN (1 << 15) /* SKL+ */
> 
>  #define EDP_PSR2_CTL			_MMIO(0x6f900)
> @@ -4232,7 +4232,7 @@ enum {
>  #define  PSR_EVENT_FRONT_BUFFER_MODIFY		(1 << 9)
>  #define  PSR_EVENT_WD_TIMER_EXPIRE		(1 << 8)
>  #define  PSR_EVENT_PIPE_REGISTERS_UPDATE	(1 << 6)
> -#define  PSR_EVENT_REGISTER_UPDATE		(1 << 5)
> +#define  PSR_EVENT_REGISTER_UPDATE		(1 << 5) /* Reserved in ICL+ */
>  #define  PSR_EVENT_HDCP_ENABLE			(1 << 4)
>  #define  PSR_EVENT_KVMR_SESSION_ENABLE		(1 << 3)
>  #define  PSR_EVENT_VBI_ENABLE			(1 << 2)
> diff --git a/drivers/gpu/drm/i915/intel_psr.c
> b/drivers/gpu/drm/i915/intel_psr.c index 358bbcd3b5f3..6f3c6f0c539f 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
> @@ -558,6 +558,7 @@ static void intel_psr_enable_source(struct intel_dp
> *intel_dp, {
>  	struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
>  	enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
> +	u32 mask;
> 
>  	/* Only HSW and BDW have PSR AUX registers that need to be setup. SKL+
>  	 * use hardcoded values PSR AUX transactions
> @@ -583,12 +584,16 @@ static void intel_psr_enable_source(struct intel_dp
> *intel_dp, * runtime_pm besides preventing  other hw tracking issues now we
> * can rely on frontbuffer tracking.
>  	 */
> -	I915_WRITE(EDP_PSR_DEBUG,
> -		   EDP_PSR_DEBUG_MASK_MEMUP |
> -		   EDP_PSR_DEBUG_MASK_HPD |
> -		   EDP_PSR_DEBUG_MASK_LPSP |
> -		   EDP_PSR_DEBUG_MASK_DISP_REG_WRITE |
> -		   EDP_PSR_DEBUG_MASK_MAX_SLEEP);
> +	mask = EDP_PSR_DEBUG_MASK_MEMUP |
> +	       EDP_PSR_DEBUG_MASK_HPD |
> +	       EDP_PSR_DEBUG_MASK_LPSP |
> +	       EDP_PSR_DEBUG_MASK_DISP_REG_WRITE |
> +	       EDP_PSR_DEBUG_MASK_MAX_SLEEP;
> +
> +	if (INTEL_GEN(dev_priv) >= 11)
> +		mask &= ~EDP_PSR_DEBUG_MASK_DISP_REG_WRITE;
> +
> +	I915_WRITE(EDP_PSR_DEBUG, mask);
>  }
> 
>  static void intel_psr_enable_locked(struct drm_i915_private *dev_priv,




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

Re: [PATCH 2/8] drm/i915/psr: Do not set MASK_DISP_REG_WRITE in ICL

[Jani Nikula]

On Thu, 20 Sep 2018, José Roberto de Souza <jose.souza@intel.com> wrote:
> ICL spec states that this bit is now reserved.
>
> Spec: 7722
>
> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_reg.h  |  4 ++--
>  drivers/gpu/drm/i915/intel_psr.c | 17 +++++++++++------
>  2 files changed, 13 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 4948b352bf4c..4dd5290a3b95 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -4195,7 +4195,7 @@ enum {
>  #define   EDP_PSR_DEBUG_MASK_LPSP              (1 << 27)
>  #define   EDP_PSR_DEBUG_MASK_MEMUP             (1 << 26)
>  #define   EDP_PSR_DEBUG_MASK_HPD               (1 << 25)
> -#define   EDP_PSR_DEBUG_MASK_DISP_REG_WRITE    (1 << 16)
> +#define   EDP_PSR_DEBUG_MASK_DISP_REG_WRITE    (1 << 16) /* Reserved in ICL+ */
>  #define   EDP_PSR_DEBUG_EXIT_ON_PIXEL_UNDERRUN (1 << 15) /* SKL+ */
>  
>  #define EDP_PSR2_CTL			_MMIO(0x6f900)
> @@ -4232,7 +4232,7 @@ enum {
>  #define  PSR_EVENT_FRONT_BUFFER_MODIFY		(1 << 9)
>  #define  PSR_EVENT_WD_TIMER_EXPIRE		(1 << 8)
>  #define  PSR_EVENT_PIPE_REGISTERS_UPDATE	(1 << 6)
> -#define  PSR_EVENT_REGISTER_UPDATE		(1 << 5)
> +#define  PSR_EVENT_REGISTER_UPDATE		(1 << 5) /* Reserved in ICL+ */
>  #define  PSR_EVENT_HDCP_ENABLE			(1 << 4)
>  #define  PSR_EVENT_KVMR_SESSION_ENABLE		(1 << 3)
>  #define  PSR_EVENT_VBI_ENABLE			(1 << 2)
> diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
> index 358bbcd3b5f3..6f3c6f0c539f 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
> @@ -558,6 +558,7 @@ static void intel_psr_enable_source(struct intel_dp *intel_dp,
>  {
>  	struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
>  	enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
> +	u32 mask;
>  
>  	/* Only HSW and BDW have PSR AUX registers that need to be setup. SKL+
>  	 * use hardcoded values PSR AUX transactions
> @@ -583,12 +584,16 @@ static void intel_psr_enable_source(struct intel_dp *intel_dp,
>  	 * runtime_pm besides preventing  other hw tracking issues now we
>  	 * can rely on frontbuffer tracking.
>  	 */
> -	I915_WRITE(EDP_PSR_DEBUG,
> -		   EDP_PSR_DEBUG_MASK_MEMUP |
> -		   EDP_PSR_DEBUG_MASK_HPD |
> -		   EDP_PSR_DEBUG_MASK_LPSP |
> -		   EDP_PSR_DEBUG_MASK_DISP_REG_WRITE |
> -		   EDP_PSR_DEBUG_MASK_MAX_SLEEP);
> +	mask = EDP_PSR_DEBUG_MASK_MEMUP |
> +	       EDP_PSR_DEBUG_MASK_HPD |
> +	       EDP_PSR_DEBUG_MASK_LPSP |
> +	       EDP_PSR_DEBUG_MASK_DISP_REG_WRITE |
> +	       EDP_PSR_DEBUG_MASK_MAX_SLEEP;
> +
> +	if (INTEL_GEN(dev_priv) >= 11)
> +		mask &= ~EDP_PSR_DEBUG_MASK_DISP_REG_WRITE;

Seems better to |= this on gen < 11 rather than &= ~ on gen >= 11.

BR,
Jani.

> +
> +	I915_WRITE(EDP_PSR_DEBUG, mask);
>  }
>  
>  static void intel_psr_enable_locked(struct drm_i915_private *dev_priv,

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

[PATCH 3/8] drm/i915/psr: Enable sink to trigger a interruption on PSR2 CRC mismatch

[José Roberto de Souza]

eDP spec states 2 different bits to enable sink to trigger a
interruption when there is a CRC mismatch.
DP_PSR_CRC_VERIFICATION is for PSR only and
DP_PSR_IRQ_HPD_WITH_CRC_ERRORS is for PSR2 only.

Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
---
 drivers/gpu/drm/i915/intel_psr.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index 6f3c6f0c539f..b4edbbda8d71 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -340,7 +340,7 @@ static void intel_psr_enable_sink(struct intel_dp *intel_dp)
 	if (dev_priv->psr.psr2_enabled) {
 		drm_dp_dpcd_writeb(&intel_dp->aux, DP_RECEIVER_ALPM_CONFIG,
 				   DP_ALPM_ENABLE);
-		dpcd_val |= DP_PSR_ENABLE_PSR2;
+		dpcd_val |= DP_PSR_ENABLE_PSR2 | DP_PSR_IRQ_HPD_WITH_CRC_ERRORS;
 	}
 
 	if (dev_priv->psr.link_standby)
-- 
2.19.0

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

Re: [PATCH 3/8] drm/i915/psr: Enable sink to trigger a interruption on PSR2 CRC mismatch

[Pandiyan, Dhinakaran]

On Thu, 2018-09-20 at 13:43 -0700, José Roberto de Souza wrote:
> eDP spec states 2 different bits to enable sink to trigger a
> interruption when there is a CRC mismatch.
> DP_PSR_CRC_VERIFICATION is for PSR only and
> DP_PSR_IRQ_HPD_WITH_CRC_ERRORS is for PSR2 only.
> 
> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_psr.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_psr.c
> b/drivers/gpu/drm/i915/intel_psr.c
> index 6f3c6f0c539f..b4edbbda8d71 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
> @@ -340,7 +340,7 @@ static void intel_psr_enable_sink(struct intel_dp
> *intel_dp)
>  	if (dev_priv->psr.psr2_enabled) {
>  		drm_dp_dpcd_writeb(&intel_dp->aux,
> DP_RECEIVER_ALPM_CONFIG,
>  				   DP_ALPM_ENABLE);
> -		dpcd_val |= DP_PSR_ENABLE_PSR2;
> +		dpcd_val |= DP_PSR_ENABLE_PSR2 |
> DP_PSR_IRQ_HPD_WITH_CRC_ERRORS;
>  	} 
	} else if (INTEL_GEN(dev_priv) >=8) {

		dpcd_val |= DP_PSR_CRC_VERIFICATION;
	}

How about doing this for clarity? 


	
>  
>  	if (dev_priv->psr.link_standby)


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

Re: [PATCH 3/8] drm/i915/psr: Enable sink to trigger a interruption on PSR2 CRC mismatch

[dhinakaran.pandiyan]

On Tue, 2018-09-25 at 00:28 +0000, Pandiyan, Dhinakaran wrote:
> On Thu, 2018-09-20 at 13:43 -0700, José Roberto de Souza wrote:
> > eDP spec states 2 different bits to enable sink to trigger a
> > interruption when there is a CRC mismatch.
> > DP_PSR_CRC_VERIFICATION is for PSR only and
> > DP_PSR_IRQ_HPD_WITH_CRC_ERRORS is for PSR2 only.

Do you have a system that triggers a short pulse for this? If yes, do
we end up calling the PSR error interrupt handler at all?

intel_dp_short_pulse()
	...
	if intel_dp_needs_link_retrain()		
		return
	intel_psr_short_pulse()
	


> > 
> > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_psr.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_psr.c
> > b/drivers/gpu/drm/i915/intel_psr.c
> > index 6f3c6f0c539f..b4edbbda8d71 100644
> > --- a/drivers/gpu/drm/i915/intel_psr.c
> > +++ b/drivers/gpu/drm/i915/intel_psr.c
> > @@ -340,7 +340,7 @@ static void intel_psr_enable_sink(struct
> > intel_dp
> > *intel_dp)
> >  	if (dev_priv->psr.psr2_enabled) {
> >  		drm_dp_dpcd_writeb(&intel_dp->aux,
> > DP_RECEIVER_ALPM_CONFIG,
> >  				   DP_ALPM_ENABLE);
> > -		dpcd_val |= DP_PSR_ENABLE_PSR2;
> > +		dpcd_val |= DP_PSR_ENABLE_PSR2 |
> > DP_PSR_IRQ_HPD_WITH_CRC_ERRORS;
> >  	} 
> 
> 	} else if (INTEL_GEN(dev_priv) >=8) {
> 
> 		dpcd_val |= DP_PSR_CRC_VERIFICATION;
> 	}
> 
> How about doing this for clarity? 
> 
> 
> 	
> >  
> >  	if (dev_priv->psr.link_standby)
> 
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 4/8] drm/i915/psr: Remove PSR2 TODO error handling

[José Roberto de Souza]

We are already handling all PSR2 errors, so we can drop this TODO.

Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
---
 drivers/gpu/drm/i915/intel_psr.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index b4edbbda8d71..0dd4211cb293 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -1127,8 +1127,6 @@ void intel_psr_short_pulse(struct intel_dp *intel_dp)
 		intel_psr_disable_locked(intel_dp);
 	/* clear status register */
 	drm_dp_dpcd_writeb(&intel_dp->aux, DP_PSR_ERROR_STATUS, val);
-
-	/* TODO: handle PSR2 errors */
 exit:
 	mutex_unlock(&psr->lock);
 }
-- 
2.19.0

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

Re: [PATCH 4/8] drm/i915/psr: Remove PSR2 TODO error handling

[dhinakaran.pandiyan]

On Thu, 2018-09-20 at 13:43 -0700, José Roberto de Souza wrote:
> We are already handling all PSR2 errors, so we can drop this TODO.

Yes, we can remove it to thanks to all the work you have been doing.

Reviewed-by: Dhinakaran Pandiyan <dhinakaran.pandiyan>

> 
> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_psr.c | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_psr.c
> b/drivers/gpu/drm/i915/intel_psr.c
> index b4edbbda8d71..0dd4211cb293 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
> @@ -1127,8 +1127,6 @@ void intel_psr_short_pulse(struct intel_dp
> *intel_dp)
>  		intel_psr_disable_locked(intel_dp);
>  	/* clear status register */
>  	drm_dp_dpcd_writeb(&intel_dp->aux, DP_PSR_ERROR_STATUS,
> val);
> -
> -	/* TODO: handle PSR2 errors */
>  exit:
>  	mutex_unlock(&psr->lock);
>  }
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 5/8] drm/i915/psr: Do not enable PSR2 if sink requires selective update X granularity

[José Roberto de Souza]

According to eDP spec, sink could required a granularity in the
start of x coordinate or in the width of the selective update region.
As it is not supported by hardware, lets not enable PSR2 in sinks
that requires it.

Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
---
 drivers/gpu/drm/i915/intel_psr.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index 0dd4211cb293..84b512426514 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -243,6 +243,8 @@ void intel_psr_init_dpcd(struct intel_dp *intel_dp)
 		bool y_req = intel_dp->psr_dpcd[1] &
 			     DP_PSR2_SU_Y_COORDINATE_REQUIRED;
 		bool alpm = intel_dp_get_alpm_status(intel_dp);
+		bool granularity_req = (intel_dp->psr_dpcd[1] &
+					DP_PSR2_SU_GRANULARITY_REQUIRED);
 
 		/*
 		 * All panels that supports PSR version 03h (PSR2 +
@@ -255,7 +257,8 @@ void intel_psr_init_dpcd(struct intel_dp *intel_dp)
 		 * Y-coordinate requirement panels we would need to enable
 		 * GTC first.
 		 */
-		dev_priv->psr.sink_psr2_support = y_req && alpm;
+		dev_priv->psr.sink_psr2_support = y_req && alpm &&
+						  !granularity_req;
 		DRM_DEBUG_KMS("PSR2 %ssupported\n",
 			      dev_priv->psr.sink_psr2_support ? "" : "not ");
 
-- 
2.19.0

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

Re: [PATCH 5/8] drm/i915/psr: Do not enable PSR2 if sink requires selective update X granularity

[dhinakaran.pandiyan]

On Thu, 2018-09-20 at 13:43 -0700, José Roberto de Souza wrote:
> According to eDP spec, sink could required a granularity in the
> start of x coordinate or in the width of the selective update region.
> As it is not supported by hardware, 

I think this warrants an explanation, what is not supported in
hardware? A certain granularity that a specific sink expects?

Even if the sink does not require a specific granularity, the source
has to support the standard granularity values (x%16 = w%4 =  y%1 = 0).
Another sink can have these standard values, supported by hardware, as
a requirement and your patch will disable PSR2 on it.


-DK

> lets not enable PSR2 in sinks
> that requires it.
> 
> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_psr.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_psr.c
> b/drivers/gpu/drm/i915/intel_psr.c
> index 0dd4211cb293..84b512426514 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
> @@ -243,6 +243,8 @@ void intel_psr_init_dpcd(struct intel_dp
> *intel_dp)
>  		bool y_req = intel_dp->psr_dpcd[1] &
>  			     DP_PSR2_SU_Y_COORDINATE_REQUIRED;
>  		bool alpm = intel_dp_get_alpm_status(intel_dp);
> +		bool granularity_req = (intel_dp->psr_dpcd[1] &
> +					DP_PSR2_SU_GRANULARITY_REQUI
> RED);
>  
>  		/*
>  		 * All panels that supports PSR version 03h (PSR2 +
> @@ -255,7 +257,8 @@ void intel_psr_init_dpcd(struct intel_dp
> *intel_dp)
>  		 * Y-coordinate requirement panels we would need to
> enable
>  		 * GTC first.
>  		 */
> -		dev_priv->psr.sink_psr2_support = y_req && alpm;
> +		dev_priv->psr.sink_psr2_support = y_req && alpm &&
> +						  !granularity_req;
>  		DRM_DEBUG_KMS("PSR2 %ssupported\n",
>  			      dev_priv->psr.sink_psr2_support ? "" :
> "not ");
>  
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 5/8] drm/i915/psr: Do not enable PSR2 if sink requires selective update X granularity

[Souza, Jose]

On Mon, 2018-09-24 at 23:23 -0700, dhinakaran.pandiyan@gmail.com wrote:
> On Thu, 2018-09-20 at 13:43 -0700, José Roberto de Souza wrote:
> > According to eDP spec, sink could required a granularity in the
> > start of x coordinate or in the width of the selective update
> > region.
> > As it is not supported by hardware, 
> 
> I think this warrants an explanation, what is not supported in
> hardware? A certain granularity that a specific sink expects?
> 
> Even if the sink does not require a specific granularity, the source
> has to support the standard granularity values (x%16 = w%4 =  y%1 =
> 0).
> Another sink can have these standard values, supported by hardware,
> as
> a requirement and your patch will disable PSR2 on it.

I add this to the commit message but source supports the standard
granularity but if sink requires a different one(checking it by reading
this bit), source should read 2 other registers with x and width
granularity send selective update respecting it.

> 
> 
> -DK
> 
> > lets not enable PSR2 in sinks
> > that requires it.
> > 
> > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_psr.c | 5 ++++-
> >  1 file changed, 4 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_psr.c
> > b/drivers/gpu/drm/i915/intel_psr.c
> > index 0dd4211cb293..84b512426514 100644
> > --- a/drivers/gpu/drm/i915/intel_psr.c
> > +++ b/drivers/gpu/drm/i915/intel_psr.c
> > @@ -243,6 +243,8 @@ void intel_psr_init_dpcd(struct intel_dp
> > *intel_dp)
> >  		bool y_req = intel_dp->psr_dpcd[1] &
> >  			     DP_PSR2_SU_Y_COORDINATE_REQUIRED;
> >  		bool alpm = intel_dp_get_alpm_status(intel_dp);
> > +		bool granularity_req = (intel_dp->psr_dpcd[1] &
> > +					DP_PSR2_SU_GRANULARITY_REQUI
> > RED);
> >  
> >  		/*
> >  		 * All panels that supports PSR version 03h (PSR2 +
> > @@ -255,7 +257,8 @@ void intel_psr_init_dpcd(struct intel_dp
> > *intel_dp)
> >  		 * Y-coordinate requirement panels we would need to
> > enable
> >  		 * GTC first.
> >  		 */
> > -		dev_priv->psr.sink_psr2_support = y_req && alpm;
> > +		dev_priv->psr.sink_psr2_support = y_req && alpm &&
> > +						  !granularity_req;
> >  		DRM_DEBUG_KMS("PSR2 %ssupported\n",
> >  			      dev_priv->psr.sink_psr2_support ? "" :
> > "not ");
> >  
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 6/8] drm/i915/psr: Use WA to force HW tracking to exit PSR2

[José Roberto de Souza]

This WA also works fine for PSR2, triggering a selective update when
possible.

Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
---
 drivers/gpu/drm/i915/intel_psr.c | 24 ++++++++++--------------
 1 file changed, 10 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index 84b512426514..cf9d6e965697 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -1026,20 +1026,16 @@ void intel_psr_flush(struct drm_i915_private *dev_priv,
 
 	/* By definition flush = invalidate + flush */
 	if (frontbuffer_bits) {
-		if (dev_priv->psr.psr2_enabled) {
-			intel_psr_exit(dev_priv);
-		} else {
-			/*
-			 * Display WA #0884: all
-			 * This documented WA for bxt can be safely applied
-			 * broadly so we can force HW tracking to exit PSR
-			 * instead of disabling and re-enabling.
-			 * Workaround tells us to write 0 to CUR_SURFLIVE_A,
-			 * but it makes more sense write to the current active
-			 * pipe.
-			 */
-			I915_WRITE(CURSURFLIVE(pipe), 0);
-		}
+		/*
+		 * Display WA #0884: all
+		 * This documented WA for bxt can be safely applied
+		 * broadly so we can force HW tracking to exit PSR
+		 * instead of disabling and re-enabling.
+		 * Workaround tells us to write 0 to CUR_SURFLIVE_A,
+		 * but it makes more sense write to the current active
+		 * pipe.
+		 */
+		I915_WRITE(CURSURFLIVE(pipe), 0);
 	}
 
 	if (!dev_priv->psr.active && !dev_priv->psr.busy_frontbuffer_bits)
-- 
2.19.0

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

Re: [PATCH 6/8] drm/i915/psr: Use WA to force HW tracking to exit PSR2

[Rodrigo Vivi]

On Thu, Sep 20, 2018 at 01:43:25PM -0700, José Roberto de Souza wrote:
> This WA also works fine for PSR2, triggering a selective update when
> possible.

Oh! really?! It didn't work when I chacked on my CNL,
but we probably had other bugs back there...

Thanks for finding this


Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>



> 
> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_psr.c | 24 ++++++++++--------------
>  1 file changed, 10 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
> index 84b512426514..cf9d6e965697 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
> @@ -1026,20 +1026,16 @@ void intel_psr_flush(struct drm_i915_private *dev_priv,
>  
>  	/* By definition flush = invalidate + flush */
>  	if (frontbuffer_bits) {
> -		if (dev_priv->psr.psr2_enabled) {
> -			intel_psr_exit(dev_priv);
> -		} else {
> -			/*
> -			 * Display WA #0884: all
> -			 * This documented WA for bxt can be safely applied
> -			 * broadly so we can force HW tracking to exit PSR
> -			 * instead of disabling and re-enabling.
> -			 * Workaround tells us to write 0 to CUR_SURFLIVE_A,
> -			 * but it makes more sense write to the current active
> -			 * pipe.
> -			 */
> -			I915_WRITE(CURSURFLIVE(pipe), 0);
> -		}
> +		/*
> +		 * Display WA #0884: all
> +		 * This documented WA for bxt can be safely applied
> +		 * broadly so we can force HW tracking to exit PSR
> +		 * instead of disabling and re-enabling.
> +		 * Workaround tells us to write 0 to CUR_SURFLIVE_A,
> +		 * but it makes more sense write to the current active
> +		 * pipe.
> +		 */
> +		I915_WRITE(CURSURFLIVE(pipe), 0);
>  	}
>  
>  	if (!dev_priv->psr.active && !dev_priv->psr.busy_frontbuffer_bits)
> -- 
> 2.19.0
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 6/8] drm/i915/psr: Use WA to force HW tracking to exit PSR2

[Souza, Jose]

On Thu, 2018-09-20 at 15:54 -0700, Rodrigo Vivi wrote:
> On Thu, Sep 20, 2018 at 01:43:25PM -0700, José Roberto de Souza
> wrote:
> > This WA also works fine for PSR2, triggering a selective update
> > when
> > possible.
> 
> Oh! really?! It didn't work when I chacked on my CNL,
> but we probably had other bugs back there...

Tested in WHL and ICL, I will give a try in CNL.

> 
> Thanks for finding this
> 
> 
> Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> 
> 
> 
> > 
> > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_psr.c | 24 ++++++++++--------------
> >  1 file changed, 10 insertions(+), 14 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_psr.c
> > b/drivers/gpu/drm/i915/intel_psr.c
> > index 84b512426514..cf9d6e965697 100644
> > --- a/drivers/gpu/drm/i915/intel_psr.c
> > +++ b/drivers/gpu/drm/i915/intel_psr.c
> > @@ -1026,20 +1026,16 @@ void intel_psr_flush(struct
> > drm_i915_private *dev_priv,
> >  
> >  	/* By definition flush = invalidate + flush */
> >  	if (frontbuffer_bits) {
> > -		if (dev_priv->psr.psr2_enabled) {
> > -			intel_psr_exit(dev_priv);
> > -		} else {
> > -			/*
> > -			 * Display WA #0884: all
> > -			 * This documented WA for bxt can be safely
> > applied
> > -			 * broadly so we can force HW tracking to exit
> > PSR
> > -			 * instead of disabling and re-enabling.
> > -			 * Workaround tells us to write 0 to
> > CUR_SURFLIVE_A,
> > -			 * but it makes more sense write to the current
> > active
> > -			 * pipe.
> > -			 */
> > -			I915_WRITE(CURSURFLIVE(pipe), 0);
> > -		}
> > +		/*
> > +		 * Display WA #0884: all
> > +		 * This documented WA for bxt can be safely applied
> > +		 * broadly so we can force HW tracking to exit PSR
> > +		 * instead of disabling and re-enabling.
> > +		 * Workaround tells us to write 0 to CUR_SURFLIVE_A,
> > +		 * but it makes more sense write to the current active
> > +		 * pipe.
> > +		 */
> > +		I915_WRITE(CURSURFLIVE(pipe), 0);
> >  	}
> >  
> >  	if (!dev_priv->psr.active && !dev_priv-
> > >psr.busy_frontbuffer_bits)
> > -- 
> > 2.19.0
> > 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 6/8] drm/i915/psr: Use WA to force HW tracking to exit PSR2

[dhinakaran.pandiyan]

On Thu, 2018-09-20 at 23:01 +0000, Souza, Jose wrote:
> On Thu, 2018-09-20 at 15:54 -0700, Rodrigo Vivi wrote:
> > On Thu, Sep 20, 2018 at 01:43:25PM -0700, José Roberto de Souza
> > wrote:
> > > This WA also works fine for PSR2, triggering a selective update
> > > when
> > > possible.
> > 
> > Oh! really?! It didn't work when I chacked on my CNL,
> > but we probably had other bugs back there...
> 
> Tested in WHL and ICL, I will give a try in CNL.
> 
> > 
> > Thanks for finding this
> > 
> > 
> > Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

I haven't checked myself but from what I understand it should work
Acked-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>

> > 
> > 
> > 
> > > 
> > > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/intel_psr.c | 24 ++++++++++--------------
> > >  1 file changed, 10 insertions(+), 14 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/intel_psr.c
> > > b/drivers/gpu/drm/i915/intel_psr.c
> > > index 84b512426514..cf9d6e965697 100644
> > > --- a/drivers/gpu/drm/i915/intel_psr.c
> > > +++ b/drivers/gpu/drm/i915/intel_psr.c
> > > @@ -1026,20 +1026,16 @@ void intel_psr_flush(struct
> > > drm_i915_private *dev_priv,
> > >  
> > >  	/* By definition flush = invalidate + flush */
> > >  	if (frontbuffer_bits) {
> > > -		if (dev_priv->psr.psr2_enabled) {
> > > -			intel_psr_exit(dev_priv);
> > > -		} else {
> > > -			/*
> > > -			 * Display WA #0884: all
> > > -			 * This documented WA for bxt can be
> > > safely
> > > applied
> > > -			 * broadly so we can force HW tracking
> > > to exit
> > > PSR
> > > -			 * instead of disabling and re-enabling.
> > > -			 * Workaround tells us to write 0 to
> > > CUR_SURFLIVE_A,
> > > -			 * but it makes more sense write to the
> > > current
> > > active
> > > -			 * pipe.
> > > -			 */
> > > -			I915_WRITE(CURSURFLIVE(pipe), 0);
> > > -		}
> > > +		/*
> > > +		 * Display WA #0884: all
> > > +		 * This documented WA for bxt can be safely
> > > applied
> > > +		 * broadly so we can force HW tracking to exit
> > > PSR
> > > +		 * instead of disabling and re-enabling.
> > > +		 * Workaround tells us to write 0 to
> > > CUR_SURFLIVE_A,
> > > +		 * but it makes more sense write to the current
> > > active
> > > +		 * pipe.
> > > +		 */
> > > +		I915_WRITE(CURSURFLIVE(pipe), 0);
> > >  	}
> > >  
> > >  	if (!dev_priv->psr.active && !dev_priv-
> > > > psr.busy_frontbuffer_bits)
> > > 
> > > -- 
> > > 2.19.0
> > > 
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 7/8] drm/i915/psr: Don't tell sink that main link will be active in PSR2

[José Roberto de Souza]

For PSR2 we don't have the option to keep main link enabled while
PSR2 is active, so don't configure sink DPCD with a wrong value.

Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
---
 drivers/gpu/drm/i915/intel_psr.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index cf9d6e965697..60cf6fd251d0 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -344,12 +344,13 @@ static void intel_psr_enable_sink(struct intel_dp *intel_dp)
 		drm_dp_dpcd_writeb(&intel_dp->aux, DP_RECEIVER_ALPM_CONFIG,
 				   DP_ALPM_ENABLE);
 		dpcd_val |= DP_PSR_ENABLE_PSR2 | DP_PSR_IRQ_HPD_WITH_CRC_ERRORS;
+	} else {
+		if (dev_priv->psr.link_standby)
+			dpcd_val |= DP_PSR_MAIN_LINK_ACTIVE;
+		if (INTEL_GEN(dev_priv) >= 8)
+			dpcd_val |= DP_PSR_CRC_VERIFICATION;
 	}
 
-	if (dev_priv->psr.link_standby)
-		dpcd_val |= DP_PSR_MAIN_LINK_ACTIVE;
-	if (!dev_priv->psr.psr2_enabled && INTEL_GEN(dev_priv) >= 8)
-		dpcd_val |= DP_PSR_CRC_VERIFICATION;
 	drm_dp_dpcd_writeb(&intel_dp->aux, DP_PSR_EN_CFG, dpcd_val);
 
 	drm_dp_dpcd_writeb(&intel_dp->aux, DP_SET_POWER, DP_SET_POWER_D0);
-- 
2.19.0

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

Re: [PATCH 7/8] drm/i915/psr: Don't tell sink that main link will be active in PSR2

[dhinakaran.pandiyan]

On Thu, 2018-09-20 at 13:43 -0700, José Roberto de Souza wrote:
> For PSR2 we don't have the option to keep main link enabled while
> PSR2 is active, so don't configure sink DPCD with a wrong value.
Is this what the DP spec says or an Intel HW restriction?

-DK

> 
> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>s
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_psr.c | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_psr.c
> b/drivers/gpu/drm/i915/intel_psr.c
> index cf9d6e965697..60cf6fd251d0 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
> @@ -344,12 +344,13 @@ static void intel_psr_enable_sink(struct
> intel_dp *intel_dp)
>  		drm_dp_dpcd_writeb(&intel_dp->aux,
> DP_RECEIVER_ALPM_CONFIG,
>  				   DP_ALPM_ENABLE);
>  		dpcd_val |= DP_PSR_ENABLE_PSR2 |
> DP_PSR_IRQ_HPD_WITH_CRC_ERRORS;
> +	} else {
> +		if (dev_priv->psr.link_standby)
> +			dpcd_val |= DP_PSR_MAIN_LINK_ACTIVE;
> +		if (INTEL_GEN(dev_priv) >= 8)
> +			dpcd_val |= DP_PSR_CRC_VERIFICATION;
>  	}
>  
> -	if (dev_priv->psr.link_standby)
> -		dpcd_val |= DP_PSR_MAIN_LINK_ACTIVE;
> -	if (!dev_priv->psr.psr2_enabled && INTEL_GEN(dev_priv) >= 8)
> -		dpcd_val |= DP_PSR_CRC_VERIFICATION;
>  	drm_dp_dpcd_writeb(&intel_dp->aux, DP_PSR_EN_CFG, dpcd_val);
>  
>  	drm_dp_dpcd_writeb(&intel_dp->aux, DP_SET_POWER,
> DP_SET_POWER_D0);
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 7/8] drm/i915/psr: Don't tell sink that main link will be active in PSR2

[Souza, Jose]

On Mon, 2018-09-24 at 23:02 -0700, dhinakaran.pandiyan@gmail.com wrote:
> On Thu, 2018-09-20 at 13:43 -0700, José Roberto de Souza wrote:
> > For PSR2 we don't have the option to keep main link enabled while
> > PSR2 is active, so don't configure sink DPCD with a wrong value.
> 
> Is this what the DP spec says or an Intel HW restriction?

This is Intel HW restriction as there is no register in PSR2 CTL to
keep link in standby, DP spec states that link can be kept on or off
while PSR2 is active, there is just a new timming restriction to turn
link off.

> 
> -DK
> 
> > 
> > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>s
> > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_psr.c | 9 +++++----
> >  1 file changed, 5 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_psr.c
> > b/drivers/gpu/drm/i915/intel_psr.c
> > index cf9d6e965697..60cf6fd251d0 100644
> > --- a/drivers/gpu/drm/i915/intel_psr.c
> > +++ b/drivers/gpu/drm/i915/intel_psr.c
> > @@ -344,12 +344,13 @@ static void intel_psr_enable_sink(struct
> > intel_dp *intel_dp)
> >  		drm_dp_dpcd_writeb(&intel_dp->aux,
> > DP_RECEIVER_ALPM_CONFIG,
> >  				   DP_ALPM_ENABLE);
> >  		dpcd_val |= DP_PSR_ENABLE_PSR2 |
> > DP_PSR_IRQ_HPD_WITH_CRC_ERRORS;
> > +	} else {
> > +		if (dev_priv->psr.link_standby)
> > +			dpcd_val |= DP_PSR_MAIN_LINK_ACTIVE;
> > +		if (INTEL_GEN(dev_priv) >= 8)
> > +			dpcd_val |= DP_PSR_CRC_VERIFICATION;
> >  	}
> >  
> > -	if (dev_priv->psr.link_standby)
> > -		dpcd_val |= DP_PSR_MAIN_LINK_ACTIVE;
> > -	if (!dev_priv->psr.psr2_enabled && INTEL_GEN(dev_priv) >= 8)
> > -		dpcd_val |= DP_PSR_CRC_VERIFICATION;
> >  	drm_dp_dpcd_writeb(&intel_dp->aux, DP_PSR_EN_CFG, dpcd_val);
> >  
> >  	drm_dp_dpcd_writeb(&intel_dp->aux, DP_SET_POWER,
> > DP_SET_POWER_D0);
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 7/8] drm/i915/psr: Don't tell sink that main link will be active in PSR2

[Souza, Jose]

On Wed, 2018-09-26 at 17:46 +0000, Souza, Jose wrote:
> On Mon, 2018-09-24 at 23:02 -0700, dhinakaran.pandiyan@gmail.com
> wrote:
> > On Thu, 2018-09-20 at 13:43 -0700, José Roberto de Souza wrote:
> > > For PSR2 we don't have the option to keep main link enabled while
> > > PSR2 is active, so don't configure sink DPCD with a wrong value.
> > 
> > Is this what the DP spec says or an Intel HW restriction?
> 
> This is Intel HW restriction as there is no register in PSR2 CTL to
> keep link in standby, DP spec states that link can be kept on or off
> while PSR2 is active, there is just a new timming restriction to turn
> link off.

I will update the commit message to make it clear:

For PSR2 there is no register to tell HW to keep main link enabled
while PSR2 is active, so don't configure sink DPCD with a
wrong value.


> 
> > 
> > -DK
> > 
> > > 
> > > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>s
> > > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/intel_psr.c | 9 +++++----
> > >  1 file changed, 5 insertions(+), 4 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/intel_psr.c
> > > b/drivers/gpu/drm/i915/intel_psr.c
> > > index cf9d6e965697..60cf6fd251d0 100644
> > > --- a/drivers/gpu/drm/i915/intel_psr.c
> > > +++ b/drivers/gpu/drm/i915/intel_psr.c
> > > @@ -344,12 +344,13 @@ static void intel_psr_enable_sink(struct
> > > intel_dp *intel_dp)
> > >  		drm_dp_dpcd_writeb(&intel_dp->aux,
> > > DP_RECEIVER_ALPM_CONFIG,
> > >  				   DP_ALPM_ENABLE);
> > >  		dpcd_val |= DP_PSR_ENABLE_PSR2 |
> > > DP_PSR_IRQ_HPD_WITH_CRC_ERRORS;
> > > +	} else {
> > > +		if (dev_priv->psr.link_standby)
> > > +			dpcd_val |= DP_PSR_MAIN_LINK_ACTIVE;
> > > +		if (INTEL_GEN(dev_priv) >= 8)
> > > +			dpcd_val |= DP_PSR_CRC_VERIFICATION;
> > >  	}
> > >  
> > > -	if (dev_priv->psr.link_standby)
> > > -		dpcd_val |= DP_PSR_MAIN_LINK_ACTIVE;
> > > -	if (!dev_priv->psr.psr2_enabled && INTEL_GEN(dev_priv) >= 8)
> > > -		dpcd_val |= DP_PSR_CRC_VERIFICATION;
> > >  	drm_dp_dpcd_writeb(&intel_dp->aux, DP_PSR_EN_CFG, dpcd_val);
> > >  
> > >  	drm_dp_dpcd_writeb(&intel_dp->aux, DP_SET_POWER,
> > > DP_SET_POWER_D0);
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 8/8] drm/i915/psr: Remove alpm from i915_psr

[José Roberto de Souza]

ALPM is a requirement and we don't need to keep it's cached, what
were done in commit 97c9de66ca80
("drm/i915/psr: Fix ALPM cap check for PSR2") but the alpm was not
removed from i915_psr.

Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 41f1082da122..4ed129cf4d12 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -630,7 +630,6 @@ struct i915_psr {
 	bool sink_psr2_support;
 	bool link_standby;
 	bool colorimetry_support;
-	bool alpm;
 	bool psr2_enabled;
 	u8 sink_sync_latency;
 	ktime_t last_entry_attempt;
-- 
2.19.0

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

Re: [PATCH 8/8] drm/i915/psr: Remove alpm from i915_psr

[dhinakaran.pandiyan]

On Thu, 2018-09-20 at 13:43 -0700, José Roberto de Souza wrote:
> ALPM is a requirement and we don't need to keep it's cached, what
> were done in commit 97c9de66ca80
> ("drm/i915/psr: Fix ALPM cap check for PSR2") but the alpm was not
> removed from i915_psr.:
You're right.

Reviewed-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>

> 
> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h
> b/drivers/gpu/drm/i915/i915_drv.h
> index 41f1082da122..4ed129cf4d12 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -630,7 +630,6 @@ struct i915_psr {
We should rename this intel_psr and move it to the same file where
struct intel_dp lives.


>  	bool sink_psr2_support;
>  	bool link_standby;
>  	bool colorimetry_support;
> -	bool alpm;
>  	bool psr2_enabled;
And rename this too?  The bool is set to enable_psr2 and does not mean
PSR2 is enabled.


>  	u8 sink_sync_latency;
>  	ktime_t last_entry_attempt;
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 8/8] drm/i915/psr: Remove alpm from i915_psr

[Souza, Jose]

On Mon, 2018-09-24 at 23:08 -0700, dhinakaran.pandiyan@gmail.com wrote:
> On Thu, 2018-09-20 at 13:43 -0700, José Roberto de Souza wrote:
> > ALPM is a requirement and we don't need to keep it's cached, what
> > were done in commit 97c9de66ca80
> > ("drm/i915/psr: Fix ALPM cap check for PSR2") but the alpm was not
> > removed from i915_psr.:
> 
> You're right.
> 
> Reviewed-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> 
> > 
> > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_drv.h | 1 -
> >  1 file changed, 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h
> > b/drivers/gpu/drm/i915/i915_drv.h
> > index 41f1082da122..4ed129cf4d12 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -630,7 +630,6 @@ struct i915_psr {
> 
> We should rename this intel_psr and move it to the same file where
> struct intel_dp lives.

Sounds a better place indeed but it would just cost more memory as we
don't enable PSR in regular DP ports also it would cause conflicts in
most of the PSR patches that we have pending, maybe we can think about
it after enable PSR by default?

> 
> 
> >  	bool sink_psr2_support;
> >  	bool link_standby;
> >  	bool colorimetry_support;
> > -	bool alpm;
> >  	bool psr2_enabled;
> 
> And rename this too?  The bool is set to enable_psr2 and does not
> mean
> PSR2 is enabled.

It have the same meaning as 'enabled', it means that sink is configured
to PSR2 and source have PSR2 hardware enabled but it do not mean that
is active at the moment.

> 
> 
> >  	u8 sink_sync_latency;
> >  	ktime_t last_entry_attempt;
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.SPARSE: warning for series starting with [1/8] drm/i915/psr: Share PSR and PSR2 exit mask

[Patchwork]

== Series Details ==

Series: series starting with [1/8] drm/i915/psr: Share PSR and PSR2 exit mask
URL   : https://patchwork.freedesktop.org/series/49993/
State : warning

== Summary ==

$ dim sparse origin/drm-tip
Commit: drm/i915/psr: Share PSR and PSR2 exit mask
Okay!

Commit: drm/i915/psr: Do not set MASK_DISP_REG_WRITE in ICL
Okay!

Commit: drm/i915/psr: Enable sink to trigger a interruption on PSR2 CRC mismatch
Okay!

Commit: drm/i915/psr: Remove PSR2 TODO error handling
Okay!

Commit: drm/i915/psr: Do not enable PSR2 if sink requires selective update X granularity
Okay!

Commit: drm/i915/psr: Use WA to force HW tracking to exit PSR2
Okay!

Commit: drm/i915/psr: Don't tell sink that main link will be active in PSR2
Okay!

Commit: drm/i915/psr: Remove alpm from i915_psr
-drivers/gpu/drm/i915/selftests/../i915_drv.h:3718:16: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/selftests/../i915_drv.h:3717:16: warning: expression using sizeof(void)

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

✓ Fi.CI.BAT: success for series starting with [1/8] drm/i915/psr: Share PSR and PSR2 exit mask

[Patchwork]

== Series Details ==

Series: series starting with [1/8] drm/i915/psr: Share PSR and PSR2 exit mask
URL   : https://patchwork.freedesktop.org/series/49993/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4854 -> Patchwork_10246 =

== Summary - SUCCESS ==

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/49993/revisions/1/mbox/

== Known issues ==

  Here are the changes found in Patchwork_10246 that come from known issues:

  === IGT changes ===

    ==== Issues hit ====

    igt@drm_mm@insert:
      {fi-skl-caroline}:  NOTRUN -> INCOMPLETE (fdo#108003)

    igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a:
      fi-byt-clapper:     PASS -> FAIL (fdo#107362, fdo#103191)
      fi-bdw-samus:       NOTRUN -> INCOMPLETE (fdo#107773)

    igt@kms_psr@primary_page_flip:
      fi-icl-u:           PASS -> FAIL (fdo#107336)
      fi-cnl-u:           PASS -> FAIL (fdo#107336)
      fi-kbl-r:           PASS -> FAIL (fdo#107336)

    
    ==== Possible fixes ====

    igt@drv_selftest@live_hugepages:
      fi-kbl-7560u:       INCOMPLETE -> PASS

    igt@gem_exec_suspend@basic-s3:
      fi-bdw-samus:       INCOMPLETE (fdo#107773) -> PASS

    igt@kms_flip@basic-flip-vs-modeset:
      fi-skl-6700hq:      DMESG-WARN (fdo#105998) -> PASS

    igt@kms_pipe_crc_basic@nonblocking-crc-pipe-b:
      fi-byt-clapper:     FAIL (fdo#107362) -> PASS

    igt@kms_psr@primary_page_flip:
      fi-kbl-7560u:       FAIL (fdo#107336) -> PASS
      fi-cfl-s3:          FAIL (fdo#107336) -> PASS

    
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  fdo#103191 https://bugs.freedesktop.org/show_bug.cgi?id=103191
  fdo#105998 https://bugs.freedesktop.org/show_bug.cgi?id=105998
  fdo#107336 https://bugs.freedesktop.org/show_bug.cgi?id=107336
  fdo#107362 https://bugs.freedesktop.org/show_bug.cgi?id=107362
  fdo#107773 https://bugs.freedesktop.org/show_bug.cgi?id=107773
  fdo#108003 https://bugs.freedesktop.org/show_bug.cgi?id=108003


== Participating hosts (52 -> 48) ==

  Additional (1): fi-skl-caroline 
  Missing    (5): fi-ctg-p8600 fi-ilk-m540 fi-byt-squawks fi-bsw-cyan fi-hsw-4200u 


== Build changes ==

    * Linux: CI_DRM_4854 -> Patchwork_10246

  CI_DRM_4854: 78a6bcff149f370d58fe0bf51c29cbe62d8bc27c @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4647: ae8187922d8de2bc739519da3bd40cf5f03f5e4f @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_10246: 2325a088dbf665a85474f22d50b1c54e8930db09 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

2325a088dbf6 drm/i915/psr: Remove alpm from i915_psr
e8974aa38bea drm/i915/psr: Don't tell sink that main link will be active in PSR2
8bae5f0d8246 drm/i915/psr: Use WA to force HW tracking to exit PSR2
dc6f8bb9a85a drm/i915/psr: Do not enable PSR2 if sink requires selective update X granularity
4ae52440e0c5 drm/i915/psr: Remove PSR2 TODO error handling
99fae4c917bd drm/i915/psr: Enable sink to trigger a interruption on PSR2 CRC mismatch
588709076949 drm/i915/psr: Do not set MASK_DISP_REG_WRITE in ICL
87d6ea8a6245 drm/i915/psr: Share PSR and PSR2 exit mask

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_10246/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.IGT: success for series starting with [1/8] drm/i915/psr: Share PSR and PSR2 exit mask

[Patchwork]

== Series Details ==

Series: series starting with [1/8] drm/i915/psr: Share PSR and PSR2 exit mask
URL   : https://patchwork.freedesktop.org/series/49993/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4854_full -> Patchwork_10246_full =

== Summary - WARNING ==

  Minor unknown changes coming with Patchwork_10246_full need to be verified
  manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_10246_full, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  

== Possible new issues ==

  Here are the unknown changes that may have been introduced in Patchwork_10246_full:

  === IGT changes ===

    ==== Warnings ====

    igt@perf_pmu@rc6:
      shard-kbl:          PASS -> SKIP

    
== Known issues ==

  Here are the changes found in Patchwork_10246_full that come from known issues:

  === IGT changes ===

    ==== Issues hit ====

    igt@kms_busy@extended-modeset-hang-newfb-render-a:
      shard-snb:          NOTRUN -> DMESG-WARN (fdo#107956)

    igt@kms_busy@extended-pageflip-hang-newfb-render-a:
      shard-apl:          PASS -> DMESG-WARN (fdo#107956)

    
    ==== Possible fixes ====

    igt@gem_eio@in-flight-1us:
      shard-glk:          FAIL (fdo#105957) -> PASS

    igt@gem_exec_await@wide-contexts:
      shard-apl:          FAIL (fdo#106680) -> PASS

    igt@gem_ppgtt@blt-vs-render-ctxn:
      shard-kbl:          INCOMPLETE (fdo#103665, fdo#106023) -> PASS

    igt@kms_available_modes_crc@available_mode_test_crc:
      shard-hsw:          FAIL (fdo#106641) -> PASS

    igt@kms_cursor_legacy@cursora-vs-flipa-toggle:
      shard-glk:          DMESG-WARN (fdo#106538, fdo#105763) -> PASS

    igt@kms_setmode@basic:
      shard-kbl:          FAIL (fdo#99912) -> PASS

    
  fdo#103665 https://bugs.freedesktop.org/show_bug.cgi?id=103665
  fdo#105763 https://bugs.freedesktop.org/show_bug.cgi?id=105763
  fdo#105957 https://bugs.freedesktop.org/show_bug.cgi?id=105957
  fdo#106023 https://bugs.freedesktop.org/show_bug.cgi?id=106023
  fdo#106538 https://bugs.freedesktop.org/show_bug.cgi?id=106538
  fdo#106641 https://bugs.freedesktop.org/show_bug.cgi?id=106641
  fdo#106680 https://bugs.freedesktop.org/show_bug.cgi?id=106680
  fdo#107956 https://bugs.freedesktop.org/show_bug.cgi?id=107956
  fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912


== Participating hosts (5 -> 5) ==

  No changes in participating hosts


== Build changes ==

    * Linux: CI_DRM_4854 -> Patchwork_10246

  CI_DRM_4854: 78a6bcff149f370d58fe0bf51c29cbe62d8bc27c @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4647: ae8187922d8de2bc739519da3bd40cf5f03f5e4f @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_10246: 2325a088dbf665a85474f22d50b1c54e8930db09 @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_10246/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/8] drm/i915/psr: Share PSR and PSR2 exit mask

[Dhinakaran Pandiyan]

On Thursday, September 20, 2018 1:43:20 PM PDT José Roberto de Souza wrote:
> Now both PSR and PSR2 have the same exit mask, so let's share then
> instead of have the same code 2 times.
> 
> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_psr.c | 34 ++++++++++++--------------------
>  1 file changed, 13 insertions(+), 21 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_psr.c
> b/drivers/gpu/drm/i915/intel_psr.c index b6838b525502..358bbcd3b5f3 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
> @@ -575,28 +575,20 @@ static void intel_psr_enable_source(struct intel_dp
> *intel_dp, else
>  			chicken &= ~VSC_DATA_SEL_SOFTWARE_CONTROL;
>  		I915_WRITE(CHICKEN_TRANS(cpu_transcoder), chicken);
> -
> -		I915_WRITE(EDP_PSR_DEBUG,
> -			   EDP_PSR_DEBUG_MASK_MEMUP |
> -			   EDP_PSR_DEBUG_MASK_HPD |
> -			   EDP_PSR_DEBUG_MASK_LPSP |
> -			   EDP_PSR_DEBUG_MASK_MAX_SLEEP |
> -			   EDP_PSR_DEBUG_MASK_DISP_REG_WRITE);
> -	} else {
> -		/*
> -		 * Per Spec: Avoid continuous PSR exit by masking MEMUP
> -		 * and HPD. also mask LPSP to avoid dependency on other
> -		 * drivers that might block runtime_pm besides
> -		 * preventing  other hw tracking issues now we can rely
> -		 * on frontbuffer tracking.
> -		 */
> -		I915_WRITE(EDP_PSR_DEBUG,
> -			   EDP_PSR_DEBUG_MASK_MEMUP |
> -			   EDP_PSR_DEBUG_MASK_HPD |
> -			   EDP_PSR_DEBUG_MASK_LPSP |
> -			   EDP_PSR_DEBUG_MASK_DISP_REG_WRITE |
> -			   EDP_PSR_DEBUG_MASK_MAX_SLEEP);
>  	}
> +
> +	/*
> +	 * Per Spec: Avoid continuous PSR exit by masking MEMUP and HPD also
> +	 * mask LPSP to avoid dependency on other drivers that might block
> +	 * runtime_pm besides preventing  other hw tracking issues now we
> +	 * can rely on frontbuffer tracking.
> +	 */

Hmm.. I don't think I understand this comment completely and we should update 
it. This patch however looks correct,
Reviewed-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>


> +	I915_WRITE(EDP_PSR_DEBUG,
> +		   EDP_PSR_DEBUG_MASK_MEMUP |
> +		   EDP_PSR_DEBUG_MASK_HPD |
> +		   EDP_PSR_DEBUG_MASK_LPSP |
> +		   EDP_PSR_DEBUG_MASK_DISP_REG_WRITE |
> +		   EDP_PSR_DEBUG_MASK_MAX_SLEEP);
>  }
> 
>  static void intel_psr_enable_locked(struct drm_i915_private *dev_priv,




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