[Intel-gfx] [PATCH 1/7] drm/i915/display: Wait PSR2 get out of deep sleep to update pipe

[Intel-gfx] [PATCH 1/7] drm/i915/display: Wait PSR2 get out of deep sleep to update pipe

[José Roberto de Souza]

Alderlake-P was getting 'max time under evasion' messages when PSR2
was enabled, this is due PIPE_SCANLINE/PIPEDSL returning 0 over a
period of time longer than VBLANK_EVASION_TIME_US.

For PSR1 we had the same issue so intel_psr_wait_for_idle() was
implemented to wait for PSR1 to get into idle state but nothing was
done for PSR2.

For PSR2 we can't only wait for idle state as PSR2 tends to keep
into sleep state that means it is ready to send selective updates.

To do so it was necessary to add intel_wait_for_condition(), this
takes as parameter a function that will return true when the desidered
condition is meet.

Cc: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>
Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
---
 .../drm/i915/display/intel_display_debugfs.c  |  3 +-
 drivers/gpu/drm/i915/display/intel_psr.c      | 64 ++++++++++++-------
 drivers/gpu/drm/i915/i915_reg.h               |  5 +-
 drivers/gpu/drm/i915/intel_uncore.c           | 47 ++++++++++++++
 drivers/gpu/drm/i915/intel_uncore.h           |  7 ++
 5 files changed, 100 insertions(+), 26 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display_debugfs.c b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
index 68f4ba8c46e75..662596adb1da6 100644
--- a/drivers/gpu/drm/i915/display/intel_display_debugfs.c
+++ b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
@@ -303,8 +303,7 @@ psr_source_status(struct intel_dp *intel_dp, struct seq_file *m)
 		};
 		val = intel_de_read(dev_priv,
 				    EDP_PSR2_STATUS(intel_dp->psr.transcoder));
-		status_val = (val & EDP_PSR2_STATUS_STATE_MASK) >>
-			      EDP_PSR2_STATUS_STATE_SHIFT;
+		status_val = REG_FIELD_GET(EDP_PSR2_STATUS_STATE_MASK, val);
 		if (status_val < ARRAY_SIZE(live_status))
 			status = live_status[status_val];
 	} else {
diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c
index 19a96d3c4acf4..a2e4ef42be60a 100644
--- a/drivers/gpu/drm/i915/display/intel_psr.c
+++ b/drivers/gpu/drm/i915/display/intel_psr.c
@@ -1796,15 +1796,33 @@ void intel_psr_post_plane_update(const struct intel_atomic_state *state)
 		_intel_psr_post_plane_update(state, crtc_state);
 }
 
