[Intel-gfx] [PATCH v2 0/3] High refresh rate PSR fixes

[Intel-gfx] [PATCH v2 0/3] High refresh rate PSR fixes

[Jouni Högander]

Fix/adjust Wa_16013835468 and Wa_14015648006. Implement Wa_1136 and
check for vblank being long enough for psr2.

v2: Implement Wa_1136

Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Stanislav Lisovskiy <stanislav.lisovskiy@intel.com>
Cc: Mika Kahola <mika.kahola@intel.com>

Jouni Högander (3):
  drm/i915/psr: Fix Wa_16013835468 and Wa_14015648006
  drm/i915/psr: Check that vblank is long enough for psr2
  drm/i915/psr: Implement Wa_1136

 .../drm/i915/display/intel_display_types.h    |  1 +
 drivers/gpu/drm/i915/display/intel_psr.c      | 35 +++++++++++++++----
 drivers/gpu/drm/i915/display/skl_watermark.c  |  6 ++--
 3 files changed, 31 insertions(+), 11 deletions(-)

-- 
2.34.1

[Intel-gfx] [PATCH v2 1/3] drm/i915/psr: Fix Wa_16013835468 and Wa_14015648006

[Jouni Högander]

PSR WM optimization should be disabled based on any wm level being
disabled. Currently it's disabled always when using delayed vblank.

Bspec: 71580

Signed-off-by: Jouni Högander <jouni.hogander@intel.com>
---
 drivers/gpu/drm/i915/display/intel_display_types.h |  1 +
 drivers/gpu/drm/i915/display/intel_psr.c           | 12 +++++-------
 drivers/gpu/drm/i915/display/skl_watermark.c       |  7 +++++--
 3 files changed, 11 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
