[PATCH 1/9] drm/i915/psr: Do not activate PSR on frontbuffer flush from fbdev.

[PATCH 1/9] drm/i915/psr: Do not activate PSR on frontbuffer flush from fbdev.

[Dhinakaran Pandiyan]

There is no corresponding invalidate call before the buffer is written
to, this results in screen freezing sometime after switching to console
mode with PSR enabled. Invalidating the front buffer in the fbdev call
backs won't work either as some of them are called in atomic contexts and
{drrs, fbc, psr}_invalidate all sleep. So don't activate PSR for now.

Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
---
 drivers/gpu/drm/i915/intel_psr.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index e9feffdea899..c12af1118647 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -881,6 +881,9 @@ void intel_psr_flush(struct drm_i915_private *dev_priv,
 	if (!CAN_PSR(dev_priv))
 		return;
 
+	if (origin == ORIGIN_DIRTYFB)
+		return;
+
 	mutex_lock(&dev_priv->psr.lock);
 	if (!dev_priv->psr.enabled) {
 		mutex_unlock(&dev_priv->psr.lock);
-- 
2.14.1

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

[PATCH 2/9] drm/i915/frontbuffer: Mark frontbuffer flush and invalidate with might_sleep()

[Dhinakaran Pandiyan]

Frontbuffer flush and invalidate call psr, fbc and drrs functions that use
mutexes but they can be called in atomic contexts in the fbdev path. The
point where the spinlocks are acquired is up in the call stack that is not
entirely easy to spot, so annotate with might_sleep().

Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
---
 drivers/gpu/drm/i915/intel_frontbuffer.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_frontbuffer.c b/drivers/gpu/drm/i915/intel_frontbuffer.c
index fcfc217e754e..3a8d3d06c26a 100644
--- a/drivers/gpu/drm/i915/intel_frontbuffer.c
+++ b/drivers/gpu/drm/i915/intel_frontbuffer.c
@@ -79,6 +79,7 @@ void __intel_fb_obj_invalidate(struct drm_i915_gem_object *obj,
 		spin_unlock(&dev_priv->fb_tracking.lock);
 	}
 
+	might_sleep();
 	intel_psr_invalidate(dev_priv, frontbuffer_bits);
 	intel_edp_drrs_invalidate(dev_priv, frontbuffer_bits);
 	intel_fbc_invalidate(dev_priv, frontbuffer_bits, origin);
@@ -108,6 +109,7 @@ static void intel_frontbuffer_flush(struct drm_i915_private *dev_priv,
 	if (!frontbuffer_bits)
 		return;
 
+	might_sleep();
 	intel_edp_drrs_flush(dev_priv, frontbuffer_bits);
 	intel_psr_flush(dev_priv, frontbuffer_bits, origin);
 	intel_fbc_flush(dev_priv, frontbuffer_bits, origin);
-- 
2.14.1

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

Re: [PATCH 2/9] drm/i915/frontbuffer: Mark frontbuffer flush and invalidate with might_sleep()

[Rodrigo Vivi]

On Sat, Jan 27, 2018 at 02:49:16AM +0000, Dhinakaran Pandiyan wrote:
> Frontbuffer flush and invalidate call psr, fbc and drrs functions that use
> mutexes but they can be called in atomic contexts in the fbdev path. The
> point where the spinlocks are acquired is up in the call stack that is not
> entirely easy to spot, so annotate with might_sleep().
>

makes sense

> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>

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

> ---
>  drivers/gpu/drm/i915/intel_frontbuffer.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_frontbuffer.c b/drivers/gpu/drm/i915/intel_frontbuffer.c
> index fcfc217e754e..3a8d3d06c26a 100644
> --- a/drivers/gpu/drm/i915/intel_frontbuffer.c
> +++ b/drivers/gpu/drm/i915/intel_frontbuffer.c
> @@ -79,6 +79,7 @@ void __intel_fb_obj_invalidate(struct drm_i915_gem_object *obj,
>  		spin_unlock(&dev_priv->fb_tracking.lock);
>  	}
>  
> +	might_sleep();
>  	intel_psr_invalidate(dev_priv, frontbuffer_bits);
>  	intel_edp_drrs_invalidate(dev_priv, frontbuffer_bits);
>  	intel_fbc_invalidate(dev_priv, frontbuffer_bits, origin);
> @@ -108,6 +109,7 @@ static void intel_frontbuffer_flush(struct drm_i915_private *dev_priv,
>  	if (!frontbuffer_bits)
>  		return;
>  
> +	might_sleep();
>  	intel_edp_drrs_flush(dev_priv, frontbuffer_bits);
>  	intel_psr_flush(dev_priv, frontbuffer_bits, origin);
>  	intel_fbc_flush(dev_priv, frontbuffer_bits, origin);
> -- 
> 2.14.1
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 3/9] drm/i915/psr: Extract PSR DPCD initialization and move it to intel_psr.c

[Dhinakaran Pandiyan]

intel_edp_init_dpcd() is cluttered with PSR specific DPCD checks and
intel_dp.c is huge.

No functional change intended.

Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c  | 64 +------------------------------------
 drivers/gpu/drm/i915/intel_drv.h |  1 +
 drivers/gpu/drm/i915/intel_psr.c | 68 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 70 insertions(+), 63 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index a2e887999915..2454326690fb 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -3135,35 +3135,6 @@ intel_dp_get_link_status(struct intel_dp *intel_dp, uint8_t link_status[DP_LINK_
 				DP_LINK_STATUS_SIZE) == DP_LINK_STATUS_SIZE;
 }
 
-static bool intel_dp_get_y_cord_status(struct intel_dp *intel_dp)
-{
-	uint8_t psr_caps = 0;
-
-	if (drm_dp_dpcd_readb(&intel_dp->aux, DP_PSR_CAPS, &psr_caps) != 1)
-		return false;
-	return psr_caps & DP_PSR2_SU_Y_COORDINATE_REQUIRED;
-}
-
-static bool intel_dp_get_colorimetry_status(struct intel_dp *intel_dp)
-{
-	uint8_t dprx = 0;
-
-	if (drm_dp_dpcd_readb(&intel_dp->aux, DP_DPRX_FEATURE_ENUMERATION_LIST,
-			      &dprx) != 1)
-		return false;
-	return dprx & DP_VSC_SDP_EXT_FOR_COLORIMETRY_SUPPORTED;
-}
-
-static bool intel_dp_get_alpm_status(struct intel_dp *intel_dp)
-{
-	uint8_t alpm_caps = 0;
-
-	if (drm_dp_dpcd_readb(&intel_dp->aux, DP_RECEIVER_ALPM_CAP,
-			      &alpm_caps) != 1)
-		return false;
-	return alpm_caps & DP_ALPM_CAP;
-}
-
 /* These are source-specific values. */
 uint8_t
 intel_dp_voltage_max(struct intel_dp *intel_dp)
@@ -3714,40 +3685,7 @@ intel_edp_init_dpcd(struct intel_dp *intel_dp)
 		dev_priv->no_aux_handshake = intel_dp->dpcd[DP_MAX_DOWNSPREAD] &
 			DP_NO_AUX_HANDSHAKE_LINK_TRAINING;
 
-	/* Check if the panel supports PSR */
-	drm_dp_dpcd_read(&intel_dp->aux, DP_PSR_SUPPORT,
-			 intel_dp->psr_dpcd,
-			 sizeof(intel_dp->psr_dpcd));
-	if (intel_dp->psr_dpcd[0] & DP_PSR_IS_SUPPORTED) {
-		dev_priv->psr.sink_support = true;
-		DRM_DEBUG_KMS("Detected EDP PSR Panel.\n");
-	}
-
-	if (INTEL_GEN(dev_priv) >= 9 &&
-	    (intel_dp->psr_dpcd[0] & DP_PSR2_IS_SUPPORTED)) {
-		uint8_t frame_sync_cap;
-
-		dev_priv->psr.sink_support = true;
-		if (drm_dp_dpcd_readb(&intel_dp->aux,
-				      DP_SINK_DEVICE_AUX_FRAME_SYNC_CAP,
-				      &frame_sync_cap) != 1)
-			frame_sync_cap = 0;
-		dev_priv->psr.aux_frame_sync = frame_sync_cap ? true : false;
-		/* PSR2 needs frame sync as well */
-		dev_priv->psr.psr2_support = dev_priv->psr.aux_frame_sync;
-		DRM_DEBUG_KMS("PSR2 %s on sink",
-			      dev_priv->psr.psr2_support ? "supported" : "not supported");
-
-		if (dev_priv->psr.psr2_support) {
-			dev_priv->psr.y_cord_support =
-				intel_dp_get_y_cord_status(intel_dp);
-			dev_priv->psr.colorimetry_support =
-				intel_dp_get_colorimetry_status(intel_dp);
-			dev_priv->psr.alpm =
-				intel_dp_get_alpm_status(intel_dp);
-		}
-
-	}
+	intel_psr_init_dpcd(intel_dp);
 
 	/*
 	 * Read the eDP display control registers.
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 3cee54bc0352..a340bc04dad8 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1851,6 +1851,7 @@ bool is_hdcp_supported(struct drm_i915_private *dev_priv, enum port port);
 
 /* intel_psr.c */
 #define CAN_PSR(dev_priv) (HAS_PSR(dev_priv) && dev_priv->psr.sink_support)
+void intel_psr_init_dpcd(struct intel_dp *intel_dp);
 void intel_psr_enable(struct intel_dp *intel_dp,
 		      const struct intel_crtc_state *crtc_state);
 void intel_psr_disable(struct intel_dp *intel_dp,
diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index c12af1118647..a1b878449e83 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -56,6 +56,74 @@
 #include "intel_drv.h"
 #include "i915_drv.h"
 
+static bool intel_dp_get_y_cord_status(struct intel_dp *intel_dp)
+{
+	uint8_t psr_caps = 0;
+
+	if (drm_dp_dpcd_readb(&intel_dp->aux, DP_PSR_CAPS, &psr_caps) != 1)
+		return false;
+	return psr_caps & DP_PSR2_SU_Y_COORDINATE_REQUIRED;
+}
+
+static bool intel_dp_get_colorimetry_status(struct intel_dp *intel_dp)
+{
+	uint8_t dprx = 0;
+
+	if (drm_dp_dpcd_readb(&intel_dp->aux, DP_DPRX_FEATURE_ENUMERATION_LIST,
+			      &dprx) != 1)
+		return false;
+	return dprx & DP_VSC_SDP_EXT_FOR_COLORIMETRY_SUPPORTED;
+}
+
+static bool intel_dp_get_alpm_status(struct intel_dp *intel_dp)
+{
+	uint8_t alpm_caps = 0;
+
+	if (drm_dp_dpcd_readb(&intel_dp->aux, DP_RECEIVER_ALPM_CAP,
+			      &alpm_caps) != 1)
+		return false;
+	return alpm_caps & DP_ALPM_CAP;
+}
+
+void intel_psr_init_dpcd(struct intel_dp *intel_dp)
+{
+	struct drm_i915_private *dev_priv =
+		to_i915(dp_to_dig_port(intel_dp)->base.base.dev);
+
+	drm_dp_dpcd_read(&intel_dp->aux, DP_PSR_SUPPORT, intel_dp->psr_dpcd,
+			 sizeof(intel_dp->psr_dpcd));
+
+	if (intel_dp->psr_dpcd[0] & DP_PSR_IS_SUPPORTED) {
+		dev_priv->psr.sink_support = true;
+		DRM_DEBUG_KMS("Detected EDP PSR Panel.\n");
+	}
+
+	if (INTEL_GEN(dev_priv) >= 9 &&
+	    (intel_dp->psr_dpcd[0] & DP_PSR2_IS_SUPPORTED)) {
+		uint8_t frame_sync_cap;
+
+		dev_priv->psr.sink_support = true;
+		if (drm_dp_dpcd_readb(&intel_dp->aux,
+				      DP_SINK_DEVICE_AUX_FRAME_SYNC_CAP,
+				      &frame_sync_cap) != 1)
+			frame_sync_cap = 0;
+		dev_priv->psr.aux_frame_sync = frame_sync_cap ? true : false;
+		/* PSR2 needs frame sync as well */
+		dev_priv->psr.psr2_support = dev_priv->psr.aux_frame_sync;
+		DRM_DEBUG_KMS("PSR2 %s on sink",
+			      dev_priv->psr.psr2_support ? "supported" : "not supported");
+
+		if (dev_priv->psr.psr2_support) {
+			dev_priv->psr.y_cord_support =
+				intel_dp_get_y_cord_status(intel_dp);
+			dev_priv->psr.colorimetry_support =
+				intel_dp_get_colorimetry_status(intel_dp);
+			dev_priv->psr.alpm =
+				intel_dp_get_alpm_status(intel_dp);
+		}
+	}
+}
+
 static bool vlv_is_psr_active_on_pipe(struct drm_device *dev, int pipe)
 {
 	struct drm_i915_private *dev_priv = to_i915(dev);
-- 
2.14.1

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

Re: [PATCH 3/9] drm/i915/psr: Extract PSR DPCD initialization and move it to intel_psr.c

[David Weinehall]

On Fri, Jan 26, 2018 at 06:49:17PM -0800, Dhinakaran Pandiyan wrote:
> intel_edp_init_dpcd() is cluttered with PSR specific DPCD checks and
> intel_dp.c is huge.

Yes please!  Good idea.

> No functional change intended.

Reviewed-by: David Weinehall <david.weinehall@linux.intel.com>

> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_dp.c  | 64 +------------------------------------
>  drivers/gpu/drm/i915/intel_drv.h |  1 +
>  drivers/gpu/drm/i915/intel_psr.c | 68 ++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 70 insertions(+), 63 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index a2e887999915..2454326690fb 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -3135,35 +3135,6 @@ intel_dp_get_link_status(struct intel_dp *intel_dp, uint8_t link_status[DP_LINK_
>  				DP_LINK_STATUS_SIZE) == DP_LINK_STATUS_SIZE;
>  }
>  
> -static bool intel_dp_get_y_cord_status(struct intel_dp *intel_dp)
> -{
> -	uint8_t psr_caps = 0;
> -
> -	if (drm_dp_dpcd_readb(&intel_dp->aux, DP_PSR_CAPS, &psr_caps) != 1)
> -		return false;
> -	return psr_caps & DP_PSR2_SU_Y_COORDINATE_REQUIRED;
> -}
> -
> -static bool intel_dp_get_colorimetry_status(struct intel_dp *intel_dp)
> -{
> -	uint8_t dprx = 0;
> -
> -	if (drm_dp_dpcd_readb(&intel_dp->aux, DP_DPRX_FEATURE_ENUMERATION_LIST,
> -			      &dprx) != 1)
> -		return false;
> -	return dprx & DP_VSC_SDP_EXT_FOR_COLORIMETRY_SUPPORTED;
> -}
> -
> -static bool intel_dp_get_alpm_status(struct intel_dp *intel_dp)
> -{
> -	uint8_t alpm_caps = 0;
> -
> -	if (drm_dp_dpcd_readb(&intel_dp->aux, DP_RECEIVER_ALPM_CAP,
> -			      &alpm_caps) != 1)
> -		return false;
> -	return alpm_caps & DP_ALPM_CAP;
> -}
> -
>  /* These are source-specific values. */
>  uint8_t
>  intel_dp_voltage_max(struct intel_dp *intel_dp)
> @@ -3714,40 +3685,7 @@ intel_edp_init_dpcd(struct intel_dp *intel_dp)
>  		dev_priv->no_aux_handshake = intel_dp->dpcd[DP_MAX_DOWNSPREAD] &
>  			DP_NO_AUX_HANDSHAKE_LINK_TRAINING;
>  
> -	/* Check if the panel supports PSR */
> -	drm_dp_dpcd_read(&intel_dp->aux, DP_PSR_SUPPORT,
> -			 intel_dp->psr_dpcd,
> -			 sizeof(intel_dp->psr_dpcd));
> -	if (intel_dp->psr_dpcd[0] & DP_PSR_IS_SUPPORTED) {
> -		dev_priv->psr.sink_support = true;
> -		DRM_DEBUG_KMS("Detected EDP PSR Panel.\n");
> -	}
> -
> -	if (INTEL_GEN(dev_priv) >= 9 &&
> -	    (intel_dp->psr_dpcd[0] & DP_PSR2_IS_SUPPORTED)) {
> -		uint8_t frame_sync_cap;
> -
> -		dev_priv->psr.sink_support = true;
> -		if (drm_dp_dpcd_readb(&intel_dp->aux,
> -				      DP_SINK_DEVICE_AUX_FRAME_SYNC_CAP,
> -				      &frame_sync_cap) != 1)
> -			frame_sync_cap = 0;
> -		dev_priv->psr.aux_frame_sync = frame_sync_cap ? true : false;
> -		/* PSR2 needs frame sync as well */
> -		dev_priv->psr.psr2_support = dev_priv->psr.aux_frame_sync;
> -		DRM_DEBUG_KMS("PSR2 %s on sink",
> -			      dev_priv->psr.psr2_support ? "supported" : "not supported");
> -
> -		if (dev_priv->psr.psr2_support) {
> -			dev_priv->psr.y_cord_support =
> -				intel_dp_get_y_cord_status(intel_dp);
> -			dev_priv->psr.colorimetry_support =
> -				intel_dp_get_colorimetry_status(intel_dp);
> -			dev_priv->psr.alpm =
> -				intel_dp_get_alpm_status(intel_dp);
> -		}
> -
> -	}
> +	intel_psr_init_dpcd(intel_dp);
>  
>  	/*
>  	 * Read the eDP display control registers.
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 3cee54bc0352..a340bc04dad8 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1851,6 +1851,7 @@ bool is_hdcp_supported(struct drm_i915_private *dev_priv, enum port port);
>  
>  /* intel_psr.c */
>  #define CAN_PSR(dev_priv) (HAS_PSR(dev_priv) && dev_priv->psr.sink_support)
> +void intel_psr_init_dpcd(struct intel_dp *intel_dp);
>  void intel_psr_enable(struct intel_dp *intel_dp,
>  		      const struct intel_crtc_state *crtc_state);
>  void intel_psr_disable(struct intel_dp *intel_dp,
> diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
> index c12af1118647..a1b878449e83 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
> @@ -56,6 +56,74 @@
>  #include "intel_drv.h"
>  #include "i915_drv.h"
>  
> +static bool intel_dp_get_y_cord_status(struct intel_dp *intel_dp)
> +{
> +	uint8_t psr_caps = 0;
> +
> +	if (drm_dp_dpcd_readb(&intel_dp->aux, DP_PSR_CAPS, &psr_caps) != 1)
> +		return false;
> +	return psr_caps & DP_PSR2_SU_Y_COORDINATE_REQUIRED;
> +}
> +
> +static bool intel_dp_get_colorimetry_status(struct intel_dp *intel_dp)
> +{
> +	uint8_t dprx = 0;
> +
> +	if (drm_dp_dpcd_readb(&intel_dp->aux, DP_DPRX_FEATURE_ENUMERATION_LIST,
> +			      &dprx) != 1)
> +		return false;
> +	return dprx & DP_VSC_SDP_EXT_FOR_COLORIMETRY_SUPPORTED;
> +}
> +
> +static bool intel_dp_get_alpm_status(struct intel_dp *intel_dp)
> +{
> +	uint8_t alpm_caps = 0;
> +
> +	if (drm_dp_dpcd_readb(&intel_dp->aux, DP_RECEIVER_ALPM_CAP,
> +			      &alpm_caps) != 1)
> +		return false;
> +	return alpm_caps & DP_ALPM_CAP;
> +}
> +
> +void intel_psr_init_dpcd(struct intel_dp *intel_dp)
> +{
> +	struct drm_i915_private *dev_priv =
> +		to_i915(dp_to_dig_port(intel_dp)->base.base.dev);
> +
> +	drm_dp_dpcd_read(&intel_dp->aux, DP_PSR_SUPPORT, intel_dp->psr_dpcd,
> +			 sizeof(intel_dp->psr_dpcd));
> +
> +	if (intel_dp->psr_dpcd[0] & DP_PSR_IS_SUPPORTED) {
> +		dev_priv->psr.sink_support = true;
> +		DRM_DEBUG_KMS("Detected EDP PSR Panel.\n");
> +	}
> +
> +	if (INTEL_GEN(dev_priv) >= 9 &&
> +	    (intel_dp->psr_dpcd[0] & DP_PSR2_IS_SUPPORTED)) {
> +		uint8_t frame_sync_cap;
> +
> +		dev_priv->psr.sink_support = true;
> +		if (drm_dp_dpcd_readb(&intel_dp->aux,
> +				      DP_SINK_DEVICE_AUX_FRAME_SYNC_CAP,
> +				      &frame_sync_cap) != 1)
> +			frame_sync_cap = 0;
> +		dev_priv->psr.aux_frame_sync = frame_sync_cap ? true : false;
> +		/* PSR2 needs frame sync as well */
> +		dev_priv->psr.psr2_support = dev_priv->psr.aux_frame_sync;
> +		DRM_DEBUG_KMS("PSR2 %s on sink",
> +			      dev_priv->psr.psr2_support ? "supported" : "not supported");
> +
> +		if (dev_priv->psr.psr2_support) {
> +			dev_priv->psr.y_cord_support =
> +				intel_dp_get_y_cord_status(intel_dp);
> +			dev_priv->psr.colorimetry_support =
> +				intel_dp_get_colorimetry_status(intel_dp);
> +			dev_priv->psr.alpm =
> +				intel_dp_get_alpm_status(intel_dp);
> +		}
> +	}
> +}
> +
>  static bool vlv_is_psr_active_on_pipe(struct drm_device *dev, int pipe)
>  {
>  	struct drm_i915_private *dev_priv = to_i915(dev);
> -- 
> 2.14.1
> 
> _______________________________________________
> 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

Re: [PATCH 3/9] drm/i915/psr: Extract PSR DPCD initialization and move it to intel_psr.c

[Rodrigo Vivi]

On Wed, Jan 31, 2018 at 10:38:24AM +0000, David Weinehall wrote:
> On Fri, Jan 26, 2018 at 06:49:17PM -0800, Dhinakaran Pandiyan wrote:
> > intel_edp_init_dpcd() is cluttered with PSR specific DPCD checks and
> > intel_dp.c is huge.
> 
> Yes please!  Good idea.
> 
> > No functional change intended.
> 
> Reviewed-by: David Weinehall <david.weinehall@linux.intel.com>

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

> 
> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_dp.c  | 64 +------------------------------------
> >  drivers/gpu/drm/i915/intel_drv.h |  1 +
> >  drivers/gpu/drm/i915/intel_psr.c | 68 ++++++++++++++++++++++++++++++++++++++++
> >  3 files changed, 70 insertions(+), 63 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> > index a2e887999915..2454326690fb 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -3135,35 +3135,6 @@ intel_dp_get_link_status(struct intel_dp *intel_dp, uint8_t link_status[DP_LINK_
> >  				DP_LINK_STATUS_SIZE) == DP_LINK_STATUS_SIZE;
> >  }
> >  
> > -static bool intel_dp_get_y_cord_status(struct intel_dp *intel_dp)
> > -{
> > -	uint8_t psr_caps = 0;
> > -
> > -	if (drm_dp_dpcd_readb(&intel_dp->aux, DP_PSR_CAPS, &psr_caps) != 1)
> > -		return false;
> > -	return psr_caps & DP_PSR2_SU_Y_COORDINATE_REQUIRED;
> > -}
> > -
> > -static bool intel_dp_get_colorimetry_status(struct intel_dp *intel_dp)
> > -{
> > -	uint8_t dprx = 0;
> > -
> > -	if (drm_dp_dpcd_readb(&intel_dp->aux, DP_DPRX_FEATURE_ENUMERATION_LIST,
> > -			      &dprx) != 1)
> > -		return false;
> > -	return dprx & DP_VSC_SDP_EXT_FOR_COLORIMETRY_SUPPORTED;
> > -}
> > -
> > -static bool intel_dp_get_alpm_status(struct intel_dp *intel_dp)
> > -{
> > -	uint8_t alpm_caps = 0;
> > -
> > -	if (drm_dp_dpcd_readb(&intel_dp->aux, DP_RECEIVER_ALPM_CAP,
> > -			      &alpm_caps) != 1)
> > -		return false;
> > -	return alpm_caps & DP_ALPM_CAP;
> > -}
> > -
> >  /* These are source-specific values. */
> >  uint8_t
> >  intel_dp_voltage_max(struct intel_dp *intel_dp)
> > @@ -3714,40 +3685,7 @@ intel_edp_init_dpcd(struct intel_dp *intel_dp)
> >  		dev_priv->no_aux_handshake = intel_dp->dpcd[DP_MAX_DOWNSPREAD] &
> >  			DP_NO_AUX_HANDSHAKE_LINK_TRAINING;
> >  
> > -	/* Check if the panel supports PSR */
> > -	drm_dp_dpcd_read(&intel_dp->aux, DP_PSR_SUPPORT,
> > -			 intel_dp->psr_dpcd,
> > -			 sizeof(intel_dp->psr_dpcd));
> > -	if (intel_dp->psr_dpcd[0] & DP_PSR_IS_SUPPORTED) {
> > -		dev_priv->psr.sink_support = true;
> > -		DRM_DEBUG_KMS("Detected EDP PSR Panel.\n");
> > -	}
> > -
> > -	if (INTEL_GEN(dev_priv) >= 9 &&
> > -	    (intel_dp->psr_dpcd[0] & DP_PSR2_IS_SUPPORTED)) {
> > -		uint8_t frame_sync_cap;
> > -
> > -		dev_priv->psr.sink_support = true;
> > -		if (drm_dp_dpcd_readb(&intel_dp->aux,
> > -				      DP_SINK_DEVICE_AUX_FRAME_SYNC_CAP,
> > -				      &frame_sync_cap) != 1)
> > -			frame_sync_cap = 0;
> > -		dev_priv->psr.aux_frame_sync = frame_sync_cap ? true : false;
> > -		/* PSR2 needs frame sync as well */
> > -		dev_priv->psr.psr2_support = dev_priv->psr.aux_frame_sync;
> > -		DRM_DEBUG_KMS("PSR2 %s on sink",
> > -			      dev_priv->psr.psr2_support ? "supported" : "not supported");
> > -
> > -		if (dev_priv->psr.psr2_support) {
> > -			dev_priv->psr.y_cord_support =
> > -				intel_dp_get_y_cord_status(intel_dp);
> > -			dev_priv->psr.colorimetry_support =
> > -				intel_dp_get_colorimetry_status(intel_dp);
> > -			dev_priv->psr.alpm =
> > -				intel_dp_get_alpm_status(intel_dp);
> > -		}
> > -
> > -	}
> > +	intel_psr_init_dpcd(intel_dp);
> >  
> >  	/*
> >  	 * Read the eDP display control registers.
> > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> > index 3cee54bc0352..a340bc04dad8 100644
> > --- a/drivers/gpu/drm/i915/intel_drv.h
> > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > @@ -1851,6 +1851,7 @@ bool is_hdcp_supported(struct drm_i915_private *dev_priv, enum port port);
> >  
> >  /* intel_psr.c */
> >  #define CAN_PSR(dev_priv) (HAS_PSR(dev_priv) && dev_priv->psr.sink_support)
> > +void intel_psr_init_dpcd(struct intel_dp *intel_dp);
> >  void intel_psr_enable(struct intel_dp *intel_dp,
> >  		      const struct intel_crtc_state *crtc_state);
> >  void intel_psr_disable(struct intel_dp *intel_dp,
> > diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
> > index c12af1118647..a1b878449e83 100644
> > --- a/drivers/gpu/drm/i915/intel_psr.c
> > +++ b/drivers/gpu/drm/i915/intel_psr.c
> > @@ -56,6 +56,74 @@
> >  #include "intel_drv.h"
> >  #include "i915_drv.h"
> >  
> > +static bool intel_dp_get_y_cord_status(struct intel_dp *intel_dp)
> > +{
> > +	uint8_t psr_caps = 0;
> > +
> > +	if (drm_dp_dpcd_readb(&intel_dp->aux, DP_PSR_CAPS, &psr_caps) != 1)
> > +		return false;
> > +	return psr_caps & DP_PSR2_SU_Y_COORDINATE_REQUIRED;
> > +}
> > +
> > +static bool intel_dp_get_colorimetry_status(struct intel_dp *intel_dp)
> > +{
> > +	uint8_t dprx = 0;
> > +
> > +	if (drm_dp_dpcd_readb(&intel_dp->aux, DP_DPRX_FEATURE_ENUMERATION_LIST,
> > +			      &dprx) != 1)
> > +		return false;
> > +	return dprx & DP_VSC_SDP_EXT_FOR_COLORIMETRY_SUPPORTED;
> > +}
> > +
> > +static bool intel_dp_get_alpm_status(struct intel_dp *intel_dp)
> > +{
> > +	uint8_t alpm_caps = 0;
> > +
> > +	if (drm_dp_dpcd_readb(&intel_dp->aux, DP_RECEIVER_ALPM_CAP,
> > +			      &alpm_caps) != 1)
> > +		return false;
> > +	return alpm_caps & DP_ALPM_CAP;
> > +}
> > +
> > +void intel_psr_init_dpcd(struct intel_dp *intel_dp)
> > +{
> > +	struct drm_i915_private *dev_priv =
> > +		to_i915(dp_to_dig_port(intel_dp)->base.base.dev);
> > +
> > +	drm_dp_dpcd_read(&intel_dp->aux, DP_PSR_SUPPORT, intel_dp->psr_dpcd,
> > +			 sizeof(intel_dp->psr_dpcd));
> > +
> > +	if (intel_dp->psr_dpcd[0] & DP_PSR_IS_SUPPORTED) {
> > +		dev_priv->psr.sink_support = true;
> > +		DRM_DEBUG_KMS("Detected EDP PSR Panel.\n");
> > +	}
> > +
> > +	if (INTEL_GEN(dev_priv) >= 9 &&
> > +	    (intel_dp->psr_dpcd[0] & DP_PSR2_IS_SUPPORTED)) {
> > +		uint8_t frame_sync_cap;
> > +
> > +		dev_priv->psr.sink_support = true;
> > +		if (drm_dp_dpcd_readb(&intel_dp->aux,
> > +				      DP_SINK_DEVICE_AUX_FRAME_SYNC_CAP,
> > +				      &frame_sync_cap) != 1)
> > +			frame_sync_cap = 0;
> > +		dev_priv->psr.aux_frame_sync = frame_sync_cap ? true : false;
> > +		/* PSR2 needs frame sync as well */
> > +		dev_priv->psr.psr2_support = dev_priv->psr.aux_frame_sync;
> > +		DRM_DEBUG_KMS("PSR2 %s on sink",
> > +			      dev_priv->psr.psr2_support ? "supported" : "not supported");
> > +
> > +		if (dev_priv->psr.psr2_support) {
> > +			dev_priv->psr.y_cord_support =
> > +				intel_dp_get_y_cord_status(intel_dp);
> > +			dev_priv->psr.colorimetry_support =
> > +				intel_dp_get_colorimetry_status(intel_dp);
> > +			dev_priv->psr.alpm =
> > +				intel_dp_get_alpm_status(intel_dp);
> > +		}
> > +	}
> > +}
> > +
> >  static bool vlv_is_psr_active_on_pipe(struct drm_device *dev, int pipe)
> >  {
> >  	struct drm_i915_private *dev_priv = to_i915(dev);
> > -- 
> > 2.14.1
> > 
> > _______________________________________________
> > 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/9] drm/i915/psr: Check for the specific AUX_FRAME_SYNC cap bit.

[Dhinakaran Pandiyan]

The cap check should be specifically for bit 0 instead of any bit.

Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@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 a1b878449e83..83874bcd1142 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -107,7 +107,7 @@ void intel_psr_init_dpcd(struct intel_dp *intel_dp)
 				      DP_SINK_DEVICE_AUX_FRAME_SYNC_CAP,
 				      &frame_sync_cap) != 1)
 			frame_sync_cap = 0;
-		dev_priv->psr.aux_frame_sync = frame_sync_cap ? true : false;
+		dev_priv->psr.aux_frame_sync = frame_sync_cap & DP_AUX_FRAME_SYNC_CAP;
 		/* PSR2 needs frame sync as well */
 		dev_priv->psr.psr2_support = dev_priv->psr.aux_frame_sync;
 		DRM_DEBUG_KMS("PSR2 %s on sink",
-- 
2.14.1

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

Re: [PATCH 4/9] drm/i915/psr: Check for the specific AUX_FRAME_SYNC cap bit.

[Rodrigo Vivi]

On Sat, Jan 27, 2018 at 02:49:18AM +0000, Dhinakaran Pandiyan wrote:
> The cap check should be specifically for bit 0 instead of any bit.
> 

Any "Fixes:" ?

> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>


Reviewed-by: Rodrigo Vivi <rodrigo.vivi@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 a1b878449e83..83874bcd1142 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
> @@ -107,7 +107,7 @@ void intel_psr_init_dpcd(struct intel_dp *intel_dp)
>  				      DP_SINK_DEVICE_AUX_FRAME_SYNC_CAP,
>  				      &frame_sync_cap) != 1)
>  			frame_sync_cap = 0;
> -		dev_priv->psr.aux_frame_sync = frame_sync_cap ? true : false;
> +		dev_priv->psr.aux_frame_sync = frame_sync_cap & DP_AUX_FRAME_SYNC_CAP;
>  		/* PSR2 needs frame sync as well */
>  		dev_priv->psr.psr2_support = dev_priv->psr.aux_frame_sync;
>  		DRM_DEBUG_KMS("PSR2 %s on sink",
> -- 
> 2.14.1
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 4/9] drm/i915/psr: Check for the specific AUX_FRAME_SYNC cap bit.

[Pandiyan, Dhinakaran]

On Wed, 2018-01-31 at 22:48 -0800, Rodrigo Vivi wrote:
> On Sat, Jan 27, 2018 at 02:49:18AM +0000, Dhinakaran Pandiyan wrote:
> > The cap check should be specifically for bit 0 instead of any bit.
> > 
> 
> Any "Fixes:" ?

Fixes: 474d1ec4a3d7 ("drm/i915/skl: Enabling PSR2 SU with frame sync")

I have to clarify that the other bits are reserved and are expected to
read 0. This patch is to make sure we do the right thing, rather than to
fix any known issue.


> 
> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> 
> 
> Reviewed-by: Rodrigo Vivi <rodrigo.vivi@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 a1b878449e83..83874bcd1142 100644
> > --- a/drivers/gpu/drm/i915/intel_psr.c
> > +++ b/drivers/gpu/drm/i915/intel_psr.c
> > @@ -107,7 +107,7 @@ void intel_psr_init_dpcd(struct intel_dp *intel_dp)
> >  				      DP_SINK_DEVICE_AUX_FRAME_SYNC_CAP,
> >  				      &frame_sync_cap) != 1)
> >  			frame_sync_cap = 0;
> > -		dev_priv->psr.aux_frame_sync = frame_sync_cap ? true : false;
> > +		dev_priv->psr.aux_frame_sync = frame_sync_cap & DP_AUX_FRAME_SYNC_CAP;
> >  		/* PSR2 needs frame sync as well */
> >  		dev_priv->psr.psr2_support = dev_priv->psr.aux_frame_sync;
> >  		DRM_DEBUG_KMS("PSR2 %s on sink",
> > -- 
> > 2.14.1
> > 
> _______________________________________________
> 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 5/9] drm/i915/psr: Inline psr2 caps checks.

[Dhinakaran Pandiyan]

Add a macro to check for a bit offset in a DPCD reg, use this macro to
eliminate three functions and a local.

Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
---
 drivers/gpu/drm/i915/intel_psr.c | 66 ++++++++++++----------------------------
 1 file changed, 19 insertions(+), 47 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
index 83874bcd1142..9f83a7430e39 100644
--- a/drivers/gpu/drm/i915/intel_psr.c
+++ b/drivers/gpu/drm/i915/intel_psr.c
@@ -56,73 +56,45 @@
 #include "intel_drv.h"
 #include "i915_drv.h"
 
-static bool intel_dp_get_y_cord_status(struct intel_dp *intel_dp)
-{
-	uint8_t psr_caps = 0;
-
-	if (drm_dp_dpcd_readb(&intel_dp->aux, DP_PSR_CAPS, &psr_caps) != 1)
-		return false;
-	return psr_caps & DP_PSR2_SU_Y_COORDINATE_REQUIRED;
-}
-
-static bool intel_dp_get_colorimetry_status(struct intel_dp *intel_dp)
-{
-	uint8_t dprx = 0;
-
-	if (drm_dp_dpcd_readb(&intel_dp->aux, DP_DPRX_FEATURE_ENUMERATION_LIST,
-			      &dprx) != 1)
-		return false;
-	return dprx & DP_VSC_SDP_EXT_FOR_COLORIMETRY_SUPPORTED;
-}
-
-static bool intel_dp_get_alpm_status(struct intel_dp *intel_dp)
-{
-	uint8_t alpm_caps = 0;
-
-	if (drm_dp_dpcd_readb(&intel_dp->aux, DP_RECEIVER_ALPM_CAP,
-			      &alpm_caps) != 1)
-		return false;
-	return alpm_caps & DP_ALPM_CAP;
-}
+#define DPCD_READB(_reg, _off) ({ u8 _buf;				       \
+	drm_dp_dpcd_readb(&intel_dp->aux, _reg, &_buf) == 1 ? _buf & _off : 0; \
+})
 
 void intel_psr_init_dpcd(struct intel_dp *intel_dp)
 {
 	struct drm_i915_private *dev_priv =
 		to_i915(dp_to_dig_port(intel_dp)->base.base.dev);
+	struct i915_psr *psr = &dev_priv->psr;
 
 	drm_dp_dpcd_read(&intel_dp->aux, DP_PSR_SUPPORT, intel_dp->psr_dpcd,
 			 sizeof(intel_dp->psr_dpcd));
 
 	if (intel_dp->psr_dpcd[0] & DP_PSR_IS_SUPPORTED) {
-		dev_priv->psr.sink_support = true;
+		psr->sink_support = true;
 		DRM_DEBUG_KMS("Detected EDP PSR Panel.\n");
 	}
 
 	if (INTEL_GEN(dev_priv) >= 9 &&
 	    (intel_dp->psr_dpcd[0] & DP_PSR2_IS_SUPPORTED)) {
-		uint8_t frame_sync_cap;
-
-		dev_priv->psr.sink_support = true;
-		if (drm_dp_dpcd_readb(&intel_dp->aux,
-				      DP_SINK_DEVICE_AUX_FRAME_SYNC_CAP,
-				      &frame_sync_cap) != 1)
-			frame_sync_cap = 0;
-		dev_priv->psr.aux_frame_sync = frame_sync_cap & DP_AUX_FRAME_SYNC_CAP;
+		psr->sink_support = true;
+		psr->aux_frame_sync = DPCD_READB(DP_SINK_DEVICE_AUX_FRAME_SYNC_CAP,
+						DP_AUX_FRAME_SYNC_CAP);
 		/* PSR2 needs frame sync as well */
-		dev_priv->psr.psr2_support = dev_priv->psr.aux_frame_sync;
+		psr->psr2_support = psr->aux_frame_sync;
 		DRM_DEBUG_KMS("PSR2 %s on sink",
-			      dev_priv->psr.psr2_support ? "supported" : "not supported");
-
-		if (dev_priv->psr.psr2_support) {
-			dev_priv->psr.y_cord_support =
-				intel_dp_get_y_cord_status(intel_dp);
-			dev_priv->psr.colorimetry_support =
-				intel_dp_get_colorimetry_status(intel_dp);
-			dev_priv->psr.alpm =
-				intel_dp_get_alpm_status(intel_dp);
+			      psr->psr2_support ? "supported" : "not supported");
+
+		if (psr->psr2_support) {
+			psr->y_cord_support = DPCD_READB(DP_PSR_CAPS,
+							DP_PSR2_SU_Y_COORDINATE_REQUIRED);
+			psr->colorimetry_support = DPCD_READB(DP_DPRX_FEATURE_ENUMERATION_LIST,
+							     DP_VSC_SDP_EXT_FOR_COLORIMETRY_SUPPORTED);
+			psr->alpm = DPCD_READB(DP_RECEIVER_ALPM_CAP,
+					      DP_ALPM_CAP);
 		}
 	}
 }
+#undef DPCD_READB
 
 static bool vlv_is_psr_active_on_pipe(struct drm_device *dev, int pipe)
 {
-- 
2.14.1

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

Re: [PATCH 5/9] drm/i915/psr: Inline psr2 caps checks.

[Jani Nikula]

On Fri, 26 Jan 2018, Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com> wrote:
> Add a macro to check for a bit offset in a DPCD reg, use this macro to
> eliminate three functions and a local.

IMO less readable with this change.

BR,
Jani.


>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_psr.c | 66 ++++++++++++----------------------------
>  1 file changed, 19 insertions(+), 47 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
> index 83874bcd1142..9f83a7430e39 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
> @@ -56,73 +56,45 @@
>  #include "intel_drv.h"
>  #include "i915_drv.h"
>  
> -static bool intel_dp_get_y_cord_status(struct intel_dp *intel_dp)
> -{
> -	uint8_t psr_caps = 0;
> -
> -	if (drm_dp_dpcd_readb(&intel_dp->aux, DP_PSR_CAPS, &psr_caps) != 1)
> -		return false;
> -	return psr_caps & DP_PSR2_SU_Y_COORDINATE_REQUIRED;
> -}
> -
> -static bool intel_dp_get_colorimetry_status(struct intel_dp *intel_dp)
> -{
> -	uint8_t dprx = 0;
> -
> -	if (drm_dp_dpcd_readb(&intel_dp->aux, DP_DPRX_FEATURE_ENUMERATION_LIST,
> -			      &dprx) != 1)
> -		return false;
> -	return dprx & DP_VSC_SDP_EXT_FOR_COLORIMETRY_SUPPORTED;
> -}
> -
> -static bool intel_dp_get_alpm_status(struct intel_dp *intel_dp)
> -{
> -	uint8_t alpm_caps = 0;
> -
> -	if (drm_dp_dpcd_readb(&intel_dp->aux, DP_RECEIVER_ALPM_CAP,
> -			      &alpm_caps) != 1)
> -		return false;
> -	return alpm_caps & DP_ALPM_CAP;
> -}
> +#define DPCD_READB(_reg, _off) ({ u8 _buf;				       \
> +	drm_dp_dpcd_readb(&intel_dp->aux, _reg, &_buf) == 1 ? _buf & _off : 0; \
> +})
>  
>  void intel_psr_init_dpcd(struct intel_dp *intel_dp)
>  {
>  	struct drm_i915_private *dev_priv =
>  		to_i915(dp_to_dig_port(intel_dp)->base.base.dev);
> +	struct i915_psr *psr = &dev_priv->psr;
>  
>  	drm_dp_dpcd_read(&intel_dp->aux, DP_PSR_SUPPORT, intel_dp->psr_dpcd,
>  			 sizeof(intel_dp->psr_dpcd));
>  
>  	if (intel_dp->psr_dpcd[0] & DP_PSR_IS_SUPPORTED) {
> -		dev_priv->psr.sink_support = true;
> +		psr->sink_support = true;
>  		DRM_DEBUG_KMS("Detected EDP PSR Panel.\n");
>  	}
>  
>  	if (INTEL_GEN(dev_priv) >= 9 &&
>  	    (intel_dp->psr_dpcd[0] & DP_PSR2_IS_SUPPORTED)) {
> -		uint8_t frame_sync_cap;
> -
> -		dev_priv->psr.sink_support = true;
> -		if (drm_dp_dpcd_readb(&intel_dp->aux,
> -				      DP_SINK_DEVICE_AUX_FRAME_SYNC_CAP,
> -				      &frame_sync_cap) != 1)
> -			frame_sync_cap = 0;
> -		dev_priv->psr.aux_frame_sync = frame_sync_cap & DP_AUX_FRAME_SYNC_CAP;
> +		psr->sink_support = true;
> +		psr->aux_frame_sync = DPCD_READB(DP_SINK_DEVICE_AUX_FRAME_SYNC_CAP,
> +						DP_AUX_FRAME_SYNC_CAP);
>  		/* PSR2 needs frame sync as well */
> -		dev_priv->psr.psr2_support = dev_priv->psr.aux_frame_sync;
> +		psr->psr2_support = psr->aux_frame_sync;
>  		DRM_DEBUG_KMS("PSR2 %s on sink",
> -			      dev_priv->psr.psr2_support ? "supported" : "not supported");
> -
> -		if (dev_priv->psr.psr2_support) {
> -			dev_priv->psr.y_cord_support =
> -				intel_dp_get_y_cord_status(intel_dp);
> -			dev_priv->psr.colorimetry_support =
> -				intel_dp_get_colorimetry_status(intel_dp);
> -			dev_priv->psr.alpm =
> -				intel_dp_get_alpm_status(intel_dp);
> +			      psr->psr2_support ? "supported" : "not supported");
> +
> +		if (psr->psr2_support) {
> +			psr->y_cord_support = DPCD_READB(DP_PSR_CAPS,
> +							DP_PSR2_SU_Y_COORDINATE_REQUIRED);
> +			psr->colorimetry_support = DPCD_READB(DP_DPRX_FEATURE_ENUMERATION_LIST,
> +							     DP_VSC_SDP_EXT_FOR_COLORIMETRY_SUPPORTED);
> +			psr->alpm = DPCD_READB(DP_RECEIVER_ALPM_CAP,
> +					      DP_ALPM_CAP);
>  		}
>  	}
>  }
> +#undef DPCD_READB
>  
>  static bool vlv_is_psr_active_on_pipe(struct drm_device *dev, int pipe)
>  {

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

[PATCH 6/9] drm/i915/dp: Remove redundant sleep after AUX transaction length check.

[Dhinakaran Pandiyan]

The core already takes care of the delay before retrying. The delay now
changes to (500, 600)us instead of (500 + 1000, 600 + 1500)us.

Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c | 8 --------
 1 file changed, 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 2454326690fb..97a4557053db 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1189,14 +1189,6 @@ intel_dp_aux_ch(struct intel_dp *intel_dp,
 	if (recv_bytes == 0 || recv_bytes > 20) {
 		DRM_DEBUG_KMS("Forbidden recv_bytes = %d on aux transaction\n",
 			      recv_bytes);
-		/*
-		 * FIXME: This patch was created on top of a series that
-		 * organize the retries at drm level. There EBUSY should
-		 * also take care for 1ms wait before retrying.
-		 * That aux retries re-org is still needed and after that is
-		 * merged we remove this sleep from here.
-		 */
-		usleep_range(1000, 1500);
 		ret = -EBUSY;
 		goto out;
 	}
-- 
2.14.1

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

Re: [PATCH 6/9] drm/i915/dp: Remove redundant sleep after AUX transaction length check.

[David Weinehall]

On Fri, Jan 26, 2018 at 06:49:20PM -0800, Dhinakaran Pandiyan wrote:
> The core already takes care of the delay before retrying. The delay now
> changes to (500, 600)us instead of (500 + 1000, 600 + 1500)us.

Reviewed-by: David Weinehall <david.weinehall@linux.intel.com>

> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_dp.c | 8 --------
>  1 file changed, 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 2454326690fb..97a4557053db 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -1189,14 +1189,6 @@ intel_dp_aux_ch(struct intel_dp *intel_dp,
>  	if (recv_bytes == 0 || recv_bytes > 20) {
>  		DRM_DEBUG_KMS("Forbidden recv_bytes = %d on aux transaction\n",
>  			      recv_bytes);
> -		/*
> -		 * FIXME: This patch was created on top of a series that
> -		 * organize the retries at drm level. There EBUSY should
> -		 * also take care for 1ms wait before retrying.
> -		 * That aux retries re-org is still needed and after that is
> -		 * merged we remove this sleep from here.
> -		 */
> -		usleep_range(1000, 1500);
>  		ret = -EBUSY;
>  		goto out;
>  	}
> -- 
> 2.14.1
> 
> _______________________________________________
> 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/9] drm/i915/dp: Move comment about hw timeout to the right place.

[Dhinakaran Pandiyan]

No functional change.

Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 97a4557053db..06619998b5a3 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1136,14 +1136,14 @@ intel_dp_aux_ch(struct intel_dp *intel_dp,
 				   DP_AUX_CH_CTL_TIME_OUT_ERROR |
 				   DP_AUX_CH_CTL_RECEIVE_ERROR);
 
-			if (status & DP_AUX_CH_CTL_TIME_OUT_ERROR)
-				continue;
-
 			/* DP CTS 1.2 Core Rev 1.1, 4.2.1.1 & 4.2.1.2
 			 *   400us delay required for errors and timeouts
 			 *   Timeout errors from the HW already meet this
 			 *   requirement so skip to next iteration
 			 */
+			if (status & DP_AUX_CH_CTL_TIME_OUT_ERROR)
+				continue;
+
 			if (status & DP_AUX_CH_CTL_RECEIVE_ERROR) {
 				usleep_range(400, 500);
 				continue;
-- 
2.14.1

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

Re: [PATCH 7/9] drm/i915/dp: Move comment about hw timeout to the right place.

[David Weinehall]

On Fri, Jan 26, 2018 at 06:49:21PM -0800, Dhinakaran Pandiyan wrote:
> No functional change.
> 
> Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>

Reviewed-by: David Weinehall <david.weinehall@linux.intel.com>

> ---
>  drivers/gpu/drm/i915/intel_dp.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 97a4557053db..06619998b5a3 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -1136,14 +1136,14 @@ intel_dp_aux_ch(struct intel_dp *intel_dp,
>  				   DP_AUX_CH_CTL_TIME_OUT_ERROR |
>  				   DP_AUX_CH_CTL_RECEIVE_ERROR);
>  
> -			if (status & DP_AUX_CH_CTL_TIME_OUT_ERROR)
> -				continue;
> -
>  			/* DP CTS 1.2 Core Rev 1.1, 4.2.1.1 & 4.2.1.2
>  			 *   400us delay required for errors and timeouts
>  			 *   Timeout errors from the HW already meet this
>  			 *   requirement so skip to next iteration
>  			 */
> +			if (status & DP_AUX_CH_CTL_TIME_OUT_ERROR)
> +				continue;
> +
>  			if (status & DP_AUX_CH_CTL_RECEIVE_ERROR) {
>  				usleep_range(400, 500);
>  				continue;
> -- 
> 2.14.1
> 
> _______________________________________________
> 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/9] drm/dp: Export AUX_RETRY_INTERVAL

[Dhinakaran Pandiyan]

Drivers can use this in their retry loops too.

Cc: dri-devel@lists.freedesktop.org
Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
---
 drivers/gpu/drm/drm_dp_helper.c | 12 +++++-------
 include/drm/drm_dp_helper.h     |  2 ++
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
index ffe14ec3e7f2..0a7c8d6e7d8c 100644
--- a/drivers/gpu/drm/drm_dp_helper.c
+++ b/drivers/gpu/drm/drm_dp_helper.c
@@ -169,8 +169,6 @@ int drm_dp_bw_code_to_link_rate(u8 link_bw)
 }
 EXPORT_SYMBOL(drm_dp_bw_code_to_link_rate);
 
-#define AUX_RETRY_INTERVAL 500 /* us */
-
 /**
  * DOC: dp helpers
  *
@@ -206,8 +204,8 @@ static int drm_dp_dpcd_access(struct drm_dp_aux *aux, u8 request,
 	 */
 	for (retry = 0; retry < 32; retry++) {
 		if (ret != 0 && ret != -ETIMEDOUT) {
-			usleep_range(AUX_RETRY_INTERVAL,
-				     AUX_RETRY_INTERVAL + 100);
+			usleep_range(DP_AUX_RETRY_INTERVAL,
+				     DP_AUX_RETRY_INTERVAL + 100);
 		}
 
 		ret = aux->transfer(aux, &msg);
@@ -718,7 +716,7 @@ static int drm_dp_i2c_retry_count(const struct drm_dp_aux_msg *msg,
 		drm_dp_aux_reply_duration(msg);
 	int i2c_time_us = drm_dp_i2c_msg_duration(msg, i2c_speed_khz);
 
-	return DIV_ROUND_UP(i2c_time_us, aux_time_us + AUX_RETRY_INTERVAL);
+	return DIV_ROUND_UP(i2c_time_us, aux_time_us + DP_AUX_RETRY_INTERVAL);
 }
 
 /*
@@ -795,7 +793,7 @@ static int drm_dp_i2c_do_msg(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg)
 			 * For now just defer for long enough to hopefully be
 			 * safe for all use-cases.
 			 */
-			usleep_range(AUX_RETRY_INTERVAL, AUX_RETRY_INTERVAL + 100);
+			usleep_range(DP_AUX_RETRY_INTERVAL, DP_AUX_RETRY_INTERVAL + 100);
 			continue;
 
 		default:
@@ -827,7 +825,7 @@ static int drm_dp_i2c_do_msg(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg)
 			aux->i2c_defer_count++;
 			if (defer_i2c < 7)
 				defer_i2c++;
-			usleep_range(AUX_RETRY_INTERVAL, AUX_RETRY_INTERVAL + 100);
+			usleep_range(DP_AUX_RETRY_INTERVAL, DP_AUX_RETRY_INTERVAL + 100);
 			drm_dp_i2c_msg_write_status_update(msg);
 
 			continue;
diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
index c239e6e24a10..2eae1aed2d26 100644
--- a/include/drm/drm_dp_helper.h
+++ b/include/drm/drm_dp_helper.h
@@ -61,6 +61,8 @@
 #define DP_AUX_I2C_REPLY_DEFER		(0x2 << 2)
 #define DP_AUX_I2C_REPLY_MASK		(0x3 << 2)
 
+#define DP_AUX_RETRY_INTERVAL		500 /* us */
+
 /* AUX CH addresses */
 /* DPCD */
 #define DP_DPCD_REV                         0x000
-- 
2.14.1

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

Re: [Intel-gfx] [PATCH 8/9] drm/dp: Export AUX_RETRY_INTERVAL

[Rodrigo Vivi]

On Sat, Jan 27, 2018 at 02:49:22AM +0000, Dhinakaran Pandiyan wrote:
> Drivers can use this in their retry loops too.

with all this layers of retries it is good that we find some consistency somewhere

is this written down on any part of eDP spec?
Last time I saw there was different retries values on different cases.
So I'm afraid this here would confuse instead of helping.

> 
> Cc: dri-devel@lists.freedesktop.org
> Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> ---
>  drivers/gpu/drm/drm_dp_helper.c | 12 +++++-------
>  include/drm/drm_dp_helper.h     |  2 ++
>  2 files changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_dp_helper.c b/drivers/gpu/drm/drm_dp_helper.c
> index ffe14ec3e7f2..0a7c8d6e7d8c 100644
> --- a/drivers/gpu/drm/drm_dp_helper.c
> +++ b/drivers/gpu/drm/drm_dp_helper.c
> @@ -169,8 +169,6 @@ int drm_dp_bw_code_to_link_rate(u8 link_bw)
>  }
>  EXPORT_SYMBOL(drm_dp_bw_code_to_link_rate);
>  
> -#define AUX_RETRY_INTERVAL 500 /* us */
> -
>  /**
>   * DOC: dp helpers
>   *
> @@ -206,8 +204,8 @@ static int drm_dp_dpcd_access(struct drm_dp_aux *aux, u8 request,
>  	 */
>  	for (retry = 0; retry < 32; retry++) {
>  		if (ret != 0 && ret != -ETIMEDOUT) {
> -			usleep_range(AUX_RETRY_INTERVAL,
> -				     AUX_RETRY_INTERVAL + 100);
> +			usleep_range(DP_AUX_RETRY_INTERVAL,
> +				     DP_AUX_RETRY_INTERVAL + 100);
>  		}
>  
>  		ret = aux->transfer(aux, &msg);
> @@ -718,7 +716,7 @@ static int drm_dp_i2c_retry_count(const struct drm_dp_aux_msg *msg,
>  		drm_dp_aux_reply_duration(msg);
>  	int i2c_time_us = drm_dp_i2c_msg_duration(msg, i2c_speed_khz);
>  
> -	return DIV_ROUND_UP(i2c_time_us, aux_time_us + AUX_RETRY_INTERVAL);
> +	return DIV_ROUND_UP(i2c_time_us, aux_time_us + DP_AUX_RETRY_INTERVAL);
>  }
>  
>  /*
> @@ -795,7 +793,7 @@ static int drm_dp_i2c_do_msg(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg)
>  			 * For now just defer for long enough to hopefully be
>  			 * safe for all use-cases.
>  			 */
> -			usleep_range(AUX_RETRY_INTERVAL, AUX_RETRY_INTERVAL + 100);
> +			usleep_range(DP_AUX_RETRY_INTERVAL, DP_AUX_RETRY_INTERVAL + 100);
>  			continue;
>  
>  		default:
> @@ -827,7 +825,7 @@ static int drm_dp_i2c_do_msg(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg)
>  			aux->i2c_defer_count++;
>  			if (defer_i2c < 7)
>  				defer_i2c++;
> -			usleep_range(AUX_RETRY_INTERVAL, AUX_RETRY_INTERVAL + 100);
> +			usleep_range(DP_AUX_RETRY_INTERVAL, DP_AUX_RETRY_INTERVAL + 100);
>  			drm_dp_i2c_msg_write_status_update(msg);
>  
>  			continue;
> diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
> index c239e6e24a10..2eae1aed2d26 100644
> --- a/include/drm/drm_dp_helper.h
> +++ b/include/drm/drm_dp_helper.h
> @@ -61,6 +61,8 @@
>  #define DP_AUX_I2C_REPLY_DEFER		(0x2 << 2)
>  #define DP_AUX_I2C_REPLY_MASK		(0x3 << 2)
>  
> +#define DP_AUX_RETRY_INTERVAL		500 /* us */
> +
>  /* AUX CH addresses */
>  /* DPCD */
>  #define DP_DPCD_REV                         0x000
> -- 
> 2.14.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 9/9] drm/i915/dp: Use the same aux retry interval as the core.

[Dhinakaran Pandiyan]

Keeps the wait times consistent.

Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 06619998b5a3..3c64b2e92575 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1145,9 +1145,11 @@ intel_dp_aux_ch(struct intel_dp *intel_dp,
 				continue;
 
 			if (status & DP_AUX_CH_CTL_RECEIVE_ERROR) {
-				usleep_range(400, 500);
+				usleep_range(DP_AUX_RETRY_INTERVAL,
+					     DP_AUX_RETRY_INTERVAL + 100);
 				continue;
 			}
+
 			if (status & DP_AUX_CH_CTL_DONE)
 				goto done;
 		}
-- 
2.14.1

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

Re: [PATCH 9/9] drm/i915/dp: Use the same aux retry interval as the core.

[Rodrigo Vivi]

On Sat, Jan 27, 2018 at 02:49:23AM +0000, Dhinakaran Pandiyan wrote:
> Keeps the wait times consistent.
> 
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>

in case the other one goes through:

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

;)

> ---
>  drivers/gpu/drm/i915/intel_dp.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 06619998b5a3..3c64b2e92575 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -1145,9 +1145,11 @@ intel_dp_aux_ch(struct intel_dp *intel_dp,
>  				continue;
>  
>  			if (status & DP_AUX_CH_CTL_RECEIVE_ERROR) {
> -				usleep_range(400, 500);
> +				usleep_range(DP_AUX_RETRY_INTERVAL,
> +					     DP_AUX_RETRY_INTERVAL + 100);
>  				continue;
>  			}
> +
>  			if (status & DP_AUX_CH_CTL_DONE)
>  				goto done;
>  		}
> -- 
> 2.14.1
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.BAT: failure for series starting with [1/9] drm/i915/psr: Do not activate PSR on frontbuffer flush from fbdev.

[Patchwork]

== Series Details ==

Series: series starting with [1/9] drm/i915/psr: Do not activate PSR on frontbuffer flush from fbdev.
URL   : https://patchwork.freedesktop.org/series/37222/
State : failure

== Summary ==

Series 37222v1 series starting with [1/9] drm/i915/psr: Do not activate PSR on frontbuffer flush from fbdev.
https://patchwork.freedesktop.org/api/1.0/series/37222/revisions/1/mbox/

Test debugfs_test:
        Subgroup read_all_entries:
                dmesg-fail -> DMESG-WARN (fi-elk-e7500) fdo#103989
Test gem_exec_suspend:
        Subgroup basic-s4-devices:
                pass       -> FAIL       (fi-cnl-y2)
Test kms_pipe_crc_basic:
        Subgroup suspend-read-crc-pipe-b:
                pass       -> INCOMPLETE (fi-snb-2520m) fdo#103713
                pass       -> FAIL       (fi-cnl-y2)
        Subgroup suspend-read-crc-pipe-c:
                pass       -> FAIL       (fi-cnl-y2)

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

fi-bdw-5557u     total:288  pass:267  dwarn:0   dfail:0   fail:0   skip:21  time:426s
fi-bdw-gvtdvm    total:288  pass:264  dwarn:0   dfail:0   fail:0   skip:24  time:424s
fi-blb-e6850     total:288  pass:223  dwarn:1   dfail:0   fail:0   skip:64  time:373s
fi-bsw-n3050     total:288  pass:242  dwarn:0   dfail:0   fail:0   skip:46  time:485s
fi-bwr-2160      total:288  pass:183  dwarn:0   dfail:0   fail:0   skip:105 time:283s
fi-bxt-dsi       total:288  pass:258  dwarn:0   dfail:0   fail:0   skip:30  time:484s
fi-bxt-j4205     total:288  pass:259  dwarn:0   dfail:0   fail:0   skip:29  time:486s
fi-byt-j1900     total:288  pass:253  dwarn:0   dfail:0   fail:0   skip:35  time:464s
fi-byt-n2820     total:288  pass:249  dwarn:0   dfail:0   fail:0   skip:39  time:452s
fi-cfl-s2        total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:565s
fi-cnl-y2        total:288  pass:258  dwarn:0   dfail:0   fail:3   skip:27  time:533s
fi-elk-e7500     total:224  pass:168  dwarn:10  dfail:0   fail:0   skip:45 
fi-gdg-551       total:288  pass:179  dwarn:0   dfail:0   fail:1   skip:108 time:275s
fi-glk-1         total:288  pass:260  dwarn:0   dfail:0   fail:0   skip:28  time:512s
fi-hsw-4770      total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:392s
fi-hsw-4770r     total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:401s
fi-ilk-650       total:288  pass:228  dwarn:0   dfail:0   fail:0   skip:60  time:410s
fi-ivb-3520m     total:288  pass:259  dwarn:0   dfail:0   fail:0   skip:29  time:460s
fi-ivb-3770      total:288  pass:255  dwarn:0   dfail:0   fail:0   skip:33  time:411s
fi-kbl-7500u     total:288  pass:263  dwarn:1   dfail:0   fail:0   skip:24  time:458s
fi-kbl-7560u     total:288  pass:269  dwarn:0   dfail:0   fail:0   skip:19  time:497s
fi-kbl-7567u     total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:462s
fi-kbl-r         total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:499s
fi-pnv-d510      total:288  pass:222  dwarn:1   dfail:0   fail:0   skip:65  time:575s
fi-skl-6260u     total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:436s
fi-skl-6600u     total:288  pass:261  dwarn:0   dfail:0   fail:0   skip:27  time:511s
fi-skl-6700hq    total:288  pass:262  dwarn:0   dfail:0   fail:0   skip:26  time:529s
fi-skl-6700k2    total:288  pass:264  dwarn:0   dfail:0   fail:0   skip:24  time:490s
fi-skl-6770hq    total:288  pass:268  dwarn:0   dfail:0   fail:0   skip:20  time:484s
fi-skl-guc       total:288  pass:260  dwarn:0   dfail:0   fail:0   skip:28  time:414s
fi-skl-gvtdvm    total:288  pass:265  dwarn:0   dfail:0   fail:0   skip:23  time:432s
fi-snb-2520m     total:245  pass:211  dwarn:0   dfail:0   fail:0   skip:33 
fi-snb-2600      total:288  pass:248  dwarn:0   dfail:0   fail:0   skip:40  time:400s
Blacklisted hosts:
fi-glk-dsi       total:288  pass:258  dwarn:0   dfail:0   fail:0   skip:30  time:473s

59275f1cec1d31adab39ddb6ab948519ac8ddffb drm-tip: 2018y-01m-26d-13h-05m-14s UTC integration manifest
613153d13cc4 drm/i915/dp: Use the same aux retry interval as the core.
df3fbe824edd drm/dp: Export AUX_RETRY_INTERVAL
d478a7b9a4f2 drm/i915/dp: Move comment about hw timeout to the right place.
0a1e9441a39c drm/i915/dp: Remove redundant sleep after AUX transaction length check.
bd9979fae5cc drm/i915/psr: Inline psr2 caps checks.
46bad20dccd5 drm/i915/psr: Check for the specific AUX_FRAME_SYNC cap bit.
f171648ca84d drm/i915/psr: Extract PSR DPCD initialization and move it to intel_psr.c
000a9a7e6e7c drm/i915/frontbuffer: Mark frontbuffer flush and invalidate with might_sleep()
8b7fcc9c09b2 drm/i915/psr: Do not activate PSR on frontbuffer flush from fbdev.

== Logs ==

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

Re: [PATCH 1/9] drm/i915/psr: Do not activate PSR on frontbuffer flush from fbdev.

[Rodrigo Vivi]

On Sat, Jan 27, 2018 at 02:49:15AM +0000, Dhinakaran Pandiyan wrote:
> There is no corresponding invalidate call before the buffer is written
> to, this results in screen freezing sometime after switching to console
> mode with PSR enabled. Invalidating the front buffer in the fbdev call
> backs won't work either as some of them are called in atomic contexts and
> {drrs, fbc, psr}_invalidate all sleep. So don't activate PSR for now.
> 
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_psr.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
> index e9feffdea899..c12af1118647 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
> @@ -881,6 +881,9 @@ void intel_psr_flush(struct drm_i915_private *dev_priv,
>  	if (!CAN_PSR(dev_priv))
>  		return;
>  
> +	if (origin == ORIGIN_DIRTYFB)
> +		return;
> +

I'd like Paulo to take a look here.

What I'm confused now is who (what fbt bit) is actually blocking PSR to work
on fbdev.

And what would be the risks of other corner cases or the risk of this
not getting psr re-enabled until next reboot...

>  	mutex_lock(&dev_priv->psr.lock);
>  	if (!dev_priv->psr.enabled) {
>  		mutex_unlock(&dev_priv->psr.lock);
> -- 
> 2.14.1
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/9] drm/i915/psr: Do not activate PSR on frontbuffer flush from fbdev.

[Pandiyan, Dhinakaran]

On Wed, 2018-01-31 at 22:56 -0800, Rodrigo Vivi wrote:
> On Sat, Jan 27, 2018 at 02:49:15AM +0000, Dhinakaran Pandiyan wrote:
> > There is no corresponding invalidate call before the buffer is written
> > to, this results in screen freezing sometime after switching to console
> > mode with PSR enabled. Invalidating the front buffer in the fbdev call
> > backs won't work either as some of them are called in atomic contexts and
> > {drrs, fbc, psr}_invalidate all sleep. So don't activate PSR for now.
> > 
> > Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_psr.c | 3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
> > index e9feffdea899..c12af1118647 100644
> > --- a/drivers/gpu/drm/i915/intel_psr.c
> > +++ b/drivers/gpu/drm/i915/intel_psr.c
> > @@ -881,6 +881,9 @@ void intel_psr_flush(struct drm_i915_private *dev_priv,
> >  	if (!CAN_PSR(dev_priv))
> >  		return;
> >  
> > +	if (origin == ORIGIN_DIRTYFB)
> > +		return;
> > +
> 
> I'd like Paulo to take a look here.
> 
> What I'm confused now is who (what fbt bit) is actually blocking PSR to work
> on fbdev.

intel_psr_flush()->intel_psr_exit() does get called as expected, so it's
not the frontbuffer bit that is preventing PSR from exiting. I don't
fully understand why exactly intel_psr_flush() is insufficient (without
psr_invalidate()).

One thing that might be of interest is the gem object has write domain
as zero. I am wondering if we are missing some initialization. But, that
does not explain why the screen freezes only with PSR

> 
> And what would be the risks of other corner cases or the risk of this
> not getting psr re-enabled until next reboot...
> 


> >  	mutex_lock(&dev_priv->psr.lock);
> >  	if (!dev_priv->psr.enabled) {
> >  		mutex_unlock(&dev_priv->psr.lock);
> > -- 
> > 2.14.1
> > 
> _______________________________________________
> 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