-/**
- * psr_wait_for_idle - wait for PSR1 to idle
- * @intel_dp: Intel DP
- * @out_value: PSR status in case of failure
- *
- * Returns: 0 on success or -ETIMEOUT if PSR status does not idle.
- *
- */
-static int psr_wait_for_idle(struct intel_dp *intel_dp, u32 *out_value)
+static bool _is_psr2_read_for_pipe_update(void *data)
+{
+	struct intel_dp *intel_dp = data;
+	struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
+	u32 val;
+
+	val = intel_uncore_read_fw(&dev_priv->uncore,
+				   EDP_PSR2_STATUS(intel_dp->psr.transcoder));
+	val &= EDP_PSR2_STATUS_STATE_MASK;
+
+	return val == EDP_PSR2_STATUS_STATE_SLEEP || val == EDP_PSR2_STATUS_STATE_IDLE;
+}
+
+static int _psr2_ready_for_pipe_update_locked(struct intel_dp *intel_dp)
+{
+	struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
+	unsigned int fw;
+
+	fw = intel_uncore_forcewake_for_reg(&dev_priv->uncore,
+					    EDP_PSR2_STATUS(intel_dp->psr.transcoder),
+					    FW_REG_READ);
+	return intel_wait_for_condition(&dev_priv->uncore,
+					_is_psr2_read_for_pipe_update,
+					intel_dp, fw, 50);
+}
+
+static int _psr1_ready_for_pipe_update_locked(struct intel_dp *intel_dp)
 {
 	struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
 
@@ -1814,15 +1832,13 @@ static int psr_wait_for_idle(struct intel_dp *intel_dp, u32 *out_value)
 	 * exit training time + 1.5 ms of aux channel handshake. 50 ms is
 	 * defensive enough to cover everything.
 	 */
-	return __intel_wait_for_register(&dev_priv->uncore,
-					 EDP_PSR_STATUS(intel_dp->psr.transcoder),
-					 EDP_PSR_STATUS_STATE_MASK,
-					 EDP_PSR_STATUS_STATE_IDLE, 2, 50,
-					 out_value);
+	return intel_de_wait_for_clear(dev_priv,
+				       EDP_PSR_STATUS(intel_dp->psr.transcoder),
+				       EDP_PSR_STATUS_STATE_MASK, 50);
 }
 
 /**
- * intel_psr_wait_for_idle - wait for PSR1 to idle
+ * intel_psr_wait_for_idle - wait for PSR be ready for a pipe update
  * @new_crtc_state: new CRTC state
  *
  * This function is expected to be called from pipe_update_start() where it is
@@ -1839,19 +1855,23 @@ void intel_psr_wait_for_idle(const struct intel_crtc_state *new_crtc_state)
 	for_each_intel_encoder_mask_with_psr(&dev_priv->drm, encoder,
 					     new_crtc_state->uapi.encoder_mask) {
 		struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
-		u32 psr_status;
+		int ret;
 
 		mutex_lock(&intel_dp->psr.lock);
-		if (!intel_dp->psr.enabled || intel_dp->psr.psr2_enabled) {
+
+		if (!intel_dp->psr.enabled) {
 			mutex_unlock(&intel_dp->psr.lock);
 			continue;
 		}
 
-		/* when the PSR1 is enabled */
-		if (psr_wait_for_idle(intel_dp, &psr_status))
-			drm_err(&dev_priv->drm,
-				"PSR idle timed out 0x%x, atomic update may fail\n",
-				psr_status);
+		if (intel_dp->psr.psr2_enabled)
+			ret = _psr2_ready_for_pipe_update_locked(intel_dp);
+		else
+			ret = _psr1_ready_for_pipe_update_locked(intel_dp);
+
+		if (ret)
+			drm_err(&dev_priv->drm, "PSR wait timed out, atomic update may fail\n");
+
 		mutex_unlock(&intel_dp->psr.lock);
 	}
 }
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index cad84c3b864bf..a827f5bf973cb 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -4698,8 +4698,9 @@ enum {
 #define _PSR2_STATUS_A			0x60940
 #define _PSR2_STATUS_EDP		0x6f940
 #define EDP_PSR2_STATUS(tran)		_MMIO_TRANS2(tran, _PSR2_STATUS_A)
-#define EDP_PSR2_STATUS_STATE_MASK     (0xf << 28)
-#define EDP_PSR2_STATUS_STATE_SHIFT    28
+#define EDP_PSR2_STATUS_STATE_MASK     REG_GENMASK(31, 28)
+#define EDP_PSR2_STATUS_STATE_SLEEP    REG_FIELD_PREP(EDP_PSR2_STATUS_STATE_MASK, 0x3)
+#define EDP_PSR2_STATUS_STATE_IDLE     REG_FIELD_PREP(EDP_PSR2_STATUS_STATE_MASK, 0x0)
 
 #define _PSR2_SU_STATUS_A		0x60914
 #define _PSR2_SU_STATUS_EDP		0x6f914
diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
index 678a99de07fee..1b3ea7318c2d5 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -2383,6 +2383,28 @@ int __intel_wait_for_register_fw(struct intel_uncore *uncore,
 #undef done
 }
 
+static int __intel_wait_for_condition_fw(bool (*func)(void *data), void *data,
+					 unsigned int fast_timeout_us,
+					 unsigned int slow_timeout_ms)
+{
+#define done (func(data))
+	int ret;
+
+	/* Catch any overuse of this function */
+	might_sleep_if(slow_timeout_ms);
+	GEM_BUG_ON(fast_timeout_us > 20000);
+	GEM_BUG_ON(!fast_timeout_us && !slow_timeout_ms);
+
+	ret = -ETIMEDOUT;
+	if (fast_timeout_us && fast_timeout_us <= 20000)
+		ret = _wait_for_atomic(done, fast_timeout_us, 0);
+	if (ret && slow_timeout_ms)
+		ret = wait_for(done, slow_timeout_ms);
+
+	return ret;
+#undef done
+}
+
 /**
  * __intel_wait_for_register - wait until register matches expected state
  * @uncore: the struct intel_uncore
@@ -2442,6 +2464,31 @@ int __intel_wait_for_register(struct intel_uncore *uncore,
 	return ret;
 }
 
+int intel_wait_for_condition(struct intel_uncore *uncore,
+			     bool (*func)(void *data),
+			     void *data,
+			     unsigned int fw,
+			     unsigned int slow_timeout_ms)
+{
+	unsigned int fast_timeout_us = 2;
+	int ret;
+
+	might_sleep_if(slow_timeout_ms);
+
+	spin_lock_irq(&uncore->lock);
+	intel_uncore_forcewake_get__locked(uncore, fw);
+
+	ret = __intel_wait_for_condition_fw(func, data, fast_timeout_us, 0);
+
+	intel_uncore_forcewake_put__locked(uncore, fw);
+	spin_unlock_irq(&uncore->lock);
+
+	if (ret && slow_timeout_ms)
+		ret = __wait_for(, (func(data)), slow_timeout_ms * 1000, 10, 1000);
+
+	return ret;
+}
+
 bool intel_uncore_unclaimed_mmio(struct intel_uncore *uncore)
 {
 	bool ret;
diff --git a/drivers/gpu/drm/i915/intel_uncore.h b/drivers/gpu/drm/i915/intel_uncore.h
index 2f31c50eeae24..d6455824cbc56 100644
--- a/drivers/gpu/drm/i915/intel_uncore.h
+++ b/drivers/gpu/drm/i915/intel_uncore.h
@@ -282,6 +282,7 @@ int __intel_wait_for_register_fw(struct intel_uncore *uncore,
 				 unsigned int fast_timeout_us,
 				 unsigned int slow_timeout_ms,
 				 u32 *out_value);
+
 static inline int
 intel_wait_for_register_fw(struct intel_uncore *uncore,
 			   i915_reg_t reg,
@@ -293,6 +294,12 @@ intel_wait_for_register_fw(struct intel_uncore *uncore,
 					    2, timeout_ms, NULL);
 }
 
+int intel_wait_for_condition(struct intel_uncore *uncore,
+			     bool (*func)(void *data),
+			     void *data,
+			     unsigned int fw,
+			     unsigned int timeout_ms);
+
 /* register access functions */
 #define __raw_read(x__, s__) \
 static inline u##x__ __raw_uncore_read##x__(const struct intel_uncore *uncore, \
-- 
2.33.0

[Intel-gfx] [PATCH 2/7] drm/i915/display: Handle frontbuffer rendering when PSR2 selective fetch is enabled

[José Roberto de Souza]

CURSURFLIVE writes has no affect when PSR2 selective fetch is enabled,
the right thing to do here would be calculate the damaged area and
program PSR2 selective fetch registers properly during vblank but
we can't do that due to performance reasons.

So for now we can workaround and offer proper rendering by disabling
PSR2 and enabling in the worker a few miliseconds later if there is
no other frontbuffer rendering.

This approach will eat some of the PSR2 power savings when userspace
makes use of frontbuffer rendering but that is the solution that we
can offer to enable PSR2 selective fetch right now while we work in
the proper solution for frontbuffer rendering and PSR2.

Cc: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
---
 drivers/gpu/drm/i915/display/intel_psr.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c
index a2e4ef42be60a..ba2da689920f9 100644
--- a/drivers/gpu/drm/i915/display/intel_psr.c
+++ b/drivers/gpu/drm/i915/display/intel_psr.c
@@ -1413,6 +1413,12 @@ static void psr_force_hw_tracking_exit(struct intel_dp *intel_dp)
 {
 	struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
 
+	/* CURSURFLIVE has no effect when Selective fetch is enabled */
+	if (intel_dp->psr.psr2_sel_fetch_enabled) {
+		intel_psr_exit(intel_dp);
+		return;
+	}
+
 	/*
 	 * Display WA #0884: skl+
 	 * This documented WA for bxt can be safely applied
-- 
2.33.0

Re: [Intel-gfx] [PATCH 2/7] drm/i915/display: Handle frontbuffer rendering when PSR2 selective fetch is enabled

[Gwan-gyeong Mun]

Looks good to me.
Reviewed-by: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>

On 9/23/21 10:46 PM, José Roberto de Souza wrote:
> CURSURFLIVE writes has no affect when PSR2 selective fetch is enabled,
> the right thing to do here would be calculate the damaged area and
> program PSR2 selective fetch registers properly during vblank but
> we can't do that due to performance reasons.
> 
> So for now we can workaround and offer proper rendering by disabling
> PSR2 and enabling in the worker a few miliseconds later if there is
> no other frontbuffer rendering.
> 
> This approach will eat some of the PSR2 power savings when userspace
> makes use of frontbuffer rendering but that is the solution that we
> can offer to enable PSR2 selective fetch right now while we work in
> the proper solution for frontbuffer rendering and PSR2.
> 
> Cc: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> ---
>   drivers/gpu/drm/i915/display/intel_psr.c | 6 ++++++
>   1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c
> index a2e4ef42be60a..ba2da689920f9 100644
> --- a/drivers/gpu/drm/i915/display/intel_psr.c
> +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> @@ -1413,6 +1413,12 @@ static void psr_force_hw_tracking_exit(struct intel_dp *intel_dp)
>   {
>   	struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
>   
> +	/* CURSURFLIVE has no effect when Selective fetch is enabled */
> +	if (intel_dp->psr.psr2_sel_fetch_enabled) {
> +		intel_psr_exit(intel_dp);
> +		return;
> +	}
> +
>   	/*
>   	 * Display WA #0884: skl+
>   	 * This documented WA for bxt can be safely applied
> 

[Intel-gfx] [PATCH 3/7] drm/i915/display: Add new fb_op_origin type and use it to optimize power savings

[José Roberto de Souza]

intel_prepare_plane_fb() was calling i915_gem_object_flush_frontbuffer()
with the generic ORIGIN_DIRTYFB, what was causing
PSR, FBC and DRRS to do their frontbuffer rendering in a framebuffer
that will be flipped.

Not handling this call as frontbuffer rendering allow us to save
more power with all 3 features.

Same applies when allocating a framebuffer for fbdev emulation, after
allocation userspace will draw on it and trigger invaldate/flush calls
with ORIGIN_DIRTYFB.

Cc: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
---
 drivers/gpu/drm/i915/display/intel_display.c     | 2 +-
 drivers/gpu/drm/i915/display/intel_drrs.c        | 5 ++++-
 drivers/gpu/drm/i915/display/intel_drrs.h        | 4 +++-
 drivers/gpu/drm/i915/display/intel_fbc.c         | 2 +-
 drivers/gpu/drm/i915/display/intel_fbdev.c       | 2 +-
 drivers/gpu/drm/i915/display/intel_frontbuffer.c | 2 +-
 drivers/gpu/drm/i915/display/intel_frontbuffer.h | 1 +
 drivers/gpu/drm/i915/display/intel_psr.c         | 5 ++++-
 8 files changed, 16 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index f27c294beb926..ecf8c69473e38 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -10666,7 +10666,7 @@ intel_prepare_plane_fb(struct drm_plane *_plane,
 		return ret;
 
 	i915_gem_object_wait_priority(obj, 0, &attr);
-	i915_gem_object_flush_frontbuffer(obj, ORIGIN_DIRTYFB);
+	i915_gem_object_flush_frontbuffer(obj, ORIGIN_PREPARE_FB);
 
 	if (!new_plane_state->uapi.fence) { /* implicit fencing */
 		struct dma_fence *fence;
diff --git a/drivers/gpu/drm/i915/display/intel_drrs.c b/drivers/gpu/drm/i915/display/intel_drrs.c
index c1439fcb5a959..46828222a098b 100644
--- a/drivers/gpu/drm/i915/display/intel_drrs.c
+++ b/drivers/gpu/drm/i915/display/intel_drrs.c
@@ -373,8 +373,11 @@ void intel_drrs_invalidate(struct drm_i915_private *dev_priv,
  * Dirty frontbuffers relevant to DRRS are tracked in busy_frontbuffer_bits.
  */
 void intel_drrs_flush(struct drm_i915_private *dev_priv,
-		      unsigned int frontbuffer_bits)
+		      unsigned int frontbuffer_bits, enum fb_op_origin origin)
 {
+	if (origin == ORIGIN_PREPARE_FB)
+		return;
+
 	intel_drrs_frontbuffer_update(dev_priv, frontbuffer_bits, false);
 }
 
diff --git a/drivers/gpu/drm/i915/display/intel_drrs.h b/drivers/gpu/drm/i915/display/intel_drrs.h
index 9ec9c447211af..e69528a98a9c2 100644
--- a/drivers/gpu/drm/i915/display/intel_drrs.h
+++ b/drivers/gpu/drm/i915/display/intel_drrs.h
@@ -8,6 +8,8 @@
 
 #include <linux/types.h>
 
+#include "intel_frontbuffer.h"
+
 struct drm_i915_private;
 struct intel_atomic_state;
 struct intel_crtc;
@@ -24,7 +26,7 @@ void intel_drrs_update(struct intel_dp *intel_dp,
 void intel_drrs_invalidate(struct drm_i915_private *dev_priv,
 			   unsigned int frontbuffer_bits);
 void intel_drrs_flush(struct drm_i915_private *dev_priv,
-		      unsigned int frontbuffer_bits);
+		      unsigned int frontbuffer_bits, enum fb_op_origin origin);
 void intel_drrs_page_flip(struct intel_atomic_state *state,
 			  struct intel_crtc *crtc);
 void intel_drrs_compute_config(struct intel_dp *intel_dp,
diff --git a/drivers/gpu/drm/i915/display/intel_fbc.c b/drivers/gpu/drm/i915/display/intel_fbc.c
index b1c1a23c36be3..56c2de2994602 100644
--- a/drivers/gpu/drm/i915/display/intel_fbc.c
+++ b/drivers/gpu/drm/i915/display/intel_fbc.c
@@ -1155,7 +1155,7 @@ void intel_fbc_flush(struct drm_i915_private *dev_priv,
 
 	fbc->busy_bits &= ~frontbuffer_bits;
 
-	if (origin == ORIGIN_FLIP)
+	if (origin == ORIGIN_FLIP || origin == ORIGIN_PREPARE_FB)
 		goto out;
 
 	if (!fbc->busy_bits && fbc->crtc &&
diff --git a/drivers/gpu/drm/i915/display/intel_fbdev.c b/drivers/gpu/drm/i915/display/intel_fbdev.c
index 60d3ded270476..3097658938ae2 100644
--- a/drivers/gpu/drm/i915/display/intel_fbdev.c
+++ b/drivers/gpu/drm/i915/display/intel_fbdev.c
@@ -230,7 +230,7 @@ static int intelfb_create(struct drm_fb_helper *helper,
 		goto out_unlock;
 	}
 
-	intel_frontbuffer_flush(to_frontbuffer(ifbdev), ORIGIN_DIRTYFB);
+	intel_frontbuffer_flush(to_frontbuffer(ifbdev), ORIGIN_PREPARE_FB);
 
 	info = drm_fb_helper_alloc_fbi(helper);
 	if (IS_ERR(info)) {
diff --git a/drivers/gpu/drm/i915/display/intel_frontbuffer.c b/drivers/gpu/drm/i915/display/intel_frontbuffer.c
index 0492446cd04ad..a88c147d6b523 100644
--- a/drivers/gpu/drm/i915/display/intel_frontbuffer.c
+++ b/drivers/gpu/drm/i915/display/intel_frontbuffer.c
@@ -92,7 +92,7 @@ static void frontbuffer_flush(struct drm_i915_private *i915,
 	trace_intel_frontbuffer_flush(frontbuffer_bits, origin);
 
 	might_sleep();
-	intel_drrs_flush(i915, frontbuffer_bits);
+	intel_drrs_flush(i915, frontbuffer_bits, origin);
 	intel_psr_flush(i915, frontbuffer_bits, origin);
 	intel_fbc_flush(i915, frontbuffer_bits, origin);
 }
diff --git a/drivers/gpu/drm/i915/display/intel_frontbuffer.h b/drivers/gpu/drm/i915/display/intel_frontbuffer.h
index 4b977c1e4d52b..950030950b0f9 100644
--- a/drivers/gpu/drm/i915/display/intel_frontbuffer.h
+++ b/drivers/gpu/drm/i915/display/intel_frontbuffer.h
@@ -37,6 +37,7 @@ enum fb_op_origin {
 	ORIGIN_CS,
 	ORIGIN_FLIP,
 	ORIGIN_DIRTYFB,
+	ORIGIN_PREPARE_FB,
 };
 
 struct intel_frontbuffer {
diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c
index ba2da689920f9..1cc4130dec7b1 100644
--- a/drivers/gpu/drm/i915/display/intel_psr.c
+++ b/drivers/gpu/drm/i915/display/intel_psr.c
@@ -2073,7 +2073,7 @@ void intel_psr_invalidate(struct drm_i915_private *dev_priv,
 {
 	struct intel_encoder *encoder;
 
-	if (origin == ORIGIN_FLIP)
+	if (origin == ORIGIN_FLIP || origin == ORIGIN_PREPARE_FB)
 		return;
 
 	for_each_intel_encoder_with_psr(&dev_priv->drm, encoder) {
@@ -2148,6 +2148,9 @@ void intel_psr_flush(struct drm_i915_private *dev_priv,
 {
 	struct intel_encoder *encoder;
 
+	if (origin == ORIGIN_PREPARE_FB)
+		return;
+
 	for_each_intel_encoder_with_psr(&dev_priv->drm, encoder) {
 		unsigned int pipe_frontbuffer_bits = frontbuffer_bits;
 		struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
-- 
2.33.0

Re: [Intel-gfx] [PATCH 3/7] drm/i915/display: Add new fb_op_origin type and use it to optimize power savings

[Ville Syrjälä]

On Thu, Sep 23, 2021 at 12:46:13PM -0700, José Roberto de Souza wrote:
> intel_prepare_plane_fb() was calling i915_gem_object_flush_frontbuffer()
> with the generic ORIGIN_DIRTYFB, what was causing
> PSR, FBC and DRRS to do their frontbuffer rendering in a framebuffer
> that will be flipped.
> 
> Not handling this call as frontbuffer rendering allow us to save
> more power with all 3 features.
> 
> Same applies when allocating a framebuffer for fbdev emulation, after
> allocation userspace will draw on it and trigger invaldate/flush calls
> with ORIGIN_DIRTYFB.
> 
> Cc: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_display.c     | 2 +-
>  drivers/gpu/drm/i915/display/intel_drrs.c        | 5 ++++-
>  drivers/gpu/drm/i915/display/intel_drrs.h        | 4 +++-
>  drivers/gpu/drm/i915/display/intel_fbc.c         | 2 +-
>  drivers/gpu/drm/i915/display/intel_fbdev.c       | 2 +-
>  drivers/gpu/drm/i915/display/intel_frontbuffer.c | 2 +-
>  drivers/gpu/drm/i915/display/intel_frontbuffer.h | 1 +
>  drivers/gpu/drm/i915/display/intel_psr.c         | 5 ++++-
>  8 files changed, 16 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index f27c294beb926..ecf8c69473e38 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -10666,7 +10666,7 @@ intel_prepare_plane_fb(struct drm_plane *_plane,
>  		return ret;
>  
>  	i915_gem_object_wait_priority(obj, 0, &attr);
> -	i915_gem_object_flush_frontbuffer(obj, ORIGIN_DIRTYFB);
> +	i915_gem_object_flush_frontbuffer(obj, ORIGIN_PREPARE_FB);

I think this should just get nuked. See
https://patchwork.freedesktop.org/patch/432166/?series=89824&rev=1

Unfortunately I wasn't able to get past glk's semi-borked
FBC with that yet.

>  
>  	if (!new_plane_state->uapi.fence) { /* implicit fencing */
>  		struct dma_fence *fence;
> diff --git a/drivers/gpu/drm/i915/display/intel_drrs.c b/drivers/gpu/drm/i915/display/intel_drrs.c
> index c1439fcb5a959..46828222a098b 100644
> --- a/drivers/gpu/drm/i915/display/intel_drrs.c
> +++ b/drivers/gpu/drm/i915/display/intel_drrs.c
> @@ -373,8 +373,11 @@ void intel_drrs_invalidate(struct drm_i915_private *dev_priv,
>   * Dirty frontbuffers relevant to DRRS are tracked in busy_frontbuffer_bits.
>   */
>  void intel_drrs_flush(struct drm_i915_private *dev_priv,
> -		      unsigned int frontbuffer_bits)
> +		      unsigned int frontbuffer_bits, enum fb_op_origin origin)
>  {
> +	if (origin == ORIGIN_PREPARE_FB)
> +		return;
> +
>  	intel_drrs_frontbuffer_update(dev_priv, frontbuffer_bits, false);
>  }
>  
> diff --git a/drivers/gpu/drm/i915/display/intel_drrs.h b/drivers/gpu/drm/i915/display/intel_drrs.h
> index 9ec9c447211af..e69528a98a9c2 100644
> --- a/drivers/gpu/drm/i915/display/intel_drrs.h
> +++ b/drivers/gpu/drm/i915/display/intel_drrs.h
> @@ -8,6 +8,8 @@
>  
>  #include <linux/types.h>
>  
> +#include "intel_frontbuffer.h"
> +
>  struct drm_i915_private;
>  struct intel_atomic_state;
>  struct intel_crtc;
> @@ -24,7 +26,7 @@ void intel_drrs_update(struct intel_dp *intel_dp,
>  void intel_drrs_invalidate(struct drm_i915_private *dev_priv,
>  			   unsigned int frontbuffer_bits);
>  void intel_drrs_flush(struct drm_i915_private *dev_priv,
> -		      unsigned int frontbuffer_bits);
> +		      unsigned int frontbuffer_bits, enum fb_op_origin origin);
>  void intel_drrs_page_flip(struct intel_atomic_state *state,
>  			  struct intel_crtc *crtc);
>  void intel_drrs_compute_config(struct intel_dp *intel_dp,
> diff --git a/drivers/gpu/drm/i915/display/intel_fbc.c b/drivers/gpu/drm/i915/display/intel_fbc.c
> index b1c1a23c36be3..56c2de2994602 100644
> --- a/drivers/gpu/drm/i915/display/intel_fbc.c
> +++ b/drivers/gpu/drm/i915/display/intel_fbc.c
> @@ -1155,7 +1155,7 @@ void intel_fbc_flush(struct drm_i915_private *dev_priv,
>  
>  	fbc->busy_bits &= ~frontbuffer_bits;
>  
> -	if (origin == ORIGIN_FLIP)
> +	if (origin == ORIGIN_FLIP || origin == ORIGIN_PREPARE_FB)
>  		goto out;
>  
>  	if (!fbc->busy_bits && fbc->crtc &&
> diff --git a/drivers/gpu/drm/i915/display/intel_fbdev.c b/drivers/gpu/drm/i915/display/intel_fbdev.c
> index 60d3ded270476..3097658938ae2 100644
> --- a/drivers/gpu/drm/i915/display/intel_fbdev.c
> +++ b/drivers/gpu/drm/i915/display/intel_fbdev.c
> @@ -230,7 +230,7 @@ static int intelfb_create(struct drm_fb_helper *helper,
>  		goto out_unlock;
>  	}
>  
> -	intel_frontbuffer_flush(to_frontbuffer(ifbdev), ORIGIN_DIRTYFB);
> +	intel_frontbuffer_flush(to_frontbuffer(ifbdev), ORIGIN_PREPARE_FB);
>  
>  	info = drm_fb_helper_alloc_fbi(helper);
>  	if (IS_ERR(info)) {
> diff --git a/drivers/gpu/drm/i915/display/intel_frontbuffer.c b/drivers/gpu/drm/i915/display/intel_frontbuffer.c
> index 0492446cd04ad..a88c147d6b523 100644
> --- a/drivers/gpu/drm/i915/display/intel_frontbuffer.c
> +++ b/drivers/gpu/drm/i915/display/intel_frontbuffer.c
> @@ -92,7 +92,7 @@ static void frontbuffer_flush(struct drm_i915_private *i915,
>  	trace_intel_frontbuffer_flush(frontbuffer_bits, origin);
>  
>  	might_sleep();
> -	intel_drrs_flush(i915, frontbuffer_bits);
> +	intel_drrs_flush(i915, frontbuffer_bits, origin);
>  	intel_psr_flush(i915, frontbuffer_bits, origin);
>  	intel_fbc_flush(i915, frontbuffer_bits, origin);
>  }
> diff --git a/drivers/gpu/drm/i915/display/intel_frontbuffer.h b/drivers/gpu/drm/i915/display/intel_frontbuffer.h
> index 4b977c1e4d52b..950030950b0f9 100644
> --- a/drivers/gpu/drm/i915/display/intel_frontbuffer.h
> +++ b/drivers/gpu/drm/i915/display/intel_frontbuffer.h
> @@ -37,6 +37,7 @@ enum fb_op_origin {
>  	ORIGIN_CS,
>  	ORIGIN_FLIP,
>  	ORIGIN_DIRTYFB,
> +	ORIGIN_PREPARE_FB,
>  };
>  
>  struct intel_frontbuffer {
> diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c
> index ba2da689920f9..1cc4130dec7b1 100644
> --- a/drivers/gpu/drm/i915/display/intel_psr.c
> +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> @@ -2073,7 +2073,7 @@ void intel_psr_invalidate(struct drm_i915_private *dev_priv,
>  {
>  	struct intel_encoder *encoder;
>  
> -	if (origin == ORIGIN_FLIP)
> +	if (origin == ORIGIN_FLIP || origin == ORIGIN_PREPARE_FB)
>  		return;
>  
>  	for_each_intel_encoder_with_psr(&dev_priv->drm, encoder) {
> @@ -2148,6 +2148,9 @@ void intel_psr_flush(struct drm_i915_private *dev_priv,
>  {
>  	struct intel_encoder *encoder;
>  
> +	if (origin == ORIGIN_PREPARE_FB)
> +		return;
> +
>  	for_each_intel_encoder_with_psr(&dev_priv->drm, encoder) {
>  		unsigned int pipe_frontbuffer_bits = frontbuffer_bits;
>  		struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
> -- 
> 2.33.0

-- 
Ville Syrjälä
Intel

Re: [Intel-gfx] [PATCH 3/7] drm/i915/display: Add new fb_op_origin type and use it to optimize power savings

[Souza, Jose]

On Fri, 2021-09-24 at 17:39 +0300, Ville Syrjälä wrote:
> On Thu, Sep 23, 2021 at 12:46:13PM -0700, José Roberto de Souza wrote:
> > intel_prepare_plane_fb() was calling i915_gem_object_flush_frontbuffer()
> > with the generic ORIGIN_DIRTYFB, what was causing
> > PSR, FBC and DRRS to do their frontbuffer rendering in a framebuffer
> > that will be flipped.
> > 
> > Not handling this call as frontbuffer rendering allow us to save
> > more power with all 3 features.
> > 
> > Same applies when allocating a framebuffer for fbdev emulation, after
> > allocation userspace will draw on it and trigger invaldate/flush calls
> > with ORIGIN_DIRTYFB.
> > 
> > Cc: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>
> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_display.c     | 2 +-
> >  drivers/gpu/drm/i915/display/intel_drrs.c        | 5 ++++-
> >  drivers/gpu/drm/i915/display/intel_drrs.h        | 4 +++-
> >  drivers/gpu/drm/i915/display/intel_fbc.c         | 2 +-
> >  drivers/gpu/drm/i915/display/intel_fbdev.c       | 2 +-
> >  drivers/gpu/drm/i915/display/intel_frontbuffer.c | 2 +-
> >  drivers/gpu/drm/i915/display/intel_frontbuffer.h | 1 +
> >  drivers/gpu/drm/i915/display/intel_psr.c         | 5 ++++-
> >  8 files changed, 16 insertions(+), 7 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> > index f27c294beb926..ecf8c69473e38 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > @@ -10666,7 +10666,7 @@ intel_prepare_plane_fb(struct drm_plane *_plane,
> >  		return ret;
> >  
> >  	i915_gem_object_wait_priority(obj, 0, &attr);
> > -	i915_gem_object_flush_frontbuffer(obj, ORIGIN_DIRTYFB);
> > +	i915_gem_object_flush_frontbuffer(obj, ORIGIN_PREPARE_FB);
> 
> I think this should just get nuked. See
> https://patchwork.freedesktop.org/patch/432166/?series=89824&rev=1
> 
> Unfortunately I wasn't able to get past glk's semi-borked
> FBC with that yet.

Cool, will try to nuke it.
This series passed in BAT and shards, hopefully we will not hit this issue today. 

> 
> >  
> >  	if (!new_plane_state->uapi.fence) { /* implicit fencing */
> >  		struct dma_fence *fence;
> > diff --git a/drivers/gpu/drm/i915/display/intel_drrs.c b/drivers/gpu/drm/i915/display/intel_drrs.c
> > index c1439fcb5a959..46828222a098b 100644
> > --- a/drivers/gpu/drm/i915/display/intel_drrs.c
> > +++ b/drivers/gpu/drm/i915/display/intel_drrs.c
> > @@ -373,8 +373,11 @@ void intel_drrs_invalidate(struct drm_i915_private *dev_priv,
> >   * Dirty frontbuffers relevant to DRRS are tracked in busy_frontbuffer_bits.
> >   */
> >  void intel_drrs_flush(struct drm_i915_private *dev_priv,
> > -		      unsigned int frontbuffer_bits)
> > +		      unsigned int frontbuffer_bits, enum fb_op_origin origin)
> >  {
> > +	if (origin == ORIGIN_PREPARE_FB)
> > +		return;
> > +
> >  	intel_drrs_frontbuffer_update(dev_priv, frontbuffer_bits, false);
> >  }
> >  
> > diff --git a/drivers/gpu/drm/i915/display/intel_drrs.h b/drivers/gpu/drm/i915/display/intel_drrs.h
> > index 9ec9c447211af..e69528a98a9c2 100644
> > --- a/drivers/gpu/drm/i915/display/intel_drrs.h
> > +++ b/drivers/gpu/drm/i915/display/intel_drrs.h
> > @@ -8,6 +8,8 @@
> >  
> >  #include <linux/types.h>
> >  
> > +#include "intel_frontbuffer.h"
> > +
> >  struct drm_i915_private;
> >  struct intel_atomic_state;
> >  struct intel_crtc;
> > @@ -24,7 +26,7 @@ void intel_drrs_update(struct intel_dp *intel_dp,
> >  void intel_drrs_invalidate(struct drm_i915_private *dev_priv,
> >  			   unsigned int frontbuffer_bits);
> >  void intel_drrs_flush(struct drm_i915_private *dev_priv,
> > -		      unsigned int frontbuffer_bits);
> > +		      unsigned int frontbuffer_bits, enum fb_op_origin origin);
> >  void intel_drrs_page_flip(struct intel_atomic_state *state,
> >  			  struct intel_crtc *crtc);
> >  void intel_drrs_compute_config(struct intel_dp *intel_dp,
> > diff --git a/drivers/gpu/drm/i915/display/intel_fbc.c b/drivers/gpu/drm/i915/display/intel_fbc.c
> > index b1c1a23c36be3..56c2de2994602 100644
> > --- a/drivers/gpu/drm/i915/display/intel_fbc.c
> > +++ b/drivers/gpu/drm/i915/display/intel_fbc.c
> > @@ -1155,7 +1155,7 @@ void intel_fbc_flush(struct drm_i915_private *dev_priv,
> >  
> >  	fbc->busy_bits &= ~frontbuffer_bits;
> >  
> > -	if (origin == ORIGIN_FLIP)
> > +	if (origin == ORIGIN_FLIP || origin == ORIGIN_PREPARE_FB)
> >  		goto out;
> >  
> >  	if (!fbc->busy_bits && fbc->crtc &&
> > diff --git a/drivers/gpu/drm/i915/display/intel_fbdev.c b/drivers/gpu/drm/i915/display/intel_fbdev.c
> > index 60d3ded270476..3097658938ae2 100644
> > --- a/drivers/gpu/drm/i915/display/intel_fbdev.c
> > +++ b/drivers/gpu/drm/i915/display/intel_fbdev.c
> > @@ -230,7 +230,7 @@ static int intelfb_create(struct drm_fb_helper *helper,
> >  		goto out_unlock;
> >  	}
> >  
> > -	intel_frontbuffer_flush(to_frontbuffer(ifbdev), ORIGIN_DIRTYFB);
> > +	intel_frontbuffer_flush(to_frontbuffer(ifbdev), ORIGIN_PREPARE_FB);
> >  
> >  	info = drm_fb_helper_alloc_fbi(helper);
> >  	if (IS_ERR(info)) {
> > diff --git a/drivers/gpu/drm/i915/display/intel_frontbuffer.c b/drivers/gpu/drm/i915/display/intel_frontbuffer.c
> > index 0492446cd04ad..a88c147d6b523 100644
> > --- a/drivers/gpu/drm/i915/display/intel_frontbuffer.c
> > +++ b/drivers/gpu/drm/i915/display/intel_frontbuffer.c
> > @@ -92,7 +92,7 @@ static void frontbuffer_flush(struct drm_i915_private *i915,
> >  	trace_intel_frontbuffer_flush(frontbuffer_bits, origin);
> >  
> >  	might_sleep();
> > -	intel_drrs_flush(i915, frontbuffer_bits);
> > +	intel_drrs_flush(i915, frontbuffer_bits, origin);
> >  	intel_psr_flush(i915, frontbuffer_bits, origin);
> >  	intel_fbc_flush(i915, frontbuffer_bits, origin);
> >  }
> > diff --git a/drivers/gpu/drm/i915/display/intel_frontbuffer.h b/drivers/gpu/drm/i915/display/intel_frontbuffer.h
> > index 4b977c1e4d52b..950030950b0f9 100644
> > --- a/drivers/gpu/drm/i915/display/intel_frontbuffer.h
> > +++ b/drivers/gpu/drm/i915/display/intel_frontbuffer.h
> > @@ -37,6 +37,7 @@ enum fb_op_origin {
> >  	ORIGIN_CS,
> >  	ORIGIN_FLIP,
> >  	ORIGIN_DIRTYFB,
> > +	ORIGIN_PREPARE_FB,
> >  };
> >  
> >  struct intel_frontbuffer {
> > diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c
> > index ba2da689920f9..1cc4130dec7b1 100644
> > --- a/drivers/gpu/drm/i915/display/intel_psr.c
> > +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> > @@ -2073,7 +2073,7 @@ void intel_psr_invalidate(struct drm_i915_private *dev_priv,
> >  {
> >  	struct intel_encoder *encoder;
> >  
> > -	if (origin == ORIGIN_FLIP)
> > +	if (origin == ORIGIN_FLIP || origin == ORIGIN_PREPARE_FB)
> >  		return;
> >  
> >  	for_each_intel_encoder_with_psr(&dev_priv->drm, encoder) {
> > @@ -2148,6 +2148,9 @@ void intel_psr_flush(struct drm_i915_private *dev_priv,
> >  {
> >  	struct intel_encoder *encoder;
> >  
> > +	if (origin == ORIGIN_PREPARE_FB)
> > +		return;
> > +
> >  	for_each_intel_encoder_with_psr(&dev_priv->drm, encoder) {
> >  		unsigned int pipe_frontbuffer_bits = frontbuffer_bits;
> >  		struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
> > -- 
> > 2.33.0
> 

[Intel-gfx] [PATCH 4/7] drm/i915/display/psr: Handle plane restrictions at every page flip

[José Roberto de Souza]

PSR2 selective is not supported over rotated and scaled planes.
We had the rotation check in intel_psr2_sel_fetch_config_valid()
but that code path is only execute when a modeset is needed and
change those plane parameters do not require a modeset.

Also need to check those restricions in the second
for_each_oldnew_intel_plane_in_state() loop because the state could
only have a plane that is not affected by those restricitons but
the damaged area intersect with planes that has those restrictions,
so a full plane fetch is required.

BSpec: 55229
Cc: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>
Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
---
 drivers/gpu/drm/i915/display/intel_psr.c | 45 ++++++++++++++----------
 1 file changed, 26 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c
index 1cc4130dec7b1..356e0e96abf4e 100644
--- a/drivers/gpu/drm/i915/display/intel_psr.c
+++ b/drivers/gpu/drm/i915/display/intel_psr.c
@@ -720,11 +720,7 @@ tgl_dc3co_exitline_compute_config(struct intel_dp *intel_dp,
 static bool intel_psr2_sel_fetch_config_valid(struct intel_dp *intel_dp,
 					      struct intel_crtc_state *crtc_state)
 {
-	struct intel_atomic_state *state = to_intel_atomic_state(crtc_state->uapi.state);
 	struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
-	struct intel_plane_state *plane_state;
-	struct intel_plane *plane;
-	int i;
 
 	if (!dev_priv->params.enable_psr2_sel_fetch &&
 	    intel_dp->psr.debug != I915_PSR_DEBUG_ENABLE_SEL_FETCH) {
@@ -739,14 +735,6 @@ static bool intel_psr2_sel_fetch_config_valid(struct intel_dp *intel_dp,
 		return false;
 	}
 
-	for_each_new_intel_plane_in_state(state, plane, plane_state, i) {
-		if (plane_state->uapi.rotation != DRM_MODE_ROTATE_0) {
-			drm_dbg_kms(&dev_priv->drm,
-				    "PSR2 sel fetch not enabled, plane rotated\n");
-			return false;
-		}
-	}
-
 	/* Wa_14010254185 Wa_14010103792 */
 	if (IS_TGL_DISPLAY_STEP(dev_priv, STEP_A0, STEP_C0)) {
 		drm_dbg_kms(&dev_priv->drm,
@@ -1586,6 +1574,26 @@ static void cursor_area_workaround(const struct intel_plane_state *new_plane_sta
 	clip_area_update(pipe_clip, damaged_area);
 }
 
+/*
+ * TODO: Not clear how to handle planes with negative position,
+ * also planes are not updated if they have a negative X
+ * position so for now doing a full update in this cases
+ *
+ * Plane scaling and rotation is not supported by selective fetch and both
+ * properties can change without a modeset, so need to be check at every
+ * atomic commmit.
+ */
+static bool psr2_sel_fetch_plane_state_supported(const struct intel_plane_state *plane_state)
+{
+	if (plane_state->uapi.dst.y1 < 0 ||
+	    plane_state->uapi.dst.x1 < 0 ||
+	    plane_state->scaler_id >= 0 ||
+	    plane_state->uapi.rotation != DRM_MODE_ROTATE_0)
+		return false;
+
+	return true;
+}
+
 int intel_psr2_sel_fetch_update(struct intel_atomic_state *state,
 				struct intel_crtc *crtc)
 {
@@ -1618,13 +1626,7 @@ int intel_psr2_sel_fetch_update(struct intel_atomic_state *state,
 		    !old_plane_state->uapi.visible)
 			continue;
 
-		/*
-		 * TODO: Not clear how to handle planes with negative position,
-		 * also planes are not updated if they have a negative X
-		 * position so for now doing a full update in this cases
-		 */
-		if (new_plane_state->uapi.dst.y1 < 0 ||
-		    new_plane_state->uapi.dst.x1 < 0) {
+		if (!psr2_sel_fetch_plane_state_supported(new_plane_state)) {
 			full_update = true;
 			break;
 		}
@@ -1703,6 +1705,11 @@ int intel_psr2_sel_fetch_update(struct intel_atomic_state *state,
 		if (!drm_rect_intersect(&inter, &new_plane_state->uapi.dst))
 			continue;
 
+		if (!psr2_sel_fetch_plane_state_supported(new_plane_state)) {
+			full_update = true;
+			break;
+		}
+
 		sel_fetch_area = &new_plane_state->psr2_sel_fetch_area;
 		sel_fetch_area->y1 = inter.y1 - new_plane_state->uapi.dst.y1;
 		sel_fetch_area->y2 = inter.y2 - new_plane_state->uapi.dst.y1;
-- 
2.33.0

Re: [Intel-gfx] [PATCH 4/7] drm/i915/display/psr: Handle plane restrictions at every page flip

[Gwan-gyeong Mun]

On 9/23/21 10:46 PM, José Roberto de Souza wrote:
> PSR2 selective is not supported over rotated and scaled planes.
> We had the rotation check in intel_psr2_sel_fetch_config_valid()
> but that code path is only execute when a modeset is needed and
> change those plane parameters do not require a modeset.
> 
> Also need to check those restricions in the second
> for_each_oldnew_intel_plane_in_state() loop because the state could
> only have a plane that is not affected by those restricitons but
> the damaged area intersect with planes that has those restrictions,
> so a full plane fetch is required.
> 
> BSpec: 55229
> Cc: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> ---
>   drivers/gpu/drm/i915/display/intel_psr.c | 45 ++++++++++++++----------
>   1 file changed, 26 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c
> index 1cc4130dec7b1..356e0e96abf4e 100644
> --- a/drivers/gpu/drm/i915/display/intel_psr.c
> +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> @@ -720,11 +720,7 @@ tgl_dc3co_exitline_compute_config(struct intel_dp *intel_dp,
>   static bool intel_psr2_sel_fetch_config_valid(struct intel_dp *intel_dp,
>   					      struct intel_crtc_state *crtc_state)
>   {
> -	struct intel_atomic_state *state = to_intel_atomic_state(crtc_state->uapi.state);
>   	struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
> -	struct intel_plane_state *plane_state;
> -	struct intel_plane *plane;
> -	int i;
>   
>   	if (!dev_priv->params.enable_psr2_sel_fetch &&
>   	    intel_dp->psr.debug != I915_PSR_DEBUG_ENABLE_SEL_FETCH) {
> @@ -739,14 +735,6 @@ static bool intel_psr2_sel_fetch_config_valid(struct intel_dp *intel_dp,
>   		return false;
>   	}
>   
> -	for_each_new_intel_plane_in_state(state, plane, plane_state, i) {
> -		if (plane_state->uapi.rotation != DRM_MODE_ROTATE_0) {
> -			drm_dbg_kms(&dev_priv->drm,
> -				    "PSR2 sel fetch not enabled, plane rotated\n");
> -			return false;
> -		}
> -	}
> -
>   	/* Wa_14010254185 Wa_14010103792 */
>   	if (IS_TGL_DISPLAY_STEP(dev_priv, STEP_A0, STEP_C0)) {
>   		drm_dbg_kms(&dev_priv->drm,
> @@ -1586,6 +1574,26 @@ static void cursor_area_workaround(const struct intel_plane_state *new_plane_sta
>   	clip_area_update(pipe_clip, damaged_area);
>   }
>   
> +/*
> + * TODO: Not clear how to handle planes with negative position,
> + * also planes are not updated if they have a negative X
> + * position so for now doing a full update in this cases
> + *
> + * Plane scaling and rotation is not supported by selective fetch and both
> + * properties can change without a modeset, so need to be check at every
> + * atomic commmit.
> + */
> +static bool psr2_sel_fetch_plane_state_supported(const struct intel_plane_state *plane_state)
> +{
> +	if (plane_state->uapi.dst.y1 < 0 ||
> +	    plane_state->uapi.dst.x1 < 0 ||
intel_atomic_plane_check_clipping() function makes 
plane_state->uapi.dst.x1 and plane_state->uapi.dst.y1 non-negative 
values, so there is no need to deal with negative positions here.

And the rest of the changes look good to me.
> +	    plane_state->scaler_id >= 0 ||
> +	    plane_state->uapi.rotation != DRM_MODE_ROTATE_0)
> +		return false;
> +
> +	return true;
> +}
> +
>   int intel_psr2_sel_fetch_update(struct intel_atomic_state *state,
>   				struct intel_crtc *crtc)
>   {
> @@ -1618,13 +1626,7 @@ int intel_psr2_sel_fetch_update(struct intel_atomic_state *state,
>   		    !old_plane_state->uapi.visible)
>   			continue;
>   
> -		/*
> -		 * TODO: Not clear how to handle planes with negative position,
> -		 * also planes are not updated if they have a negative X
> -		 * position so for now doing a full update in this cases
> -		 */
> -		if (new_plane_state->uapi.dst.y1 < 0 ||
> -		    new_plane_state->uapi.dst.x1 < 0) {
> +		if (!psr2_sel_fetch_plane_state_supported(new_plane_state)) {
>   			full_update = true;
>   			break;
>   		}
> @@ -1703,6 +1705,11 @@ int intel_psr2_sel_fetch_update(struct intel_atomic_state *state,
>   		if (!drm_rect_intersect(&inter, &new_plane_state->uapi.dst))
>   			continue;
>   
> +		if (!psr2_sel_fetch_plane_state_supported(new_plane_state)) {
> +			full_update = true;
> +			break;
> +		}
> +
>   		sel_fetch_area = &new_plane_state->psr2_sel_fetch_area;
>   		sel_fetch_area->y1 = inter.y1 - new_plane_state->uapi.dst.y1;
>   		sel_fetch_area->y2 = inter.y2 - new_plane_state->uapi.dst.y1;
> 

Re: [Intel-gfx] [PATCH 4/7] drm/i915/display/psr: Handle plane restrictions at every page flip

[Ville Syrjälä]

On Wed, Sep 29, 2021 at 08:50:12PM +0300, Gwan-gyeong Mun wrote:
> 
> 
> On 9/23/21 10:46 PM, José Roberto de Souza wrote:
> > PSR2 selective is not supported over rotated and scaled planes.
> > We had the rotation check in intel_psr2_sel_fetch_config_valid()
> > but that code path is only execute when a modeset is needed and
> > change those plane parameters do not require a modeset.
> > 
> > Also need to check those restricions in the second
> > for_each_oldnew_intel_plane_in_state() loop because the state could
> > only have a plane that is not affected by those restricitons but
> > the damaged area intersect with planes that has those restrictions,
> > so a full plane fetch is required.
> > 
> > BSpec: 55229
> > Cc: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>
> > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > ---
> >   drivers/gpu/drm/i915/display/intel_psr.c | 45 ++++++++++++++----------
> >   1 file changed, 26 insertions(+), 19 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c
> > index 1cc4130dec7b1..356e0e96abf4e 100644
> > --- a/drivers/gpu/drm/i915/display/intel_psr.c
> > +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> > @@ -720,11 +720,7 @@ tgl_dc3co_exitline_compute_config(struct intel_dp *intel_dp,
> >   static bool intel_psr2_sel_fetch_config_valid(struct intel_dp *intel_dp,
> >   					      struct intel_crtc_state *crtc_state)
> >   {
> > -	struct intel_atomic_state *state = to_intel_atomic_state(crtc_state->uapi.state);
> >   	struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
> > -	struct intel_plane_state *plane_state;
> > -	struct intel_plane *plane;
> > -	int i;
> >   
> >   	if (!dev_priv->params.enable_psr2_sel_fetch &&
> >   	    intel_dp->psr.debug != I915_PSR_DEBUG_ENABLE_SEL_FETCH) {
> > @@ -739,14 +735,6 @@ static bool intel_psr2_sel_fetch_config_valid(struct intel_dp *intel_dp,
> >   		return false;
> >   	}
> >   
> > -	for_each_new_intel_plane_in_state(state, plane, plane_state, i) {
> > -		if (plane_state->uapi.rotation != DRM_MODE_ROTATE_0) {
> > -			drm_dbg_kms(&dev_priv->drm,
> > -				    "PSR2 sel fetch not enabled, plane rotated\n");
> > -			return false;
> > -		}
> > -	}
> > -
> >   	/* Wa_14010254185 Wa_14010103792 */
> >   	if (IS_TGL_DISPLAY_STEP(dev_priv, STEP_A0, STEP_C0)) {
> >   		drm_dbg_kms(&dev_priv->drm,
> > @@ -1586,6 +1574,26 @@ static void cursor_area_workaround(const struct intel_plane_state *new_plane_sta
> >   	clip_area_update(pipe_clip, damaged_area);
> >   }
> >   
> > +/*
> > + * TODO: Not clear how to handle planes with negative position,
> > + * also planes are not updated if they have a negative X
> > + * position so for now doing a full update in this cases
> > + *
> > + * Plane scaling and rotation is not supported by selective fetch and both
> > + * properties can change without a modeset, so need to be check at every
> > + * atomic commmit.
> > + */
> > +static bool psr2_sel_fetch_plane_state_supported(const struct intel_plane_state *plane_state)
> > +{
> > +	if (plane_state->uapi.dst.y1 < 0 ||
> > +	    plane_state->uapi.dst.x1 < 0 ||
> intel_atomic_plane_check_clipping() function makes 
> plane_state->uapi.dst.x1 and plane_state->uapi.dst.y1 non-negative 
> values, so there is no need to deal with negative positions here.

Cursor can have negative coordinates as it's hardware that will do the
clipping for us.

> 
> And the rest of the changes look good to me.
> > +	    plane_state->scaler_id >= 0 ||
> > +	    plane_state->uapi.rotation != DRM_MODE_ROTATE_0)
> > +		return false;
> > +
> > +	return true;
> > +}
> > +
> >   int intel_psr2_sel_fetch_update(struct intel_atomic_state *state,
> >   				struct intel_crtc *crtc)
> >   {
> > @@ -1618,13 +1626,7 @@ int intel_psr2_sel_fetch_update(struct intel_atomic_state *state,
> >   		    !old_plane_state->uapi.visible)
> >   			continue;
> >   
> > -		/*
> > -		 * TODO: Not clear how to handle planes with negative position,
> > -		 * also planes are not updated if they have a negative X
> > -		 * position so for now doing a full update in this cases
> > -		 */
> > -		if (new_plane_state->uapi.dst.y1 < 0 ||
> > -		    new_plane_state->uapi.dst.x1 < 0) {
> > +		if (!psr2_sel_fetch_plane_state_supported(new_plane_state)) {
> >   			full_update = true;
> >   			break;
> >   		}
> > @@ -1703,6 +1705,11 @@ int intel_psr2_sel_fetch_update(struct intel_atomic_state *state,
> >   		if (!drm_rect_intersect(&inter, &new_plane_state->uapi.dst))
> >   			continue;
> >   
> > +		if (!psr2_sel_fetch_plane_state_supported(new_plane_state)) {
> > +			full_update = true;
> > +			break;
> > +		}
> > +
> >   		sel_fetch_area = &new_plane_state->psr2_sel_fetch_area;
> >   		sel_fetch_area->y1 = inter.y1 - new_plane_state->uapi.dst.y1;
> >   		sel_fetch_area->y2 = inter.y2 - new_plane_state->uapi.dst.y1;
> > 

-- 
Ville Syrjälä
Intel

Re: [Intel-gfx] [PATCH 4/7] drm/i915/display/psr: Handle plane restrictions at every page flip

[Gwan-gyeong Mun]

On 9/29/21 9:20 PM, Ville Syrjälä wrote:
> On Wed, Sep 29, 2021 at 08:50:12PM +0300, Gwan-gyeong Mun wrote:
>>
>>
>> On 9/23/21 10:46 PM, José Roberto de Souza wrote:
>>> PSR2 selective is not supported over rotated and scaled planes.
>>> We had the rotation check in intel_psr2_sel_fetch_config_valid()
>>> but that code path is only execute when a modeset is needed and
>>> change those plane parameters do not require a modeset.
>>>
>>> Also need to check those restricions in the second
>>> for_each_oldnew_intel_plane_in_state() loop because the state could
>>> only have a plane that is not affected by those restricitons but
>>> the damaged area intersect with planes that has those restrictions,
>>> so a full plane fetch is required.
>>>
>>> BSpec: 55229
>>> Cc: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>
>>> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
>>> ---
>>>    drivers/gpu/drm/i915/display/intel_psr.c | 45 ++++++++++++++----------
>>>    1 file changed, 26 insertions(+), 19 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c
>>> index 1cc4130dec7b1..356e0e96abf4e 100644
>>> --- a/drivers/gpu/drm/i915/display/intel_psr.c
>>> +++ b/drivers/gpu/drm/i915/display/intel_psr.c
>>> @@ -720,11 +720,7 @@ tgl_dc3co_exitline_compute_config(struct intel_dp *intel_dp,
>>>    static bool intel_psr2_sel_fetch_config_valid(struct intel_dp *intel_dp,
>>>    					      struct intel_crtc_state *crtc_state)
>>>    {
>>> -	struct intel_atomic_state *state = to_intel_atomic_state(crtc_state->uapi.state);
>>>    	struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
>>> -	struct intel_plane_state *plane_state;
>>> -	struct intel_plane *plane;
>>> -	int i;
>>>    
>>>    	if (!dev_priv->params.enable_psr2_sel_fetch &&
>>>    	    intel_dp->psr.debug != I915_PSR_DEBUG_ENABLE_SEL_FETCH) {
>>> @@ -739,14 +735,6 @@ static bool intel_psr2_sel_fetch_config_valid(struct intel_dp *intel_dp,
>>>    		return false;
>>>    	}
>>>    
>>> -	for_each_new_intel_plane_in_state(state, plane, plane_state, i) {
>>> -		if (plane_state->uapi.rotation != DRM_MODE_ROTATE_0) {
>>> -			drm_dbg_kms(&dev_priv->drm,
>>> -				    "PSR2 sel fetch not enabled, plane rotated\n");
>>> -			return false;
>>> -		}
>>> -	}
>>> -
>>>    	/* Wa_14010254185 Wa_14010103792 */
>>>    	if (IS_TGL_DISPLAY_STEP(dev_priv, STEP_A0, STEP_C0)) {
>>>    		drm_dbg_kms(&dev_priv->drm,
>>> @@ -1586,6 +1574,26 @@ static void cursor_area_workaround(const struct intel_plane_state *new_plane_sta
>>>    	clip_area_update(pipe_clip, damaged_area);
>>>    }
>>>    
>>> +/*
>>> + * TODO: Not clear how to handle planes with negative position,
>>> + * also planes are not updated if they have a negative X
>>> + * position so for now doing a full update in this cases
>>> + *
>>> + * Plane scaling and rotation is not supported by selective fetch and both
>>> + * properties can change without a modeset, so need to be check at every
>>> + * atomic commmit.
>>> + */
>>> +static bool psr2_sel_fetch_plane_state_supported(const struct intel_plane_state *plane_state)
>>> +{
>>> +	if (plane_state->uapi.dst.y1 < 0 ||
>>> +	    plane_state->uapi.dst.x1 < 0 ||
>> intel_atomic_plane_check_clipping() function makes
>> plane_state->uapi.dst.x1 and plane_state->uapi.dst.y1 non-negative
>> values, so there is no need to deal with negative positions here.
> 
> Cursor can have negative coordinates as it's hardware that will do the
> clipping for us.
> 
Yes, you are right, thanks for pointing out it. The cursor plane's 
CUR_POS register has X and Y Position Sign bits.

Reviewed-by: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>
>>
>> And the rest of the changes look good to me.
>>> +	    plane_state->scaler_id >= 0 ||
>>> +	    plane_state->uapi.rotation != DRM_MODE_ROTATE_0)
>>> +		return false;
>>> +
>>> +	return true;
>>> +}
>>> +
>>>    int intel_psr2_sel_fetch_update(struct intel_atomic_state *state,
>>>    				struct intel_crtc *crtc)
>>>    {
>>> @@ -1618,13 +1626,7 @@ int intel_psr2_sel_fetch_update(struct intel_atomic_state *state,
>>>    		    !old_plane_state->uapi.visible)
>>>    			continue;
>>>    
>>> -		/*
>>> -		 * TODO: Not clear how to handle planes with negative position,
>>> -		 * also planes are not updated if they have a negative X
>>> -		 * position so for now doing a full update in this cases
>>> -		 */
>>> -		if (new_plane_state->uapi.dst.y1 < 0 ||
>>> -		    new_plane_state->uapi.dst.x1 < 0) {
>>> +		if (!psr2_sel_fetch_plane_state_supported(new_plane_state)) {
>>>    			full_update = true;
>>>    			break;
>>>    		}
>>> @@ -1703,6 +1705,11 @@ int intel_psr2_sel_fetch_update(struct intel_atomic_state *state,
>>>    		if (!drm_rect_intersect(&inter, &new_plane_state->uapi.dst))
>>>    			continue;
>>>    
>>> +		if (!psr2_sel_fetch_plane_state_supported(new_plane_state)) {
>>> +			full_update = true;
>>> +			break;
>>> +		}
>>> +
>>>    		sel_fetch_area = &new_plane_state->psr2_sel_fetch_area;
>>>    		sel_fetch_area->y1 = inter.y1 - new_plane_state->uapi.dst.y1;
>>>    		sel_fetch_area->y2 = inter.y2 - new_plane_state->uapi.dst.y1;
>>>
> 

Re: [Intel-gfx] [PATCH 4/7] drm/i915/display/psr: Handle plane restrictions at every page flip

[Ville Syrjälä]

On Thu, Sep 30, 2021 at 08:35:01AM +0300, Gwan-gyeong Mun wrote:
> 
> 
> On 9/29/21 9:20 PM, Ville Syrjälä wrote:
> > On Wed, Sep 29, 2021 at 08:50:12PM +0300, Gwan-gyeong Mun wrote:
> >>
> >>
> >> On 9/23/21 10:46 PM, José Roberto de Souza wrote:
> >>> PSR2 selective is not supported over rotated and scaled planes.
> >>> We had the rotation check in intel_psr2_sel_fetch_config_valid()
> >>> but that code path is only execute when a modeset is needed and
> >>> change those plane parameters do not require a modeset.
> >>>
> >>> Also need to check those restricions in the second
> >>> for_each_oldnew_intel_plane_in_state() loop because the state could
> >>> only have a plane that is not affected by those restricitons but
> >>> the damaged area intersect with planes that has those restrictions,
> >>> so a full plane fetch is required.
> >>>
> >>> BSpec: 55229
> >>> Cc: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>
> >>> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> >>> ---
> >>>    drivers/gpu/drm/i915/display/intel_psr.c | 45 ++++++++++++++----------
> >>>    1 file changed, 26 insertions(+), 19 deletions(-)
> >>>
> >>> diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c
> >>> index 1cc4130dec7b1..356e0e96abf4e 100644
> >>> --- a/drivers/gpu/drm/i915/display/intel_psr.c
> >>> +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> >>> @@ -720,11 +720,7 @@ tgl_dc3co_exitline_compute_config(struct intel_dp *intel_dp,
> >>>    static bool intel_psr2_sel_fetch_config_valid(struct intel_dp *intel_dp,
> >>>    					      struct intel_crtc_state *crtc_state)
> >>>    {
> >>> -	struct intel_atomic_state *state = to_intel_atomic_state(crtc_state->uapi.state);
> >>>    	struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
> >>> -	struct intel_plane_state *plane_state;
> >>> -	struct intel_plane *plane;
> >>> -	int i;
> >>>    
> >>>    	if (!dev_priv->params.enable_psr2_sel_fetch &&
> >>>    	    intel_dp->psr.debug != I915_PSR_DEBUG_ENABLE_SEL_FETCH) {
> >>> @@ -739,14 +735,6 @@ static bool intel_psr2_sel_fetch_config_valid(struct intel_dp *intel_dp,
> >>>    		return false;
> >>>    	}
> >>>    
> >>> -	for_each_new_intel_plane_in_state(state, plane, plane_state, i) {
> >>> -		if (plane_state->uapi.rotation != DRM_MODE_ROTATE_0) {
> >>> -			drm_dbg_kms(&dev_priv->drm,
> >>> -				    "PSR2 sel fetch not enabled, plane rotated\n");
> >>> -			return false;
> >>> -		}
> >>> -	}
> >>> -
> >>>    	/* Wa_14010254185 Wa_14010103792 */
> >>>    	if (IS_TGL_DISPLAY_STEP(dev_priv, STEP_A0, STEP_C0)) {
> >>>    		drm_dbg_kms(&dev_priv->drm,
> >>> @@ -1586,6 +1574,26 @@ static void cursor_area_workaround(const struct intel_plane_state *new_plane_sta
> >>>    	clip_area_update(pipe_clip, damaged_area);
> >>>    }
> >>>    
> >>> +/*
> >>> + * TODO: Not clear how to handle planes with negative position,
> >>> + * also planes are not updated if they have a negative X
> >>> + * position so for now doing a full update in this cases
> >>> + *
> >>> + * Plane scaling and rotation is not supported by selective fetch and both
> >>> + * properties can change without a modeset, so need to be check at every
> >>> + * atomic commmit.
> >>> + */
> >>> +static bool psr2_sel_fetch_plane_state_supported(const struct intel_plane_state *plane_state)
> >>> +{
> >>> +	if (plane_state->uapi.dst.y1 < 0 ||
> >>> +	    plane_state->uapi.dst.x1 < 0 ||
> >> intel_atomic_plane_check_clipping() function makes
> >> plane_state->uapi.dst.x1 and plane_state->uapi.dst.y1 non-negative
> >> values, so there is no need to deal with negative positions here.
> > 
> > Cursor can have negative coordinates as it's hardware that will do the
> > clipping for us.
> > 
> Yes, you are right, thanks for pointing out it. The cursor plane's 
> CUR_POS register has X and Y Position Sign bits.

Oh and btw the cursor will go partially offscreen in the other direction
as well. If the code can't deal with the top/left out of bounds case I
suspect it might have problems with the bottom/right case too.

> 
> Reviewed-by: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>
> >>
> >> And the rest of the changes look good to me.
> >>> +	    plane_state->scaler_id >= 0 ||
> >>> +	    plane_state->uapi.rotation != DRM_MODE_ROTATE_0)
> >>> +		return false;
> >>> +
> >>> +	return true;
> >>> +}
> >>> +
> >>>    int intel_psr2_sel_fetch_update(struct intel_atomic_state *state,
> >>>    				struct intel_crtc *crtc)
> >>>    {
> >>> @@ -1618,13 +1626,7 @@ int intel_psr2_sel_fetch_update(struct intel_atomic_state *state,
> >>>    		    !old_plane_state->uapi.visible)
> >>>    			continue;
> >>>    
> >>> -		/*
> >>> -		 * TODO: Not clear how to handle planes with negative position,
> >>> -		 * also planes are not updated if they have a negative X
> >>> -		 * position so for now doing a full update in this cases
> >>> -		 */
> >>> -		if (new_plane_state->uapi.dst.y1 < 0 ||
> >>> -		    new_plane_state->uapi.dst.x1 < 0) {
> >>> +		if (!psr2_sel_fetch_plane_state_supported(new_plane_state)) {
> >>>    			full_update = true;
> >>>    			break;
> >>>    		}
> >>> @@ -1703,6 +1705,11 @@ int intel_psr2_sel_fetch_update(struct intel_atomic_state *state,
> >>>    		if (!drm_rect_intersect(&inter, &new_plane_state->uapi.dst))
> >>>    			continue;
> >>>    
> >>> +		if (!psr2_sel_fetch_plane_state_supported(new_plane_state)) {
> >>> +			full_update = true;
> >>> +			break;
> >>> +		}
> >>> +
> >>>    		sel_fetch_area = &new_plane_state->psr2_sel_fetch_area;
> >>>    		sel_fetch_area->y1 = inter.y1 - new_plane_state->uapi.dst.y1;
> >>>    		sel_fetch_area->y2 = inter.y2 - new_plane_state->uapi.dst.y1;
> >>>
> > 

-- 
Ville Syrjälä
Intel

[Intel-gfx] [PATCH 5/7] drm/i915/display/psr: Do full fetch when handling biplanar formats

[José Roberto de Souza]

From: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>

We are still missing the PSR2 selective fetch handling of biplanar
formats but until proper handle is added we can workaround it by
doing full frames fetch when state has biplanar formats.

Signed-off-by: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>
---
 drivers/gpu/drm/i915/display/intel_psr.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c
index 356e0e96abf4e..001d81f128989 100644
--- a/drivers/gpu/drm/i915/display/intel_psr.c
+++ b/drivers/gpu/drm/i915/display/intel_psr.c
@@ -1579,6 +1579,9 @@ static void cursor_area_workaround(const struct intel_plane_state *new_plane_sta
  * also planes are not updated if they have a negative X
  * position so for now doing a full update in this cases
  *
+ * TODO: We are missing biplanar formats handling, until it is
+ * implemented it will send full frame updates.
+ *
  * Plane scaling and rotation is not supported by selective fetch and both
  * properties can change without a modeset, so need to be check at every
  * atomic commmit.
@@ -1588,6 +1591,7 @@ static bool psr2_sel_fetch_plane_state_supported(const struct intel_plane_state
 	if (plane_state->uapi.dst.y1 < 0 ||
 	    plane_state->uapi.dst.x1 < 0 ||
 	    plane_state->scaler_id >= 0 ||
+	    plane_state->hw.fb->format->is_yuv ||
 	    plane_state->uapi.rotation != DRM_MODE_ROTATE_0)
 		return false;
 
-- 
2.33.0

Re: [Intel-gfx] [PATCH 5/7] drm/i915/display/psr: Do full fetch when handling biplanar formats

[Ville Syrjälä]

On Thu, Sep 23, 2021 at 12:46:15PM -0700, José Roberto de Souza wrote:
> From: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>
> 
> We are still missing the PSR2 selective fetch handling of biplanar
> formats but until proper handle is added we can workaround it by
> doing full frames fetch when state has biplanar formats.
> 
> Signed-off-by: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_psr.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c
> index 356e0e96abf4e..001d81f128989 100644
> --- a/drivers/gpu/drm/i915/display/intel_psr.c
> +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> @@ -1579,6 +1579,9 @@ static void cursor_area_workaround(const struct intel_plane_state *new_plane_sta
>   * also planes are not updated if they have a negative X
>   * position so for now doing a full update in this cases
>   *
> + * TODO: We are missing biplanar formats handling, until it is
> + * implemented it will send full frame updates.
> + *
>   * Plane scaling and rotation is not supported by selective fetch and both
>   * properties can change without a modeset, so need to be check at every
>   * atomic commmit.
> @@ -1588,6 +1591,7 @@ static bool psr2_sel_fetch_plane_state_supported(const struct intel_plane_state
>  	if (plane_state->uapi.dst.y1 < 0 ||
>  	    plane_state->uapi.dst.x1 < 0 ||
>  	    plane_state->scaler_id >= 0 ||
> +	    plane_state->hw.fb->format->is_yuv ||

is_yuv != planar

>  	    plane_state->uapi.rotation != DRM_MODE_ROTATE_0)
>  		return false;
>  
> -- 
> 2.33.0

-- 
Ville Syrjälä
Intel

[Intel-gfx] [PATCH 6/7] drm/i915/display/adlp: Allow PSR2 to be enabled

[José Roberto de Souza]

With all the recent fixes PSR2 is properly working in Alderlake-P but
due to some issues that don't have software workarounds it will not be
supported in display steppings older than B0.

Even with this patch PSR2 will no be enabled by default in ADL-P, it
still requires enable_psr2_sel_fetch to be set to true, what some
of our tests does.

Cc: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>
Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
---
 drivers/gpu/drm/i915/display/intel_psr.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c
index 001d81f128989..37727ff2b2ec9 100644
--- a/drivers/gpu/drm/i915/display/intel_psr.c
+++ b/drivers/gpu/drm/i915/display/intel_psr.c
@@ -830,12 +830,8 @@ static bool intel_psr2_config_valid(struct intel_dp *intel_dp,
 		return false;
 	}
 
-	/*
-	 * We are missing the implementation of some workarounds to enabled PSR2
-	 * in Alderlake_P, until ready PSR2 should be kept disabled.
-	 */
-	if (IS_ALDERLAKE_P(dev_priv)) {
-		drm_dbg_kms(&dev_priv->drm, "PSR2 is missing the implementation of workarounds\n");
+	if (IS_ADLP_DISPLAY_STEP(dev_priv, STEP_A0, STEP_B0)) {
+		drm_dbg_kms(&dev_priv->drm, "PSR2 not completely functional in this stepping\n");
 		return false;
 	}
 
-- 
2.33.0

Re: [Intel-gfx] [PATCH 6/7] drm/i915/display/adlp: Allow PSR2 to be enabled

[Gwan-gyeong Mun]

Reviewed-by: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>

On 9/23/21 10:46 PM, José Roberto de Souza wrote:
> With all the recent fixes PSR2 is properly working in Alderlake-P but
> due to some issues that don't have software workarounds it will not be
> supported in display steppings older than B0.
> 
> Even with this patch PSR2 will no be enabled by default in ADL-P, it
> still requires enable_psr2_sel_fetch to be set to true, what some
> of our tests does.
> 
> Cc: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> ---
>   drivers/gpu/drm/i915/display/intel_psr.c | 8 ++------
>   1 file changed, 2 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c
> index 001d81f128989..37727ff2b2ec9 100644
> --- a/drivers/gpu/drm/i915/display/intel_psr.c
> +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> @@ -830,12 +830,8 @@ static bool intel_psr2_config_valid(struct intel_dp *intel_dp,
>   		return false;
>   	}
>   
> -	/*
> -	 * We are missing the implementation of some workarounds to enabled PSR2
> -	 * in Alderlake_P, until ready PSR2 should be kept disabled.
> -	 */
> -	if (IS_ALDERLAKE_P(dev_priv)) {
> -		drm_dbg_kms(&dev_priv->drm, "PSR2 is missing the implementation of workarounds\n");
> +	if (IS_ADLP_DISPLAY_STEP(dev_priv, STEP_A0, STEP_B0)) {
> +		drm_dbg_kms(&dev_priv->drm, "PSR2 not completely functional in this stepping\n");
>   		return false;
>   	}
>   
> 

[Intel-gfx] [PATCH 7/7] drm/i915/display: Enable PSR2 selective fetch by default

[José Roberto de Souza]

With all the past fixes now this feature is functional and can be
enabled by default in desktop enviroments that uses compositor.

Cc: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
---
 drivers/gpu/drm/i915/i915_params.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_params.h b/drivers/gpu/drm/i915/i915_params.h
index f27eceb82c0f5..8d725b64592d8 100644
--- a/drivers/gpu/drm/i915/i915_params.h
+++ b/drivers/gpu/drm/i915/i915_params.h
@@ -55,7 +55,7 @@ struct drm_printer;
 	param(int, enable_fbc, -1, 0600) \
 	param(int, enable_psr, -1, 0600) \
 	param(bool, psr_safest_params, false, 0400) \
-	param(bool, enable_psr2_sel_fetch, false, 0400) \
+	param(bool, enable_psr2_sel_fetch, true, 0400) \
 	param(int, disable_power_well, -1, 0400) \
 	param(int, enable_ips, 1, 0600) \
 	param(int, invert_brightness, 0, 0600) \
-- 
2.33.0

Re: [Intel-gfx] [PATCH 7/7] drm/i915/display: Enable PSR2 selective fetch by default

[Gwan-gyeong Mun]

Reviewed-by: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>

On 9/23/21 10:46 PM, José Roberto de Souza wrote:
> With all the past fixes now this feature is functional and can be
> enabled by default in desktop enviroments that uses compositor.
> 
> Cc: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> ---
>   drivers/gpu/drm/i915/i915_params.h | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_params.h b/drivers/gpu/drm/i915/i915_params.h
> index f27eceb82c0f5..8d725b64592d8 100644
> --- a/drivers/gpu/drm/i915/i915_params.h
> +++ b/drivers/gpu/drm/i915/i915_params.h
> @@ -55,7 +55,7 @@ struct drm_printer;
>   	param(int, enable_fbc, -1, 0600) \
>   	param(int, enable_psr, -1, 0600) \
>   	param(bool, psr_safest_params, false, 0400) \
> -	param(bool, enable_psr2_sel_fetch, false, 0400) \
> +	param(bool, enable_psr2_sel_fetch, true, 0400) \
>   	param(int, disable_power_well, -1, 0400) \
>   	param(int, enable_ips, 1, 0600) \
>   	param(int, invert_brightness, 0, 0600) \
> 

[Intel-gfx] ✗ Fi.CI.DOCS: warning for series starting with [1/7] drm/i915/display: Wait PSR2 get out of deep sleep to update pipe

[Patchwork]

== Series Details ==

Series: series starting with [1/7] drm/i915/display: Wait PSR2 get out of deep sleep to update pipe
URL   : https://patchwork.freedesktop.org/series/95002/
State : warning

== Summary ==

$ make htmldocs 2>&1 > /dev/null | grep i915
./drivers/gpu/drm/i915/display/intel_drrs.c:377: warning: Function parameter or member 'origin' not described in 'intel_drrs_flush'

[Intel-gfx] ✓ Fi.CI.BAT: success for series starting with [1/7] drm/i915/display: Wait PSR2 get out of deep sleep to update pipe

[Patchwork]

[-- Attachment #1: Type: text/plain, Size: 6432 bytes --]

== Series Details ==

Series: series starting with [1/7] drm/i915/display: Wait PSR2 get out of deep sleep to update pipe
URL   : https://patchwork.freedesktop.org/series/95002/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_10632 -> Patchwork_21148
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21148/index.html

Known issues
------------

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

### IGT changes ###

#### Issues hit ####

  * igt@amdgpu/amd_basic@query-info:
    - fi-tgl-1115g4:      NOTRUN -> [SKIP][1] ([fdo#109315])
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21148/fi-tgl-1115g4/igt@amdgpu/amd_basic@query-info.html

  * igt@amdgpu/amd_cs_nop@nop-gfx0:
    - fi-tgl-1115g4:      NOTRUN -> [SKIP][2] ([fdo#109315] / [i915#2575]) +16 similar issues
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21148/fi-tgl-1115g4/igt@amdgpu/amd_cs_nop@nop-gfx0.html

  * igt@gem_exec_suspend@basic-s0:
    - fi-tgl-1115g4:      NOTRUN -> [FAIL][3] ([i915#1888])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21148/fi-tgl-1115g4/igt@gem_exec_suspend@basic-s0.html

  * igt@gem_huc_copy@huc-copy:
    - fi-tgl-1115g4:      NOTRUN -> [SKIP][4] ([i915#2190])
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21148/fi-tgl-1115g4/igt@gem_huc_copy@huc-copy.html

  * igt@i915_pm_backlight@basic-brightness:
    - fi-tgl-1115g4:      NOTRUN -> [SKIP][5] ([i915#1155])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21148/fi-tgl-1115g4/igt@i915_pm_backlight@basic-brightness.html

  * igt@kms_chamelium@common-hpd-after-suspend:
    - fi-tgl-1115g4:      NOTRUN -> [SKIP][6] ([fdo#111827]) +8 similar issues
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21148/fi-tgl-1115g4/igt@kms_chamelium@common-hpd-after-suspend.html

  * igt@kms_chamelium@dp-crc-fast:
    - fi-kbl-7500u:       [PASS][7] -> [FAIL][8] ([i915#1372])
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10632/fi-kbl-7500u/igt@kms_chamelium@dp-crc-fast.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21148/fi-kbl-7500u/igt@kms_chamelium@dp-crc-fast.html

  * igt@kms_cursor_legacy@basic-busy-flip-before-cursor-atomic:
    - fi-tgl-1115g4:      NOTRUN -> [SKIP][9] ([i915#4103]) +1 similar issue
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21148/fi-tgl-1115g4/igt@kms_cursor_legacy@basic-busy-flip-before-cursor-atomic.html

  * igt@kms_cursor_legacy@basic-flip-before-cursor-legacy:
    - fi-tgl-u2:          [PASS][10] -> [FAIL][11] ([i915#2346]) +3 similar issues
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10632/fi-tgl-u2/igt@kms_cursor_legacy@basic-flip-before-cursor-legacy.html
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21148/fi-tgl-u2/igt@kms_cursor_legacy@basic-flip-before-cursor-legacy.html

  * igt@kms_force_connector_basic@force-load-detect:
    - fi-tgl-1115g4:      NOTRUN -> [SKIP][12] ([fdo#109285])
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21148/fi-tgl-1115g4/igt@kms_force_connector_basic@force-load-detect.html

  * igt@kms_psr@primary_mmap_gtt:
    - fi-tgl-1115g4:      NOTRUN -> [SKIP][13] ([i915#1072]) +3 similar issues
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21148/fi-tgl-1115g4/igt@kms_psr@primary_mmap_gtt.html

  * igt@prime_vgem@basic-userptr:
    - fi-tgl-1115g4:      NOTRUN -> [SKIP][14] ([i915#3301])
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21148/fi-tgl-1115g4/igt@prime_vgem@basic-userptr.html

  
#### Possible fixes ####

  * igt@kms_chamelium@hdmi-edid-read:
    - fi-kbl-7500u:       [FAIL][15] ([i915#3449]) -> [PASS][16]
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10632/fi-kbl-7500u/igt@kms_chamelium@hdmi-edid-read.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21148/fi-kbl-7500u/igt@kms_chamelium@hdmi-edid-read.html

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

  [fdo#109285]: https://bugs.freedesktop.org/show_bug.cgi?id=109285
  [fdo#109315]: https://bugs.freedesktop.org/show_bug.cgi?id=109315
  [fdo#111827]: https://bugs.freedesktop.org/show_bug.cgi?id=111827
  [i915#1072]: https://gitlab.freedesktop.org/drm/intel/issues/1072
  [i915#1155]: https://gitlab.freedesktop.org/drm/intel/issues/1155
  [i915#1372]: https://gitlab.freedesktop.org/drm/intel/issues/1372
  [i915#1888]: https://gitlab.freedesktop.org/drm/intel/issues/1888
  [i915#2190]: https://gitlab.freedesktop.org/drm/intel/issues/2190
  [i915#2346]: https://gitlab.freedesktop.org/drm/intel/issues/2346
  [i915#2575]: https://gitlab.freedesktop.org/drm/intel/issues/2575
  [i915#3301]: https://gitlab.freedesktop.org/drm/intel/issues/3301
  [i915#3449]: https://gitlab.freedesktop.org/drm/intel/issues/3449
  [i915#4103]: https://gitlab.freedesktop.org/drm/intel/issues/4103
  [i915#541]: https://gitlab.freedesktop.org/drm/intel/issues/541


Participating hosts (40 -> 34)
------------------------------

  Additional (1): fi-tgl-1115g4 
  Missing    (7): bat-dg1-6 bat-dg1-5 fi-bsw-cyan bat-adlp-4 fi-bdw-samus bat-jsl-2 bat-jsl-1 


Build changes
-------------

  * Linux: CI_DRM_10632 -> Patchwork_21148

  CI-20190529: 20190529
  CI_DRM_10632: 3e62264203fbb2a56cab68878fe3415ba1e64a32 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_6217: e02037bfd63fb26dd32b9e9ceb1ca131ddbf30c3 @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
  Patchwork_21148: 50322e62e00f732b953d93219d755a04689381f3 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

50322e62e00f drm/i915/display: Enable PSR2 selective fetch by default
184cf45a8f1b drm/i915/display/adlp: Allow PSR2 to be enabled
46843eaab5c7 drm/i915/display/psr: Do full fetch when handling biplanar formats
6831b9e9a35c drm/i915/display/psr: Handle plane restrictions at every page flip
73ef03f8841a drm/i915/display: Add new fb_op_origin type and use it to optimize power savings
9a9ea87ef89a drm/i915/display: Handle frontbuffer rendering when PSR2 selective fetch is enabled
afa26ae95a67 drm/i915/display: Wait PSR2 get out of deep sleep to update pipe

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21148/index.html

[-- Attachment #2: Type: text/html, Size: 7401 bytes --]

[Intel-gfx] ✓ Fi.CI.IGT: success for series starting with [1/7] drm/i915/display: Wait PSR2 get out of deep sleep to update pipe

[Patchwork]

[-- Attachment #1: Type: text/plain, Size: 30314 bytes --]

== Series Details ==

Series: series starting with [1/7] drm/i915/display: Wait PSR2 get out of deep sleep to update pipe
URL   : https://patchwork.freedesktop.org/series/95002/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_10632_full -> Patchwork_21148_full
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  

Known issues
------------

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

### IGT changes ###

#### Issues hit ####

  * igt@gem_ctx_persistence@idempotent:
    - shard-snb:          NOTRUN -> [SKIP][1] ([fdo#109271] / [i915#1099])
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21148/shard-snb6/igt@gem_ctx_persistence@idempotent.html

  * igt@gem_ctx_persistence@legacy-engines-queued@vebox:
    - shard-skl:          [PASS][2] -> [DMESG-WARN][3] ([i915#1982]) +2 similar issues
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10632/shard-skl8/igt@gem_ctx_persistence@legacy-engines-queued@vebox.html
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21148/shard-skl8/igt@gem_ctx_persistence@legacy-engines-queued@vebox.html

  * igt@gem_ctx_sseu@engines:
    - shard-tglb:         NOTRUN -> [SKIP][4] ([i915#280])
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21148/shard-tglb1/igt@gem_ctx_sseu@engines.html

  * igt@gem_exec_fair@basic-deadline:
    - shard-apl:          NOTRUN -> [FAIL][5] ([i915#2846])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21148/shard-apl3/igt@gem_exec_fair@basic-deadline.html

  * igt@gem_exec_fair@basic-none-solo@rcs0:
    - shard-kbl:          NOTRUN -> [FAIL][6] ([i915#2842])
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21148/shard-kbl1/igt@gem_exec_fair@basic-none-solo@rcs0.html

  * igt@gem_exec_fair@basic-none-vip@rcs0:
    - shard-glk:          [PASS][7] -> [FAIL][8] ([i915#2842])
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10632/shard-glk5/igt@gem_exec_fair@basic-none-vip@rcs0.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21148/shard-glk4/igt@gem_exec_fair@basic-none-vip@rcs0.html

  * igt@gem_exec_fair@basic-pace-share@rcs0:
    - shard-tglb:         [PASS][9] -> [FAIL][10] ([i915#2842])
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10632/shard-tglb3/igt@gem_exec_fair@basic-pace-share@rcs0.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21148/shard-tglb5/igt@gem_exec_fair@basic-pace-share@rcs0.html

  * igt@gem_exec_fair@basic-pace@bcs0:
    - shard-tglb:         NOTRUN -> [FAIL][11] ([i915#2842]) +4 similar issues
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21148/shard-tglb1/igt@gem_exec_fair@basic-pace@bcs0.html

  * igt@gem_exec_fair@basic-pace@rcs0:
    - shard-kbl:          [PASS][12] -> [FAIL][13] ([i915#2851])
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10632/shard-kbl7/igt@gem_exec_fair@basic-pace@rcs0.html
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21148/shard-kbl7/igt@gem_exec_fair@basic-pace@rcs0.html
    - shard-glk:          [PASS][14] -> [FAIL][15] ([i915#2851])
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10632/shard-glk2/igt@gem_exec_fair@basic-pace@rcs0.html
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21148/shard-glk6/igt@gem_exec_fair@basic-pace@rcs0.html

  * igt@gem_pread@exhaustion:
    - shard-apl:          NOTRUN -> [WARN][16] ([i915#2658]) +1 similar issue
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21148/shard-apl6/igt@gem_pread@exhaustion.html

  * igt@gem_pwrite@basic-exhaustion:
    - shard-snb:          NOTRUN -> [WARN][17] ([i915#2658]) +1 similar issue
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21148/shard-snb6/igt@gem_pwrite@basic-exhaustion.html
    - shard-kbl:          NOTRUN -> [WARN][18] ([i915#2658]) +1 similar issue
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21148/shard-kbl1/igt@gem_pwrite@basic-exhaustion.html

  * igt@gem_softpin@evict-snoop-interruptible:
    - shard-tglb:         NOTRUN -> [SKIP][19] ([fdo#109312])
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21148/shard-tglb3/igt@gem_softpin@evict-snoop-interruptible.html

  * igt@gem_softpin@noreloc-s3:
    - shard-apl:          NOTRUN -> [DMESG-WARN][20] ([i915#180])
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21148/shard-apl7/igt@gem_softpin@noreloc-s3.html

  * igt@gem_userptr_blits@dmabuf-sync:
    - shard-apl:          NOTRUN -> [SKIP][21] ([fdo#109271] / [i915#3323])
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21148/shard-apl7/igt@gem_userptr_blits@dmabuf-sync.html

  * igt@gem_userptr_blits@unsync-unmap-cycles:
    - shard-tglb:         NOTRUN -> [SKIP][22] ([i915#3297])
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21148/shard-tglb3/igt@gem_userptr_blits@unsync-unmap-cycles.html

  * igt@gem_workarounds@suspend-resume-context:
    - shard-skl:          [PASS][23] -> [INCOMPLETE][24] ([i915#198] / [i915#4173])
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10632/shard-skl6/igt@gem_workarounds@suspend-resume-context.html
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21148/shard-skl7/igt@gem_workarounds@suspend-resume-context.html

  * igt@gen7_exec_parse@basic-allocation:
    - shard-tglb:         NOTRUN -> [SKIP][25] ([fdo#109289]) +2 similar issues
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21148/shard-tglb3/igt@gen7_exec_parse@basic-allocation.html

  * igt@gen9_exec_parse@bb-start-param:
    - shard-tglb:         NOTRUN -> [SKIP][26] ([i915#2856])
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21148/shard-tglb3/igt@gen9_exec_parse@bb-start-param.html

  * igt@i915_pm_dc@dc5-dpms:
    - shard-skl:          [PASS][27] -> [INCOMPLETE][28] ([i915#198])
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10632/shard-skl7/igt@i915_pm_dc@dc5-dpms.html
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21148/shard-skl9/igt@i915_pm_dc@dc5-dpms.html

  * igt@i915_pm_lpsp@kms-lpsp@kms-lpsp-dp:
    - shard-apl:          NOTRUN -> [SKIP][29] ([fdo#109271] / [i915#1937])
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21148/shard-apl3/igt@i915_pm_lpsp@kms-lpsp@kms-lpsp-dp.html

  * igt@kms_big_fb@linear-16bpp-rotate-270:
    - shard-tglb:         NOTRUN -> [SKIP][30] ([fdo#111614]) +2 similar issues
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21148/shard-tglb1/igt@kms_big_fb@linear-16bpp-rotate-270.html

  * igt@kms_big_fb@x-tiled-max-hw-stride-32bpp-rotate-180-hflip:
    - shard-kbl:          NOTRUN -> [SKIP][31] ([fdo#109271] / [i915#3777]) +2 similar issues
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21148/shard-kbl2/igt@kms_big_fb@x-tiled-max-hw-stride-32bpp-rotate-180-hflip.html

  * igt@kms_big_fb@y-tiled-32bpp-rotate-0:
    - shard-glk:          [PASS][32] -> [DMESG-WARN][33] ([i915#118] / [i915#95])
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10632/shard-glk8/igt@kms_big_fb@y-tiled-32bpp-rotate-0.html
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21148/shard-glk4/igt@kms_big_fb@y-tiled-32bpp-rotate-0.html

  * igt@kms_big_fb@y-tiled-max-hw-stride-32bpp-rotate-0-hflip:
    - shard-apl:          NOTRUN -> [SKIP][34] ([fdo#109271] / [i915#3777]) +2 similar issues
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21148/shard-apl3/igt@kms_big_fb@y-tiled-max-hw-stride-32bpp-rotate-0-hflip.html

  * igt@kms_big_fb@y-tiled-max-hw-stride-32bpp-rotate-180-hflip:
    - shard-skl:          NOTRUN -> [SKIP][35] ([fdo#109271] / [i915#3777])
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21148/shard-skl9/igt@kms_big_fb@y-tiled-max-hw-stride-32bpp-rotate-180-hflip.html

  * igt@kms_big_fb@yf-tiled-64bpp-rotate-0:
    - shard-iclb:         NOTRUN -> [SKIP][36] ([fdo#110723])
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21148/shard-iclb8/igt@kms_big_fb@yf-tiled-64bpp-rotate-0.html

  * igt@kms_big_fb@yf-tiled-64bpp-rotate-270:
    - shard-tglb:         NOTRUN -> [SKIP][37] ([fdo#111615]) +4 similar issues
   [37]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21148/shard-tglb1/igt@kms_big_fb@yf-tiled-64bpp-rotate-270.html

  * igt@kms_ccs@pipe-a-bad-pixel-format-y_tiled_gen12_mc_ccs:
    - shard-tglb:         NOTRUN -> [SKIP][38] ([i915#3689] / [i915#3886]) +3 similar issues
   [38]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21148/shard-tglb1/igt@kms_ccs@pipe-a-bad-pixel-format-y_tiled_gen12_mc_ccs.html

  * igt@kms_ccs@pipe-a-crc-primary-rotation-180-y_tiled_gen12_mc_ccs:
    - shard-kbl:          NOTRUN -> [SKIP][39] ([fdo#109271] / [i915#3886]) +2 similar issues
   [39]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21148/shard-kbl2/igt@kms_ccs@pipe-a-crc-primary-rotation-180-y_tiled_gen12_mc_ccs.html

  * igt@kms_ccs@pipe-a-missing-ccs-buffer-y_tiled_gen12_rc_ccs_cc:
    - shard-apl:          NOTRUN -> [SKIP][40] ([fdo#109271] / [i915#3886]) +7 similar issues
   [40]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21148/shard-apl7/igt@kms_ccs@pipe-a-missing-ccs-buffer-y_tiled_gen12_rc_ccs_cc.html

  * igt@kms_ccs@pipe-b-bad-aux-stride-y_tiled_gen12_rc_ccs_cc:
    - shard-skl:          NOTRUN -> [SKIP][41] ([fdo#109271] / [i915#3886]) +2 similar issues
   [41]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21148/shard-skl9/igt@kms_ccs@pipe-b-bad-aux-stride-y_tiled_gen12_rc_ccs_cc.html

  * igt@kms_ccs@pipe-d-crc-primary-basic-y_tiled_gen12_mc_ccs:
    - shard-iclb:         NOTRUN -> [SKIP][42] ([fdo#109278]) +2 similar issues
   [42]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21148/shard-iclb8/igt@kms_ccs@pipe-d-crc-primary-basic-y_tiled_gen12_mc_ccs.html

  * igt@kms_ccs@pipe-d-missing-ccs-buffer-yf_tiled_ccs:
    - shard-tglb:         NOTRUN -> [SKIP][43] ([i915#3689]) +4 similar issues
   [43]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21148/shard-tglb1/igt@kms_ccs@pipe-d-missing-ccs-buffer-yf_tiled_ccs.html

  * igt@kms_chamelium@dp-hpd-enable-disable-mode:
    - shard-iclb:         NOTRUN -> [SKIP][44] ([fdo#109284] / [fdo#111827])
   [44]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21148/shard-iclb8/igt@kms_chamelium@dp-hpd-enable-disable-mode.html

  * igt@kms_chamelium@dp-hpd-storm-disable:
    - shard-skl:          NOTRUN -> [SKIP][45] ([fdo#109271] / [fdo#111827]) +4 similar issues
   [45]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21148/shard-skl9/igt@kms_chamelium@dp-hpd-storm-disable.html

  * igt@kms_chamelium@hdmi-edid-change-during-suspend:
    - shard-apl:          NOTRUN -> [SKIP][46] ([fdo#109271] / [fdo#111827]) +23 similar issues
   [46]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21148/shard-apl6/igt@kms_chamelium@hdmi-edid-change-during-suspend.html

  * igt@kms_chamelium@vga-hpd:
    - shard-tglb:         NOTRUN -> [SKIP][47] ([fdo#109284] / [fdo#111827]) +6 similar issues
   [47]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21148/shard-tglb1/igt@kms_chamelium@vga-hpd.html

  * igt@kms_color_chamelium@pipe-a-ctm-blue-to-red:
    - shard-snb:          NOTRUN -> [SKIP][48] ([fdo#109271] / [fdo#111827]) +12 similar issues
   [48]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21148/shard-snb6/igt@kms_color_chamelium@pipe-a-ctm-blue-to-red.html
    - shard-kbl:          NOTRUN -> [SKIP][49] ([fdo#109271] / [fdo#111827]) +19 similar issues
   [49]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21148/shard-kbl1/igt@kms_color_chamelium@pipe-a-ctm-blue-to-red.html

  * igt@kms_color_chamelium@pipe-d-ctm-blue-to-red:
    - shard-iclb:         NOTRUN -> [SKIP][50] ([fdo#109278] / [fdo#109284] / [fdo#111827])
   [50]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21148/shard-iclb8/igt@kms_color_chamelium@pipe-d-ctm-blue-to-red.html

  * igt@kms_cursor_crc@pipe-a-cursor-suspend:
    - shard-apl:          [PASS][51] -> [DMESG-WARN][52] ([i915#180])
   [51]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10632/shard-apl2/igt@kms_cursor_crc@pipe-a-cursor-suspend.html
   [52]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21148/shard-apl6/igt@kms_cursor_crc@pipe-a-cursor-suspend.html
    - shard-skl:          [PASS][53] -> [INCOMPLETE][54] ([i915#2828] / [i915#300])
   [53]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10632/shard-skl6/igt@kms_cursor_crc@pipe-a-cursor-suspend.html
   [54]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21148/shard-skl4/igt@kms_cursor_crc@pipe-a-cursor-suspend.html

  * igt@kms_cursor_crc@pipe-b-cursor-32x32-rapid-movement:
    - shard-tglb:         NOTRUN -> [SKIP][55] ([i915#3319]) +1 similar issue
   [55]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21148/shard-tglb3/igt@kms_cursor_crc@pipe-b-cursor-32x32-rapid-movement.html

  * igt@kms_cursor_crc@pipe-c-cursor-512x512-random:
    - shard-tglb:         NOTRUN -> [SKIP][56] ([fdo#109279] / [i915#3359]) +1 similar issue
   [56]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21148/shard-tglb1/igt@kms_cursor_crc@pipe-c-cursor-512x512-random.html

  * igt@kms_cursor_crc@pipe-c-cursor-512x512-rapid-movement:
    - shard-iclb:         NOTRUN -> [SKIP][57] ([fdo#109278] / [fdo#109279])
   [57]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21148/shard-iclb8/igt@kms_cursor_crc@pipe-c-cursor-512x512-rapid-movement.html

  * igt@kms_cursor_crc@pipe-c-cursor-suspend:
    - shard-kbl:          NOTRUN -> [DMESG-WARN][58] ([i915#180])
   [58]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21148/shard-kbl3/igt@kms_cursor_crc@pipe-c-cursor-suspend.html

  * igt@kms_cursor_crc@pipe-d-cursor-512x170-rapid-movement:
    - shard-tglb:         NOTRUN -> [SKIP][59] ([i915#3359]) +2 similar issues
   [59]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21148/shard-tglb1/igt@kms_cursor_crc@pipe-d-cursor-512x170-rapid-movement.html

  * igt@kms_cursor_edge_walk@pipe-d-128x128-right-edge:
    - shard-snb:          NOTRUN -> [SKIP][60] ([fdo#109271]) +312 similar issues
   [60]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21148/shard-snb6/igt@kms_cursor_edge_walk@pipe-d-128x128-right-edge.html

  * igt@kms_cursor_legacy@cursorb-vs-flipa-legacy:
    - shard-iclb:         NOTRUN -> [SKIP][61] ([fdo#109274] / [fdo#109278])
   [61]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21148/shard-iclb8/igt@kms_cursor_legacy@cursorb-vs-flipa-legacy.html

  * igt@kms_cursor_legacy@short-busy-flip-before-cursor-atomic-transitions:
    - shard-tglb:         NOTRUN -> [SKIP][62] ([i915#4103])
   [62]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21148/shard-tglb1/igt@kms_cursor_legacy@short-busy-flip-before-cursor-atomic-transitions.html

  * igt@kms_fbcon_fbt@fbc-suspend:
    - shard-tglb:         [PASS][63] -> [INCOMPLETE][64] ([i915#4173] / [i915#456])
   [63]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10632/shard-tglb1/igt@kms_fbcon_fbt@fbc-suspend.html
   [64]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21148/shard-tglb7/igt@kms_fbcon_fbt@fbc-suspend.html

  * igt@kms_flip@2x-blocking-absolute-wf_vblank:
    - shard-iclb:         NOTRUN -> [SKIP][65] ([fdo#109274])
   [65]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21148/shard-iclb8/igt@kms_flip@2x-blocking-absolute-wf_vblank.html

  * igt@kms_frontbuffer_tracking@fbc-2p-primscrn-pri-indfb-draw-mmap-wc:
    - shard-tglb:         NOTRUN -> [SKIP][66] ([fdo#111825]) +16 similar issues
   [66]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21148/shard-tglb3/igt@kms_frontbuffer_tracking@fbc-2p-primscrn-pri-indfb-draw-mmap-wc.html

  * igt@kms_frontbuffer_tracking@fbc-suspend:
    - shard-kbl:          [PASS][67] -> [DMESG-WARN][68] ([i915#180]) +3 similar issues
   [67]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10632/shard-kbl3/igt@kms_frontbuffer_tracking@fbc-suspend.html
   [68]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21148/shard-kbl6/igt@kms_frontbuffer_tracking@fbc-suspend.html

  * igt@kms_frontbuffer_tracking@fbcpsr-2p-scndscrn-shrfb-pgflip-blt:
    - shard-iclb:         NOTRUN -> [SKIP][69] ([fdo#109280]) +2 similar issues
   [69]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21148/shard-iclb8/igt@kms_frontbuffer_tracking@fbcpsr-2p-scndscrn-shrfb-pgflip-blt.html

  * igt@kms_frontbuffer_tracking@psr-rgb101010-draw-mmap-wc:
    - shard-kbl:          NOTRUN -> [SKIP][70] ([fdo#109271]) +154 similar issues
   [70]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21148/shard-kbl3/igt@kms_frontbuffer_tracking@psr-rgb101010-draw-mmap-wc.html

  * igt@kms_hdr@static-swap:
    - shard-tglb:         NOTRUN -> [SKIP][71] ([i915#1187])
   [71]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21148/shard-tglb3/igt@kms_hdr@static-swap.html

  * igt@kms_pipe_crc_basic@nonblocking-crc-pipe-d-frame-sequence:
    - shard-apl:          NOTRUN -> [SKIP][72] ([fdo#109271] / [i915#533])
   [72]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21148/shard-apl1/igt@kms_pipe_crc_basic@nonblocking-crc-pipe-d-frame-sequence.html

  * igt@kms_plane_alpha_blend@pipe-a-alpha-transparent-fb:
    - shard-apl:          NOTRUN -> [FAIL][73] ([i915#265])
   [73]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21148/shard-apl6/igt@kms_plane_alpha_blend@pipe-a-alpha-transparent-fb.html

  * igt@kms_plane_alpha_blend@pipe-b-alpha-7efc:
    - shard-apl:          NOTRUN -> [FAIL][74] ([fdo#108145] / [i915#265]) +1 similar issue
   [74]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21148/shard-apl6/igt@kms_plane_alpha_blend@pipe-b-alpha-7efc.html

  * igt@kms_plane_alpha_blend@pipe-c-alpha-7efc:
    - shard-kbl:          NOTRUN -> [FAIL][75] ([fdo#108145] / [i915#265]) +4 similar issues
   [75]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21148/shard-kbl2/igt@kms_plane_alpha_blend@pipe-c-alpha-7efc.html

  * igt@kms_plane_alpha_blend@pipe-c-coverage-7efc:
    - shard-skl:          [PASS][76] -> [FAIL][77] ([fdo#108145] / [i915#265]) +1 similar issue
   [76]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10632/shard-skl2/igt@kms_plane_alpha_blend@pipe-c-coverage-7efc.html
   [77]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21148/shard-skl7/igt@kms_plane_alpha_blend@pipe-c-coverage-7efc.html

  * igt@kms_plane_lowres@pipe-c-tiling-y:
    - shard-tglb:         NOTRUN -> [SKIP][78] ([i915#3536])
   [78]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21148/shard-tglb1/igt@kms_plane_lowres@pipe-c-tiling-y.html

  * igt@kms_plane_multiple@atomic-pipe-b-tiling-yf:
    - shard-tglb:         NOTRUN -> [SKIP][79] ([fdo#112054])
   [79]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21148/shard-tglb3/igt@kms_plane_multiple@atomic-pipe-b-tiling-yf.html

  * igt@kms_psr2_sf@overlay-plane-update-sf-dmg-area-3:
    - shard-skl:          NOTRUN -> [SKIP][80] ([fdo#109271] / [i915#658])
   [80]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21148/shard-skl9/igt@kms_psr2_sf@overlay-plane-update-sf-dmg-area-3.html

  * igt@kms_psr2_sf@overlay-primary-update-sf-dmg-area-4:
    - shard-apl:          NOTRUN -> [SKIP][81] ([fdo#109271] / [i915#658]) +1 similar issue
   [81]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21148/shard-apl3/igt@kms_psr2_sf@overlay-primary-update-sf-dmg-area-4.html

  * igt@kms_psr2_sf@overlay-primary-update-sf-dmg-area-5:
    - shard-kbl:          NOTRUN -> [SKIP][82] ([fdo#109271] / [i915#658]) +1 similar issue
   [82]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21148/shard-kbl2/igt@kms_psr2_sf@overlay-primary-update-sf-dmg-area-5.html

  * igt@kms_psr2_sf@primary-plane-update-sf-dmg-area-2:
    - shard-tglb:         NOTRUN -> [SKIP][83] ([i915#2920])
   [83]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21148/shard-tglb1/igt@kms_psr2_sf@primary-plane-update-sf-dmg-area-2.html

  * igt@kms_psr2_su@frontbuffer:
    - shard-iclb:         [PASS][84] -> [SKIP][85] ([fdo#109642] / [fdo#111068] / [i915#658])
   [84]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10632/shard-iclb2/igt@kms_psr2_su@frontbuffer.html
   [85]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21148/shard-iclb3/igt@kms_psr2_su@frontbuffer.html

  * igt@kms_psr@psr2_cursor_mmap_cpu:
    - shard-tglb:         NOTRUN -> [FAIL][86] ([i915#132] / [i915#3467])
   [86]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21148/shard-tglb3/igt@kms_psr@psr2_cursor_mmap_cpu.html

  * igt@kms_psr@psr2_sprite_mmap_gtt:
    - shard-iclb:         [PASS][87] -> [SKIP][88] ([fdo#109441]) +1 similar issue
   [87]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10632/shard-iclb2/igt@kms_psr@psr2_sprite_mmap_gtt.html
   [88]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21148/shard-iclb6/igt@kms_psr@psr2_sprite_mmap_gtt.html

  * igt@kms_vblank@pipe-d-ts-continuation-suspend:
    - shard-tglb:         [PASS][89] -> [INCOMPLETE][90] ([i915#3896])
   [89]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10632/shard-tglb6/igt@kms_vblank@pipe-d-ts-continuation-suspend.html
   [90]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21148/shard-tglb7/igt@kms_vblank@pipe-d-ts-continuation-suspend.html

  * igt@kms_writeback@writeback-check-output:
    - shard-apl:          NOTRUN -> [SKIP][91] ([fdo#109271] / [i915#2437])
   [91]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21148/shard-apl3/igt@kms_writeback@writeback-check-output.html

  * igt@nouveau_crc@pipe-a-ctx-flip-skip-current-frame:
    - shard-tglb:         NOTRUN -> [SKIP][92] ([i915#2530]) +1 similar issue
   [92]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21148/shard-tglb1/igt@nouveau_crc@pipe-a-ctx-flip-skip-current-frame.html

  * igt@prime_nv_api@i915_nv_reimport_twice_check_flink_name:
    - shard-apl:          NOTRUN -> [SKIP][93] ([fdo#109271]) +234 similar issues
   [93]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21148/shard-apl7/igt@prime_nv_api@i915_nv_reimport_twice_check_flink_name.html

  * igt@prime_nv_api@i915_self_import:
    - shard-skl:          NOTRUN -> [SKIP][94] ([fdo#109271]) +59 similar issues
   [94]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21148/shard-skl3/igt@prime_nv_api@i915_self_import.html

  * igt@prime_nv_pcopy@test3_2:
    - shard-iclb:         NOTRUN -> [SKIP][95] ([fdo#109291]) +1 similar issue
   [95]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21148/shard-iclb8/igt@prime_nv_pcopy@test3_2.html

  * igt@prime_nv_test@nv_write_i915_gtt_mmap_read:
    - shard-tglb:         NOTRUN -> [SKIP][96] ([fdo#109291]) +1 similar issue
   [96]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21148/shard-tglb3/igt@prime_nv_test@nv_write_i915_gtt_mmap_read.html

  * igt@sysfs_clients@create:
    - shard-skl:          NOTRUN -> [SKIP][97] ([fdo#109271] / [i915#2994])
   [97]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21148/shard-skl9/igt@sysfs_clients@create.html

  * igt@sysfs_clients@fair-7:
    - shard-apl:          NOTRUN -> [SKIP][98] ([fdo#109271] / [i915#2994]) +3 similar issues
   [98]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21148/shard-apl6/igt@sysfs_clients@fair-7.html
    - shard-kbl:          NOTRUN -> [SKIP][99] ([fdo#109271] / [i915#2994]) +1 similar issue
   [99]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21148/shard-kbl1/igt@sysfs_clients@fair-7.html

  
#### Possible fixes ####

  * igt@gem_exec_fair@basic-deadline:
    - shard-tglb:         [FAIL][100] ([i915#2846]) -> [PASS][101]
   [100]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10632/shard-tglb5/igt@gem_exec_fair@basic-deadline.html
   [101]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21148/shard-tglb1/igt@gem_exec_fair@basic-deadline.html
    - shard-glk:          [FAIL][102] ([i915#2846]) -> [PASS][103]
   [102]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10632/shard-glk2/igt@gem_exec_fair@basic-deadline.html
   [103]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21148/shard-glk3/igt@gem_exec_fair@basic-deadline.html

  * igt@gem_exec_fair@basic-none-share@rcs0:
    - shard-iclb:         [FAIL][104] ([i915#2842]) -> [PASS][105]
   [104]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10632/shard-iclb1/igt@gem_exec_fair@basic-none-share@rcs0.html
   [105]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21148/shard-iclb3/igt@gem_exec_fair@basic-none-share@rcs0.html

  * igt@gem_exec_fair@basic-pace@vecs0:
    - shard-kbl:          [FAIL][106] ([i915#2842]) -> [PASS][107]
   [106]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10632/shard-kbl7/igt@gem_exec_fair@basic-pace@vecs0.html
   [107]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21148/shard-kbl7/igt@gem_exec_fair@basic-pace@vecs0.html

  * igt@gem_exec_fair@basic-throttle@rcs0:
    - shard-tglb:         [FAIL][108] ([i915#2842]) -> [PASS][109]
   [108]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10632/shard-tglb1/igt@gem_exec_fair@basic-throttle@rcs0.html
   [109]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21148/shard-tglb7/igt@gem_exec_fair@basic-throttle@rcs0.html

  * igt@gem_exec_suspend@basic-s3:
    - shard-kbl:          [INCOMPLETE][110] ([i915#155] / [i915#4173]) -> [PASS][111]
   [110]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10632/shard-kbl3/igt@gem_exec_suspend@basic-s3.html
   [111]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21148/shard-kbl6/igt@gem_exec_suspend@basic-s3.html

  * igt@gen9_exec_parse@allowed-all:
    - shard-kbl:          [DMESG-WARN][112] ([i915#1436] / [i915#716]) -> [PASS][113]
   [112]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10632/shard-kbl7/igt@gen9_exec_parse@allowed-all.html
   [113]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21148/shard-kbl2/igt@gen9_exec_parse@allowed-all.html

  * igt@i915_pm_rpm@system-suspend-execbuf:
    - shard-tglb:         [INCOMPLETE][114] ([i915#2411] / [i915#456] / [i915#750]) -> [PASS][115]
   [114]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10632/shard-tglb7/igt@i915_pm_rpm@system-suspend-execbuf.html
   [115]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21148/shard-tglb3/igt@i915_pm_rpm@system-suspend-execbuf.html

  * igt@kms_cursor_crc@pipe-a-cursor-suspend:
    - shard-tglb:         [INCOMPLETE][116] ([i915#2411] / [i915#2828] / [i915#456]) -> [PASS][117]
   [116]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10632/shard-tglb7/igt@kms_cursor_crc@pipe-a-cursor-suspend.html
   [117]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21148/shard-tglb1/igt@kms_cursor_crc@pipe-a-cursor-suspend.html

  * igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions:
    - shard-skl:          [FAIL][118] ([i915#2346]) -> [PASS][119]
   [118]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10632/shard-skl2/igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions.html
   [119]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21148/shard-skl1/igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions.html

  * igt@kms_flip_scaled_crc@flip-64bpp-ytile-to-32bpp-ytile:
    - shard-iclb:         [SKIP][120] ([i915#3701]) -> [PASS][121] +1 similar issue
   [120]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10632/shard-iclb2/igt@kms_flip_scaled_crc@flip-64bpp-ytile-to-32bpp-ytile.html
   [121]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21148/shard-iclb6/igt@kms_flip_scaled_crc@flip-64bpp-ytile-to-32bpp-ytile.html

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-mmap-wc:
    - shard-glk:          [FAIL][122] ([i915#2546]) -> [PASS][123]
   [122]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10632/shard-glk8/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-mmap-wc.html
   [123]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21148/shard-glk4/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-mmap-wc.html

  * igt@kms_hdr@bpc-switch:
    - shard-skl:          [FAIL][124] ([i915#1188]) -> [PASS][125]
   [124]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10632/shard-skl1/igt@kms_hdr@bpc-switch.html
   [125]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21148/shard-skl5/igt@kms_hdr@bpc-switch.html

  * igt@kms_plane@plane-panning-bottom-right-suspend@pipe-b-planes:
    - shard-kbl:          [DMESG-WARN][126] ([i915#180]) -> [PASS][127] +5 similar issues
   [126]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10632/shard-kbl6/igt@kms_plane@plane-panning-bottom-right-suspend@pipe-b-planes.html
   [127]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21148/shard-kbl2/igt@kms_plane@plane-panning-bottom-right-suspend@pipe-b-planes.html

  * igt@kms_plane_alpha_blend@pipe-c-constant-alpha-min:
    - shard-skl:          [FAIL][128] ([fdo#108145] / [i915#265]) -> [PASS][129]
   [128]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10632/shard-skl1/igt@kms_plane_alpha_blend@pipe-c-constant-alpha-min.html
   [129]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21148/shard-skl2/igt@kms_plane_alpha_blend@pipe-c-constant-alpha-min.html

  * igt@kms_psr@psr2_cursor_plane_move:
    - shard-iclb:         [SKIP][130] ([fdo#109441]) -> [PASS][131] +1 similar issue
   [130]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10632/shard-iclb4/igt@kms_psr@psr2_cursor_plane_move.html
   [131]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21148/shard-iclb2/igt@kms_psr@psr2_cursor_plane_move.html

  * igt@perf@polling-parameterized:
    - shard-glk:          [FAIL][132] ([i915#1542]) -> [PASS][133]
   [132]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10632/shard-glk1/igt@perf@polling-parameterized.html
   [133]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21148/shard-glk7/igt@perf@polling-parameterized.html

  * igt@syncobj_wait@wait-all-delayed-signal:
    - shard-glk:          [DMESG-WARN][134] ([i915#118] / [i915#95]) -> [PASS][135] +2 similar issues
   [134]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10632/shard-glk5/igt@syncobj_wait@wait-all-delayed-signal.html
   [135]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21148/shard-glk9/igt@syncobj_wait@wait-all-delayed-signal.html

  
#### Warnings ####

  * igt@gem_exec_fair@basic-throttle@rcs0:
    - shard-iclb:         [FAIL][136] ([i915#2849]) -> [FAIL][137] ([i915#2842])
   [136]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_10632/shard-iclb2/igt

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_21148/index.html

[-- Attachment #2: Type: text/html, Size: 33690 bytes --]

Re: [Intel-gfx] [PATCH 1/7] drm/i915/display: Wait PSR2 get out of deep sleep to update pipe

[Ville Syrjälä]

On Thu, Sep 23, 2021 at 12:46:11PM -0700, José Roberto de Souza wrote:
> Alderlake-P was getting 'max time under evasion' messages when PSR2
> was enabled, this is due PIPE_SCANLINE/PIPEDSL returning 0 over a
> period of time longer than VBLANK_EVASION_TIME_US.
> 
> For PSR1 we had the same issue so intel_psr_wait_for_idle() was
> implemented to wait for PSR1 to get into idle state but nothing was
> done for PSR2.
> 
> For PSR2 we can't only wait for idle state as PSR2 tends to keep
> into sleep state that means it is ready to send selective updates.
> 
> To do so it was necessary to add intel_wait_for_condition(), this
> takes as parameter a function that will return true when the desidered
> condition is meet.
> 
> Cc: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> ---
>  .../drm/i915/display/intel_display_debugfs.c  |  3 +-
>  drivers/gpu/drm/i915/display/intel_psr.c      | 64 ++++++++++++-------
>  drivers/gpu/drm/i915/i915_reg.h               |  5 +-
>  drivers/gpu/drm/i915/intel_uncore.c           | 47 ++++++++++++++
>  drivers/gpu/drm/i915/intel_uncore.h           |  7 ++
>  5 files changed, 100 insertions(+), 26 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display_debugfs.c b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> index 68f4ba8c46e75..662596adb1da6 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> +++ b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> @@ -303,8 +303,7 @@ psr_source_status(struct intel_dp *intel_dp, struct seq_file *m)
>  		};
>  		val = intel_de_read(dev_priv,
>  				    EDP_PSR2_STATUS(intel_dp->psr.transcoder));
> -		status_val = (val & EDP_PSR2_STATUS_STATE_MASK) >>
> -			      EDP_PSR2_STATUS_STATE_SHIFT;
> +		status_val = REG_FIELD_GET(EDP_PSR2_STATUS_STATE_MASK, val);
>  		if (status_val < ARRAY_SIZE(live_status))
>  			status = live_status[status_val];
>  	} else {
> diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c
> index 19a96d3c4acf4..a2e4ef42be60a 100644
> --- a/drivers/gpu/drm/i915/display/intel_psr.c
> +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> @@ -1796,15 +1796,33 @@ void intel_psr_post_plane_update(const struct intel_atomic_state *state)
>  		_intel_psr_post_plane_update(state, crtc_state);
>  }
>  
> -/**
> - * psr_wait_for_idle - wait for PSR1 to idle
> - * @intel_dp: Intel DP
> - * @out_value: PSR status in case of failure
> - *
> - * Returns: 0 on success or -ETIMEOUT if PSR status does not idle.
> - *
> - */
> -static int psr_wait_for_idle(struct intel_dp *intel_dp, u32 *out_value)
> +static bool _is_psr2_read_for_pipe_update(void *data)
> +{
> +	struct intel_dp *intel_dp = data;
> +	struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
> +	u32 val;
> +
> +	val = intel_uncore_read_fw(&dev_priv->uncore,
> +				   EDP_PSR2_STATUS(intel_dp->psr.transcoder));
> +	val &= EDP_PSR2_STATUS_STATE_MASK;
> +
> +	return val == EDP_PSR2_STATUS_STATE_SLEEP || val == EDP_PSR2_STATUS_STATE_IDLE;
> +}
> +
> +static int _psr2_ready_for_pipe_update_locked(struct intel_dp *intel_dp)
> +{
> +	struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
> +	unsigned int fw;
> +
> +	fw = intel_uncore_forcewake_for_reg(&dev_priv->uncore,
> +					    EDP_PSR2_STATUS(intel_dp->psr.transcoder),
> +					    FW_REG_READ);
> +	return intel_wait_for_condition(&dev_priv->uncore,
> +					_is_psr2_read_for_pipe_update,
> +					intel_dp, fw, 50);
> +}
> +
> +static int _psr1_ready_for_pipe_update_locked(struct intel_dp *intel_dp)
>  {
>  	struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
>  
> @@ -1814,15 +1832,13 @@ static int psr_wait_for_idle(struct intel_dp *intel_dp, u32 *out_value)
>  	 * exit training time + 1.5 ms of aux channel handshake. 50 ms is
>  	 * defensive enough to cover everything.
>  	 */
> -	return __intel_wait_for_register(&dev_priv->uncore,
> -					 EDP_PSR_STATUS(intel_dp->psr.transcoder),
> -					 EDP_PSR_STATUS_STATE_MASK,
> -					 EDP_PSR_STATUS_STATE_IDLE, 2, 50,
> -					 out_value);
> +	return intel_de_wait_for_clear(dev_priv,
> +				       EDP_PSR_STATUS(intel_dp->psr.transcoder),
> +				       EDP_PSR_STATUS_STATE_MASK, 50);
>  }
>  
>  /**
> - * intel_psr_wait_for_idle - wait for PSR1 to idle
> + * intel_psr_wait_for_idle - wait for PSR be ready for a pipe update
>   * @new_crtc_state: new CRTC state
>   *
>   * This function is expected to be called from pipe_update_start() where it is
> @@ -1839,19 +1855,23 @@ void intel_psr_wait_for_idle(const struct intel_crtc_state *new_crtc_state)
>  	for_each_intel_encoder_mask_with_psr(&dev_priv->drm, encoder,
>  					     new_crtc_state->uapi.encoder_mask) {
>  		struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
> -		u32 psr_status;
> +		int ret;
>  
>  		mutex_lock(&intel_dp->psr.lock);
> -		if (!intel_dp->psr.enabled || intel_dp->psr.psr2_enabled) {
> +
> +		if (!intel_dp->psr.enabled) {
>  			mutex_unlock(&intel_dp->psr.lock);
>  			continue;
>  		}
>  
> -		/* when the PSR1 is enabled */
> -		if (psr_wait_for_idle(intel_dp, &psr_status))
> -			drm_err(&dev_priv->drm,
> -				"PSR idle timed out 0x%x, atomic update may fail\n",
> -				psr_status);
> +		if (intel_dp->psr.psr2_enabled)
> +			ret = _psr2_ready_for_pipe_update_locked(intel_dp);
> +		else
> +			ret = _psr1_ready_for_pipe_update_locked(intel_dp);
> +
> +		if (ret)
> +			drm_err(&dev_priv->drm, "PSR wait timed out, atomic update may fail\n");
> +
>  		mutex_unlock(&intel_dp->psr.lock);
>  	}
>  }
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index cad84c3b864bf..a827f5bf973cb 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -4698,8 +4698,9 @@ enum {
>  #define _PSR2_STATUS_A			0x60940
>  #define _PSR2_STATUS_EDP		0x6f940
>  #define EDP_PSR2_STATUS(tran)		_MMIO_TRANS2(tran, _PSR2_STATUS_A)
> -#define EDP_PSR2_STATUS_STATE_MASK     (0xf << 28)
> -#define EDP_PSR2_STATUS_STATE_SHIFT    28
> +#define EDP_PSR2_STATUS_STATE_MASK     REG_GENMASK(31, 28)
> +#define EDP_PSR2_STATUS_STATE_SLEEP    REG_FIELD_PREP(EDP_PSR2_STATUS_STATE_MASK, 0x3)
> +#define EDP_PSR2_STATUS_STATE_IDLE     REG_FIELD_PREP(EDP_PSR2_STATUS_STATE_MASK, 0x0)
>  
>  #define _PSR2_SU_STATUS_A		0x60914
>  #define _PSR2_SU_STATUS_EDP		0x6f914
> diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
> index 678a99de07fee..1b3ea7318c2d5 100644
> --- a/drivers/gpu/drm/i915/intel_uncore.c
> +++ b/drivers/gpu/drm/i915/intel_uncore.c
> @@ -2383,6 +2383,28 @@ int __intel_wait_for_register_fw(struct intel_uncore *uncore,
>  #undef done
>  }
>  
> +static int __intel_wait_for_condition_fw(bool (*func)(void *data), void *data,
> +					 unsigned int fast_timeout_us,
> +					 unsigned int slow_timeout_ms)
> +{
> +#define done (func(data))
> +	int ret;
> +
> +	/* Catch any overuse of this function */
> +	might_sleep_if(slow_timeout_ms);
> +	GEM_BUG_ON(fast_timeout_us > 20000);
> +	GEM_BUG_ON(!fast_timeout_us && !slow_timeout_ms);
> +
> +	ret = -ETIMEDOUT;
> +	if (fast_timeout_us && fast_timeout_us <= 20000)
> +		ret = _wait_for_atomic(done, fast_timeout_us, 0);
> +	if (ret && slow_timeout_ms)
> +		ret = wait_for(done, slow_timeout_ms);

Is there a particular reason for these complicated wrappers
instead of just using wait_for() directly?

> +
> +	return ret;
> +#undef done
> +}
> +
>  /**
>   * __intel_wait_for_register - wait until register matches expected state
>   * @uncore: the struct intel_uncore
> @@ -2442,6 +2464,31 @@ int __intel_wait_for_register(struct intel_uncore *uncore,
>  	return ret;
>  }
>  
> +int intel_wait_for_condition(struct intel_uncore *uncore,
> +			     bool (*func)(void *data),
> +			     void *data,
> +			     unsigned int fw,
> +			     unsigned int slow_timeout_ms)
> +{
> +	unsigned int fast_timeout_us = 2;
> +	int ret;
> +
> +	might_sleep_if(slow_timeout_ms);
> +
> +	spin_lock_irq(&uncore->lock);
> +	intel_uncore_forcewake_get__locked(uncore, fw);
> +
> +	ret = __intel_wait_for_condition_fw(func, data, fast_timeout_us, 0);
> +
> +	intel_uncore_forcewake_put__locked(uncore, fw);
> +	spin_unlock_irq(&uncore->lock);
> +
> +	if (ret && slow_timeout_ms)
> +		ret = __wait_for(, (func(data)), slow_timeout_ms * 1000, 10, 1000);
> +
> +	return ret;
> +}
> +
>  bool intel_uncore_unclaimed_mmio(struct intel_uncore *uncore)
>  {
>  	bool ret;
> diff --git a/drivers/gpu/drm/i915/intel_uncore.h b/drivers/gpu/drm/i915/intel_uncore.h
> index 2f31c50eeae24..d6455824cbc56 100644
> --- a/drivers/gpu/drm/i915/intel_uncore.h
> +++ b/drivers/gpu/drm/i915/intel_uncore.h
> @@ -282,6 +282,7 @@ int __intel_wait_for_register_fw(struct intel_uncore *uncore,
>  				 unsigned int fast_timeout_us,
>  				 unsigned int slow_timeout_ms,
>  				 u32 *out_value);
> +
>  static inline int
>  intel_wait_for_register_fw(struct intel_uncore *uncore,
>  			   i915_reg_t reg,
> @@ -293,6 +294,12 @@ intel_wait_for_register_fw(struct intel_uncore *uncore,
>  					    2, timeout_ms, NULL);
>  }
>  
> +int intel_wait_for_condition(struct intel_uncore *uncore,
> +			     bool (*func)(void *data),
> +			     void *data,
> +			     unsigned int fw,
> +			     unsigned int timeout_ms);
> +
>  /* register access functions */
>  #define __raw_read(x__, s__) \
>  static inline u##x__ __raw_uncore_read##x__(const struct intel_uncore *uncore, \
> -- 
> 2.33.0

-- 
Ville Syrjälä
Intel

Re: [Intel-gfx] [PATCH 1/7] drm/i915/display: Wait PSR2 get out of deep sleep to update pipe

[Souza, Jose]

On Fri, 2021-09-24 at 17:35 +0300, Ville Syrjälä wrote:
> On Thu, Sep 23, 2021 at 12:46:11PM -0700, José Roberto de Souza wrote:
> > Alderlake-P was getting 'max time under evasion' messages when PSR2
> > was enabled, this is due PIPE_SCANLINE/PIPEDSL returning 0 over a
> > period of time longer than VBLANK_EVASION_TIME_US.
> > 
> > For PSR1 we had the same issue so intel_psr_wait_for_idle() was
> > implemented to wait for PSR1 to get into idle state but nothing was
> > done for PSR2.
> > 
> > For PSR2 we can't only wait for idle state as PSR2 tends to keep
> > into sleep state that means it is ready to send selective updates.
> > 
> > To do so it was necessary to add intel_wait_for_condition(), this
> > takes as parameter a function that will return true when the desidered
> > condition is meet.
> > 
> > Cc: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>
> > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > ---
> >  .../drm/i915/display/intel_display_debugfs.c  |  3 +-
> >  drivers/gpu/drm/i915/display/intel_psr.c      | 64 ++++++++++++-------
> >  drivers/gpu/drm/i915/i915_reg.h               |  5 +-
> >  drivers/gpu/drm/i915/intel_uncore.c           | 47 ++++++++++++++
> >  drivers/gpu/drm/i915/intel_uncore.h           |  7 ++
> >  5 files changed, 100 insertions(+), 26 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_display_debugfs.c b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> > index 68f4ba8c46e75..662596adb1da6 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> > +++ b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> > @@ -303,8 +303,7 @@ psr_source_status(struct intel_dp *intel_dp, struct seq_file *m)
> >  		};
> >  		val = intel_de_read(dev_priv,
> >  				    EDP_PSR2_STATUS(intel_dp->psr.transcoder));
> > -		status_val = (val & EDP_PSR2_STATUS_STATE_MASK) >>
> > -			      EDP_PSR2_STATUS_STATE_SHIFT;
> > +		status_val = REG_FIELD_GET(EDP_PSR2_STATUS_STATE_MASK, val);
> >  		if (status_val < ARRAY_SIZE(live_status))
> >  			status = live_status[status_val];
> >  	} else {
> > diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c
> > index 19a96d3c4acf4..a2e4ef42be60a 100644
> > --- a/drivers/gpu/drm/i915/display/intel_psr.c
> > +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> > @@ -1796,15 +1796,33 @@ void intel_psr_post_plane_update(const struct intel_atomic_state *state)
> >  		_intel_psr_post_plane_update(state, crtc_state);
> >  }
> >  
> > -/**
> > - * psr_wait_for_idle - wait for PSR1 to idle
> > - * @intel_dp: Intel DP
> > - * @out_value: PSR status in case of failure
> > - *
> > - * Returns: 0 on success or -ETIMEOUT if PSR status does not idle.
> > - *
> > - */
> > -static int psr_wait_for_idle(struct intel_dp *intel_dp, u32 *out_value)
> > +static bool _is_psr2_read_for_pipe_update(void *data)
> > +{
> > +	struct intel_dp *intel_dp = data;
> > +	struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
> > +	u32 val;
> > +
> > +	val = intel_uncore_read_fw(&dev_priv->uncore,
> > +				   EDP_PSR2_STATUS(intel_dp->psr.transcoder));
> > +	val &= EDP_PSR2_STATUS_STATE_MASK;
> > +
> > +	return val == EDP_PSR2_STATUS_STATE_SLEEP || val == EDP_PSR2_STATUS_STATE_IDLE;
> > +}
> > +
> > +static int _psr2_ready_for_pipe_update_locked(struct intel_dp *intel_dp)
> > +{
> > +	struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
> > +	unsigned int fw;
> > +
> > +	fw = intel_uncore_forcewake_for_reg(&dev_priv->uncore,
> > +					    EDP_PSR2_STATUS(intel_dp->psr.transcoder),
> > +					    FW_REG_READ);
> > +	return intel_wait_for_condition(&dev_priv->uncore,
> > +					_is_psr2_read_for_pipe_update,
> > +					intel_dp, fw, 50);
> > +}
> > +
> > +static int _psr1_ready_for_pipe_update_locked(struct intel_dp *intel_dp)
> >  {
> >  	struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
> >  
> > @@ -1814,15 +1832,13 @@ static int psr_wait_for_idle(struct intel_dp *intel_dp, u32 *out_value)
> >  	 * exit training time + 1.5 ms of aux channel handshake. 50 ms is
> >  	 * defensive enough to cover everything.
> >  	 */
> > -	return __intel_wait_for_register(&dev_priv->uncore,
> > -					 EDP_PSR_STATUS(intel_dp->psr.transcoder),
> > -					 EDP_PSR_STATUS_STATE_MASK,
> > -					 EDP_PSR_STATUS_STATE_IDLE, 2, 50,
> > -					 out_value);
> > +	return intel_de_wait_for_clear(dev_priv,
> > +				       EDP_PSR_STATUS(intel_dp->psr.transcoder),
> > +				       EDP_PSR_STATUS_STATE_MASK, 50);
> >  }
> >  
> >  /**
> > - * intel_psr_wait_for_idle - wait for PSR1 to idle
> > + * intel_psr_wait_for_idle - wait for PSR be ready for a pipe update
> >   * @new_crtc_state: new CRTC state
> >   *
> >   * This function is expected to be called from pipe_update_start() where it is
> > @@ -1839,19 +1855,23 @@ void intel_psr_wait_for_idle(const struct intel_crtc_state *new_crtc_state)
> >  	for_each_intel_encoder_mask_with_psr(&dev_priv->drm, encoder,
> >  					     new_crtc_state->uapi.encoder_mask) {
> >  		struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
> > -		u32 psr_status;
> > +		int ret;
> >  
> >  		mutex_lock(&intel_dp->psr.lock);
> > -		if (!intel_dp->psr.enabled || intel_dp->psr.psr2_enabled) {
> > +
> > +		if (!intel_dp->psr.enabled) {
> >  			mutex_unlock(&intel_dp->psr.lock);
> >  			continue;
> >  		}
> >  
> > -		/* when the PSR1 is enabled */
> > -		if (psr_wait_for_idle(intel_dp, &psr_status))
> > -			drm_err(&dev_priv->drm,
> > -				"PSR idle timed out 0x%x, atomic update may fail\n",
> > -				psr_status);
> > +		if (intel_dp->psr.psr2_enabled)
> > +			ret = _psr2_ready_for_pipe_update_locked(intel_dp);
> > +		else
> > +			ret = _psr1_ready_for_pipe_update_locked(intel_dp);
> > +
> > +		if (ret)
> > +			drm_err(&dev_priv->drm, "PSR wait timed out, atomic update may fail\n");
> > +
> >  		mutex_unlock(&intel_dp->psr.lock);
> >  	}
> >  }
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> > index cad84c3b864bf..a827f5bf973cb 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -4698,8 +4698,9 @@ enum {
> >  #define _PSR2_STATUS_A			0x60940
> >  #define _PSR2_STATUS_EDP		0x6f940
> >  #define EDP_PSR2_STATUS(tran)		_MMIO_TRANS2(tran, _PSR2_STATUS_A)
> > -#define EDP_PSR2_STATUS_STATE_MASK     (0xf << 28)
> > -#define EDP_PSR2_STATUS_STATE_SHIFT    28
> > +#define EDP_PSR2_STATUS_STATE_MASK     REG_GENMASK(31, 28)
> > +#define EDP_PSR2_STATUS_STATE_SLEEP    REG_FIELD_PREP(EDP_PSR2_STATUS_STATE_MASK, 0x3)
> > +#define EDP_PSR2_STATUS_STATE_IDLE     REG_FIELD_PREP(EDP_PSR2_STATUS_STATE_MASK, 0x0)
> >  
> >  #define _PSR2_SU_STATUS_A		0x60914
> >  #define _PSR2_SU_STATUS_EDP		0x6f914
> > diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
> > index 678a99de07fee..1b3ea7318c2d5 100644
> > --- a/drivers/gpu/drm/i915/intel_uncore.c
> > +++ b/drivers/gpu/drm/i915/intel_uncore.c
> > @@ -2383,6 +2383,28 @@ int __intel_wait_for_register_fw(struct intel_uncore *uncore,
> >  #undef done
> >  }
> >  
> > +static int __intel_wait_for_condition_fw(bool (*func)(void *data), void *data,
> > +					 unsigned int fast_timeout_us,
> > +					 unsigned int slow_timeout_ms)
> > +{
> > +#define done (func(data))
> > +	int ret;
> > +
> > +	/* Catch any overuse of this function */
> > +	might_sleep_if(slow_timeout_ms);
> > +	GEM_BUG_ON(fast_timeout_us > 20000);
> > +	GEM_BUG_ON(!fast_timeout_us && !slow_timeout_ms);
> > +
> > +	ret = -ETIMEDOUT;
> > +	if (fast_timeout_us && fast_timeout_us <= 20000)
> > +		ret = _wait_for_atomic(done, fast_timeout_us, 0);
> > +	if (ret && slow_timeout_ms)
> > +		ret = wait_for(done, slow_timeout_ms);
> 
> Is there a particular reason for these complicated wrappers
> instead of just using wait_for() directly?

Just replicated what __intel_wait_for_register_fw() do.
I guess the first one sleep less for cases that condition is meet in a few usecs.

> 
> > +
> > +	return ret;
> > +#undef done
> > +}
> > +
> >  /**
> >   * __intel_wait_for_register - wait until register matches expected state
> >   * @uncore: the struct intel_uncore
> > @@ -2442,6 +2464,31 @@ int __intel_wait_for_register(struct intel_uncore *uncore,
> >  	return ret;
> >  }
> >  
> > +int intel_wait_for_condition(struct intel_uncore *uncore,
> > +			     bool (*func)(void *data),
> > +			     void *data,
> > +			     unsigned int fw,
> > +			     unsigned int slow_timeout_ms)
> > +{
> > +	unsigned int fast_timeout_us = 2;
> > +	int ret;
> > +
> > +	might_sleep_if(slow_timeout_ms);
> > +
> > +	spin_lock_irq(&uncore->lock);
> > +	intel_uncore_forcewake_get__locked(uncore, fw);
> > +
> > +	ret = __intel_wait_for_condition_fw(func, data, fast_timeout_us, 0);
> > +
> > +	intel_uncore_forcewake_put__locked(uncore, fw);
> > +	spin_unlock_irq(&uncore->lock);
> > +
> > +	if (ret && slow_timeout_ms)
> > +		ret = __wait_for(, (func(data)), slow_timeout_ms * 1000, 10, 1000);
> > +
> > +	return ret;
> > +}
> > +
> >  bool intel_uncore_unclaimed_mmio(struct intel_uncore *uncore)
> >  {
> >  	bool ret;
> > diff --git a/drivers/gpu/drm/i915/intel_uncore.h b/drivers/gpu/drm/i915/intel_uncore.h
> > index 2f31c50eeae24..d6455824cbc56 100644
> > --- a/drivers/gpu/drm/i915/intel_uncore.h
> > +++ b/drivers/gpu/drm/i915/intel_uncore.h
> > @@ -282,6 +282,7 @@ int __intel_wait_for_register_fw(struct intel_uncore *uncore,
> >  				 unsigned int fast_timeout_us,
> >  				 unsigned int slow_timeout_ms,
> >  				 u32 *out_value);
> > +
> >  static inline int
> >  intel_wait_for_register_fw(struct intel_uncore *uncore,
> >  			   i915_reg_t reg,
> > @@ -293,6 +294,12 @@ intel_wait_for_register_fw(struct intel_uncore *uncore,
> >  					    2, timeout_ms, NULL);
> >  }
> >  
> > +int intel_wait_for_condition(struct intel_uncore *uncore,
> > +			     bool (*func)(void *data),
> > +			     void *data,
> > +			     unsigned int fw,
> > +			     unsigned int timeout_ms);
> > +
> >  /* register access functions */
> >  #define __raw_read(x__, s__) \
> >  static inline u##x__ __raw_uncore_read##x__(const struct intel_uncore *uncore, \
> > -- 
> > 2.33.0
> 

Re: [Intel-gfx] [PATCH 1/7] drm/i915/display: Wait PSR2 get out of deep sleep to update pipe

[Ville Syrjälä]

On Fri, Sep 24, 2021 at 05:10:44PM +0000, Souza, Jose wrote:
> On Fri, 2021-09-24 at 17:35 +0300, Ville Syrjälä wrote:
> > On Thu, Sep 23, 2021 at 12:46:11PM -0700, José Roberto de Souza wrote:
> > > Alderlake-P was getting 'max time under evasion' messages when PSR2
> > > was enabled, this is due PIPE_SCANLINE/PIPEDSL returning 0 over a
> > > period of time longer than VBLANK_EVASION_TIME_US.
> > > 
> > > For PSR1 we had the same issue so intel_psr_wait_for_idle() was
> > > implemented to wait for PSR1 to get into idle state but nothing was
> > > done for PSR2.
> > > 
> > > For PSR2 we can't only wait for idle state as PSR2 tends to keep
> > > into sleep state that means it is ready to send selective updates.
> > > 
> > > To do so it was necessary to add intel_wait_for_condition(), this
> > > takes as parameter a function that will return true when the desidered
> > > condition is meet.
> > > 
> > > Cc: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>
> > > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > > ---
> > >  .../drm/i915/display/intel_display_debugfs.c  |  3 +-
> > >  drivers/gpu/drm/i915/display/intel_psr.c      | 64 ++++++++++++-------
> > >  drivers/gpu/drm/i915/i915_reg.h               |  5 +-
> > >  drivers/gpu/drm/i915/intel_uncore.c           | 47 ++++++++++++++
> > >  drivers/gpu/drm/i915/intel_uncore.h           |  7 ++
> > >  5 files changed, 100 insertions(+), 26 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/display/intel_display_debugfs.c b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> > > index 68f4ba8c46e75..662596adb1da6 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_display_debugfs.c
> > > @@ -303,8 +303,7 @@ psr_source_status(struct intel_dp *intel_dp, struct seq_file *m)
> > >  		};
> > >  		val = intel_de_read(dev_priv,
> > >  				    EDP_PSR2_STATUS(intel_dp->psr.transcoder));
> > > -		status_val = (val & EDP_PSR2_STATUS_STATE_MASK) >>
> > > -			      EDP_PSR2_STATUS_STATE_SHIFT;
> > > +		status_val = REG_FIELD_GET(EDP_PSR2_STATUS_STATE_MASK, val);
> > >  		if (status_val < ARRAY_SIZE(live_status))
> > >  			status = live_status[status_val];
> > >  	} else {
> > > diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c
> > > index 19a96d3c4acf4..a2e4ef42be60a 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_psr.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> > > @@ -1796,15 +1796,33 @@ void intel_psr_post_plane_update(const struct intel_atomic_state *state)
> > >  		_intel_psr_post_plane_update(state, crtc_state);
> > >  }
> > >  
> > > -/**
> > > - * psr_wait_for_idle - wait for PSR1 to idle
> > > - * @intel_dp: Intel DP
> > > - * @out_value: PSR status in case of failure
> > > - *
> > > - * Returns: 0 on success or -ETIMEOUT if PSR status does not idle.
> > > - *
> > > - */
> > > -static int psr_wait_for_idle(struct intel_dp *intel_dp, u32 *out_value)
> > > +static bool _is_psr2_read_for_pipe_update(void *data)
> > > +{
> > > +	struct intel_dp *intel_dp = data;
> > > +	struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
> > > +	u32 val;
> > > +
> > > +	val = intel_uncore_read_fw(&dev_priv->uncore,
> > > +				   EDP_PSR2_STATUS(intel_dp->psr.transcoder));
> > > +	val &= EDP_PSR2_STATUS_STATE_MASK;
> > > +
> > > +	return val == EDP_PSR2_STATUS_STATE_SLEEP || val == EDP_PSR2_STATUS_STATE_IDLE;
> > > +}
> > > +
> > > +static int _psr2_ready_for_pipe_update_locked(struct intel_dp *intel_dp)
> > > +{
> > > +	struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
> > > +	unsigned int fw;
> > > +
> > > +	fw = intel_uncore_forcewake_for_reg(&dev_priv->uncore,
> > > +					    EDP_PSR2_STATUS(intel_dp->psr.transcoder),
> > > +					    FW_REG_READ);
> > > +	return intel_wait_for_condition(&dev_priv->uncore,
> > > +					_is_psr2_read_for_pipe_update,
> > > +					intel_dp, fw, 50);
> > > +}
> > > +
> > > +static int _psr1_ready_for_pipe_update_locked(struct intel_dp *intel_dp)
> > >  {
> > >  	struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
> > >  
> > > @@ -1814,15 +1832,13 @@ static int psr_wait_for_idle(struct intel_dp *intel_dp, u32 *out_value)
> > >  	 * exit training time + 1.5 ms of aux channel handshake. 50 ms is
> > >  	 * defensive enough to cover everything.
> > >  	 */
> > > -	return __intel_wait_for_register(&dev_priv->uncore,
> > > -					 EDP_PSR_STATUS(intel_dp->psr.transcoder),
> > > -					 EDP_PSR_STATUS_STATE_MASK,
> > > -					 EDP_PSR_STATUS_STATE_IDLE, 2, 50,
> > > -					 out_value);
> > > +	return intel_de_wait_for_clear(dev_priv,
> > > +				       EDP_PSR_STATUS(intel_dp->psr.transcoder),
> > > +				       EDP_PSR_STATUS_STATE_MASK, 50);
> > >  }
> > >  
> > >  /**
> > > - * intel_psr_wait_for_idle - wait for PSR1 to idle
> > > + * intel_psr_wait_for_idle - wait for PSR be ready for a pipe update
> > >   * @new_crtc_state: new CRTC state
> > >   *
> > >   * This function is expected to be called from pipe_update_start() where it is
> > > @@ -1839,19 +1855,23 @@ void intel_psr_wait_for_idle(const struct intel_crtc_state *new_crtc_state)
> > >  	for_each_intel_encoder_mask_with_psr(&dev_priv->drm, encoder,
> > >  					     new_crtc_state->uapi.encoder_mask) {
> > >  		struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
> > > -		u32 psr_status;
> > > +		int ret;
> > >  
> > >  		mutex_lock(&intel_dp->psr.lock);
> > > -		if (!intel_dp->psr.enabled || intel_dp->psr.psr2_enabled) {
> > > +
> > > +		if (!intel_dp->psr.enabled) {
> > >  			mutex_unlock(&intel_dp->psr.lock);
> > >  			continue;
> > >  		}
> > >  
> > > -		/* when the PSR1 is enabled */
> > > -		if (psr_wait_for_idle(intel_dp, &psr_status))
> > > -			drm_err(&dev_priv->drm,
> > > -				"PSR idle timed out 0x%x, atomic update may fail\n",
> > > -				psr_status);
> > > +		if (intel_dp->psr.psr2_enabled)
> > > +			ret = _psr2_ready_for_pipe_update_locked(intel_dp);
> > > +		else
> > > +			ret = _psr1_ready_for_pipe_update_locked(intel_dp);
> > > +
> > > +		if (ret)
> > > +			drm_err(&dev_priv->drm, "PSR wait timed out, atomic update may fail\n");
> > > +
> > >  		mutex_unlock(&intel_dp->psr.lock);
> > >  	}
> > >  }
> > > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> > > index cad84c3b864bf..a827f5bf973cb 100644
> > > --- a/drivers/gpu/drm/i915/i915_reg.h
> > > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > > @@ -4698,8 +4698,9 @@ enum {
> > >  #define _PSR2_STATUS_A			0x60940
> > >  #define _PSR2_STATUS_EDP		0x6f940
> > >  #define EDP_PSR2_STATUS(tran)		_MMIO_TRANS2(tran, _PSR2_STATUS_A)
> > > -#define EDP_PSR2_STATUS_STATE_MASK     (0xf << 28)
> > > -#define EDP_PSR2_STATUS_STATE_SHIFT    28
> > > +#define EDP_PSR2_STATUS_STATE_MASK     REG_GENMASK(31, 28)
> > > +#define EDP_PSR2_STATUS_STATE_SLEEP    REG_FIELD_PREP(EDP_PSR2_STATUS_STATE_MASK, 0x3)
> > > +#define EDP_PSR2_STATUS_STATE_IDLE     REG_FIELD_PREP(EDP_PSR2_STATUS_STATE_MASK, 0x0)
> > >  
> > >  #define _PSR2_SU_STATUS_A		0x60914
> > >  #define _PSR2_SU_STATUS_EDP		0x6f914
> > > diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
> > > index 678a99de07fee..1b3ea7318c2d5 100644
> > > --- a/drivers/gpu/drm/i915/intel_uncore.c
> > > +++ b/drivers/gpu/drm/i915/intel_uncore.c
> > > @@ -2383,6 +2383,28 @@ int __intel_wait_for_register_fw(struct intel_uncore *uncore,
> > >  #undef done
> > >  }
> > >  
> > > +static int __intel_wait_for_condition_fw(bool (*func)(void *data), void *data,
> > > +					 unsigned int fast_timeout_us,
> > > +					 unsigned int slow_timeout_ms)
> > > +{
> > > +#define done (func(data))
> > > +	int ret;
> > > +
> > > +	/* Catch any overuse of this function */
> > > +	might_sleep_if(slow_timeout_ms);
> > > +	GEM_BUG_ON(fast_timeout_us > 20000);
> > > +	GEM_BUG_ON(!fast_timeout_us && !slow_timeout_ms);
> > > +
> > > +	ret = -ETIMEDOUT;
> > > +	if (fast_timeout_us && fast_timeout_us <= 20000)
> > > +		ret = _wait_for_atomic(done, fast_timeout_us, 0);
> > > +	if (ret && slow_timeout_ms)
> > > +		ret = wait_for(done, slow_timeout_ms);
> > 
> > Is there a particular reason for these complicated wrappers
> > instead of just using wait_for() directly?
> 
> Just replicated what __intel_wait_for_register_fw() do.
> I guess the first one sleep less for cases that condition is meet in a few usecs.

wait_for() should already start with a short delay with
exponential backoff the longer it has to wait. So unless there
is a demonstrable problem with wait_for() I don't think we
should be adding even more complex wait macros.

-- 
Ville Syrjälä
Intel