All of lore.kernel.org
 help / color / mirror / Atom feed
* [Intel-gfx] [PATCH 1/2] drm/i915/display/psr: Set partial frame enable when forcing full frame fetch
@ 2022-03-24 18:13 José Roberto de Souza
  2022-03-24 18:13 ` [Intel-gfx] [PATCH 2/2] drm/i915/display/psr: Use continuos full frame to handle frontbuffer invalidations José Roberto de Souza
                   ` (4 more replies)
  0 siblings, 5 replies; 9+ messages in thread
From: José Roberto de Souza @ 2022-03-24 18:13 UTC (permalink / raw)
  To: intel-gfx

Following up what was done in commit 804f46885317 ("drm/i915/psr: Set
"SF Partial Frame Enable" also on full update") and also setting
partial frame enable when psr_force_hw_tracking_exit() is called.

Cc: Jouni Högander <jouni.hogander@intel.com>
Cc: Mika Kahola <mika.kahola@intel.com>
Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
---
 drivers/gpu/drm/i915/display/intel_psr.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c
index 80002ca6a6ebe..d87b357806c91 100644
--- a/drivers/gpu/drm/i915/display/intel_psr.c
+++ b/drivers/gpu/drm/i915/display/intel_psr.c
@@ -1457,7 +1457,8 @@ static void psr_force_hw_tracking_exit(struct intel_dp *intel_dp)
 	if (intel_dp->psr.psr2_sel_fetch_enabled)
 		intel_de_rmw(dev_priv,
 			     PSR2_MAN_TRK_CTL(intel_dp->psr.transcoder), 0,
-			     man_trk_ctl_single_full_frame_bit_get(dev_priv));
+			     man_trk_ctl_single_full_frame_bit_get(dev_priv) |
+			     man_trk_ctl_partial_frame_bit_get(dev_priv));
 
 	/*
 	 * Display WA #0884: skl+
-- 
2.35.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [Intel-gfx] [PATCH 2/2] drm/i915/display/psr: Use continuos full frame to handle frontbuffer invalidations
  2022-03-24 18:13 [Intel-gfx] [PATCH 1/2] drm/i915/display/psr: Set partial frame enable when forcing full frame fetch José Roberto de Souza
@ 2022-03-24 18:13 ` José Roberto de Souza
  2022-03-25 14:21   ` Hogander, Jouni
  2022-03-24 18:59 ` [Intel-gfx] ✗ Fi.CI.DOCS: warning for series starting with [1/2] drm/i915/display/psr: Set partial frame enable when forcing full frame fetch Patchwork
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 9+ messages in thread
From: José Roberto de Souza @ 2022-03-24 18:13 UTC (permalink / raw)
  To: intel-gfx

Instead of exit PSR when a frontbuffer invalidation happens, we can
enable the PSR2 selective fetch continuous full frame, that will keep
the panel updated like PSR was disabled but without keeping PSR active.

So as soon as the frontbuffer flush happens we can disable the
continuous full frame and start to do selective fetches much quicker
than the path that would enable PSR, that will wait a few frames
to actually activate PSR.

Also this approach has proven to fix some glitches found in Alderlake-P
when there are a lot of invalidations happening together with page
flips.

Some may ask why it is writing to CURSURFLIVE(), it is because
that is the way that hardware team provided us to poke display to
handle PSR updates, and it is being used since display 9.

Cc: Khaled Almahallawy <khaled.almahallawy@intel.com>
Cc: Shawn C Lee <shawn.c.lee@intel.com>
Cc: Jouni Högander <jouni.hogander@intel.com>
Cc: Mika Kahola <mika.kahola@intel.com>
Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
---
 drivers/gpu/drm/i915/display/intel_psr.c | 109 ++++++++++++++++++++---
 1 file changed, 95 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c
index d87b357806c91..f7b7b374374b1 100644
--- a/drivers/gpu/drm/i915/display/intel_psr.c
+++ b/drivers/gpu/drm/i915/display/intel_psr.c
@@ -1450,6 +1450,22 @@ static inline u32 man_trk_ctl_partial_frame_bit_get(struct drm_i915_private *dev
 	       PSR2_MAN_TRK_CTL_SF_PARTIAL_FRAME_UPDATE;
 }
 
+static inline u32 man_trk_ctl_continuos_full_frame(struct drm_i915_private *dev_priv)
+{
+	return IS_ALDERLAKE_P(dev_priv) ?
+	       ADLP_PSR2_MAN_TRK_CTL_SF_CONTINUOS_FULL_FRAME :
+	       PSR2_MAN_TRK_CTL_SF_CONTINUOS_FULL_FRAME;
+}
+
+static inline u32 man_trk_ctl_su_region_start_end_mask(struct drm_i915_private *dev_priv)
+{
+	if (IS_ALDERLAKE_P(dev_priv))
+		return ADLP_PSR2_MAN_TRK_CTL_SU_REGION_START_ADDR_MASK |
+		       ADLP_PSR2_MAN_TRK_CTL_SU_REGION_END_ADDR_MASK;
+	return PSR2_MAN_TRK_CTL_SU_REGION_START_ADDR_MASK |
+	       PSR2_MAN_TRK_CTL_SU_REGION_END_ADDR_MASK;
+}
+
 static void psr_force_hw_tracking_exit(struct intel_dp *intel_dp)
 {
 	struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
@@ -1546,8 +1562,9 @@ void intel_psr2_program_trans_man_trk_ctl(const struct intel_crtc_state *crtc_st
 	if (!crtc_state->enable_psr2_sel_fetch)
 		return;
 
-	intel_de_write(dev_priv, PSR2_MAN_TRK_CTL(crtc_state->cpu_transcoder),
-		       crtc_state->psr2_man_track_ctl);
+	intel_de_rmw(dev_priv, PSR2_MAN_TRK_CTL(crtc_state->cpu_transcoder),
+		     man_trk_ctl_su_region_start_end_mask(dev_priv),
+		     crtc_state->psr2_man_track_ctl);
 }
 
 static void psr2_man_trk_ctl_calc(struct intel_crtc_state *crtc_state,
@@ -2127,6 +2144,26 @@ static void intel_psr_work(struct work_struct *work)
 	mutex_unlock(&intel_dp->psr.lock);
 }
 
+static void _psr_invalidate_handle(struct intel_dp *intel_dp,
+				   unsigned int prev_busy_frontbuffer_bits)
+{
+	struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
+
+	if (intel_dp->psr.psr2_sel_fetch_enabled) {
+		u32 val = man_trk_ctl_continuos_full_frame(dev_priv) |
+			  man_trk_ctl_partial_frame_bit_get(dev_priv);
+
+		/* continuos full frame is already enabled */
+		if (prev_busy_frontbuffer_bits)
+			return;
+
+		intel_de_rmw(dev_priv, PSR2_MAN_TRK_CTL(intel_dp->psr.transcoder), 0, val);
+		intel_de_write(dev_priv, CURSURFLIVE(intel_dp->psr.pipe), 0);
+	} else {
+		intel_psr_exit(intel_dp);
+	}
+}
+
 /**
  * intel_psr_invalidate - Invalidade PSR
  * @dev_priv: i915 device
@@ -2151,6 +2188,7 @@ void intel_psr_invalidate(struct drm_i915_private *dev_priv,
 	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);
+		unsigned int prev_busy_frontbuffer_bits;
 
 		mutex_lock(&intel_dp->psr.lock);
 		if (!intel_dp->psr.enabled) {
@@ -2158,12 +2196,13 @@ void intel_psr_invalidate(struct drm_i915_private *dev_priv,
 			continue;
 		}
 
+		prev_busy_frontbuffer_bits = intel_dp->psr.busy_frontbuffer_bits;
 		pipe_frontbuffer_bits &=
 			INTEL_FRONTBUFFER_ALL_MASK(intel_dp->psr.pipe);
 		intel_dp->psr.busy_frontbuffer_bits |= pipe_frontbuffer_bits;
 
 		if (pipe_frontbuffer_bits)
-			intel_psr_exit(intel_dp);
+			_psr_invalidate_handle(intel_dp, prev_busy_frontbuffer_bits);
 
 		mutex_unlock(&intel_dp->psr.lock);
 	}
@@ -2195,6 +2234,49 @@ tgl_dc3co_flush_locked(struct intel_dp *intel_dp, unsigned int frontbuffer_bits,
 			 intel_dp->psr.dc3co_exit_delay);
 }
 
+static void _psr_flush_handle(struct intel_dp *intel_dp,
+			      unsigned int prev_busy_frontbuffer_bits)
+{
+	struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
+
+	if (intel_dp->psr.psr2_sel_fetch_enabled) {
+		/* is continuos full frame enabled? */
+		if (prev_busy_frontbuffer_bits) {
+			/* it is, can we turn it off? */
+			if (intel_dp->psr.busy_frontbuffer_bits == 0) {
+				u32 clear = man_trk_ctl_continuos_full_frame(dev_priv);
+				u32 set = man_trk_ctl_single_full_frame_bit_get(dev_priv) |
+					  man_trk_ctl_partial_frame_bit_get(dev_priv);
+
+				/*
+				 * turn continuos full frame off and do a single
+				 * full frame
+				 */
+				intel_de_rmw(dev_priv,
+					     PSR2_MAN_TRK_CTL(intel_dp->psr.transcoder),
+					     clear, set);
+				intel_de_write(dev_priv, CURSURFLIVE(intel_dp->psr.pipe), 0);
+			}
+		} else {
+			/*
+			 * continuos full frame is disabled, only a single full
+			 * frame is required
+			 */
+			psr_force_hw_tracking_exit(intel_dp);
+		}
+	} else {
+		/*
+		 * if prev_busy_frontbuffer_bits is set, it means that PSR is
+		 * inactive
+		 */
+		if (prev_busy_frontbuffer_bits == 0)
+			psr_force_hw_tracking_exit(intel_dp);
+
+		if (!intel_dp->psr.active && !intel_dp->psr.busy_frontbuffer_bits)
+			schedule_work(&intel_dp->psr.work);
+	}
+}
+
 /**
  * intel_psr_flush - Flush PSR
  * @dev_priv: i915 device
@@ -2216,6 +2298,7 @@ void intel_psr_flush(struct drm_i915_private *dev_priv,
 	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);
+		unsigned int prev_busy_frontbuffer_bits;
 
 		mutex_lock(&intel_dp->psr.lock);
 		if (!intel_dp->psr.enabled) {
@@ -2223,6 +2306,7 @@ void intel_psr_flush(struct drm_i915_private *dev_priv,
 			continue;
 		}
 
+		prev_busy_frontbuffer_bits = intel_dp->psr.busy_frontbuffer_bits;
 		pipe_frontbuffer_bits &=
 			INTEL_FRONTBUFFER_ALL_MASK(intel_dp->psr.pipe);
 		intel_dp->psr.busy_frontbuffer_bits &= ~pipe_frontbuffer_bits;
@@ -2232,25 +2316,22 @@ void intel_psr_flush(struct drm_i915_private *dev_priv,
 		 * we have to ensure that the PSR is not activated until
 		 * intel_psr_resume() is called.
 		 */
-		if (intel_dp->psr.paused) {
-			mutex_unlock(&intel_dp->psr.lock);
-			continue;
-		}
+		if (intel_dp->psr.paused)
+			goto exit;
 
 		if (origin == ORIGIN_FLIP ||
 		    (origin == ORIGIN_CURSOR_UPDATE &&
 		     !intel_dp->psr.psr2_sel_fetch_enabled)) {
 			tgl_dc3co_flush_locked(intel_dp, frontbuffer_bits, origin);
-			mutex_unlock(&intel_dp->psr.lock);
-			continue;
+			goto exit;
 		}
 
-		/* By definition flush = invalidate + flush */
-		if (pipe_frontbuffer_bits)
-			psr_force_hw_tracking_exit(intel_dp);
+		if (pipe_frontbuffer_bits == 0)
+			goto exit;
 
-		if (!intel_dp->psr.active && !intel_dp->psr.busy_frontbuffer_bits)
-			schedule_work(&intel_dp->psr.work);
+		/* By definition flush = invalidate + flush */
+		_psr_flush_handle(intel_dp, prev_busy_frontbuffer_bits);
+exit:
 		mutex_unlock(&intel_dp->psr.lock);
 	}
 }
