Re: [Intel-gfx] [PATCH v3 6/7] drm/i915/psr/bdw+: Enable CRC check in the static frame on the sink side

2018-05-22 Thread Dhinakaran Pandiyan
On Thu, 2018-05-17 at 15:21 -0700, José Roberto de Souza wrote:
> Sink can be configured to calculate the CRC over the static frame and
> compare with the CRC calculated and transmited in the VSC SDP by
> source, if there is a mismatch sink will do a short pulse in HPD
> and set DP_PSR_LINK_CRC_ERROR on DP_PSR_ERROR_STATUS.
> 
> Also spec recommends to disable MAX_SLEEP as a trigger to exit PSR
> when
> CRC check is enabled to improve power savings.
> 
> Spec: 7723
> 
> v3:
> disabling PSR instead of exiting on error
> 
> Cc: Dhinakaran Pandiyan 
> Cc: Rodrigo Vivi 
> Signed-off-by: José Roberto de Souza 
> ---
>  drivers/gpu/drm/i915/i915_reg.h  |  1 +
>  drivers/gpu/drm/i915/intel_psr.c | 29 -
>  2 files changed, 21 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h
> b/drivers/gpu/drm/i915/i915_reg.h
> index 196a0eb79272..1add22e664ea 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -4020,6 +4020,7 @@ enum {
>  #define   EDP_PSR_SKIP_AUX_EXIT  (1<<12)
>  #define   EDP_PSR_TP1_TP2_SEL(0<<11)
>  #define   EDP_PSR_TP1_TP3_SEL(1<<11)
> +#define   EDP_PSR_CRC_ENABLE (1<<10) /* BDW+
> */
>  #define   EDP_PSR_TP2_TP3_TIME_500us (0<<8)
>  #define   EDP_PSR_TP2_TP3_TIME_100us (1<<8)
>  #define   EDP_PSR_TP2_TP3_TIME_2500us(2<<8)
> diff --git a/drivers/gpu/drm/i915/intel_psr.c
> b/drivers/gpu/drm/i915/intel_psr.c
> index f72e3f91809f..2f29dcd6f69e 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
> @@ -363,6 +363,8 @@ static void hsw_psr_enable_sink(struct intel_dp
> *intel_dp)
>   dpcd_val |= DP_PSR_ENABLE_PSR2;
>   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);
> @@ -418,6 +420,9 @@ static void hsw_activate_psr1(struct intel_dp
> *intel_dp)
>   else
>   val |= EDP_PSR_TP1_TP2_SEL;
>  
> + if (INTEL_GEN(dev_priv) >= 8)
> + val |= EDP_PSR_CRC_ENABLE;
> +
>   val |= I915_READ(EDP_PSR_CTL) &
> EDP_PSR_RESTORE_PSR_ACTIVE_CTX_MASK;
>   I915_WRITE(EDP_PSR_CTL, val);
>  }
> @@ -635,11 +640,14 @@ static void hsw_psr_enable_source(struct
> intel_dp *intel_dp,
>    * 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);
> + u32 val = EDP_PSR_DEBUG_MASK_MEMUP |
> +   EDP_PSR_DEBUG_MASK_HPD |
> +   EDP_PSR_DEBUG_MASK_LPSP |
> +   EDP_PSR_DEBUG_MASK_DISP_REG_WRITE;
> +
> + if (INTEL_GEN(dev_priv) >= 8)
> + val |= EDP_PSR_DEBUG_MASK_MAX_SLEEP;
Move Patch 7/7 ahead of this one to avoid removing this check again?