index c32bfba06ca1..60504c390408 100644
--- a/drivers/gpu/drm/i915/display/intel_display_types.h
+++ b/drivers/gpu/drm/i915/display/intel_display_types.h
@@ -1152,6 +1152,7 @@ struct intel_crtc_state {
 	bool has_psr2;
 	bool enable_psr2_sel_fetch;
 	bool req_psr2_sdp_prior_scanline;
+	bool wm_level_disabled;
 	u32 dc3co_exitline;
 	u16 su_y_granularity;
 	struct drm_dp_vsc_sdp psr_vsc;
diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c
index 44610b20cd29..a6edd65b8edb 100644
--- a/drivers/gpu/drm/i915/display/intel_psr.c
+++ b/drivers/gpu/drm/i915/display/intel_psr.c
@@ -1177,13 +1177,11 @@ static void intel_psr_enable_source(struct intel_dp *intel_dp,
 	 * Wa_16013835468
 	 * Wa_14015648006
 	 */
-	if (IS_MTL_DISPLAY_STEP(dev_priv, STEP_A0, STEP_B0) ||
-	    IS_DISPLAY_VER(dev_priv, 12, 13)) {
-		if (crtc_state->hw.adjusted_mode.crtc_vblank_start !=
-		    crtc_state->hw.adjusted_mode.crtc_vdisplay)
-			intel_de_rmw(dev_priv, GEN8_CHICKEN_DCPR_1, 0,
-				     wa_16013835468_bit_get(intel_dp));
-	}
+	if ((IS_MTL_DISPLAY_STEP(dev_priv, STEP_A0, STEP_B0) ||
+	     IS_DISPLAY_VER(dev_priv, 12, 13)) &&
+	    crtc_state->wm_level_disabled)
+		intel_de_rmw(dev_priv, GEN8_CHICKEN_DCPR_1, 0,
+			     wa_16013835468_bit_get(intel_dp));
 
 	if (intel_dp->psr.psr2_enabled) {
 		if (DISPLAY_VER(dev_priv) == 9)
diff --git a/drivers/gpu/drm/i915/display/skl_watermark.c b/drivers/gpu/drm/i915/display/skl_watermark.c
index 50a9a6adbe32..afb751c024ba 100644
--- a/drivers/gpu/drm/i915/display/skl_watermark.c
+++ b/drivers/gpu/drm/i915/display/skl_watermark.c
@@ -2273,9 +2273,12 @@ static int skl_wm_check_vblank(struct intel_crtc_state *crtc_state)
 		return level;
 
 	/*
-	 * FIXME PSR needs to toggle LATENCY_REPORTING_REMOVED_PIPE_*
+	 * PSR needs to toggle LATENCY_REPORTING_REMOVED_PIPE_*
 	 * based on whether we're limited by the vblank duration.
-	 *
+	 */
+	crtc_state->wm_level_disabled = level < i915->display.wm.num_levels - 1;
+
+	/*
 	 * FIXME also related to skl+ w/a 1136 (also unimplemented as of
 	 * now) perhaps?
 	 */
-- 
2.34.1

[Intel-gfx] [PATCH v2 2/3] drm/i915/psr: Check that vblank is long enough for psr2

[Jouni Högander]

Ensure vblank >= psr2 vblank
where
Psr2 vblank = PSR2_CTL Block Count Number maximum line count.

Bspec: 71580, 49274

Signed-off-by: Jouni Högander <jouni.hogander@intel.com>
---
 drivers/gpu/drm/i915/display/intel_psr.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c
index a6edd65b8edb..a385cb8dbf13 100644
--- a/drivers/gpu/drm/i915/display/intel_psr.c
+++ b/drivers/gpu/drm/i915/display/intel_psr.c
@@ -958,6 +958,14 @@ static bool intel_psr2_config_valid(struct intel_dp *intel_dp,
 		return false;
 	}
 
+	/* Vblank >= PSR2_CTL Block Count Number maximum line count */
+	if (crtc_state->hw.adjusted_mode.crtc_vblank_end -
+	    crtc_state->hw.adjusted_mode.crtc_vblank_start < 12) {
+		drm_dbg_kms(&dev_priv->drm,
+			    "PSR2 not enabled, too short vblank time\n");
+		return false;
+	}
+
 	if (HAS_PSR2_SEL_FETCH(dev_priv)) {
 		if (!intel_psr2_sel_fetch_config_valid(intel_dp, crtc_state) &&
 		    !HAS_PSR_HW_TRACKING(dev_priv)) {
-- 
2.34.1

[Intel-gfx] [PATCH v2 3/3] drm/i915/psr: Implement Wa_1136

[Jouni Högander]

Implement Wa_1136 for SKL/BXT/ICL.

Bspec: 21664

Signed-off-by: Jouni Högander <jouni.hogander@intel.com>
---
 drivers/gpu/drm/i915/display/intel_psr.c     | 15 +++++++++++++++
 drivers/gpu/drm/i915/display/skl_watermark.c |  5 -----
 2 files changed, 15 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c
index a385cb8dbf13..e6bd46441392 100644
--- a/drivers/gpu/drm/i915/display/intel_psr.c
+++ b/drivers/gpu/drm/i915/display/intel_psr.c
@@ -1049,6 +1049,13 @@ void intel_psr_compute_config(struct intel_dp *intel_dp,
 		return;
 	}
 
+	/* Wa_1136 */
+	if (DISPLAY_VER(dev_priv) < 12 && crtc_state->wm_level_disabled) {
+		drm_dbg_kms(&dev_priv->drm,
+			    "PSR condition failed: WM level disabled and no HW WA available\n");
+		return;
+	}
+
 	crtc_state->has_psr = true;
 	crtc_state->has_psr2 = intel_psr2_config_valid(intel_dp, crtc_state);
 
@@ -1260,6 +1267,10 @@ static void intel_psr_enable_locked(struct intel_dp *intel_dp,
 
 	drm_WARN_ON(&dev_priv->drm, intel_dp->psr.enabled);
 
+	/* Wa_1136 */
+	if (DISPLAY_VER(dev_priv) < 12 && crtc_state->wm_level_disabled)
+		return;
+
 	intel_dp->psr.psr2_enabled = crtc_state->has_psr2;
 	intel_dp->psr.busy_frontbuffer_bits = 0;
 	intel_dp->psr.pipe = to_intel_crtc(crtc_state->uapi.crtc)->pipe;
@@ -1940,6 +1951,10 @@ void intel_psr_pre_plane_update(struct intel_atomic_state *state,
 		needs_to_disable |= !new_crtc_state->active_planes;
 		needs_to_disable |= new_crtc_state->has_psr2 != psr->psr2_enabled;
 
+		/* Wa_1136 */
+		needs_to_disable |= DISPLAY_VER(i915) < 12 &&
+			new_crtc_state->wm_level_disabled;
+
 		if (psr->enabled && needs_to_disable)
 			intel_psr_disable_locked(intel_dp);
 
diff --git a/drivers/gpu/drm/i915/display/skl_watermark.c b/drivers/gpu/drm/i915/display/skl_watermark.c
index afb751c024ba..ced61da8b496 100644
--- a/drivers/gpu/drm/i915/display/skl_watermark.c
+++ b/drivers/gpu/drm/i915/display/skl_watermark.c
@@ -2278,11 +2278,6 @@ static int skl_wm_check_vblank(struct intel_crtc_state *crtc_state)
 	 */
 	crtc_state->wm_level_disabled = level < i915->display.wm.num_levels - 1;
 
-	/*
-	 * FIXME also related to skl+ w/a 1136 (also unimplemented as of
-	 * now) perhaps?
-	 */
-
 	for (level++; level < i915->display.wm.num_levels; level++) {
 		enum plane_id plane_id;
 
-- 
2.34.1

Re: [Intel-gfx] [PATCH v2 3/3] drm/i915/psr: Implement Wa_1136

[Ville Syrjälä]

On Fri, Mar 17, 2023 at 01:04:37PM +0200, Jouni Högander wrote:
> Implement Wa_1136 for SKL/BXT/ICL.
> 
> Bspec: 21664
> 
> Signed-off-by: Jouni Högander <jouni.hogander@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_psr.c     | 15 +++++++++++++++
>  drivers/gpu/drm/i915/display/skl_watermark.c |  5 -----
>  2 files changed, 15 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c
> index a385cb8dbf13..e6bd46441392 100644
> --- a/drivers/gpu/drm/i915/display/intel_psr.c
> +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> @@ -1049,6 +1049,13 @@ void intel_psr_compute_config(struct intel_dp *intel_dp,
>  		return;
>  	}
>  
> +	/* Wa_1136 */

The syntax we've used for the old w/as is different.

> +	if (DISPLAY_VER(dev_priv) < 12 && crtc_state->wm_level_disabled) {

This won't have been calculated yet.

As for the platform check. I think the one hsd we still have left
indicates that icl already got some kind of full fix. So probably
that should at least be safe. And I do think the KBL+ should also
work fine.

But we could do that as followups:
1. do this
2. switch to the chicken bit approach for icl
3. switch to the chicken bit approach for kbl+

Then of any issue later come up that point to a problem
with the chicken bits we could more easily revert to full
psr disable.

> +		drm_dbg_kms(&dev_priv->drm,
> +			    "PSR condition failed: WM level disabled and no HW WA available\n");
> +		return;
> +	}
> +
>  	crtc_state->has_psr = true;
>  	crtc_state->has_psr2 = intel_psr2_config_valid(intel_dp, crtc_state);
>  
> @@ -1260,6 +1267,10 @@ static void intel_psr_enable_locked(struct intel_dp *intel_dp,
>  
>  	drm_WARN_ON(&dev_priv->drm, intel_dp->psr.enabled);
>  
> +	/* Wa_1136 */
> +	if (DISPLAY_VER(dev_priv) < 12 && crtc_state->wm_level_disabled)

It's a bit weird to handle this differently than the active_planes case.
Though the fact that the pre and post updatre hooks also do things
in different ways is also confusing. Seems to me some general cleanup
in this area could be worthwile.

> +		return;
> +
>  	intel_dp->psr.psr2_enabled = crtc_state->has_psr2;
>  	intel_dp->psr.busy_frontbuffer_bits = 0;
>  	intel_dp->psr.pipe = to_intel_crtc(crtc_state->uapi.crtc)->pipe;
> @@ -1940,6 +1951,10 @@ void intel_psr_pre_plane_update(struct intel_atomic_state *state,
>  		needs_to_disable |= !new_crtc_state->active_planes;
>  		needs_to_disable |= new_crtc_state->has_psr2 != psr->psr2_enabled;
>  
> +		/* Wa_1136 */
> +		needs_to_disable |= DISPLAY_VER(i915) < 12 &&
> +			new_crtc_state->wm_level_disabled;
> +
>  		if (psr->enabled && needs_to_disable)
>  			intel_psr_disable_locked(intel_dp);
>  
> diff --git a/drivers/gpu/drm/i915/display/skl_watermark.c b/drivers/gpu/drm/i915/display/skl_watermark.c
> index afb751c024ba..ced61da8b496 100644
> --- a/drivers/gpu/drm/i915/display/skl_watermark.c
> +++ b/drivers/gpu/drm/i915/display/skl_watermark.c
> @@ -2278,11 +2278,6 @@ static int skl_wm_check_vblank(struct intel_crtc_state *crtc_state)
>  	 */
>  	crtc_state->wm_level_disabled = level < i915->display.wm.num_levels - 1;
>  
> -	/*
> -	 * FIXME also related to skl+ w/a 1136 (also unimplemented as of
> -	 * now) perhaps?
> -	 */
> -
>  	for (level++; level < i915->display.wm.num_levels; level++) {
>  		enum plane_id plane_id;
>  
> -- 
> 2.34.1

-- 
Ville Syrjälä
Intel

[Intel-gfx] ✗ Fi.CI.BUILD: failure for High refresh rate PSR fixes (rev2)

[Patchwork]

== Series Details ==

Series: High refresh rate PSR fixes (rev2)
URL   : https://patchwork.freedesktop.org/series/115109/
State : failure

== Summary ==

Error: patch https://patchwork.freedesktop.org/api/1.0/series/115109/revisions/2/mbox/ not applied
Committer identity unknown

*** Please tell me who you are.

Run

  git config --global user.email "you@example.com"
  git config --global user.name "Your Name"

to set your account's default identity.
Omit --global to set the identity only in this repository.

fatal: unable to auto-detect email address (got 'kbuild2@gfx-ci.(none)')
Build failed, no error log produced