-- 
2.35.1


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [Intel-gfx] ✗ Fi.CI.DOCS: warning for series starting with [1/2] drm/i915/display/psr: Set partial frame enable when forcing full frame fetch
  2022-03-24 18:13 [Intel-gfx] [PATCH 1/2] drm/i915/display/psr: Set partial frame enable when forcing full frame fetch José Roberto de Souza
  2022-03-24 18:13 ` [Intel-gfx] [PATCH 2/2] drm/i915/display/psr: Use continuos full frame to handle frontbuffer invalidations José Roberto de Souza
@ 2022-03-24 18:59 ` Patchwork
  2022-03-24 19:26 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 9+ messages in thread
From: Patchwork @ 2022-03-24 18:59 UTC (permalink / raw)
  To: José Roberto de Souza; +Cc: intel-gfx

== Series Details ==

Series: series starting with [1/2] drm/i915/display/psr: Set partial frame enable when forcing full frame fetch
URL   : https://patchwork.freedesktop.org/series/101752/
State : warning

== Summary ==

$ make htmldocs 2>&1 > /dev/null | grep i915
./drivers/gpu/drm/i915/display/intel_drrs.c:1: warning: 'intel_drrs_enable' not found
./drivers/gpu/drm/i915/display/intel_drrs.c:1: warning: 'intel_drrs_disable' not found



^ permalink raw reply	[flat|nested] 9+ messages in thread

* [Intel-gfx] ✓ Fi.CI.BAT: success for series starting with [1/2] drm/i915/display/psr: Set partial frame enable when forcing full frame fetch
  2022-03-24 18:13 [Intel-gfx] [PATCH 1/2] drm/i915/display/psr: Set partial frame enable when forcing full frame fetch José Roberto de Souza
  2022-03-24 18:13 ` [Intel-gfx] [PATCH 2/2] drm/i915/display/psr: Use continuos full frame to handle frontbuffer invalidations José Roberto de Souza
  2022-03-24 18:59 ` [Intel-gfx] ✗ Fi.CI.DOCS: warning for series starting with [1/2] drm/i915/display/psr: Set partial frame enable when forcing full frame fetch Patchwork
@ 2022-03-24 19:26 ` Patchwork
  2022-03-24 21:48 ` [Intel-gfx] ✗ Fi.CI.IGT: failure " Patchwork
  2022-03-25  9:04 ` [Intel-gfx] [PATCH 1/2] " Hogander, Jouni
  4 siblings, 0 replies; 9+ messages in thread
From: Patchwork @ 2022-03-24 19:26 UTC (permalink / raw)
  To: José Roberto de Souza; +Cc: intel-gfx

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

== Series Details ==

Series: series starting with [1/2] drm/i915/display/psr: Set partial frame enable when forcing full frame fetch
URL   : https://patchwork.freedesktop.org/series/101752/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_11402 -> Patchwork_22673
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

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

Participating hosts (45 -> 41)
------------------------------

  Additional (1): fi-hsw-4770 
  Missing    (5): shard-tglu fi-bsw-cyan shard-rkl bat-jsl-2 fi-bdw-samus 

Possible new issues
-------------------

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

### IGT changes ###

#### Suppressed ####

  The following results come from untrusted machines, tests, or statuses.
  They do not affect the overall result.

  * igt@i915_pm_rpm@module-reload:
    - {bat-rpls-2}:       [PASS][1] -> [INCOMPLETE][2]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11402/bat-rpls-2/igt@i915_pm_rpm@module-reload.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22673/bat-rpls-2/igt@i915_pm_rpm@module-reload.html

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

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

### IGT changes ###

#### Issues hit ####

  * igt@amdgpu/amd_basic@semaphore:
    - fi-hsw-4770:        NOTRUN -> [SKIP][3] ([fdo#109271] / [fdo#109315]) +17 similar issues
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22673/fi-hsw-4770/igt@amdgpu/amd_basic@semaphore.html

  * igt@gem_softpin@allocator-basic-reserve:
    - fi-hsw-4770:        NOTRUN -> [SKIP][4] ([fdo#109271]) +9 similar issues
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22673/fi-hsw-4770/igt@gem_softpin@allocator-basic-reserve.html

  * igt@i915_pm_backlight@basic-brightness:
    - fi-hsw-4770:        NOTRUN -> [SKIP][5] ([fdo#109271] / [i915#3012])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22673/fi-hsw-4770/igt@i915_pm_backlight@basic-brightness.html

  * igt@i915_selftest@live@late_gt_pm:
    - fi-bsw-nick:        [PASS][6] -> [DMESG-FAIL][7] ([i915#2927] / [i915#3428])
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11402/fi-bsw-nick/igt@i915_selftest@live@late_gt_pm.html
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22673/fi-bsw-nick/igt@i915_selftest@live@late_gt_pm.html

  * igt@i915_selftest@live@requests:
    - fi-blb-e6850:       [PASS][8] -> [DMESG-FAIL][9] ([i915#4528])
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11402/fi-blb-e6850/igt@i915_selftest@live@requests.html
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22673/fi-blb-e6850/igt@i915_selftest@live@requests.html

  * igt@kms_chamelium@dp-crc-fast:
    - fi-hsw-4770:        NOTRUN -> [SKIP][10] ([fdo#109271] / [fdo#111827]) +8 similar issues
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22673/fi-hsw-4770/igt@kms_chamelium@dp-crc-fast.html

  * igt@kms_pipe_crc_basic@compare-crc-sanitycheck-pipe-d:
    - fi-hsw-4770:        NOTRUN -> [SKIP][11] ([fdo#109271] / [i915#533])
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22673/fi-hsw-4770/igt@kms_pipe_crc_basic@compare-crc-sanitycheck-pipe-d.html

  * igt@kms_psr@primary_mmap_gtt:
    - fi-hsw-4770:        NOTRUN -> [SKIP][12] ([fdo#109271] / [i915#1072]) +3 similar issues
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22673/fi-hsw-4770/igt@kms_psr@primary_mmap_gtt.html

  * igt@runner@aborted:
    - fi-blb-e6850:       NOTRUN -> [FAIL][13] ([fdo#109271] / [i915#2403] / [i915#4312])
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22673/fi-blb-e6850/igt@runner@aborted.html
    - fi-bsw-nick:        NOTRUN -> [FAIL][14] ([fdo#109271] / [i915#1436] / [i915#3428] / [i915#4312])
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22673/fi-bsw-nick/igt@runner@aborted.html

  
#### Possible fixes ####

  * igt@i915_pm_rpm@module-reload:
    - {bat-adlp-6}:       [DMESG-WARN][15] ([i915#3576]) -> [PASS][16]
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11402/bat-adlp-6/igt@i915_pm_rpm@module-reload.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22673/bat-adlp-6/igt@i915_pm_rpm@module-reload.html

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

  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [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#1436]: https://gitlab.freedesktop.org/drm/intel/issues/1436
  [i915#2403]: https://gitlab.freedesktop.org/drm/intel/issues/2403
  [i915#2927]: https://gitlab.freedesktop.org/drm/intel/issues/2927
  [i915#3012]: https://gitlab.freedesktop.org/drm/intel/issues/3012
  [i915#3428]: https://gitlab.freedesktop.org/drm/intel/issues/3428
  [i915#3576]: https://gitlab.freedesktop.org/drm/intel/issues/3576
  [i915#4312]: https://gitlab.freedesktop.org/drm/intel/issues/4312
  [i915#4391]: https://gitlab.freedesktop.org/drm/intel/issues/4391
  [i915#4528]: https://gitlab.freedesktop.org/drm/intel/issues/4528
  [i915#533]: https://gitlab.freedesktop.org/drm/intel/issues/533
  [i915#5339]: https://gitlab.freedesktop.org/drm/intel/issues/5339
  [i915#5342]: https://gitlab.freedesktop.org/drm/intel/issues/5342


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

  * Linux: CI_DRM_11402 -> Patchwork_22673

  CI-20190529: 20190529
  CI_DRM_11402: 07be21d8f94f3d18394522fe5d6c75a08036c3a6 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_6392: 5a78ea9ff9c0a77bec5b094bf7e9d82c9848702b @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
  Patchwork_22673: 73945a0514107dcf07f99dcd4607fb3ec11a484c @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

73945a051410 drm/i915/display/psr: Use continuos full frame to handle frontbuffer invalidations
65b61c2ea352 drm/i915/display/psr: Set partial frame enable when forcing full frame fetch

== Logs ==

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

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [Intel-gfx] ✗ Fi.CI.IGT: failure for series starting with [1/2] drm/i915/display/psr: Set partial frame enable when forcing full frame fetch
  2022-03-24 18:13 [Intel-gfx] [PATCH 1/2] drm/i915/display/psr: Set partial frame enable when forcing full frame fetch José Roberto de Souza
                   ` (2 preceding siblings ...)
  2022-03-24 19:26 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
@ 2022-03-24 21:48 ` Patchwork
  2022-03-25  9:04 ` [Intel-gfx] [PATCH 1/2] " Hogander, Jouni
  4 siblings, 0 replies; 9+ messages in thread
From: Patchwork @ 2022-03-24 21:48 UTC (permalink / raw)
  To: José Roberto de Souza; +Cc: intel-gfx

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

== Series Details ==

Series: series starting with [1/2] drm/i915/display/psr: Set partial frame enable when forcing full frame fetch
URL   : https://patchwork.freedesktop.org/series/101752/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_11402_full -> Patchwork_22673_full
====================================================

Summary
-------

  **FAILURE**

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

  

Participating hosts (11 -> 11)
------------------------------

  No changes in participating hosts

Possible new issues
-------------------

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

### IGT changes ###

#### Possible regressions ####

  * igt@gem_softpin@allocator-evict@bcs0:
    - shard-kbl:          [PASS][1] -> [INCOMPLETE][2]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11402/shard-kbl4/igt@gem_softpin@allocator-evict@bcs0.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22673/shard-kbl6/igt@gem_softpin@allocator-evict@bcs0.html

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

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

### IGT changes ###