> + I915_WRITE(EDP_PSR_DEBUG, val);
>   }
>  }
>  
> @@ -1051,16 +1059,19 @@ void intel_psr_short_pulse(struct intel_dp
> *intel_dp)
>   goto exit;
>   }
>  
> - if (val & DP_PSR_RFB_STORAGE_ERROR) {
> - DRM_DEBUG_KMS("PSR RFB storage error, exiting
> PSR\n");
> + if (val & (DP_PSR_RFB_STORAGE_ERROR |
> DP_PSR_LINK_CRC_ERROR)) {
> + if (val & DP_PSR_RFB_STORAGE_ERROR)
> + DRM_DEBUG_KMS("PSR RFB storage error,
> disabling PSR\n");
> + if (val & DP_PSR_LINK_CRC_ERROR)
> + DRM_DEBUG_KMS("PSR Link CRC error, disabling
> PSR\n");
>   psr_disable(intel_dp);
>   }
> - if (val & (DP_PSR_VSC_SDP_UNCORRECTABLE_ERROR |
> DP_PSR_LINK_CRC_ERROR))
> + if (val & DP_PSR_VSC_SDP_UNCORRECTABLE_ERROR)
>   DRM_ERROR("PSR_ERROR_STATUS not handled %x\n", val);
>   /* clear status register */
>   drm_dp_dpcd_writeb(&intel_dp->aux, DP_PSR_ERROR_STATUS,
> val);
>  
> - /* TODO: handle other PSR/PSR2 errors */
> + /* 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


[Intel-gfx] [PATCH v3 6/7] drm/i915/psr/bdw+: Enable CRC check in the static frame on the sink side

2018-05-17 Thread José Roberto de Souza
Sink can be configured to calculate the CRC over the static frame and
compare with the CRC calculated and transmited in the VSC SDP by
source, if there is a mismatch sink will do a short pulse in HPD
and set DP_PSR_LINK_CRC_ERROR on DP_PSR_ERROR_STATUS.

Also spec recommends to disable MAX_SLEEP as a trigger to exit PSR when
CRC check is enabled to improve power savings.

Spec: 7723

v3:
disabling PSR instead of exiting on error

Cc: Dhinakaran Pandiyan 
Cc: Rodrigo Vivi 
Signed-off-by: José Roberto de Souza 
---
 drivers/gpu/drm/i915/i915_reg.h  |  1 +
 drivers/gpu/drm/i915/intel_psr.c | 29 -
 2 files changed, 21 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 196a0eb79272..1add22e664ea 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -4020,6 +4020,7 @@ enum {
 #define   EDP_PSR_SKIP_AUX_EXIT(1<<12)
 #define   EDP_PSR_TP1_TP2_SEL  (0<<11)
 #define   EDP_PSR_TP1_TP3_SEL  (1<<11)
+#define   EDP_PSR_CRC_ENABLE   (1<<10) /* BDW+ */
 #define   EDP_PSR_TP2_TP3_TIME_500us   (0<<8)
 #define   EDP_PSR_TP2_TP3_TIME_100us   (1<<8)
 #define   EDP_PSR_TP2_TP3_TIME_2500us  (2<<8)
diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index f72e3f91809f..2f29dcd6f69e 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -363,6 +363,8 @@ static void hsw_psr_enable_sink(struct intel_dp *intel_dp)
dpcd_val |= DP_PSR_ENABLE_PSR2;
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);
@@ -418,6 +420,9 @@ static void hsw_activate_psr1(struct intel_dp *intel_dp)
else
val |= EDP_PSR_TP1_TP2_SEL;
 
+   if (INTEL_GEN(dev_priv) >= 8)
+   val |= EDP_PSR_CRC_ENABLE;
+
val |= I915_READ(EDP_PSR_CTL) & EDP_PSR_RESTORE_PSR_ACTIVE_CTX_MASK;
I915_WRITE(EDP_PSR_CTL, val);
 }
@@ -635,11 +640,14 @@ static void hsw_psr_enable_source(struct intel_dp 
*intel_dp,
 * 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);
+   u32 val = EDP_PSR_DEBUG_MASK_MEMUP |
+ EDP_PSR_DEBUG_MASK_HPD |
+ EDP_PSR_DEBUG_MASK_LPSP |
+ EDP_PSR_DEBUG_MASK_DISP_REG_WRITE;
+
+   if (INTEL_GEN(dev_priv) >= 8)
+   val |= EDP_PSR_DEBUG_MASK_MAX_SLEEP;
+   I915_WRITE(EDP_PSR_DEBUG, val);
}
 }
 
@@ -1051,16 +1059,19 @@ void intel_psr_short_pulse(struct intel_dp *intel_dp)
goto exit;
}
 
-   if (val & DP_PSR_RFB_STORAGE_ERROR) {
-   DRM_DEBUG_KMS("PSR RFB storage error, exiting PSR\n");
+   if (val & (DP_PSR_RFB_STORAGE_ERROR | DP_PSR_LINK_CRC_ERROR)) {
+   if (val & DP_PSR_RFB_STORAGE_ERROR)
+   DRM_DEBUG_KMS("PSR RFB storage error, disabling PSR\n");
+   if (val & DP_PSR_LINK_CRC_ERROR)
+   DRM_DEBUG_KMS("PSR Link CRC error, disabling PSR\n");
psr_disable(intel_dp);
}
-   if (val & (DP_PSR_VSC_SDP_UNCORRECTABLE_ERROR | DP_PSR_LINK_CRC_ERROR))
+   if (val & DP_PSR_VSC_SDP_UNCORRECTABLE_ERROR)
DRM_ERROR("PSR_ERROR_STATUS not handled %x\n", val);
/* clear status register */
drm_dp_dpcd_writeb(&intel_dp->aux, DP_PSR_ERROR_STATUS, val);
 
-   /* TODO: handle other PSR/PSR2 errors */
+   /* TODO: handle PSR2 errors */
 exit:
mutex_unlock(&psr->lock);
 }
-- 
2.17.0

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