#### Issues hit ####

  * igt@feature_discovery@psr2:
    - shard-iclb:         NOTRUN -> [SKIP][3] ([i915#658])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22673/shard-iclb6/igt@feature_discovery@psr2.html

  * igt@gem_ctx_isolation@preservation-s3@rcs0:
    - shard-kbl:          [PASS][4] -> [INCOMPLETE][5] ([i915#1373] / [i915#794])
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11402/shard-kbl1/igt@gem_ctx_isolation@preservation-s3@rcs0.html
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22673/shard-kbl4/igt@gem_ctx_isolation@preservation-s3@rcs0.html

  * igt@gem_eio@in-flight-contexts-immediate:
    - shard-tglb:         [PASS][6] -> [TIMEOUT][7] ([i915#3063])
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11402/shard-tglb3/igt@gem_eio@in-flight-contexts-immediate.html
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22673/shard-tglb5/igt@gem_eio@in-flight-contexts-immediate.html

  * igt@gem_eio@kms:
    - shard-tglb:         [PASS][8] -> [FAIL][9] ([i915#232])
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11402/shard-tglb6/igt@gem_eio@kms.html
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22673/shard-tglb1/igt@gem_eio@kms.html

  * igt@gem_exec_balancer@parallel:
    - shard-kbl:          NOTRUN -> [DMESG-WARN][10] ([i915#5076])
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22673/shard-kbl7/igt@gem_exec_balancer@parallel.html

  * igt@gem_exec_balancer@parallel-bb-first:
    - shard-iclb:         NOTRUN -> [SKIP][11] ([i915#4525])
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22673/shard-iclb8/igt@gem_exec_balancer@parallel-bb-first.html

  * igt@gem_exec_capture@pi@rcs0:
    - shard-skl:          NOTRUN -> [INCOMPLETE][12] ([i915#1888] / [i915#4547])
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22673/shard-skl4/igt@gem_exec_capture@pi@rcs0.html

  * igt@gem_exec_fair@basic-deadline:
    - shard-tglb:         [PASS][13] -> [FAIL][14] ([i915#2846])
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11402/shard-tglb2/igt@gem_exec_fair@basic-deadline.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22673/shard-tglb1/igt@gem_exec_fair@basic-deadline.html
    - shard-skl:          NOTRUN -> [FAIL][15] ([i915#2846])
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22673/shard-skl7/igt@gem_exec_fair@basic-deadline.html

  * igt@gem_exec_fair@basic-none-solo@rcs0:
    - shard-iclb:         NOTRUN -> [FAIL][16] ([i915#2842]) +1 similar issue
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22673/shard-iclb4/igt@gem_exec_fair@basic-none-solo@rcs0.html

  * igt@gem_exec_fair@basic-none-vip@rcs0:
    - shard-tglb:         NOTRUN -> [FAIL][17] ([i915#2842]) +1 similar issue
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22673/shard-tglb1/igt@gem_exec_fair@basic-none-vip@rcs0.html

  * igt@gem_exec_fair@basic-none@rcs0:
    - shard-glk:          [PASS][18] -> [FAIL][19] ([i915#2842])
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11402/shard-glk5/igt@gem_exec_fair@basic-none@rcs0.html
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22673/shard-glk1/igt@gem_exec_fair@basic-none@rcs0.html

  * igt@gem_exec_fair@basic-none@vcs0:
    - shard-kbl:          [PASS][20] -> [FAIL][21] ([i915#2842]) +2 similar issues
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11402/shard-kbl6/igt@gem_exec_fair@basic-none@vcs0.html
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22673/shard-kbl3/igt@gem_exec_fair@basic-none@vcs0.html

  * igt@gem_lmem_swapping@parallel-random-verify:
    - shard-skl:          NOTRUN -> [SKIP][22] ([fdo#109271] / [i915#4613])
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22673/shard-skl4/igt@gem_lmem_swapping@parallel-random-verify.html

  * igt@gem_lmem_swapping@random:
    - shard-tglb:         NOTRUN -> [SKIP][23] ([i915#4613])
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22673/shard-tglb6/igt@gem_lmem_swapping@random.html
    - shard-iclb:         NOTRUN -> [SKIP][24] ([i915#4613])
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22673/shard-iclb4/igt@gem_lmem_swapping@random.html

  * igt@gem_pread@exhaustion:
    - shard-glk:          NOTRUN -> [WARN][25] ([i915#2658])
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22673/shard-glk7/igt@gem_pread@exhaustion.html
    - shard-apl:          NOTRUN -> [WARN][26] ([i915#2658])
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22673/shard-apl6/igt@gem_pread@exhaustion.html

  * igt@gem_pwrite@basic-exhaustion:
    - shard-skl:          NOTRUN -> [WARN][27] ([i915#2658])
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22673/shard-skl9/igt@gem_pwrite@basic-exhaustion.html
    - shard-iclb:         NOTRUN -> [WARN][28] ([i915#2658])
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22673/shard-iclb8/igt@gem_pwrite@basic-exhaustion.html

  * igt@gem_render_copy@x-tiled-to-vebox-yf-tiled:
    - shard-iclb:         NOTRUN -> [SKIP][29] ([i915#768]) +1 similar issue
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22673/shard-iclb8/igt@gem_render_copy@x-tiled-to-vebox-yf-tiled.html

  * igt@gem_userptr_blits@dmabuf-sync:
    - shard-kbl:          NOTRUN -> [SKIP][30] ([fdo#109271] / [i915#3323])
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22673/shard-kbl7/igt@gem_userptr_blits@dmabuf-sync.html
    - shard-apl:          NOTRUN -> [SKIP][31] ([fdo#109271] / [i915#3323])
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22673/shard-apl6/igt@gem_userptr_blits@dmabuf-sync.html
    - shard-glk:          NOTRUN -> [SKIP][32] ([fdo#109271] / [i915#3323])
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22673/shard-glk7/igt@gem_userptr_blits@dmabuf-sync.html

  * igt@gem_userptr_blits@input-checking:
    - shard-skl:          NOTRUN -> [DMESG-WARN][33] ([i915#4991])
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22673/shard-skl6/igt@gem_userptr_blits@input-checking.html

  * igt@gen7_exec_parse@basic-offset:
    - shard-iclb:         NOTRUN -> [SKIP][34] ([fdo#109289])
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22673/shard-iclb6/igt@gen7_exec_parse@basic-offset.html

  * igt@gen9_exec_parse@bb-start-param:
    - shard-tglb:         NOTRUN -> [SKIP][35] ([i915#2527] / [i915#2856])
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22673/shard-tglb6/igt@gen9_exec_parse@bb-start-param.html
    - shard-iclb:         NOTRUN -> [SKIP][36] ([i915#2856])
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22673/shard-iclb4/igt@gen9_exec_parse@bb-start-param.html

  * igt@i915_module_load@reload-no-display:
    - shard-tglb:         [PASS][37] -> [DMESG-WARN][38] ([i915#2867])
   [37]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11402/shard-tglb5/igt@i915_module_load@reload-no-display.html
   [38]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22673/shard-tglb2/igt@i915_module_load@reload-no-display.html

  * igt@i915_pm_dc@dc6-psr:
    - shard-skl:          NOTRUN -> [FAIL][39] ([i915#454])
   [39]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22673/shard-skl8/igt@i915_pm_dc@dc6-psr.html

  * igt@kms_big_fb@4-tiled-32bpp-rotate-90:
    - shard-iclb:         NOTRUN -> [SKIP][40] ([i915#5286])
   [40]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22673/shard-iclb6/igt@kms_big_fb@4-tiled-32bpp-rotate-90.html
    - shard-tglb:         NOTRUN -> [SKIP][41] ([i915#5286])
   [41]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22673/shard-tglb1/igt@kms_big_fb@4-tiled-32bpp-rotate-90.html

  * igt@kms_big_fb@4-tiled-max-hw-stride-64bpp-rotate-0-async-flip:
    - shard-apl:          NOTRUN -> [SKIP][42] ([fdo#109271]) +46 similar issues
   [42]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22673/shard-apl7/igt@kms_big_fb@4-tiled-max-hw-stride-64bpp-rotate-0-async-flip.html

  * igt@kms_big_fb@y-tiled-32bpp-rotate-0:
    - shard-glk:          [PASS][43] -> [DMESG-WARN][44] ([i915#118])
   [43]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11402/shard-glk8/igt@kms_big_fb@y-tiled-32bpp-rotate-0.html
   [44]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22673/shard-glk7/igt@kms_big_fb@y-tiled-32bpp-rotate-0.html

  * igt@kms_big_fb@y-tiled-max-hw-stride-32bpp-rotate-0-async-flip:
    - shard-skl:          NOTRUN -> [FAIL][45] ([i915#3743]) +1 similar issue
   [45]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22673/shard-skl4/igt@kms_big_fb@y-tiled-max-hw-stride-32bpp-rotate-0-async-flip.html

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

  * igt@kms_big_fb@yf-tiled-max-hw-stride-32bpp-rotate-180-hflip-async-flip:
    - shard-skl:          NOTRUN -> [SKIP][47] ([fdo#109271] / [i915#3777]) +1 similar issue
   [47]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22673/shard-skl6/igt@kms_big_fb@yf-tiled-max-hw-stride-32bpp-rotate-180-hflip-async-flip.html

  * igt@kms_ccs@pipe-a-bad-aux-stride-yf_tiled_ccs:
    - shard-tglb:         NOTRUN -> [SKIP][48] ([fdo#111615] / [i915#3689]) +1 similar issue
   [48]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22673/shard-tglb1/igt@kms_ccs@pipe-a-bad-aux-stride-yf_tiled_ccs.html

  * igt@kms_ccs@pipe-a-bad-rotation-90-y_tiled_gen12_mc_ccs:
    - shard-tglb:         NOTRUN -> [SKIP][49] ([i915#3689] / [i915#3886])
   [49]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22673/shard-tglb6/igt@kms_ccs@pipe-a-bad-rotation-90-y_tiled_gen12_mc_ccs.html
    - shard-iclb:         NOTRUN -> [SKIP][50] ([fdo#109278] / [i915#3886]) +1 similar issue
   [50]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22673/shard-iclb4/igt@kms_ccs@pipe-a-bad-rotation-90-y_tiled_gen12_mc_ccs.html

  * igt@kms_ccs@pipe-a-crc-primary-rotation-180-y_tiled_gen12_rc_ccs_cc:
    - shard-skl:          NOTRUN -> [SKIP][51] ([fdo#109271] / [i915#3886]) +10 similar issues
   [51]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22673/shard-skl4/igt@kms_ccs@pipe-a-crc-primary-rotation-180-y_tiled_gen12_rc_ccs_cc.html

  * igt@kms_ccs@pipe-a-random-ccs-data-y_tiled_ccs:
    - shard-tglb:         NOTRUN -> [SKIP][52] ([i915#3689]) +2 similar issues
   [52]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22673/shard-tglb6/igt@kms_ccs@pipe-a-random-ccs-data-y_tiled_ccs.html

  * igt@kms_ccs@pipe-b-crc-primary-rotation-180-y_tiled_gen12_rc_ccs_cc:
    - shard-glk:          NOTRUN -> [SKIP][53] ([fdo#109271] / [i915#3886]) +1 similar issue
   [53]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22673/shard-glk7/igt@kms_ccs@pipe-b-crc-primary-rotation-180-y_tiled_gen12_rc_ccs_cc.html

  * igt@kms_ccs@pipe-c-bad-rotation-90-y_tiled_gen12_mc_ccs:
    - shard-apl:          NOTRUN -> [SKIP][54] ([fdo#109271] / [i915#3886]) +4 similar issues
   [54]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22673/shard-apl6/igt@kms_ccs@pipe-c-bad-rotation-90-y_tiled_gen12_mc_ccs.html

  * igt@kms_chamelium@hdmi-hpd-enable-disable-mode:
    - shard-iclb:         NOTRUN -> [SKIP][55] ([fdo#109284] / [fdo#111827]) +3 similar issues
   [55]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22673/shard-iclb4/igt@kms_chamelium@hdmi-hpd-enable-disable-mode.html
    - shard-tglb:         NOTRUN -> [SKIP][56] ([fdo#109284] / [fdo#111827]) +2 similar issues
   [56]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22673/shard-tglb6/igt@kms_chamelium@hdmi-hpd-enable-disable-mode.html

  * igt@kms_chamelium@hdmi-hpd-storm:
    - shard-skl:          NOTRUN -> [SKIP][57] ([fdo#109271] / [fdo#111827] / [i915#1888])
   [57]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22673/shard-skl6/igt@kms_chamelium@hdmi-hpd-storm.html

  * igt@kms_chamelium@vga-hpd-with-enabled-mode:
    - shard-apl:          NOTRUN -> [SKIP][58] ([fdo#109271] / [fdo#111827]) +1 similar issue
   [58]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22673/shard-apl7/igt@kms_chamelium@vga-hpd-with-enabled-mode.html

  * igt@kms_color@pipe-d-ctm-0-75:
    - shard-iclb:         NOTRUN -> [SKIP][59] ([fdo#109278] / [i915#1149])
   [59]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22673/shard-iclb8/igt@kms_color@pipe-d-ctm-0-75.html

  * igt@kms_color_chamelium@pipe-b-ctm-0-5:
    - shard-glk:          NOTRUN -> [SKIP][60] ([fdo#109271] / [fdo#111827]) +2 similar issues
   [60]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22673/shard-glk7/igt@kms_color_chamelium@pipe-b-ctm-0-5.html

  * igt@kms_color_chamelium@pipe-b-ctm-max:
    - shard-skl:          NOTRUN -> [SKIP][61] ([fdo#109271] / [fdo#111827]) +16 similar issues
   [61]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22673/shard-skl8/igt@kms_color_chamelium@pipe-b-ctm-max.html

  * igt@kms_content_protection@atomic:
    - shard-iclb:         NOTRUN -> [SKIP][62] ([fdo#109300] / [fdo#111066])
   [62]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22673/shard-iclb4/igt@kms_content_protection@atomic.html
    - shard-tglb:         NOTRUN -> [SKIP][63] ([i915#1063])
   [63]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22673/shard-tglb6/igt@kms_content_protection@atomic.html

  * igt@kms_content_protection@srm:
    - shard-apl:          NOTRUN -> [TIMEOUT][64] ([i915#1319])
   [64]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22673/shard-apl7/igt@kms_content_protection@srm.html

  * igt@kms_cursor_crc@pipe-b-cursor-32x32-rapid-movement:
    - shard-tglb:         NOTRUN -> [SKIP][65] ([i915#3319])
   [65]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22673/shard-tglb6/igt@kms_cursor_crc@pipe-b-cursor-32x32-rapid-movement.html

  * igt@kms_cursor_crc@pipe-c-cursor-suspend:
    - shard-apl:          [PASS][66] -> [DMESG-WARN][67] ([i915#180]) +1 similar issue
   [66]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11402/shard-apl1/igt@kms_cursor_crc@pipe-c-cursor-suspend.html
   [67]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22673/shard-apl8/igt@kms_cursor_crc@pipe-c-cursor-suspend.html

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

  * igt@kms_cursor_crc@pipe-d-cursor-max-size-rapid-movement:
    - shard-tglb:         NOTRUN -> [SKIP][69] ([i915#3359]) +1 similar issue
   [69]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22673/shard-tglb6/igt@kms_cursor_crc@pipe-d-cursor-max-size-rapid-movement.html

  * igt@kms_cursor_edge_walk@pipe-d-128x128-top-edge:
    - shard-iclb:         NOTRUN -> [SKIP][70] ([fdo#109278]) +13 similar issues
   [70]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22673/shard-iclb6/igt@kms_cursor_edge_walk@pipe-d-128x128-top-edge.html

  * igt@kms_cursor_legacy@cursorb-vs-flipb-varying-size:
    - shard-iclb:         NOTRUN -> [SKIP][71] ([fdo#109274] / [fdo#109278])
   [71]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22673/shard-iclb8/igt@kms_cursor_legacy@cursorb-vs-flipb-varying-size.html

  * igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions-varying-size:
    - shard-skl:          NOTRUN -> [FAIL][72] ([i915#2346] / [i915#533])
   [72]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22673/shard-skl7/igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions-varying-size.html

  * igt@kms_cursor_legacy@flip-vs-cursor-varying-size:
    - shard-iclb:         [PASS][73] -> [FAIL][74] ([i915#2346])
   [73]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11402/shard-iclb8/igt@kms_cursor_legacy@flip-vs-cursor-varying-size.html
   [74]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22673/shard-iclb7/igt@kms_cursor_legacy@flip-vs-cursor-varying-size.html

  * igt@kms_cursor_legacy@pipe-d-torture-move:
    - shard-skl:          NOTRUN -> [SKIP][75] ([fdo#109271]) +206 similar issues
   [75]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22673/shard-skl9/igt@kms_cursor_legacy@pipe-d-torture-move.html

  * igt@kms_fbcon_fbt@fbc-suspend:
    - shard-kbl:          [PASS][76] -> [INCOMPLETE][77] ([i915#180] / [i915#636])
   [76]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11402/shard-kbl6/igt@kms_fbcon_fbt@fbc-suspend.html
   [77]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22673/shard-kbl4/igt@kms_fbcon_fbt@fbc-suspend.html

  * igt@kms_flip@2x-flip-vs-blocking-wf-vblank@ab-hdmi-a1-hdmi-a2:
    - shard-glk:          [PASS][78] -> [FAIL][79] ([i915#2122])
   [78]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11402/shard-glk4/igt@kms_flip@2x-flip-vs-blocking-wf-vblank@ab-hdmi-a1-hdmi-a2.html
   [79]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22673/shard-glk2/igt@kms_flip@2x-flip-vs-blocking-wf-vblank@ab-hdmi-a1-hdmi-a2.html

  * igt@kms_flip@2x-plain-flip-interruptible:
    - shard-tglb:         NOTRUN -> [SKIP][80] ([fdo#109274] / [fdo#111825])
   [80]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22673/shard-tglb6/igt@kms_flip@2x-plain-flip-interruptible.html
    - shard-iclb:         NOTRUN -> [SKIP][81] ([fdo#109274]) +1 similar issue
   [81]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22673/shard-iclb4/igt@kms_flip@2x-plain-flip-interruptible.html

  * igt@kms_flip_scaled_crc@flip-32bpp-ytile-to-64bpp-ytile-downscaling:
    - shard-iclb:         [PASS][82] -> [SKIP][83] ([i915#3701]) +1 similar issue
   [82]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11402/shard-iclb5/igt@kms_flip_scaled_crc@flip-32bpp-ytile-to-64bpp-ytile-downscaling.html
   [83]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22673/shard-iclb2/igt@kms_flip_scaled_crc@flip-32bpp-ytile-to-64bpp-ytile-downscaling.html

  * igt@kms_force_connector_basic@force-load-detect:
    - shard-iclb:         NOTRUN -> [SKIP][84] ([fdo#109285])
   [84]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22673/shard-iclb8/igt@kms_force_connector_basic@force-load-detect.html

  * igt@kms_frontbuffer_tracking@fbc-2p-primscrn-spr-indfb-fullscreen:
    - shard-kbl:          NOTRUN -> [SKIP][85] ([fdo#109271]) +19 similar issues
   [85]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22673/shard-kbl7/igt@kms_frontbuffer_tracking@fbc-2p-primscrn-spr-indfb-fullscreen.html

  * igt@kms_frontbuffer_tracking@fbcpsr-2p-primscrn-pri-indfb-draw-blt:
    - shard-tglb:         NOTRUN -> [SKIP][86] ([fdo#109280] / [fdo#111825]) +3 similar issues
   [86]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22673/shard-tglb1/igt@kms_frontbuffer_tracking@fbcpsr-2p-primscrn-pri-indfb-draw-blt.html

  * igt@kms_frontbuffer_tracking@psr-2p-scndscrn-pri-indfb-draw-pwrite:
    - shard-iclb:         NOTRUN -> [SKIP][87] ([fdo#109280]) +3 similar issues
   [87]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22673/shard-iclb6/igt@kms_frontbuffer_tracking@psr-2p-scndscrn-pri-indfb-draw-pwrite.html

  * igt@kms_frontbuffer_tracking@psr-2p-scndscrn-spr-indfb-draw-pwrite:
    - shard-glk:          NOTRUN -> [SKIP][88] ([fdo#109271]) +29 similar issues
   [88]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22673/shard-glk7/igt@kms_frontbuffer_tracking@psr-2p-scndscrn-spr-indfb-draw-pwrite.html

  * igt@kms_hdr@bpc-switch-suspend@bpc-switch-suspend-edp-1-pipe-a:
    - shard-skl:          NOTRUN -> [FAIL][89] ([i915#1188])
   [89]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22673/shard-skl4/igt@kms_hdr@bpc-switch-suspend@bpc-switch-suspend-edp-1-pipe-a.html

  * igt@kms_hdr@static-toggle:
    - shard-iclb:         NOTRUN -> [SKIP][90] ([i915#3555])
   [90]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22673/shard-iclb8/igt@kms_hdr@static-toggle.html

  * igt@kms_pipe_crc_basic@compare-crc-sanitycheck-pipe-d:
    - shard-skl:          NOTRUN -> [SKIP][91] ([fdo#109271] / [i915#533]) +2 similar issues
   [91]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22673/shard-skl4/igt@kms_pipe_crc_basic@compare-crc-sanitycheck-pipe-d.html

  * igt@kms_pipe_crc_basic@nonblocking-crc-pipe-d-frame-sequence:
    - shard-apl:          NOTRUN -> [SKIP][92] ([fdo#109271] / [i915#533]) +1 similar issue
   [92]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22673/shard-apl7/igt@kms_pipe_crc_basic@nonblocking-crc-pipe-d-frame-sequence.html

  * igt@kms_plane@plane-panning-bottom-right-suspend@pipe-b-planes:
    - shard-kbl:          [PASS][93] -> [DMESG-WARN][94] ([i915#180]) +1 similar issue
   [93]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11402/shard-kbl3/igt@kms_plane@plane-panning-bottom-right-suspend@pipe-b-planes.html
   [94]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22673/shard-kbl1/igt@kms_plane@plane-panning-bottom-right-suspend@pipe-b-planes.html

  * igt@kms_plane_alpha_blend@pipe-b-alpha-basic:
    - shard-skl:          NOTRUN -> [FAIL][95] ([fdo#108145] / [i915#265])
   [95]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22673/shard-skl8/igt@kms_plane_alpha_blend@pipe-b-alpha-basic.html

  * igt@kms_plane_alpha_blend@pipe-c-alpha-transparent-fb:
    - shard-skl:          NOTRUN -> [FAIL][96] ([i915#265])
   [96]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22673/shard-skl7/igt@kms_plane_alpha_blend@pipe-c-alpha-transparent-fb.html

  * igt@kms_plane_alpha_blend@pipe-c-coverage-7efc:
    - shard-skl:          [PASS][97] -> [FAIL][98] ([fdo#108145] / [i915#265])
   [97]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11402/shard-skl4/igt@kms_plane_alpha_blend@pipe-c-coverage-7efc.html
   [98]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22673/shard-skl6/igt@kms_plane_alpha_blend@pipe-c-coverage-7efc.html

  * igt@kms_plane_scaling@downscale-with-pixel-format-factor-0-75@pipe-b-edp-1-downscale-with-pixel-format:
    - shard-iclb:         [PASS][99] -> [INCOMPLETE][100] ([i915#5293])
   [99]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11402/shard-iclb6/igt@kms_plane_scaling@downscale-with-pixel-format-factor-0-75@pipe-b-edp-1-downscale-with-pixel-format.html
   [100]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22673/shard-iclb2/igt@kms_plane_scaling@downscale-with-pixel-format-factor-0-75@pipe-b-edp-1-downscale-with-pixel-format.html

  * igt@kms_plane_scaling@planes-upscale-20x20-downscale-factor-0-25@pipe-a-edp-1-planes-upscale-downscale:
    - shard-iclb:         NOTRUN -> [SKIP][101] ([i915#5235]) +2 similar issues
   [101]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22673/shard-iclb6/igt@kms_plane_scaling@planes-upscale-20x20-downscale-factor-0-25@pipe-a-edp-1-planes-upscale-downscale.html

  * igt@kms_psr2_sf@overlay-plane-update-sf-dmg-area:
    - shard-skl:          NOTRUN -> [SKIP][102] ([fdo#109271] / [i915#658]) +2 similar issues
   [102]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22673/shard-skl4/igt@kms_psr2_sf@overlay-plane-update-sf-dmg-area.html

  * igt@kms_psr2_su@page_flip-p010:
    - shard-apl:          NOTRUN -> [SKIP][103] ([fdo#109271] / [i915#658])
   [103]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22673/shard-apl7/igt@kms_psr2_su@page_flip-p010.html

  * igt@kms_psr@psr2_cursor_plane_onoff:
    - shard-tglb:         NOTRUN -> [FAIL][104] ([i915#132] / [i915#3467]) +1 similar issue
   [104]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22673/shard-tglb6/igt@kms_psr@psr2_cursor_plane_onoff.html
    - shard-iclb:         NOTRUN -> [SKIP][105] ([fdo#109441]) +1 similar issue
   [105]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22673/shard-iclb4/igt@kms_psr@psr2_cursor_plane_onoff.html

  * igt@kms_sysfs_edid_timing:
    - shard-skl:          NOTRUN -> [FAIL][106] ([IGT#2])
   [106]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22673/shard-skl6/igt@kms_sysfs_edid_timing.html

  * igt@kms_writeback@writeback-fb-id:
    - shard-skl:          NOTRUN -> [SKIP][107] ([fdo#109271] / [i915#2437])
   [107]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22673/shard-skl4/igt@kms_writeback@writeback-fb-id.html

  * igt@nouveau_crc@pipe-a-source-rg:
    - shard-iclb:         NOTRUN -> [SKIP][108] ([i915#2530])
   [108]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22673/shard-iclb8/igt@nouveau_crc@pipe-a-source-rg.html

  * igt@prime_nv_test@i915_nv_sharing:
    - shard-iclb:         NOTRUN -> [SKIP][109] ([fdo#109291]) +1 similar issue
   [109]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22673/shard-iclb4/igt@prime_nv_test@i915_nv_sharing.html
    - shard-tglb:         NOTRUN -> [SKIP][110] ([fdo#109291])
   [110]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22673/shard-tglb6/igt@prime_nv_test@i915_nv_sharing.html

  * igt@syncobj_timeline@transfer-timeline-point:
    - shard-iclb:         NOTRUN -> [DMESG-FAIL][111] ([i915#5098])
   [111]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22673/shard-iclb8/igt@syncobj_timeline@transfer-timeline-point.html
    - shard-skl:          NOTRUN -> [DMESG-FAIL][112] ([i915#5098])
   [112]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22673/shard-skl9/igt@syncobj_timeline@transfer-timeline-point.html

  * igt@sysfs_clients@create:
    - shard-tglb:         NOTRUN -> [SKIP][113] ([i915#2994])
   [113]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22673/shard-tglb6/igt@sysfs_clients@create.html

  * igt@sysfs_clients@fair-0:
    - shard-kbl:          NOTRUN -> [SKIP][114] ([fdo#109271] / [i915#2994])
   [114]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22673/shard-kbl7/igt@sysfs_clients@fair-0.html

  * igt@sysfs_clients@recycle-many:
    - shard-apl:          NOTRUN -> [SKIP][115] ([fdo#109271] / [i915#2994])
   [115]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22673/shard-apl6/igt@sysfs_clients@recycle-many.html
    - shard-glk:          NOTRUN -> [SKIP][116] ([fdo#109271] / [i915#2994])
   [116]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22673/shard-glk7/igt@sysfs_clients@recycle-many.html

  * igt@sysfs_clients@sema-50:
    - shard-iclb:         NOTRUN -> [SKIP][117] ([i915#2994]) +1 similar issue
   [117]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22673/shard-iclb6/igt@sysfs_clients@sema-50.html

  * igt@sysfs_clients@split-50:
    - shard-skl:          NOTRUN -> [SKIP][118] ([fdo#109271] / [i915#2994])
   [118]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22673/shard-skl8/igt@sysfs_clients@split-50.html

  
#### Possible fixes ####

  * igt@gem_ctx_isolation@dirty-create@vcs0:
    - shard-glk:          [DMESG-WARN][119] ([i915#118]) -> [PASS][120]
   [119]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11402/shard-glk7/igt@gem_ctx_isolation@dirty-create@vcs0.html
   [120]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22673/shard-glk8/igt@gem_ctx_isolation@dirty-create@vcs0.html

  * igt@gem_exec_balancer@parallel-balancer:
    - shard-iclb:         [SKIP][121] ([i915#4525]) -> [PASS][122]
   [121]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11402/shard-iclb6/igt@gem_exec_balancer@parallel-balancer.html
   [122]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22673/shard-iclb1/igt@gem_exec_balancer@parallel-balancer.html

  * igt@gem_exec_fair@basic-none-share@rcs0:
    - shard-iclb:         [FAIL][123] ([i915#2842]) -> [PASS][124]
   [123]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11402/shard-iclb8/igt@gem_exec_fair@basic-none-share@rcs0.html
   [124]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22673/shard-iclb7/igt@gem_exec_fair@basic-none-share@rcs0.html

  * igt@gem_exec_fair@basic-pace-share@rcs0:
    - shard-tglb:         [FAIL][125] ([i915#2842]) -> [PASS][126]
   [125]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11402/shard-tglb3/igt@gem_exec_fair@basic-pace-share@rcs0.html
   [126]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22673/shard-tglb2/igt@gem_exec_fair@basic-pace-share@rcs0.html

  * igt@gem_exec_fair@basic-pace@vecs0:
    - shard-kbl:          [SKIP][127] ([fdo#109271]) -> [PASS][128]
   [127]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11402/shard-kbl1/igt@gem_exec_fair@basic-pace@vecs0.html
   [128]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22673/shard-kbl3/igt@gem_exec_fair@basic-pace@vecs0.html

  * igt@gem_softpin@allocator-evict-all-engines:
    - shard-glk:          [FAIL][129] ([i915#4171]) -> [PASS][130]
   [129]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11402/shard-glk3/igt@gem_softpin@allocator-evict-all-engines.html
   [130]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22673/shard-glk5/igt@gem_softpin@allocator-evict-all-engines.html

  * igt@gen9_exec_parse@allowed-all:
    - shard-glk:          [DMESG-WARN][131] ([i915#1436] / [i915#716]) -> [PASS][132]
   [131]: https://intel-gfx-

== Logs ==

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

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Intel-gfx] [PATCH 1/2] drm/i915/display/psr: Set partial frame enable when forcing full frame fetch
  2022-03-24 18:13 [Intel-gfx] [PATCH 1/2] drm/i915/display/psr: Set partial frame enable when forcing full frame fetch José Roberto de Souza
                   ` (3 preceding siblings ...)
  2022-03-24 21:48 ` [Intel-gfx] ✗ Fi.CI.IGT: failure " Patchwork
@ 2022-03-25  9:04 ` Hogander, Jouni
  4 siblings, 0 replies; 9+ messages in thread
From: Hogander, Jouni @ 2022-03-25  9:04 UTC (permalink / raw)
  To: intel-gfx, Souza, Jose

On Thu, 2022-03-24 at 11:13 -0700, José Roberto de Souza wrote:
> Following up what was done in commit 804f46885317 ("drm/i915/psr: Set
> "SF Partial Frame Enable" also on full update") and also setting
> partial frame enable when psr_force_hw_tracking_exit() is called.

Reviewed-by: Jouni Högander <jouni.hogander@intel.com>

> Cc: Jouni Högander <jouni.hogander@intel.com>
> Cc: Mika Kahola <mika.kahola@intel.com>
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_psr.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_psr.c
> b/drivers/gpu/drm/i915/display/intel_psr.c
> index 80002ca6a6ebe..d87b357806c91 100644
> --- a/drivers/gpu/drm/i915/display/intel_psr.c
> +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> @@ -1457,7 +1457,8 @@ static void psr_force_hw_tracking_exit(struct
> intel_dp *intel_dp)
>  	if (intel_dp->psr.psr2_sel_fetch_enabled)
>  		intel_de_rmw(dev_priv,
>  			     PSR2_MAN_TRK_CTL(intel_dp-
> >psr.transcoder), 0,
> -			     man_trk_ctl_single_full_frame_bit_get(dev_
> priv));
> +			     man_trk_ctl_single_full_frame_bit_get(dev_
> priv) |
> +			     man_trk_ctl_partial_frame_bit_get(dev_priv
> ));
>  
>  	/*
>  	 * Display WA #0884: skl+


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Intel-gfx] [PATCH 2/2] drm/i915/display/psr: Use continuos full frame to handle frontbuffer invalidations
  2022-03-24 18:13 ` [Intel-gfx] [PATCH 2/2] drm/i915/display/psr: Use continuos full frame to handle frontbuffer invalidations José Roberto de Souza
@ 2022-03-25 14:21   ` Hogander, Jouni
  2022-03-25 14:46     ` Souza, Jose
  0 siblings, 1 reply; 9+ messages in thread
From: Hogander, Jouni @ 2022-03-25 14:21 UTC (permalink / raw)
  To: intel-gfx, Souza, Jose

Hello Jose,

See my comments below.

On Thu, 2022-03-24 at 11:13 -0700, José Roberto de Souza wrote:
> Instead of exit PSR when a frontbuffer invalidation happens, we can
> enable the PSR2 selective fetch continuous full frame, that will keep
> the panel updated like PSR was disabled but without keeping PSR
> active.

with keeping PSR active? I don't think it's like PSR was disabled. New
full frame is updated only via atomic commit. Having PSR disabled new
full frame is updated all the time as PSR wasn't existing at all.

> 
> So as soon as the frontbuffer flush happens we can disable the
> continuous full frame and start to do selective fetches much quicker
> than the path that would enable PSR, that will wait a few frames
> to actually activate PSR.
> 
> Also this approach has proven to fix some glitches found in
> Alderlake-P
> when there are a lot of invalidations happening together with page
> flips.
> 
> Some may ask why it is writing to CURSURFLIVE(), it is because
> that is the way that hardware team provided us to poke display to
> handle PSR updates, and it is being used since display 9.

Generic comments:

Current logic is to disable psr2 in invalidate callback and start
sending fullframe updates on every vblank period. This is done until
flush callback where psr2 is re-enabled. Intention is to update
possible frontbuffer writes between invalidate/flush instantly. 

Now you are changing the logic to update one full frame when
frontbuffer write starts (_psr_invalidate_handle) and another one when
it stops (_psr_flush_handle) without disabling psr at all. Have I
understood your patch correctly?

Propably we wont notice this change as we have these invalidate/flush
calls scattered around in the code. Also parallel atomic commits are
triggering updates. In theory we could observe latency in updates
between invalidate/flush? Do we care? What do you think?

Do we need to send update in invalidate at all? Isn't that usually
called before doing any frontbuffer writing? I.e. we would be sending
frame that is already in RFB?

> 
> Cc: Khaled Almahallawy <khaled.almahallawy@intel.com>
> Cc: Shawn C Lee <shawn.c.lee@intel.com>
> Cc: Jouni Högander <jouni.hogander@intel.com>
> Cc: Mika Kahola <mika.kahola@intel.com>
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_psr.c | 109 ++++++++++++++++++++-
> --
>  1 file changed, 95 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_psr.c
> b/drivers/gpu/drm/i915/display/intel_psr.c
> index d87b357806c91..f7b7b374374b1 100644
> --- a/drivers/gpu/drm/i915/display/intel_psr.c
> +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> @@ -1450,6 +1450,22 @@ static inline u32
> man_trk_ctl_partial_frame_bit_get(struct drm_i915_private *dev
>  	       PSR2_MAN_TRK_CTL_SF_PARTIAL_FRAME_UPDATE;
>  }
>  
> +static inline u32 man_trk_ctl_continuos_full_frame(struct
> drm_i915_private *dev_priv)
> +{
> +	return IS_ALDERLAKE_P(dev_priv) ?
> +	       ADLP_PSR2_MAN_TRK_CTL_SF_CONTINUOS_FULL_FRAME :
> +	       PSR2_MAN_TRK_CTL_SF_CONTINUOS_FULL_FRAME;
> +}
> +
> +static inline u32 man_trk_ctl_su_region_start_end_mask(struct
> drm_i915_private *dev_priv)
> +{
> +	if (IS_ALDERLAKE_P(dev_priv))
> +		return ADLP_PSR2_MAN_TRK_CTL_SU_REGION_START_ADDR_MASK
> |
> +		       ADLP_PSR2_MAN_TRK_CTL_SU_REGION_END_ADDR_MASK;
> +	return PSR2_MAN_TRK_CTL_SU_REGION_START_ADDR_MASK |
> +	       PSR2_MAN_TRK_CTL_SU_REGION_END_ADDR_MASK;
> +}
> +
>  static void psr_force_hw_tracking_exit(struct intel_dp *intel_dp)
>  {
>  	struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
> @@ -1546,8 +1562,9 @@ void intel_psr2_program_trans_man_trk_ctl(const
> struct intel_crtc_state *crtc_st
>  	if (!crtc_state->enable_psr2_sel_fetch)
>  		return;
>  
> -	intel_de_write(dev_priv, PSR2_MAN_TRK_CTL(crtc_state-
> >cpu_transcoder),
> -		       crtc_state->psr2_man_track_ctl);
> +	intel_de_rmw(dev_priv, PSR2_MAN_TRK_CTL(crtc_state-
> >cpu_transcoder),
> +		     man_trk_ctl_su_region_start_end_mask(dev_priv),
> +		     crtc_state->psr2_man_track_ctl);

Should we actually now consider taking psr->lock here?

>  }
>  
>  static void psr2_man_trk_ctl_calc(struct intel_crtc_state
> *crtc_state,
> @@ -2127,6 +2144,26 @@ static void intel_psr_work(struct work_struct
> *work)
>  	mutex_unlock(&intel_dp->psr.lock);
>  }
>  
> +static void _psr_invalidate_handle(struct intel_dp *intel_dp,
> +				   unsigned int
> prev_busy_frontbuffer_bits)
> +{
> +	struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
> +
> +	if (intel_dp->psr.psr2_sel_fetch_enabled) {
> +		u32 val = man_trk_ctl_continuos_full_frame(dev_priv) |
> +			  man_trk_ctl_partial_frame_bit_get(dev_priv);
> +
> +		/* continuos full frame is already enabled */
> +		if (prev_busy_frontbuffer_bits)
> +			return;

Should we still trigger the update using CURSURFLIVE? Or do we need
that at all in the first place?

> +
> +		intel_de_rmw(dev_priv, PSR2_MAN_TRK_CTL(intel_dp-
> >psr.transcoder), 0, val);
> +		intel_de_write(dev_priv, CURSURFLIVE(intel_dp-
> >psr.pipe), 0);

So these two register writes here are triggering one full frame update.
and leaving full frame update bit set so that coming updates are also
full frame. Did I understood it correctly?

As invalidate is called before frontbuffer is writen: Isn't it actually
re-updating same frame which is already supposed to be in panel RFB?

> +	} else {
> +		intel_psr_exit(intel_dp);
> +	}
> +}
> +
>  /**
>   * intel_psr_invalidate - Invalidade PSR
>   * @dev_priv: i915 device
> @@ -2151,6 +2188,7 @@ void intel_psr_invalidate(struct
> drm_i915_private *dev_priv,
>  	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);
> +		unsigned int prev_busy_frontbuffer_bits;
>  
>  		mutex_lock(&intel_dp->psr.lock);
>  		if (!intel_dp->psr.enabled) {
> @@ -2158,12 +2196,13 @@ void intel_psr_invalidate(struct
> drm_i915_private *dev_priv,
>  			continue;
>  		}
>  
> +		prev_busy_frontbuffer_bits = intel_dp-
> >psr.busy_frontbuffer_bits;
>  		pipe_frontbuffer_bits &=
>  			INTEL_FRONTBUFFER_ALL_MASK(intel_dp->psr.pipe);
>  		intel_dp->psr.busy_frontbuffer_bits |=
> pipe_frontbuffer_bits;
>  
>  		if (pipe_frontbuffer_bits)
> -			intel_psr_exit(intel_dp);
> +			_psr_invalidate_handle(intel_dp,
> prev_busy_frontbuffer_bits);
>  
>  		mutex_unlock(&intel_dp->psr.lock);
>  	}
> @@ -2195,6 +2234,49 @@ tgl_dc3co_flush_locked(struct intel_dp
> *intel_dp, unsigned int frontbuffer_bits,
>  			 intel_dp->psr.dc3co_exit_delay);
>  }
>  
> +static void _psr_flush_handle(struct intel_dp *intel_dp,
> +			      unsigned int prev_busy_frontbuffer_bits)
> +{
> +	struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
> +
> +	if (intel_dp->psr.psr2_sel_fetch_enabled) {
> +		/* is continuos full frame enabled? */
> +		if (prev_busy_frontbuffer_bits) {
> +			/* it is, can we turn it off? */

As you are using this prev_busy_frontbuffer_bits only to check if
"continuos full frame" is enabled and nothing else: Maybe you could
just name it as it is e.g. bool cff_enabled or bool
continuous_full_frame or...same comment in _psr_invalidate_handle. This
would allow you to drop couple of comments.

> +			if (intel_dp->psr.busy_frontbuffer_bits == 0) {
> +				u32 clear =
> man_trk_ctl_continuos_full_frame(dev_priv);
> +				u32 set =
> man_trk_ctl_single_full_frame_bit_get(dev_priv) |
> +					  man_trk_ctl_partial_frame_bit
> _get(dev_priv);


> +
> +				/*
> +				 * turn continuos full frame off and do
> a single
> +				 * full frame
> +				 */
> +				intel_de_rmw(dev_priv,
> +					     PSR2_MAN_TRK_CTL(intel_dp-
> >psr.transcoder),
> +					     clear, set);
> +				intel_de_write(dev_priv,
> CURSURFLIVE(intel_dp->psr.pipe), 0);
> +			}
> +		} else {
> +			/*
> +			 * continuos full frame is disabled, only a
> single full
> +			 * frame is required
> +			 */
> +			psr_force_hw_tracking_exit(intel_dp);
> +		}
> +	} else {
> +		/*
> +		 * if prev_busy_frontbuffer_bits is set, it means that
> PSR is
> +		 * inactive
> +		 */
> +		if (prev_busy_frontbuffer_bits == 0)
> +			psr_force_hw_tracking_exit(intel_dp);
> +
> +		if (!intel_dp->psr.active && !intel_dp-
> >psr.busy_frontbuffer_bits)
> +			schedule_work(&intel_dp->psr.work);
> +	}
> +}
> +
>  /**
>   * intel_psr_flush - Flush PSR
>   * @dev_priv: i915 device
> @@ -2216,6 +2298,7 @@ void intel_psr_flush(struct drm_i915_private
> *dev_priv,
>  	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);
> +		unsigned int prev_busy_frontbuffer_bits;
>  
>  		mutex_lock(&intel_dp->psr.lock);
>  		if (!intel_dp->psr.enabled) {
> @@ -2223,6 +2306,7 @@ void intel_psr_flush(struct drm_i915_private
> *dev_priv,
>  			continue;
>  		}
>  
> +		prev_busy_frontbuffer_bits = intel_dp-
> >psr.busy_frontbuffer_bits;
>  		pipe_frontbuffer_bits &=
>  			INTEL_FRONTBUFFER_ALL_MASK(intel_dp->psr.pipe);
>  		intel_dp->psr.busy_frontbuffer_bits &=
> ~pipe_frontbuffer_bits;
> @@ -2232,25 +2316,22 @@ void intel_psr_flush(struct drm_i915_private
> *dev_priv,
>  		 * we have to ensure that the PSR is not activated
> until
>  		 * intel_psr_resume() is called.
>  		 */
> -		if (intel_dp->psr.paused) {
> -			mutex_unlock(&intel_dp->psr.lock);
> -			continue;
> -		}
> +		if (intel_dp->psr.paused)
> +			goto exit;
>  
>  		if (origin == ORIGIN_FLIP ||
>  		    (origin == ORIGIN_CURSOR_UPDATE &&
>  		     !intel_dp->psr.psr2_sel_fetch_enabled)) {
>  			tgl_dc3co_flush_locked(intel_dp,
> frontbuffer_bits, origin);
> -			mutex_unlock(&intel_dp->psr.lock);
> -			continue;
> +			goto exit;
>  		}
>  
> -		/* By definition flush = invalidate + flush */
> -		if (pipe_frontbuffer_bits)
> -			psr_force_hw_tracking_exit(intel_dp);
> +		if (pipe_frontbuffer_bits == 0)
> +			goto exit;
>  
> -		if (!intel_dp->psr.active && !intel_dp-
> >psr.busy_frontbuffer_bits)
> -			schedule_work(&intel_dp->psr.work);
> +		/* By definition flush = invalidate + flush */
> +		_psr_flush_handle(intel_dp,
> prev_busy_frontbuffer_bits);
> +exit:

I think you should name it as unlock.

>  		mutex_unlock(&intel_dp->psr.lock);
>  	}
>  }

BR,

Jouni Högander


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Intel-gfx] [PATCH 2/2] drm/i915/display/psr: Use continuos full frame to handle frontbuffer invalidations
  2022-03-25 14:21   ` Hogander, Jouni
@ 2022-03-25 14:46     ` Souza, Jose
  2022-03-30  6:23       ` Hogander, Jouni
  0 siblings, 1 reply; 9+ messages in thread
From: Souza, Jose @ 2022-03-25 14:46 UTC (permalink / raw)
  To: intel-gfx, Hogander, Jouni

On Fri, 2022-03-25 at 14:21 +0000, Hogander, Jouni wrote:
> Hello Jose,
> 
> See my comments below.
> 
> On Thu, 2022-03-24 at 11:13 -0700, José Roberto de Souza wrote:
> > Instead of exit PSR when a frontbuffer invalidation happens, we can
> > enable the PSR2 selective fetch continuous full frame, that will keep
> > the panel updated like PSR was disabled but without keeping PSR
> > active.
> 
> with keeping PSR active? I don't think it's like PSR was disabled. New
> full frame is updated only via atomic commit. Having PSR disabled new
> full frame is updated all the time as PSR wasn't existing at all.
> 
> > 
> > So as soon as the frontbuffer flush happens we can disable the
> > continuous full frame and start to do selective fetches much quicker
> > than the path that would enable PSR, that will wait a few frames
> > to actually activate PSR.
> > 
> > Also this approach has proven to fix some glitches found in
> > Alderlake-P
> > when there are a lot of invalidations happening together with page
> > flips.
> > 
> > Some may ask why it is writing to CURSURFLIVE(), it is because
> > that is the way that hardware team provided us to poke display to
> > handle PSR updates, and it is being used since display 9.
> 
> Generic comments:
> 
> Current logic is to disable psr2 in invalidate callback and start
> sending fullframe updates on every vblank period. This is done until
> flush callback where psr2 is re-enabled. Intention is to update
> possible frontbuffer writes between invalidate/flush instantly.
> 
> Now you are changing the logic to update one full frame when

It is not enabling the one full frame, it is enabling the continuous full frame so at every vblank panel will be updated until this bit cleared.

> frontbuffer write starts (_psr_invalidate_handle) and another one when
> it stops (_psr_flush_handle) without disabling psr at all. Have I
> understood your patch correctly?
> 
> Propably we wont notice this change as we have these invalidate/flush
> calls scattered around in the code. Also parallel atomic commits are
> triggering updates. In theory we could observe latency in updates
> between invalidate/flush? Do we care? What do you think?
> 
> Do we need to send update in invalidate at all? Isn't that usually
> called before doing any frontbuffer writing? I.e. we would be sending
> frame that is already in RFB?
> 
> > 
> > Cc: Khaled Almahallawy <khaled.almahallawy@intel.com>
> > Cc: Shawn C Lee <shawn.c.lee@intel.com>
> > Cc: Jouni Högander <jouni.hogander@intel.com>
> > Cc: Mika Kahola <mika.kahola@intel.com>
> > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_psr.c | 109 ++++++++++++++++++++-
> > --
> >  1 file changed, 95 insertions(+), 14 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_psr.c
> > b/drivers/gpu/drm/i915/display/intel_psr.c
> > index d87b357806c91..f7b7b374374b1 100644
> > --- a/drivers/gpu/drm/i915/display/intel_psr.c
> > +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> > @@ -1450,6 +1450,22 @@ static inline u32
> > man_trk_ctl_partial_frame_bit_get(struct drm_i915_private *dev
> >              PSR2_MAN_TRK_CTL_SF_PARTIAL_FRAME_UPDATE;
> >  }
> > 
> > +static inline u32 man_trk_ctl_continuos_full_frame(struct
> > drm_i915_private *dev_priv)
> > +{
> > +     return IS_ALDERLAKE_P(dev_priv) ?
> > +            ADLP_PSR2_MAN_TRK_CTL_SF_CONTINUOS_FULL_FRAME :
> > +            PSR2_MAN_TRK_CTL_SF_CONTINUOS_FULL_FRAME;
> > +}
> > +
> > +static inline u32 man_trk_ctl_su_region_start_end_mask(struct
> > drm_i915_private *dev_priv)
> > +{
> > +     if (IS_ALDERLAKE_P(dev_priv))
> > +             return ADLP_PSR2_MAN_TRK_CTL_SU_REGION_START_ADDR_MASK
> > > 
> > +                    ADLP_PSR2_MAN_TRK_CTL_SU_REGION_END_ADDR_MASK;
> > +     return PSR2_MAN_TRK_CTL_SU_REGION_START_ADDR_MASK |
> > +            PSR2_MAN_TRK_CTL_SU_REGION_END_ADDR_MASK;
> > +}
> > +
> >  static void psr_force_hw_tracking_exit(struct intel_dp *intel_dp)
> >  {
> >       struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
> > @@ -1546,8 +1562,9 @@ void intel_psr2_program_trans_man_trk_ctl(const
> > struct intel_crtc_state *crtc_st
> >       if (!crtc_state->enable_psr2_sel_fetch)
> >               return;
> > 
> > -     intel_de_write(dev_priv, PSR2_MAN_TRK_CTL(crtc_state-
> > > cpu_transcoder),
> > -                    crtc_state->psr2_man_track_ctl);
> > +     intel_de_rmw(dev_priv, PSR2_MAN_TRK_CTL(crtc_state-
> > > cpu_transcoder),
> > +                  man_trk_ctl_su_region_start_end_mask(dev_priv),
> > +                  crtc_state->psr2_man_track_ctl);
> 
> Should we actually now consider taking psr->lock here?

I don't think we need. mmio writes are syncronized, this will never set continuous full frame and will only clear su region start and end.
Also this function is called from a time sensitive section if we spend too much time here it will cause vblank evasion warnings.

> 
> >  }
> > 
> >  static void psr2_man_trk_ctl_calc(struct intel_crtc_state
> > *crtc_state,
> > @@ -2127,6 +2144,26 @@ static void intel_psr_work(struct work_struct
> > *work)
> >       mutex_unlock(&intel_dp->psr.lock);
> >  }
> > 
> > +static void _psr_invalidate_handle(struct intel_dp *intel_dp,
> > +                                unsigned int
> > prev_busy_frontbuffer_bits)
> > +{
> > +     struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
> > +
> > +     if (intel_dp->psr.psr2_sel_fetch_enabled) {
> > +             u32 val = man_trk_ctl_continuos_full_frame(dev_priv) |
> > +                       man_trk_ctl_partial_frame_bit_get(dev_priv);
> > +
> > +             /* continuos full frame is already enabled */
> > +             if (prev_busy_frontbuffer_bits)
> > +                     return;
> 
> Should we still trigger the update using CURSURFLIVE? Or do we need
> that at all in the first place?

*continuos full frame*

> 
> > +
> > +             intel_de_rmw(dev_priv, PSR2_MAN_TRK_CTL(intel_dp-
> > > psr.transcoder), 0, val);
> > +             intel_de_write(dev_priv, CURSURFLIVE(intel_dp-
> > > psr.pipe), 0);
> 
> So these two register writes here are triggering one full frame update.
> and leaving full frame update bit set so that coming updates are also
> full frame. Did I understood it correctly?

*continuos full frame*

> 
> As invalidate is called before frontbuffer is writen: Isn't it actually
> re-updating same frame which is already supposed to be in panel RFB?
> 
> > +     } else {
> > +             intel_psr_exit(intel_dp);
> > +     }
> > +}
> > +
> >  /**
> >   * intel_psr_invalidate - Invalidade PSR
> >   * @dev_priv: i915 device
> > @@ -2151,6 +2188,7 @@ void intel_psr_invalidate(struct
> > drm_i915_private *dev_priv,
> >       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);
> > +             unsigned int prev_busy_frontbuffer_bits;
> > 
> >               mutex_lock(&intel_dp->psr.lock);
> >               if (!intel_dp->psr.enabled) {
> > @@ -2158,12 +2196,13 @@ void intel_psr_invalidate(struct
> > drm_i915_private *dev_priv,
> >                       continue;
> >               }
> > 
> > +             prev_busy_frontbuffer_bits = intel_dp-
> > > psr.busy_frontbuffer_bits;
> >               pipe_frontbuffer_bits &=
> >                       INTEL_FRONTBUFFER_ALL_MASK(intel_dp->psr.pipe);
> >               intel_dp->psr.busy_frontbuffer_bits |=
> > pipe_frontbuffer_bits;
> > 
> >               if (pipe_frontbuffer_bits)
> > -                     intel_psr_exit(intel_dp);
> > +                     _psr_invalidate_handle(intel_dp,
> > prev_busy_frontbuffer_bits);
> > 
> >               mutex_unlock(&intel_dp->psr.lock);
> >       }
> > @@ -2195,6 +2234,49 @@ tgl_dc3co_flush_locked(struct intel_dp
> > *intel_dp, unsigned int frontbuffer_bits,
> >                        intel_dp->psr.dc3co_exit_delay);
> >  }
> > 
> > +static void _psr_flush_handle(struct intel_dp *intel_dp,
> > +                           unsigned int prev_busy_frontbuffer_bits)
> > +{
> > +     struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
> > +
> > +     if (intel_dp->psr.psr2_sel_fetch_enabled) {
> > +             /* is continuos full frame enabled? */
> > +             if (prev_busy_frontbuffer_bits) {
> > +                     /* it is, can we turn it off? */
> 
> As you are using this prev_busy_frontbuffer_bits only to check if
> "continuos full frame" is enabled and nothing else: Maybe you could
> just name it as it is e.g. bool cff_enabled or bool
> continuous_full_frame or...same comment in _psr_invalidate_handle. This
> would allow you to drop couple of comments.

prev_busy_frontbuffer_bits or a bool anyway works for me, if you think is easier to understand I can change to bool.

> 
> > +                     if (intel_dp->psr.busy_frontbuffer_bits == 0) {
> > +                             u32 clear =
> > man_trk_ctl_continuos_full_frame(dev_priv);
> > +                             u32 set =
> > man_trk_ctl_single_full_frame_bit_get(dev_priv) |
> > +                                       man_trk_ctl_partial_frame_bit
> > _get(dev_priv);
> 
> 
> > +
> > +                             /*
> > +                              * turn continuos full frame off and do
> > a single
> > +                              * full frame
> > +                              */
> > +                             intel_de_rmw(dev_priv,
> > +                                          PSR2_MAN_TRK_CTL(intel_dp-
> > > psr.transcoder),
> > +                                          clear, set);
> > +                             intel_de_write(dev_priv,
> > CURSURFLIVE(intel_dp->psr.pipe), 0);
> > +                     }
> > +             } else {
> > +                     /*
> > +                      * continuos full frame is disabled, only a
> > single full
> > +                      * frame is required
> > +                      */
> > +                     psr_force_hw_tracking_exit(intel_dp);
> > +             }
> > +     } else {
> > +             /*
> > +              * if prev_busy_frontbuffer_bits is set, it means that
> > PSR is
> > +              * inactive
> > +              */
> > +             if (prev_busy_frontbuffer_bits == 0)
> > +                     psr_force_hw_tracking_exit(intel_dp);
> > +
> > +             if (!intel_dp->psr.active && !intel_dp-
> > > psr.busy_frontbuffer_bits)
> > +                     schedule_work(&intel_dp->psr.work);
> > +     }
> > +}
> > +
> >  /**
> >   * intel_psr_flush - Flush PSR
> >   * @dev_priv: i915 device
> > @@ -2216,6 +2298,7 @@ void intel_psr_flush(struct drm_i915_private
> > *dev_priv,
> >       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);
> > +             unsigned int prev_busy_frontbuffer_bits;
> > 
> >               mutex_lock(&intel_dp->psr.lock);
> >               if (!intel_dp->psr.enabled) {
> > @@ -2223,6 +2306,7 @@ void intel_psr_flush(struct drm_i915_private
> > *dev_priv,
> >                       continue;
> >               }
> > 
> > +             prev_busy_frontbuffer_bits = intel_dp-
> > > psr.busy_frontbuffer_bits;
> >               pipe_frontbuffer_bits &=
> >                       INTEL_FRONTBUFFER_ALL_MASK(intel_dp->psr.pipe);
> >               intel_dp->psr.busy_frontbuffer_bits &=
> > ~pipe_frontbuffer_bits;
> > @@ -2232,25 +2316,22 @@ void intel_psr_flush(struct drm_i915_private
> > *dev_priv,
> >                * we have to ensure that the PSR is not activated
> > until
> >                * intel_psr_resume() is called.
> >                */
> > -             if (intel_dp->psr.paused) {
> > -                     mutex_unlock(&intel_dp->psr.lock);
> > -                     continue;
> > -             }
> > +             if (intel_dp->psr.paused)
> > +                     goto exit;
> > 
> >               if (origin == ORIGIN_FLIP ||
> >                   (origin == ORIGIN_CURSOR_UPDATE &&
> >                    !intel_dp->psr.psr2_sel_fetch_enabled)) {
> >                       tgl_dc3co_flush_locked(intel_dp,
> > frontbuffer_bits, origin);
> > -                     mutex_unlock(&intel_dp->psr.lock);
> > -                     continue;
> > +                     goto exit;
> >               }
> > 
> > -             /* By definition flush = invalidate + flush */
> > -             if (pipe_frontbuffer_bits)
> > -                     psr_force_hw_tracking_exit(intel_dp);
> > +             if (pipe_frontbuffer_bits == 0)
> > +                     goto exit;
> > 
> > -             if (!intel_dp->psr.active && !intel_dp-
> > > psr.busy_frontbuffer_bits)
> > -                     schedule_work(&intel_dp->psr.work);
> > +             /* By definition flush = invalidate + flush */
> > +             _psr_flush_handle(intel_dp,
> > prev_busy_frontbuffer_bits);
> > +exit:
> 
> I think you should name it as unlock.
> 
> >               mutex_unlock(&intel_dp->psr.lock);
> >       }
> >  }
> 
> BR,
> 
> Jouni Högander
> 


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [Intel-gfx] [PATCH 2/2] drm/i915/display/psr: Use continuos full frame to handle frontbuffer invalidations
  2022-03-25 14:46     ` Souza, Jose
@ 2022-03-30  6:23       ` Hogander, Jouni
  0 siblings, 0 replies; 9+ messages in thread
From: Hogander, Jouni @ 2022-03-30  6:23 UTC (permalink / raw)
  To: intel-gfx, Souza, Jose

Hello Jose,

Thank you for your response. I'm sorry for the delay on my side. See my
comment below.
 
On Fri, 2022-03-25 at 14:46 +0000, Souza, Jose wrote:
> On Fri, 2022-03-25 at 14:21 +0000, Hogander, Jouni wrote:
> > Hello Jose,
> > 
> > See my comments below.
> > 
> > On Thu, 2022-03-24 at 11:13 -0700, José Roberto de Souza wrote:
> > > Instead of exit PSR when a frontbuffer invalidation happens, we
> > > can
> > > enable the PSR2 selective fetch continuous full frame, that will
> > > keep
> > > the panel updated like PSR was disabled but without keeping PSR
> > > active.
> > 
> > with keeping PSR active? I don't think it's like PSR was disabled.
> > New
> > full frame is updated only via atomic commit. Having PSR disabled
> > new
> > full frame is updated all the time as PSR wasn't existing at all.
> > 
> > > So as soon as the frontbuffer flush happens we can disable the
> > > continuous full frame and start to do selective fetches much
> > > quicker
> > > than the path that would enable PSR, that will wait a few frames
> > > to actually activate PSR.
> > > 
> > > Also this approach has proven to fix some glitches found in
> > > Alderlake-P
> > > when there are a lot of invalidations happening together with
> > > page
> > > flips.
> > > 
> > > Some may ask why it is writing to CURSURFLIVE(), it is because
> > > that is the way that hardware team provided us to poke display to
> > > handle PSR updates, and it is being used since display 9.
> > 
> > Generic comments:
> > 
> > Current logic is to disable psr2 in invalidate callback and start
> > sending fullframe updates on every vblank period. This is done
> > until
> > flush callback where psr2 is re-enabled. Intention is to update
> > possible frontbuffer writes between invalidate/flush instantly.
> > 
> > Now you are changing the logic to update one full frame when
> 
> It is not enabling the one full frame, it is enabling the continuous
> full frame so at every vblank panel will be updated until this bit
> cleared.

There is comment in bspec stating otherwise (55229). I think it is
important to clarify so that we really understand what this WA is doing
and to avoid hiding the issue just for this specific case.

Currently I'm suspecting that "SF Continuous Full Frame" just
configures next selective update to full frame and then write to
CURSURFLIVE triggers flip which is sending that full frame (only one
frame). Updates between invalidate and flush are just depending if
there are flips in between. After getting this clarified we can
properly review this WA patch.

> 
> > frontbuffer write starts (_psr_invalidate_handle) and another one
> > when
> > it stops (_psr_flush_handle) without disabling psr at all. Have I
> > understood your patch correctly?
> > 
> > Propably we wont notice this change as we have these
> > invalidate/flush
> > calls scattered around in the code. Also parallel atomic commits
> > are
> > triggering updates. In theory we could observe latency in updates
> > between invalidate/flush? Do we care? What do you think?
> > 
> > Do we need to send update in invalidate at all? Isn't that usually
> > called before doing any frontbuffer writing? I.e. we would be
> > sending
> > frame that is already in RFB?
> > 
> > > Cc: Khaled Almahallawy <khaled.almahallawy@intel.com>
> > > Cc: Shawn C Lee <shawn.c.lee@intel.com>
> > > Cc: Jouni Högander <jouni.hogander@intel.com>
> > > Cc: Mika Kahola <mika.kahola@intel.com>
> > > Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/display/intel_psr.c | 109
> > > ++++++++++++++++++++-
> > > --
> > >  1 file changed, 95 insertions(+), 14 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/display/intel_psr.c
> > > b/drivers/gpu/drm/i915/display/intel_psr.c
> > > index d87b357806c91..f7b7b374374b1 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_psr.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> > > @@ -1450,6 +1450,22 @@ static inline u32
> > > man_trk_ctl_partial_frame_bit_get(struct drm_i915_private *dev
> > >              PSR2_MAN_TRK_CTL_SF_PARTIAL_FRAME_UPDATE;
> > >  }
> > > 
> > > +static inline u32 man_trk_ctl_continuos_full_frame(struct
> > > drm_i915_private *dev_priv)
> > > +{
> > > +     return IS_ALDERLAKE_P(dev_priv) ?
> > > +            ADLP_PSR2_MAN_TRK_CTL_SF_CONTINUOS_FULL_FRAME :
> > > +            PSR2_MAN_TRK_CTL_SF_CONTINUOS_FULL_FRAME;
> > > +}
> > > +
> > > +static inline u32 man_trk_ctl_su_region_start_end_mask(struct
> > > drm_i915_private *dev_priv)
> > > +{
> > > +     if (IS_ALDERLAKE_P(dev_priv))
> > > +             return
> > > ADLP_PSR2_MAN_TRK_CTL_SU_REGION_START_ADDR_MASK
> > > +                    ADLP_PSR2_MAN_TRK_CTL_SU_REGION_END_ADDR_MAS
> > > K;
> > > +     return PSR2_MAN_TRK_CTL_SU_REGION_START_ADDR_MASK |
> > > +            PSR2_MAN_TRK_CTL_SU_REGION_END_ADDR_MASK;
> > > +}
> > > +
> > >  static void psr_force_hw_tracking_exit(struct intel_dp
> > > *intel_dp)
> > >  {
> > >       struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
> > > @@ -1546,8 +1562,9 @@ void
> > > intel_psr2_program_trans_man_trk_ctl(const
> > > struct intel_crtc_state *crtc_st
> > >       if (!crtc_state->enable_psr2_sel_fetch)
> > >               return;
> > > 
> > > -     intel_de_write(dev_priv, PSR2_MAN_TRK_CTL(crtc_state-
> > > > cpu_transcoder),
> > > -                    crtc_state->psr2_man_track_ctl);
> > > +     intel_de_rmw(dev_priv, PSR2_MAN_TRK_CTL(crtc_state-
> > > > cpu_transcoder),
> > > +                  man_trk_ctl_su_region_start_end_mask(dev_priv)
> > > ,
> > > +                  crtc_state->psr2_man_track_ctl);
> > 
> > Should we actually now consider taking psr->lock here?
> 
> I don't think we need. mmio writes are syncronized, this will never
> set continuous full frame and will only clear su region start and
> end.
> Also this function is called from a time sensitive section if we
> spend too much time here it will cause vblank evasion warnings.
> 

Ok, how about hitting flush/invalidate in between read/write in
intel_de_rmw? Is that possible?

> > >  }
> > > 
> > >  static void psr2_man_trk_ctl_calc(struct intel_crtc_state
> > > *crtc_state,
> > > @@ -2127,6 +2144,26 @@ static void intel_psr_work(struct
> > > work_struct
> > > *work)
> > >       mutex_unlock(&intel_dp->psr.lock);
> > >  }
> > > 
> > > +static void _psr_invalidate_handle(struct intel_dp *intel_dp,
> > > +                                unsigned int
> > > prev_busy_frontbuffer_bits)
> > > +{
> > > +     struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
> > > +
> > > +     if (intel_dp->psr.psr2_sel_fetch_enabled) {
> > > +             u32 val =
> > > man_trk_ctl_continuos_full_frame(dev_priv) |
> > > +                       man_trk_ctl_partial_frame_bit_get(dev_pri
> > > v);
> > > +
> > > +             /* continuos full frame is already enabled */
> > > +             if (prev_busy_frontbuffer_bits)
> > > +                     return;
> > 
> > Should we still trigger the update using CURSURFLIVE? Or do we need
> > that at all in the first place?
> 
> *continuos full frame*
> 
> > > +
> > > +             intel_de_rmw(dev_priv, PSR2_MAN_TRK_CTL(intel_dp-
> > > > psr.transcoder), 0, val);
> > > +             intel_de_write(dev_priv, CURSURFLIVE(intel_dp-
> > > > psr.pipe), 0);
> > 
> > So these two register writes here are triggering one full frame
> > update.
> > and leaving full frame update bit set so that coming updates are
> > also
> > full frame. Did I understood it correctly?
> 
> *continuos full frame*
> 
> > As invalidate is called before frontbuffer is writen: Isn't it
> > actually
> > re-updating same frame which is already supposed to be in panel
> > RFB?
> > 
> > > +     } else {
> > > +             intel_psr_exit(intel_dp);
> > > +     }
> > > +}
> > > +
> > >  /**
> > >   * intel_psr_invalidate - Invalidade PSR
> > >   * @dev_priv: i915 device
> > > @@ -2151,6 +2188,7 @@ void intel_psr_invalidate(struct
> > > drm_i915_private *dev_priv,
> > >       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);
> > > +             unsigned int prev_busy_frontbuffer_bits;
> > > 
> > >               mutex_lock(&intel_dp->psr.lock);
> > >               if (!intel_dp->psr.enabled) {
> > > @@ -2158,12 +2196,13 @@ void intel_psr_invalidate(struct
> > > drm_i915_private *dev_priv,
> > >                       continue;
> > >               }
> > > 
> > > +             prev_busy_frontbuffer_bits = intel_dp-
> > > > psr.busy_frontbuffer_bits;
> > >               pipe_frontbuffer_bits &=
> > >                       INTEL_FRONTBUFFER_ALL_MASK(intel_dp-
> > > >psr.pipe);
> > >               intel_dp->psr.busy_frontbuffer_bits |=
> > > pipe_frontbuffer_bits;
> > > 
> > >               if (pipe_frontbuffer_bits)
> > > -                     intel_psr_exit(intel_dp);
> > > +                     _psr_invalidate_handle(intel_dp,
> > > prev_busy_frontbuffer_bits);
> > > 
> > >               mutex_unlock(&intel_dp->psr.lock);
> > >       }
> > > @@ -2195,6 +2234,49 @@ tgl_dc3co_flush_locked(struct intel_dp
> > > *intel_dp, unsigned int frontbuffer_bits,
> > >                        intel_dp->psr.dc3co_exit_delay);
> > >  }
> > > 
> > > +static void _psr_flush_handle(struct intel_dp *intel_dp,
> > > +                           unsigned int
> > > prev_busy_frontbuffer_bits)
> > > +{
> > > +     struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
> > > +
> > > +     if (intel_dp->psr.psr2_sel_fetch_enabled) {
> > > +             /* is continuos full frame enabled? */
> > > +             if (prev_busy_frontbuffer_bits) {
> > > +                     /* it is, can we turn it off? */
> > 
> > As you are using this prev_busy_frontbuffer_bits only to check if
> > "continuos full frame" is enabled and nothing else: Maybe you could
> > just name it as it is e.g. bool cff_enabled or bool
> > continuous_full_frame or...same comment in _psr_invalidate_handle.
> > This
> > would allow you to drop couple of comments.
> 
> prev_busy_frontbuffer_bits or a bool anyway works for me, if you
> think is easier to understand I can change to bool.

To my opinion it actually helps understanding the code. This was just
idea. I'm not insisting to change if you think current one is better.

> 
> > > +                     if (intel_dp->psr.busy_frontbuffer_bits ==
> > > 0) {
> > > +                             u32 clear =
> > > man_trk_ctl_continuos_full_frame(dev_priv);
> > > +                             u32 set =
> > > man_trk_ctl_single_full_frame_bit_get(dev_priv) |
> > > +                                       man_trk_ctl_partial_frame
> > > _bit
> > > _get(dev_priv);
> > > +
> > > +                             /*
> > > +                              * turn continuos full frame off
> > > and do
> > > a single
> > > +                              * full frame
> > > +                              */
> > > +                             intel_de_rmw(dev_priv,
> > > +                                          PSR2_MAN_TRK_CTL(intel
> > > _dp-
> > > > psr.transcoder),
> > > +                                          clear, set);
> > > +                             intel_de_write(dev_priv,
> > > CURSURFLIVE(intel_dp->psr.pipe), 0);
> > > +                     }
> > > +             } else {
> > > +                     /*
> > > +                      * continuos full frame is disabled, only a
> > > single full
> > > +                      * frame is required
> > > +                      */
> > > +                     psr_force_hw_tracking_exit(intel_dp);
> > > +             }
> > > +     } else {
> > > +             /*
> > > +              * if prev_busy_frontbuffer_bits is set, it means
> > > that
> > > PSR is
> > > +              * inactive
> > > +              */
> > > +             if (prev_busy_frontbuffer_bits == 0)
> > > +                     psr_force_hw_tracking_exit(intel_dp);
> > > +
> > > +             if (!intel_dp->psr.active && !intel_dp-
> > > > psr.busy_frontbuffer_bits)
> > > +                     schedule_work(&intel_dp->psr.work);
> > > +     }
> > > +}
> > > +
> > >  /**
> > >   * intel_psr_flush - Flush PSR
> > >   * @dev_priv: i915 device
> > > @@ -2216,6 +2298,7 @@ void intel_psr_flush(struct
> > > drm_i915_private
> > > *dev_priv,
> > >       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);
> > > +             unsigned int prev_busy_frontbuffer_bits;
> > > 
> > >               mutex_lock(&intel_dp->psr.lock);
> > >               if (!intel_dp->psr.enabled) {
> > > @@ -2223,6 +2306,7 @@ void intel_psr_flush(struct
> > > drm_i915_private
> > > *dev_priv,
> > >                       continue;
> > >               }
> > > 
> > > +             prev_busy_frontbuffer_bits = intel_dp-
> > > > psr.busy_frontbuffer_bits;
> > >               pipe_frontbuffer_bits &=
> > >                       INTEL_FRONTBUFFER_ALL_MASK(intel_dp-
> > > >psr.pipe);
> > >               intel_dp->psr.busy_frontbuffer_bits &=
> > > ~pipe_frontbuffer_bits;
> > > @@ -2232,25 +2316,22 @@ void intel_psr_flush(struct
> > > drm_i915_private
> > > *dev_priv,
> > >                * we have to ensure that the PSR is not activated
> > > until
> > >                * intel_psr_resume() is called.
> > >                */
> > > -             if (intel_dp->psr.paused) {
> > > -                     mutex_unlock(&intel_dp->psr.lock);
> > > -                     continue;
> > > -             }
> > > +             if (intel_dp->psr.paused)
> > > +                     goto exit;
> > > 
> > >               if (origin == ORIGIN_FLIP ||
> > >                   (origin == ORIGIN_CURSOR_UPDATE &&
> > >                    !intel_dp->psr.psr2_sel_fetch_enabled)) {
> > >                       tgl_dc3co_flush_locked(intel_dp,
> > > frontbuffer_bits, origin);
> > > -                     mutex_unlock(&intel_dp->psr.lock);
> > > -                     continue;
> > > +                     goto exit;
> > >               }
> > > 
> > > -             /* By definition flush = invalidate + flush */
> > > -             if (pipe_frontbuffer_bits)
> > > -                     psr_force_hw_tracking_exit(intel_dp);
> > > +             if (pipe_frontbuffer_bits == 0)
> > > +                     goto exit;
> > > 
> > > -             if (!intel_dp->psr.active && !intel_dp-
> > > > psr.busy_frontbuffer_bits)
> > > -                     schedule_work(&intel_dp->psr.work);
> > > +             /* By definition flush = invalidate + flush */
> > > +             _psr_flush_handle(intel_dp,
> > > prev_busy_frontbuffer_bits);
> > > +exit:
> > 
> > I think you should name it as unlock.
> > 
> > >               mutex_unlock(&intel_dp->psr.lock);
> > >       }
> > >  }
> > 
> > BR,
> > 
> > Jouni Högander
> > 


^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2022-03-30  6:24 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-03-24 18:13 [Intel-gfx] [PATCH 1/2] drm/i915/display/psr: Set partial frame enable when forcing full frame fetch José Roberto de Souza
2022-03-24 18:13 ` [Intel-gfx] [PATCH 2/2] drm/i915/display/psr: Use continuos full frame to handle frontbuffer invalidations José Roberto de Souza
2022-03-25 14:21   ` Hogander, Jouni
2022-03-25 14:46     ` Souza, Jose
2022-03-30  6:23       ` Hogander, Jouni
2022-03-24 18:59 ` [Intel-gfx] ✗ Fi.CI.DOCS: warning for series starting with [1/2] drm/i915/display/psr: Set partial frame enable when forcing full frame fetch Patchwork
2022-03-24 19:26 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
2022-03-24 21:48 ` [Intel-gfx] ✗ Fi.CI.IGT: failure " Patchwork
2022-03-25  9:04 ` [Intel-gfx] [PATCH 1/2] " Hogander, Jouni

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.