All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] Revert "drm/i915/psr: Implement WA to help reach PC10"
@ 2024-09-30  7:13 Jouni Högander
  2024-09-30  7:20 ` ✓ CI.Patch_applied: success for " Patchwork
                   ` (5 more replies)
  0 siblings, 6 replies; 9+ messages in thread
From: Jouni Högander @ 2024-09-30  7:13 UTC (permalink / raw)
  To: intel-xe; +Cc: Jouni Högander

This reverts commit c2579a217799ba577fa39a2a12643a277334e691.

Reverting this commit as it is suspected being culprit on regression.

Signed-off-by: Jouni Högander <jouni.hogander@intel.com>
---
 .../drm/i915/display/intel_display_types.h    |   3 -
 drivers/gpu/drm/i915/display/intel_psr.c      | 119 +-----------------
 2 files changed, 1 insertion(+), 121 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
index 7ff97e5b83dd5..f60d66f4b980a 100644
--- a/drivers/gpu/drm/i915/display/intel_display_types.h
+++ b/drivers/gpu/drm/i915/display/intel_display_types.h
@@ -1576,9 +1576,6 @@ struct intel_psr {
 #define I915_PSR_DEBUG_PANEL_REPLAY_DISABLE	0x40
 
 	u32 debug;
-	bool is_dpkgc_configured;
-	bool is_dc5_entry_possible;
-	bool is_wa_delayed_vblank_limit;
 	bool sink_support;
 	bool source_support;
 	bool enabled;
diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c
index 39aa46de15e43..e3357f3b5c705 100644
--- a/drivers/gpu/drm/i915/display/intel_psr.c
+++ b/drivers/gpu/drm/i915/display/intel_psr.c
@@ -26,7 +26,6 @@
 #include <drm/drm_atomic_helper.h>
 #include <drm/drm_damage_helper.h>
 #include <drm/drm_debugfs.h>
-#include <drm/drm_vblank.h>
 
 #include "i915_drv.h"
 #include "i915_reg.h"
@@ -897,89 +896,6 @@ static u8 psr_compute_idle_frames(struct intel_dp *intel_dp)
 	return idle_frames;
 }
 
-static bool
-intel_psr_check_wa_delayed_vblank(const struct drm_display_mode *adjusted_mode)
-{
-	return (adjusted_mode->crtc_vblank_start - adjusted_mode->crtc_vdisplay) >= 6;
-}
-
-/*
- * PKG_C_LATENCY is configured only when DISPLAY_VER >= 20 and
- * VRR is not enabled
- */
-static bool intel_psr_is_dpkgc_configured(struct intel_display *display,
-					  struct intel_atomic_state *state)
-{
-	struct intel_crtc *intel_crtc;
-	struct intel_crtc_state *crtc_state;
-	int i;
-
-	if (DISPLAY_VER(display) < 20)
-		return false;
-
-	for_each_new_intel_crtc_in_state(state, intel_crtc, crtc_state, i) {
-		if (!intel_crtc->active)
-			continue;
-
-		if (crtc_state->vrr.enable)
-			return false;
-	}
-
-	return true;
-}
-
-static bool wa_22019444797_psr1_check(const struct intel_crtc_state *crtc_state,
-				      struct intel_psr *psr)
-{
-	struct intel_display *display = to_intel_display(crtc_state);
-
-	return DISPLAY_VER(display) == 20 && psr->is_dpkgc_configured &&
-		(psr->is_wa_delayed_vblank_limit || !psr->is_dc5_entry_possible) &&
-		!crtc_state->has_sel_update && !crtc_state->has_panel_replay;
-}
-
-/*
- * DC5 entry is only possible if vblank interrupt is disabled
- * and either psr1, psr2, edp 1.5 pr alpm is enabled on all
- * enabled encoders.
- */
-static bool
-intel_psr_is_dc5_entry_possible(struct intel_display *display,
-				struct intel_atomic_state *state)
-{
-	struct intel_crtc *intel_crtc;
-	struct intel_crtc_state *crtc_state;
-	int i;
-
-	if ((display->power.domains.target_dc_state &
-	     DC_STATE_EN_UPTO_DC5_DC6_MASK) == 0)
-		return false;
-
-	for_each_new_intel_crtc_in_state(state, intel_crtc, crtc_state, i) {
-		struct drm_crtc *crtc = &intel_crtc->base;
-		struct drm_vblank_crtc *vblank;
-		struct intel_encoder *encoder;
-
-		if (!intel_crtc->active)
-			continue;
-
-		vblank = drm_crtc_vblank_crtc(crtc);
-
-		if (vblank->enabled)
-			return false;
-
-		if (!crtc_state->has_psr)
-			return false;
-
-		for_each_encoder_on_crtc(display->drm, crtc, encoder)
-			if (encoder->type != INTEL_OUTPUT_EDP ||
-			    !CAN_PSR(enc_to_intel_dp(encoder)))
-				return false;
-	}
-
-	return true;
-}
-
 static void hsw_activate_psr1(struct intel_dp *intel_dp)
 {
 	struct intel_display *display = to_intel_display(intel_dp);
@@ -1092,15 +1008,7 @@ static void hsw_activate_psr2(struct intel_dp *intel_dp)
 	u32 val = EDP_PSR2_ENABLE;
 	u32 psr_val = 0;
 
-	/*
-	 * Wa_22019444797
-	 * TODO: Disable idle frames when vblank gets enabled while
-	 * PSR2 is enabled
-	 */
-	if (DISPLAY_VER(dev_priv) != 20 ||
-	    !intel_dp->psr.is_dpkgc_configured ||
-	    intel_dp->psr.is_dc5_entry_possible)
-		val |= EDP_PSR2_IDLE_FRAMES(psr_compute_idle_frames(intel_dp));
+	val |= EDP_PSR2_IDLE_FRAMES(psr_compute_idle_frames(intel_dp));
 
 	if (DISPLAY_VER(display) < 14 && !IS_ALDERLAKE_P(dev_priv))
 		val |= EDP_SU_TRACK_ENABLE;
@@ -2815,20 +2723,10 @@ void intel_psr_pre_plane_update(struct intel_atomic_state *state,
 	const struct intel_crtc_state *new_crtc_state =
 		intel_atomic_get_new_crtc_state(state, crtc);
 	struct intel_encoder *encoder;
-	bool dpkgc_configured = false, dc5_entry_possible = false;
-	bool wa_delayed_vblank_limit = false;
 
 	if (!HAS_PSR(display))
 		return;
 
-	if (DISPLAY_VER(display) == 20) {
-		dpkgc_configured = intel_psr_is_dpkgc_configured(display, state);
-		dc5_entry_possible =
-			intel_psr_is_dc5_entry_possible(display, state);
-		wa_delayed_vblank_limit =
-			intel_psr_check_wa_delayed_vblank(&new_crtc_state->hw.adjusted_mode);
-	}
-
 	for_each_intel_encoder_mask_with_psr(state->base.dev, encoder,
 					     old_crtc_state->uapi.encoder_mask) {
 		struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
@@ -2837,12 +2735,6 @@ void intel_psr_pre_plane_update(struct intel_atomic_state *state,
 
 		mutex_lock(&psr->lock);
 
-		if (DISPLAY_VER(i915) == 20) {
-			psr->is_dpkgc_configured = dpkgc_configured;
-			psr->is_dc5_entry_possible = dc5_entry_possible;
-			psr->is_wa_delayed_vblank_limit = wa_delayed_vblank_limit;
-		}
-
 		/*
 		 * Reasons to disable:
 		 * - PSR disabled in new state
@@ -2850,7 +2742,6 @@ void intel_psr_pre_plane_update(struct intel_atomic_state *state,
 		 * - Changing between PSR versions
 		 * - Region Early Transport changing
 		 * - Display WA #1136: skl, bxt
-		 * - Display WA_22019444797
 		 */
 		needs_to_disable |= intel_crtc_needs_modeset(new_crtc_state);
 		needs_to_disable |= !new_crtc_state->has_psr;
@@ -2860,8 +2751,6 @@ void intel_psr_pre_plane_update(struct intel_atomic_state *state,
 			psr->su_region_et_enabled;
 		needs_to_disable |= DISPLAY_VER(i915) < 11 &&
 			new_crtc_state->wm_level_disabled;
-		/* TODO: Disable PSR1 when vblank gets enabled while PSR1 is enabled */
-		needs_to_disable |= wa_22019444797_psr1_check(new_crtc_state, psr);
 
 		if (psr->enabled && needs_to_disable)
 			intel_psr_disable_locked(intel_dp);
@@ -2902,12 +2791,6 @@ void intel_psr_post_plane_update(struct intel_atomic_state *state,
 		keep_disabled |= DISPLAY_VER(display) < 11 &&
 			crtc_state->wm_level_disabled;
 
-		/*
-		 * Wa_22019444797
-		 * TODO: Disable PSR1 when vblank gets enabled while PSR1 is enabled
-		 */
-		keep_disabled |= wa_22019444797_psr1_check(crtc_state, psr);
-
 		if (!psr->enabled && !keep_disabled)
 			intel_psr_enable_locked(intel_dp, crtc_state);
 		else if (psr->enabled && !crtc_state->wm_level_disabled)
-- 
2.34.1


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

* ✓ CI.Patch_applied: success for Revert "drm/i915/psr: Implement WA to help reach PC10"
  2024-09-30  7:13 [PATCH] Revert "drm/i915/psr: Implement WA to help reach PC10" Jouni Högander
@ 2024-09-30  7:20 ` Patchwork
  2024-09-30  7:20 ` ✓ CI.checkpatch: " Patchwork
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 9+ messages in thread
From: Patchwork @ 2024-09-30  7:20 UTC (permalink / raw)
  To: Jouni Högander; +Cc: intel-xe

== Series Details ==

Series: Revert "drm/i915/psr: Implement WA to help reach PC10"
URL   : https://patchwork.freedesktop.org/series/139275/
State : success

== Summary ==

=== Applying kernel patches on branch 'drm-tip' with base: ===
Base commit: cd8c80767801 drm-tip: 2024y-09m-30d-06h-48m-45s UTC integration manifest
=== git am output follows ===
Applying: Revert "drm/i915/psr: Implement WA to help reach PC10"



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

* ✓ CI.checkpatch: success for Revert "drm/i915/psr: Implement WA to help reach PC10"
  2024-09-30  7:13 [PATCH] Revert "drm/i915/psr: Implement WA to help reach PC10" Jouni Högander
  2024-09-30  7:20 ` ✓ CI.Patch_applied: success for " Patchwork
@ 2024-09-30  7:20 ` Patchwork
  2024-09-30  7:20 ` ✗ CI.KUnit: failure " Patchwork
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 9+ messages in thread
From: Patchwork @ 2024-09-30  7:20 UTC (permalink / raw)
  To: Jouni Högander; +Cc: intel-xe

== Series Details ==

Series: Revert "drm/i915/psr: Implement WA to help reach PC10"
URL   : https://patchwork.freedesktop.org/series/139275/
State : success

== Summary ==

+ KERNEL=/kernel
+ git clone https://gitlab.freedesktop.org/drm/maintainer-tools mt
Cloning into 'mt'...
warning: redirecting to https://gitlab.freedesktop.org/drm/maintainer-tools.git/
+ git -C mt rev-list -n1 origin/master
30ab6715fc09baee6cc14cb3c89ad8858688d474
+ cd /kernel
+ git config --global --add safe.directory /kernel
+ git log -n1
commit decaca296c7e42c6780c40d76331bb8966415b28
Author: Jouni Högander <jouni.hogander@intel.com>
Date:   Mon Sep 30 10:13:29 2024 +0300

    Revert "drm/i915/psr: Implement WA to help reach PC10"
    
    This reverts commit c2579a217799ba577fa39a2a12643a277334e691.
    
    Reverting this commit as it is suspected being culprit on regression.
    
    Signed-off-by: Jouni Högander <jouni.hogander@intel.com>
+ /mt/dim checkpatch cd8c8076780100d1307482e68ec349113d797ead drm-intel
decaca296c7e Revert "drm/i915/psr: Implement WA to help reach PC10"



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

* ✗ CI.KUnit: failure for Revert "drm/i915/psr: Implement WA to help reach PC10"
  2024-09-30  7:13 [PATCH] Revert "drm/i915/psr: Implement WA to help reach PC10" Jouni Högander
  2024-09-30  7:20 ` ✓ CI.Patch_applied: success for " Patchwork
  2024-09-30  7:20 ` ✓ CI.checkpatch: " Patchwork
@ 2024-09-30  7:20 ` Patchwork
  2024-09-30 13:46 ` [PATCH] " Kandpal, Suraj
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 9+ messages in thread
From: Patchwork @ 2024-09-30  7:20 UTC (permalink / raw)
  To: Jouni Högander; +Cc: intel-xe

== Series Details ==

Series: Revert "drm/i915/psr: Implement WA to help reach PC10"
URL   : https://patchwork.freedesktop.org/series/139275/
State : failure

== Summary ==

+ trap cleanup EXIT
+ /kernel/tools/testing/kunit/kunit.py run --kunitconfig /kernel/drivers/gpu/drm/xe/.kunitconfig
[07:20:29] Configuring KUnit Kernel ...
Generating .config ...
Populating config with:
$ make ARCH=um O=.kunit olddefconfig
[07:20:33] Building KUnit Kernel ...
Populating config with:
$ make ARCH=um O=.kunit olddefconfig
Building with:
$ make all compile_commands.json ARCH=um O=.kunit --jobs=48
ERROR:root:../drivers/gpu/drm/xe/xe_gt_idle.c:56:27: error: redefinition of ‘str_up_down’
   56 | static inline const char *str_up_down(bool v)
      |                           ^~~~~~~~~~~
In file included from ../include/linux/string_helpers.h:7,
                 from ../drivers/gpu/drm/xe/xe_assert.h:9,
                 from ../drivers/gpu/drm/xe/xe_force_wake.h:9,
                 from ../drivers/gpu/drm/xe/xe_gt_idle.c:8:
../include/linux/string_choices.h:62:27: note: previous definition of ‘str_up_down’ with type ‘const char *(bool)’ {aka ‘const char *(_Bool)’}
   62 | static inline const char *str_up_down(bool v)
      |                           ^~~~~~~~~~~
make[7]: *** [../scripts/Makefile.build:229: drivers/gpu/drm/xe/xe_gt_idle.o] Error 1
make[7]: *** Waiting for unfinished jobs....
make[6]: *** [../scripts/Makefile.build:478: drivers/gpu/drm/xe] Error 2
make[6]: *** Waiting for unfinished jobs....
../lib/iomap.c:156:5: warning: no previous prototype for ‘ioread64_lo_hi’ [-Wmissing-prototypes]
  156 | u64 ioread64_lo_hi(const void __iomem *addr)
      |     ^~~~~~~~~~~~~~
../lib/iomap.c:163:5: warning: no previous prototype for ‘ioread64_hi_lo’ [-Wmissing-prototypes]
  163 | u64 ioread64_hi_lo(const void __iomem *addr)
      |     ^~~~~~~~~~~~~~
../lib/iomap.c:170:5: warning: no previous prototype for ‘ioread64be_lo_hi’ [-Wmissing-prototypes]
  170 | u64 ioread64be_lo_hi(const void __iomem *addr)
      |     ^~~~~~~~~~~~~~~~
../lib/iomap.c:178:5: warning: no previous prototype for ‘ioread64be_hi_lo’ [-Wmissing-prototypes]
  178 | u64 ioread64be_hi_lo(const void __iomem *addr)
      |     ^~~~~~~~~~~~~~~~
../lib/iomap.c:264:6: warning: no previous prototype for ‘iowrite64_lo_hi’ [-Wmissing-prototypes]
  264 | void iowrite64_lo_hi(u64 val, void __iomem *addr)
      |      ^~~~~~~~~~~~~~~
../lib/iomap.c:272:6: warning: no previous prototype for ‘iowrite64_hi_lo’ [-Wmissing-prototypes]
  272 | void iowrite64_hi_lo(u64 val, void __iomem *addr)
      |      ^~~~~~~~~~~~~~~
../lib/iomap.c:280:6: warning: no previous prototype for ‘iowrite64be_lo_hi’ [-Wmissing-prototypes]
  280 | void iowrite64be_lo_hi(u64 val, void __iomem *addr)
      |      ^~~~~~~~~~~~~~~~~
../lib/iomap.c:288:6: warning: no previous prototype for ‘iowrite64be_hi_lo’ [-Wmissing-prototypes]
  288 | void iowrite64be_hi_lo(u64 val, void __iomem *addr)
      |      ^~~~~~~~~~~~~~~~~
make[5]: *** [../scripts/Makefile.build:478: drivers/gpu/drm] Error 2
make[4]: *** [../scripts/Makefile.build:478: drivers/gpu] Error 2
make[3]: *** [../scripts/Makefile.build:478: drivers] Error 2
make[3]: *** Waiting for unfinished jobs....
make[2]: *** [/kernel/Makefile:1936: .] Error 2
make[1]: *** [/kernel/Makefile:224: __sub-make] Error 2
make: *** [Makefile:224: __sub-make] Error 2

+ cleanup
++ stat -c %u:%g /kernel
+ chown -R 1003:1003 /kernel



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

* RE: [PATCH] Revert "drm/i915/psr: Implement WA to help reach PC10"
  2024-09-30  7:13 [PATCH] Revert "drm/i915/psr: Implement WA to help reach PC10" Jouni Högander
                   ` (2 preceding siblings ...)
  2024-09-30  7:20 ` ✗ CI.KUnit: failure " Patchwork
@ 2024-09-30 13:46 ` Kandpal, Suraj
  2024-09-30 15:21 ` Rodrigo Vivi
  2024-09-30 17:26 ` ✗ CI.Patch_applied: failure for Revert "drm/i915/psr: Implement WA to help reach PC10" (rev2) Patchwork
  5 siblings, 0 replies; 9+ messages in thread
From: Kandpal, Suraj @ 2024-09-30 13:46 UTC (permalink / raw)
  To: Hogander, Jouni, intel-xe@lists.freedesktop.org; +Cc: Hogander, Jouni



> -----Original Message-----
> From: Intel-xe <intel-xe-bounces@lists.freedesktop.org> On Behalf Of Jouni
> Högander
> Sent: Monday, September 30, 2024 12:43 PM
> To: intel-xe@lists.freedesktop.org
> Cc: Hogander, Jouni <jouni.hogander@intel.com>
> Subject: [PATCH] Revert "drm/i915/psr: Implement WA to help reach PC10"
> 
> This reverts commit c2579a217799ba577fa39a2a12643a277334e691.
> 
> Reverting this commit as it is suspected being culprit on regression.

LGTM,
Reviewed-by: Suraj Kandpal <suraj.kandpal@intel.com>

> 
> Signed-off-by: Jouni Högander <jouni.hogander@intel.com>
> ---
>  .../drm/i915/display/intel_display_types.h    |   3 -
>  drivers/gpu/drm/i915/display/intel_psr.c      | 119 +-----------------
>  2 files changed, 1 insertion(+), 121 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h
> b/drivers/gpu/drm/i915/display/intel_display_types.h
> index 7ff97e5b83dd5..f60d66f4b980a 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> @@ -1576,9 +1576,6 @@ struct intel_psr {
>  #define I915_PSR_DEBUG_PANEL_REPLAY_DISABLE	0x40
> 
>  	u32 debug;
> -	bool is_dpkgc_configured;
> -	bool is_dc5_entry_possible;
> -	bool is_wa_delayed_vblank_limit;
>  	bool sink_support;
>  	bool source_support;
>  	bool enabled;
> diff --git a/drivers/gpu/drm/i915/display/intel_psr.c
> b/drivers/gpu/drm/i915/display/intel_psr.c
> index 39aa46de15e43..e3357f3b5c705 100644
> --- a/drivers/gpu/drm/i915/display/intel_psr.c
> +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> @@ -26,7 +26,6 @@
>  #include <drm/drm_atomic_helper.h>
>  #include <drm/drm_damage_helper.h>
>  #include <drm/drm_debugfs.h>
> -#include <drm/drm_vblank.h>
> 
>  #include "i915_drv.h"
>  #include "i915_reg.h"
> @@ -897,89 +896,6 @@ static u8 psr_compute_idle_frames(struct intel_dp
> *intel_dp)
>  	return idle_frames;
>  }
> 
> -static bool
> -intel_psr_check_wa_delayed_vblank(const struct drm_display_mode
> *adjusted_mode) -{
> -	return (adjusted_mode->crtc_vblank_start - adjusted_mode-
> >crtc_vdisplay) >= 6;
> -}
> -
> -/*
> - * PKG_C_LATENCY is configured only when DISPLAY_VER >= 20 and
> - * VRR is not enabled
> - */
> -static bool intel_psr_is_dpkgc_configured(struct intel_display *display,
> -					  struct intel_atomic_state *state)
> -{
> -	struct intel_crtc *intel_crtc;
> -	struct intel_crtc_state *crtc_state;
> -	int i;
> -
> -	if (DISPLAY_VER(display) < 20)
> -		return false;
> -
> -	for_each_new_intel_crtc_in_state(state, intel_crtc, crtc_state, i) {
> -		if (!intel_crtc->active)
> -			continue;
> -
> -		if (crtc_state->vrr.enable)
> -			return false;
> -	}
> -
> -	return true;
> -}
> -
> -static bool wa_22019444797_psr1_check(const struct intel_crtc_state
> *crtc_state,
> -				      struct intel_psr *psr)
> -{
> -	struct intel_display *display = to_intel_display(crtc_state);
> -
> -	return DISPLAY_VER(display) == 20 && psr->is_dpkgc_configured &&
> -		(psr->is_wa_delayed_vblank_limit || !psr-
> >is_dc5_entry_possible) &&
> -		!crtc_state->has_sel_update && !crtc_state-
> >has_panel_replay;
> -}
> -
> -/*
> - * DC5 entry is only possible if vblank interrupt is disabled
> - * and either psr1, psr2, edp 1.5 pr alpm is enabled on all
> - * enabled encoders.
> - */
> -static bool
> -intel_psr_is_dc5_entry_possible(struct intel_display *display,
> -				struct intel_atomic_state *state)
> -{
> -	struct intel_crtc *intel_crtc;
> -	struct intel_crtc_state *crtc_state;
> -	int i;
> -
> -	if ((display->power.domains.target_dc_state &
> -	     DC_STATE_EN_UPTO_DC5_DC6_MASK) == 0)
> -		return false;
> -
> -	for_each_new_intel_crtc_in_state(state, intel_crtc, crtc_state, i) {
> -		struct drm_crtc *crtc = &intel_crtc->base;
> -		struct drm_vblank_crtc *vblank;
> -		struct intel_encoder *encoder;
> -
> -		if (!intel_crtc->active)
> -			continue;
> -
> -		vblank = drm_crtc_vblank_crtc(crtc);
> -
> -		if (vblank->enabled)
> -			return false;
> -
> -		if (!crtc_state->has_psr)
> -			return false;
> -
> -		for_each_encoder_on_crtc(display->drm, crtc, encoder)
> -			if (encoder->type != INTEL_OUTPUT_EDP ||
> -			    !CAN_PSR(enc_to_intel_dp(encoder)))
> -				return false;
> -	}
> -
> -	return true;
> -}
> -
>  static void hsw_activate_psr1(struct intel_dp *intel_dp)  {
>  	struct intel_display *display = to_intel_display(intel_dp); @@ -
> 1092,15 +1008,7 @@ static void hsw_activate_psr2(struct intel_dp *intel_dp)
>  	u32 val = EDP_PSR2_ENABLE;
>  	u32 psr_val = 0;
> 
> -	/*
> -	 * Wa_22019444797
> -	 * TODO: Disable idle frames when vblank gets enabled while
> -	 * PSR2 is enabled
> -	 */
> -	if (DISPLAY_VER(dev_priv) != 20 ||
> -	    !intel_dp->psr.is_dpkgc_configured ||
> -	    intel_dp->psr.is_dc5_entry_possible)
> -		val |=
> EDP_PSR2_IDLE_FRAMES(psr_compute_idle_frames(intel_dp));
> +	val |= EDP_PSR2_IDLE_FRAMES(psr_compute_idle_frames(intel_dp));
> 
>  	if (DISPLAY_VER(display) < 14 && !IS_ALDERLAKE_P(dev_priv))
>  		val |= EDP_SU_TRACK_ENABLE;
> @@ -2815,20 +2723,10 @@ void intel_psr_pre_plane_update(struct
> intel_atomic_state *state,
>  	const struct intel_crtc_state *new_crtc_state =
>  		intel_atomic_get_new_crtc_state(state, crtc);
>  	struct intel_encoder *encoder;
> -	bool dpkgc_configured = false, dc5_entry_possible = false;
> -	bool wa_delayed_vblank_limit = false;
> 
>  	if (!HAS_PSR(display))
>  		return;
> 
> -	if (DISPLAY_VER(display) == 20) {
> -		dpkgc_configured = intel_psr_is_dpkgc_configured(display,
> state);
> -		dc5_entry_possible =
> -			intel_psr_is_dc5_entry_possible(display, state);
> -		wa_delayed_vblank_limit =
> -
> 	intel_psr_check_wa_delayed_vblank(&new_crtc_state-
> >hw.adjusted_mode);
> -	}
> -
>  	for_each_intel_encoder_mask_with_psr(state->base.dev, encoder,
>  					     old_crtc_state-
> >uapi.encoder_mask) {
>  		struct intel_dp *intel_dp = enc_to_intel_dp(encoder); @@ -
> 2837,12 +2735,6 @@ void intel_psr_pre_plane_update(struct
> intel_atomic_state *state,
> 
>  		mutex_lock(&psr->lock);
> 
> -		if (DISPLAY_VER(i915) == 20) {
> -			psr->is_dpkgc_configured = dpkgc_configured;
> -			psr->is_dc5_entry_possible = dc5_entry_possible;
> -			psr->is_wa_delayed_vblank_limit =
> wa_delayed_vblank_limit;
> -		}
> -
>  		/*
>  		 * Reasons to disable:
>  		 * - PSR disabled in new state
> @@ -2850,7 +2742,6 @@ void intel_psr_pre_plane_update(struct
> intel_atomic_state *state,
>  		 * - Changing between PSR versions
>  		 * - Region Early Transport changing
>  		 * - Display WA #1136: skl, bxt
> -		 * - Display WA_22019444797
>  		 */
>  		needs_to_disable |=
> intel_crtc_needs_modeset(new_crtc_state);
>  		needs_to_disable |= !new_crtc_state->has_psr; @@ -2860,8
> +2751,6 @@ void intel_psr_pre_plane_update(struct intel_atomic_state
> *state,
>  			psr->su_region_et_enabled;
>  		needs_to_disable |= DISPLAY_VER(i915) < 11 &&
>  			new_crtc_state->wm_level_disabled;
> -		/* TODO: Disable PSR1 when vblank gets enabled while PSR1
> is enabled */
> -		needs_to_disable |=
> wa_22019444797_psr1_check(new_crtc_state, psr);
> 
>  		if (psr->enabled && needs_to_disable)
>  			intel_psr_disable_locked(intel_dp);
> @@ -2902,12 +2791,6 @@ void intel_psr_post_plane_update(struct
> intel_atomic_state *state,
>  		keep_disabled |= DISPLAY_VER(display) < 11 &&
>  			crtc_state->wm_level_disabled;
> 
> -		/*
> -		 * Wa_22019444797
> -		 * TODO: Disable PSR1 when vblank gets enabled while PSR1
> is enabled
> -		 */
> -		keep_disabled |= wa_22019444797_psr1_check(crtc_state,
> psr);
> -
>  		if (!psr->enabled && !keep_disabled)
>  			intel_psr_enable_locked(intel_dp, crtc_state);
>  		else if (psr->enabled && !crtc_state->wm_level_disabled)
> --
> 2.34.1


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

* Re: [PATCH] Revert "drm/i915/psr: Implement WA to help reach PC10"
  2024-09-30  7:13 [PATCH] Revert "drm/i915/psr: Implement WA to help reach PC10" Jouni Högander
                   ` (3 preceding siblings ...)
  2024-09-30 13:46 ` [PATCH] " Kandpal, Suraj
@ 2024-09-30 15:21 ` Rodrigo Vivi
  2024-09-30 15:28   ` Saarinen, Jani
  2024-09-30 17:26 ` ✗ CI.Patch_applied: failure for Revert "drm/i915/psr: Implement WA to help reach PC10" (rev2) Patchwork
  5 siblings, 1 reply; 9+ messages in thread
From: Rodrigo Vivi @ 2024-09-30 15:21 UTC (permalink / raw)
  To: Jouni Högander; +Cc: intel-xe

On Mon, Sep 30, 2024 at 10:13:29AM +0300, Jouni Högander wrote:
> This reverts commit c2579a217799ba577fa39a2a12643a277334e691.
> 
> Reverting this commit as it is suspected being culprit on regression.

Any link to a gitlab issue or any information about the regression itself?

> 
> Signed-off-by: Jouni Högander <jouni.hogander@intel.com>
> ---
>  .../drm/i915/display/intel_display_types.h    |   3 -
>  drivers/gpu/drm/i915/display/intel_psr.c      | 119 +-----------------
>  2 files changed, 1 insertion(+), 121 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
> index 7ff97e5b83dd5..f60d66f4b980a 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> @@ -1576,9 +1576,6 @@ struct intel_psr {
>  #define I915_PSR_DEBUG_PANEL_REPLAY_DISABLE	0x40
>  
>  	u32 debug;
> -	bool is_dpkgc_configured;
> -	bool is_dc5_entry_possible;
> -	bool is_wa_delayed_vblank_limit;
>  	bool sink_support;
>  	bool source_support;
>  	bool enabled;
> diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c
> index 39aa46de15e43..e3357f3b5c705 100644
> --- a/drivers/gpu/drm/i915/display/intel_psr.c
> +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> @@ -26,7 +26,6 @@
>  #include <drm/drm_atomic_helper.h>
>  #include <drm/drm_damage_helper.h>
>  #include <drm/drm_debugfs.h>
> -#include <drm/drm_vblank.h>
>  
>  #include "i915_drv.h"
>  #include "i915_reg.h"
> @@ -897,89 +896,6 @@ static u8 psr_compute_idle_frames(struct intel_dp *intel_dp)
>  	return idle_frames;
>  }
>  
> -static bool
> -intel_psr_check_wa_delayed_vblank(const struct drm_display_mode *adjusted_mode)
> -{
> -	return (adjusted_mode->crtc_vblank_start - adjusted_mode->crtc_vdisplay) >= 6;
> -}
> -
> -/*
> - * PKG_C_LATENCY is configured only when DISPLAY_VER >= 20 and
> - * VRR is not enabled
> - */
> -static bool intel_psr_is_dpkgc_configured(struct intel_display *display,
> -					  struct intel_atomic_state *state)
> -{
> -	struct intel_crtc *intel_crtc;
> -	struct intel_crtc_state *crtc_state;
> -	int i;
> -
> -	if (DISPLAY_VER(display) < 20)
> -		return false;
> -
> -	for_each_new_intel_crtc_in_state(state, intel_crtc, crtc_state, i) {
> -		if (!intel_crtc->active)
> -			continue;
> -
> -		if (crtc_state->vrr.enable)
> -			return false;
> -	}
> -
> -	return true;
> -}
> -
> -static bool wa_22019444797_psr1_check(const struct intel_crtc_state *crtc_state,
> -				      struct intel_psr *psr)
> -{
> -	struct intel_display *display = to_intel_display(crtc_state);
> -
> -	return DISPLAY_VER(display) == 20 && psr->is_dpkgc_configured &&
> -		(psr->is_wa_delayed_vblank_limit || !psr->is_dc5_entry_possible) &&
> -		!crtc_state->has_sel_update && !crtc_state->has_panel_replay;
> -}
> -
> -/*
> - * DC5 entry is only possible if vblank interrupt is disabled
> - * and either psr1, psr2, edp 1.5 pr alpm is enabled on all
> - * enabled encoders.
> - */
> -static bool
> -intel_psr_is_dc5_entry_possible(struct intel_display *display,
> -				struct intel_atomic_state *state)
> -{
> -	struct intel_crtc *intel_crtc;
> -	struct intel_crtc_state *crtc_state;
> -	int i;
> -
> -	if ((display->power.domains.target_dc_state &
> -	     DC_STATE_EN_UPTO_DC5_DC6_MASK) == 0)
> -		return false;
> -
> -	for_each_new_intel_crtc_in_state(state, intel_crtc, crtc_state, i) {
> -		struct drm_crtc *crtc = &intel_crtc->base;
> -		struct drm_vblank_crtc *vblank;
> -		struct intel_encoder *encoder;
> -
> -		if (!intel_crtc->active)
> -			continue;
> -
> -		vblank = drm_crtc_vblank_crtc(crtc);
> -
> -		if (vblank->enabled)
> -			return false;
> -
> -		if (!crtc_state->has_psr)
> -			return false;
> -
> -		for_each_encoder_on_crtc(display->drm, crtc, encoder)
> -			if (encoder->type != INTEL_OUTPUT_EDP ||
> -			    !CAN_PSR(enc_to_intel_dp(encoder)))
> -				return false;
> -	}
> -
> -	return true;
> -}
> -
>  static void hsw_activate_psr1(struct intel_dp *intel_dp)
>  {
>  	struct intel_display *display = to_intel_display(intel_dp);
> @@ -1092,15 +1008,7 @@ static void hsw_activate_psr2(struct intel_dp *intel_dp)
>  	u32 val = EDP_PSR2_ENABLE;
>  	u32 psr_val = 0;
>  
> -	/*
> -	 * Wa_22019444797
> -	 * TODO: Disable idle frames when vblank gets enabled while
> -	 * PSR2 is enabled
> -	 */
> -	if (DISPLAY_VER(dev_priv) != 20 ||
> -	    !intel_dp->psr.is_dpkgc_configured ||
> -	    intel_dp->psr.is_dc5_entry_possible)
> -		val |= EDP_PSR2_IDLE_FRAMES(psr_compute_idle_frames(intel_dp));
> +	val |= EDP_PSR2_IDLE_FRAMES(psr_compute_idle_frames(intel_dp));
>  
>  	if (DISPLAY_VER(display) < 14 && !IS_ALDERLAKE_P(dev_priv))
>  		val |= EDP_SU_TRACK_ENABLE;
> @@ -2815,20 +2723,10 @@ void intel_psr_pre_plane_update(struct intel_atomic_state *state,
>  	const struct intel_crtc_state *new_crtc_state =
>  		intel_atomic_get_new_crtc_state(state, crtc);
>  	struct intel_encoder *encoder;
> -	bool dpkgc_configured = false, dc5_entry_possible = false;
> -	bool wa_delayed_vblank_limit = false;
>  
>  	if (!HAS_PSR(display))
>  		return;
>  
> -	if (DISPLAY_VER(display) == 20) {
> -		dpkgc_configured = intel_psr_is_dpkgc_configured(display, state);
> -		dc5_entry_possible =
> -			intel_psr_is_dc5_entry_possible(display, state);
> -		wa_delayed_vblank_limit =
> -			intel_psr_check_wa_delayed_vblank(&new_crtc_state->hw.adjusted_mode);
> -	}
> -
>  	for_each_intel_encoder_mask_with_psr(state->base.dev, encoder,
>  					     old_crtc_state->uapi.encoder_mask) {
>  		struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
> @@ -2837,12 +2735,6 @@ void intel_psr_pre_plane_update(struct intel_atomic_state *state,
>  
>  		mutex_lock(&psr->lock);
>  
> -		if (DISPLAY_VER(i915) == 20) {
> -			psr->is_dpkgc_configured = dpkgc_configured;
> -			psr->is_dc5_entry_possible = dc5_entry_possible;
> -			psr->is_wa_delayed_vblank_limit = wa_delayed_vblank_limit;
> -		}
> -
>  		/*
>  		 * Reasons to disable:
>  		 * - PSR disabled in new state
> @@ -2850,7 +2742,6 @@ void intel_psr_pre_plane_update(struct intel_atomic_state *state,
>  		 * - Changing between PSR versions
>  		 * - Region Early Transport changing
>  		 * - Display WA #1136: skl, bxt
> -		 * - Display WA_22019444797
>  		 */
>  		needs_to_disable |= intel_crtc_needs_modeset(new_crtc_state);
>  		needs_to_disable |= !new_crtc_state->has_psr;
> @@ -2860,8 +2751,6 @@ void intel_psr_pre_plane_update(struct intel_atomic_state *state,
>  			psr->su_region_et_enabled;
>  		needs_to_disable |= DISPLAY_VER(i915) < 11 &&
>  			new_crtc_state->wm_level_disabled;
> -		/* TODO: Disable PSR1 when vblank gets enabled while PSR1 is enabled */
> -		needs_to_disable |= wa_22019444797_psr1_check(new_crtc_state, psr);
>  
>  		if (psr->enabled && needs_to_disable)
>  			intel_psr_disable_locked(intel_dp);
> @@ -2902,12 +2791,6 @@ void intel_psr_post_plane_update(struct intel_atomic_state *state,
>  		keep_disabled |= DISPLAY_VER(display) < 11 &&
>  			crtc_state->wm_level_disabled;
>  
> -		/*
> -		 * Wa_22019444797
> -		 * TODO: Disable PSR1 when vblank gets enabled while PSR1 is enabled
> -		 */
> -		keep_disabled |= wa_22019444797_psr1_check(crtc_state, psr);
> -
>  		if (!psr->enabled && !keep_disabled)
>  			intel_psr_enable_locked(intel_dp, crtc_state);
>  		else if (psr->enabled && !crtc_state->wm_level_disabled)
> -- 
> 2.34.1
> 

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

* RE: [PATCH] Revert "drm/i915/psr: Implement WA to help reach PC10"
  2024-09-30 15:21 ` Rodrigo Vivi
@ 2024-09-30 15:28   ` Saarinen, Jani
  2024-09-30 16:44     ` Rodrigo Vivi
  0 siblings, 1 reply; 9+ messages in thread
From: Saarinen, Jani @ 2024-09-30 15:28 UTC (permalink / raw)
  To: Vivi, Rodrigo, Hogander, Jouni; +Cc: intel-xe@lists.freedesktop.org

HI, 
> -----Original Message-----
> From: Intel-xe <intel-xe-bounces@lists.freedesktop.org> On Behalf Of Rodrigo
> Vivi
> Sent: Monday, 30 September 2024 18.22
> To: Hogander, Jouni <jouni.hogander@intel.com>
> Cc: intel-xe@lists.freedesktop.org
> Subject: Re: [PATCH] Revert "drm/i915/psr: Implement WA to help reach
> PC10"
> 
> On Mon, Sep 30, 2024 at 10:13:29AM +0300, Jouni Högander wrote:
> > This reverts commit c2579a217799ba577fa39a2a12643a277334e691.
> >
> > Reverting this commit as it is suspected being culprit on regression.
> 
> Any link to a gitlab issue or any information about the regression itself?
Some here https://intel-gfx-ci.01.org/tree/intel-xe/index.html?testfilter=kms_psr&hosts=lnl 
Gitlab : https://gitlab.freedesktop.org/drm/xe/kernel/issues/1649 
> 
> >
> > Signed-off-by: Jouni Högander <jouni.hogander@intel.com>
> > ---
> >  .../drm/i915/display/intel_display_types.h    |   3 -
> >  drivers/gpu/drm/i915/display/intel_psr.c      | 119 +-----------------
> >  2 files changed, 1 insertion(+), 121 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h
> > b/drivers/gpu/drm/i915/display/intel_display_types.h
> > index 7ff97e5b83dd5..f60d66f4b980a 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> > +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> > @@ -1576,9 +1576,6 @@ struct intel_psr {
> >  #define I915_PSR_DEBUG_PANEL_REPLAY_DISABLE	0x40
> >
> >  	u32 debug;
> > -	bool is_dpkgc_configured;
> > -	bool is_dc5_entry_possible;
> > -	bool is_wa_delayed_vblank_limit;
> >  	bool sink_support;
> >  	bool source_support;
> >  	bool enabled;
> > diff --git a/drivers/gpu/drm/i915/display/intel_psr.c
> > b/drivers/gpu/drm/i915/display/intel_psr.c
> > index 39aa46de15e43..e3357f3b5c705 100644
> > --- a/drivers/gpu/drm/i915/display/intel_psr.c
> > +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> > @@ -26,7 +26,6 @@
> >  #include <drm/drm_atomic_helper.h>
> >  #include <drm/drm_damage_helper.h>
> >  #include <drm/drm_debugfs.h>
> > -#include <drm/drm_vblank.h>
> >
> >  #include "i915_drv.h"
> >  #include "i915_reg.h"
> > @@ -897,89 +896,6 @@ static u8 psr_compute_idle_frames(struct intel_dp
> *intel_dp)
> >  	return idle_frames;
> >  }
> >
> > -static bool
> > -intel_psr_check_wa_delayed_vblank(const struct drm_display_mode
> > *adjusted_mode) -{
> > -	return (adjusted_mode->crtc_vblank_start - adjusted_mode-
> >crtc_vdisplay) >= 6;
> > -}
> > -
> > -/*
> > - * PKG_C_LATENCY is configured only when DISPLAY_VER >= 20 and
> > - * VRR is not enabled
> > - */
> > -static bool intel_psr_is_dpkgc_configured(struct intel_display *display,
> > -					  struct intel_atomic_state *state)
> > -{
> > -	struct intel_crtc *intel_crtc;
> > -	struct intel_crtc_state *crtc_state;
> > -	int i;
> > -
> > -	if (DISPLAY_VER(display) < 20)
> > -		return false;
> > -
> > -	for_each_new_intel_crtc_in_state(state, intel_crtc, crtc_state, i) {
> > -		if (!intel_crtc->active)
> > -			continue;
> > -
> > -		if (crtc_state->vrr.enable)
> > -			return false;
> > -	}
> > -
> > -	return true;
> > -}
> > -
> > -static bool wa_22019444797_psr1_check(const struct intel_crtc_state
> *crtc_state,
> > -				      struct intel_psr *psr)
> > -{
> > -	struct intel_display *display = to_intel_display(crtc_state);
> > -
> > -	return DISPLAY_VER(display) == 20 && psr->is_dpkgc_configured &&
> > -		(psr->is_wa_delayed_vblank_limit || !psr-
> >is_dc5_entry_possible) &&
> > -		!crtc_state->has_sel_update && !crtc_state-
> >has_panel_replay;
> > -}
> > -
> > -/*
> > - * DC5 entry is only possible if vblank interrupt is disabled
> > - * and either psr1, psr2, edp 1.5 pr alpm is enabled on all
> > - * enabled encoders.
> > - */
> > -static bool
> > -intel_psr_is_dc5_entry_possible(struct intel_display *display,
> > -				struct intel_atomic_state *state)
> > -{
> > -	struct intel_crtc *intel_crtc;
> > -	struct intel_crtc_state *crtc_state;
> > -	int i;
> > -
> > -	if ((display->power.domains.target_dc_state &
> > -	     DC_STATE_EN_UPTO_DC5_DC6_MASK) == 0)
> > -		return false;
> > -
> > -	for_each_new_intel_crtc_in_state(state, intel_crtc, crtc_state, i) {
> > -		struct drm_crtc *crtc = &intel_crtc->base;
> > -		struct drm_vblank_crtc *vblank;
> > -		struct intel_encoder *encoder;
> > -
> > -		if (!intel_crtc->active)
> > -			continue;
> > -
> > -		vblank = drm_crtc_vblank_crtc(crtc);
> > -
> > -		if (vblank->enabled)
> > -			return false;
> > -
> > -		if (!crtc_state->has_psr)
> > -			return false;
> > -
> > -		for_each_encoder_on_crtc(display->drm, crtc, encoder)
> > -			if (encoder->type != INTEL_OUTPUT_EDP ||
> > -			    !CAN_PSR(enc_to_intel_dp(encoder)))
> > -				return false;
> > -	}
> > -
> > -	return true;
> > -}
> > -
> >  static void hsw_activate_psr1(struct intel_dp *intel_dp)  {
> >  	struct intel_display *display = to_intel_display(intel_dp); @@
> > -1092,15 +1008,7 @@ static void hsw_activate_psr2(struct intel_dp
> *intel_dp)
> >  	u32 val = EDP_PSR2_ENABLE;
> >  	u32 psr_val = 0;
> >
> > -	/*
> > -	 * Wa_22019444797
> > -	 * TODO: Disable idle frames when vblank gets enabled while
> > -	 * PSR2 is enabled
> > -	 */
> > -	if (DISPLAY_VER(dev_priv) != 20 ||
> > -	    !intel_dp->psr.is_dpkgc_configured ||
> > -	    intel_dp->psr.is_dc5_entry_possible)
> > -		val |=
> EDP_PSR2_IDLE_FRAMES(psr_compute_idle_frames(intel_dp));
> > +	val |= EDP_PSR2_IDLE_FRAMES(psr_compute_idle_frames(intel_dp));
> >
> >  	if (DISPLAY_VER(display) < 14 && !IS_ALDERLAKE_P(dev_priv))
> >  		val |= EDP_SU_TRACK_ENABLE;
> > @@ -2815,20 +2723,10 @@ void intel_psr_pre_plane_update(struct
> intel_atomic_state *state,
> >  	const struct intel_crtc_state *new_crtc_state =
> >  		intel_atomic_get_new_crtc_state(state, crtc);
> >  	struct intel_encoder *encoder;
> > -	bool dpkgc_configured = false, dc5_entry_possible = false;
> > -	bool wa_delayed_vblank_limit = false;
> >
> >  	if (!HAS_PSR(display))
> >  		return;
> >
> > -	if (DISPLAY_VER(display) == 20) {
> > -		dpkgc_configured = intel_psr_is_dpkgc_configured(display,
> state);
> > -		dc5_entry_possible =
> > -			intel_psr_is_dc5_entry_possible(display, state);
> > -		wa_delayed_vblank_limit =
> > -
> 	intel_psr_check_wa_delayed_vblank(&new_crtc_state-
> >hw.adjusted_mode);
> > -	}
> > -
> >  	for_each_intel_encoder_mask_with_psr(state->base.dev, encoder,
> >  					     old_crtc_state-
> >uapi.encoder_mask) {
> >  		struct intel_dp *intel_dp = enc_to_intel_dp(encoder); @@ -
> 2837,12
> > +2735,6 @@ void intel_psr_pre_plane_update(struct intel_atomic_state
> > *state,
> >
> >  		mutex_lock(&psr->lock);
> >
> > -		if (DISPLAY_VER(i915) == 20) {
> > -			psr->is_dpkgc_configured = dpkgc_configured;
> > -			psr->is_dc5_entry_possible = dc5_entry_possible;
> > -			psr->is_wa_delayed_vblank_limit =
> wa_delayed_vblank_limit;
> > -		}
> > -
> >  		/*
> >  		 * Reasons to disable:
> >  		 * - PSR disabled in new state
> > @@ -2850,7 +2742,6 @@ void intel_psr_pre_plane_update(struct
> intel_atomic_state *state,
> >  		 * - Changing between PSR versions
> >  		 * - Region Early Transport changing
> >  		 * - Display WA #1136: skl, bxt
> > -		 * - Display WA_22019444797
> >  		 */
> >  		needs_to_disable |=
> intel_crtc_needs_modeset(new_crtc_state);
> >  		needs_to_disable |= !new_crtc_state->has_psr; @@ -2860,8
> +2751,6 @@
> > void intel_psr_pre_plane_update(struct intel_atomic_state *state,
> >  			psr->su_region_et_enabled;
> >  		needs_to_disable |= DISPLAY_VER(i915) < 11 &&
> >  			new_crtc_state->wm_level_disabled;
> > -		/* TODO: Disable PSR1 when vblank gets enabled while PSR1
> is enabled */
> > -		needs_to_disable |=
> wa_22019444797_psr1_check(new_crtc_state, psr);
> >
> >  		if (psr->enabled && needs_to_disable)
> >  			intel_psr_disable_locked(intel_dp);
> > @@ -2902,12 +2791,6 @@ void intel_psr_post_plane_update(struct
> intel_atomic_state *state,
> >  		keep_disabled |= DISPLAY_VER(display) < 11 &&
> >  			crtc_state->wm_level_disabled;
> >
> > -		/*
> > -		 * Wa_22019444797
> > -		 * TODO: Disable PSR1 when vblank gets enabled while PSR1 is
> enabled
> > -		 */
> > -		keep_disabled |= wa_22019444797_psr1_check(crtc_state,
> psr);
> > -
> >  		if (!psr->enabled && !keep_disabled)
> >  			intel_psr_enable_locked(intel_dp, crtc_state);
> >  		else if (psr->enabled && !crtc_state->wm_level_disabled)
> > --
> > 2.34.1
> >

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

* Re: [PATCH] Revert "drm/i915/psr: Implement WA to help reach PC10"
  2024-09-30 15:28   ` Saarinen, Jani
@ 2024-09-30 16:44     ` Rodrigo Vivi
  0 siblings, 0 replies; 9+ messages in thread
From: Rodrigo Vivi @ 2024-09-30 16:44 UTC (permalink / raw)
  To: Saarinen, Jani; +Cc: Hogander, Jouni, intel-xe@lists.freedesktop.org

On Mon, Sep 30, 2024 at 11:28:56AM -0400, Saarinen, Jani wrote:
> HI, 
> > -----Original Message-----
> > From: Intel-xe <intel-xe-bounces@lists.freedesktop.org> On Behalf Of Rodrigo
> > Vivi
> > Sent: Monday, 30 September 2024 18.22
> > To: Hogander, Jouni <jouni.hogander@intel.com>
> > Cc: intel-xe@lists.freedesktop.org
> > Subject: Re: [PATCH] Revert "drm/i915/psr: Implement WA to help reach
> > PC10"
> > 
> > On Mon, Sep 30, 2024 at 10:13:29AM +0300, Jouni Högander wrote:
> > > This reverts commit c2579a217799ba577fa39a2a12643a277334e691.
> > >
> > > Reverting this commit as it is suspected being culprit on regression.
> > 
> > Any link to a gitlab issue or any information about the regression itself?
> Some here https://intel-gfx-ci.01.org/tree/intel-xe/index.html?testfilter=kms_psr&hosts=lnl 
> Gitlab : https://gitlab.freedesktop.org/drm/xe/kernel/issues/1649 

Thank you

pushed with:
Closes: https://gitlab.freedesktop.org/drm/xe/kernel/issues/1649

> > 
> > >
> > > Signed-off-by: Jouni Högander <jouni.hogander@intel.com>
> > > ---
> > >  .../drm/i915/display/intel_display_types.h    |   3 -
> > >  drivers/gpu/drm/i915/display/intel_psr.c      | 119 +-----------------
> > >  2 files changed, 1 insertion(+), 121 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h
> > > b/drivers/gpu/drm/i915/display/intel_display_types.h
> > > index 7ff97e5b83dd5..f60d66f4b980a 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> > > +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> > > @@ -1576,9 +1576,6 @@ struct intel_psr {
> > >  #define I915_PSR_DEBUG_PANEL_REPLAY_DISABLE	0x40
> > >
> > >  	u32 debug;
> > > -	bool is_dpkgc_configured;
> > > -	bool is_dc5_entry_possible;
> > > -	bool is_wa_delayed_vblank_limit;
> > >  	bool sink_support;
> > >  	bool source_support;
> > >  	bool enabled;
> > > diff --git a/drivers/gpu/drm/i915/display/intel_psr.c
> > > b/drivers/gpu/drm/i915/display/intel_psr.c
> > > index 39aa46de15e43..e3357f3b5c705 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_psr.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> > > @@ -26,7 +26,6 @@
> > >  #include <drm/drm_atomic_helper.h>
> > >  #include <drm/drm_damage_helper.h>
> > >  #include <drm/drm_debugfs.h>
> > > -#include <drm/drm_vblank.h>
> > >
> > >  #include "i915_drv.h"
> > >  #include "i915_reg.h"
> > > @@ -897,89 +896,6 @@ static u8 psr_compute_idle_frames(struct intel_dp
> > *intel_dp)
> > >  	return idle_frames;
> > >  }
> > >
> > > -static bool
> > > -intel_psr_check_wa_delayed_vblank(const struct drm_display_mode
> > > *adjusted_mode) -{
> > > -	return (adjusted_mode->crtc_vblank_start - adjusted_mode-
> > >crtc_vdisplay) >= 6;
> > > -}
> > > -
> > > -/*
> > > - * PKG_C_LATENCY is configured only when DISPLAY_VER >= 20 and
> > > - * VRR is not enabled
> > > - */
> > > -static bool intel_psr_is_dpkgc_configured(struct intel_display *display,
> > > -					  struct intel_atomic_state *state)
> > > -{
> > > -	struct intel_crtc *intel_crtc;
> > > -	struct intel_crtc_state *crtc_state;
> > > -	int i;
> > > -
> > > -	if (DISPLAY_VER(display) < 20)
> > > -		return false;
> > > -
> > > -	for_each_new_intel_crtc_in_state(state, intel_crtc, crtc_state, i) {
> > > -		if (!intel_crtc->active)
> > > -			continue;
> > > -
> > > -		if (crtc_state->vrr.enable)
> > > -			return false;
> > > -	}
> > > -
> > > -	return true;
> > > -}
> > > -
> > > -static bool wa_22019444797_psr1_check(const struct intel_crtc_state
> > *crtc_state,
> > > -				      struct intel_psr *psr)
> > > -{
> > > -	struct intel_display *display = to_intel_display(crtc_state);
> > > -
> > > -	return DISPLAY_VER(display) == 20 && psr->is_dpkgc_configured &&
> > > -		(psr->is_wa_delayed_vblank_limit || !psr-
> > >is_dc5_entry_possible) &&
> > > -		!crtc_state->has_sel_update && !crtc_state-
> > >has_panel_replay;
> > > -}
> > > -
> > > -/*
> > > - * DC5 entry is only possible if vblank interrupt is disabled
> > > - * and either psr1, psr2, edp 1.5 pr alpm is enabled on all
> > > - * enabled encoders.
> > > - */
> > > -static bool
> > > -intel_psr_is_dc5_entry_possible(struct intel_display *display,
> > > -				struct intel_atomic_state *state)
> > > -{
> > > -	struct intel_crtc *intel_crtc;
> > > -	struct intel_crtc_state *crtc_state;
> > > -	int i;
> > > -
> > > -	if ((display->power.domains.target_dc_state &
> > > -	     DC_STATE_EN_UPTO_DC5_DC6_MASK) == 0)
> > > -		return false;
> > > -
> > > -	for_each_new_intel_crtc_in_state(state, intel_crtc, crtc_state, i) {
> > > -		struct drm_crtc *crtc = &intel_crtc->base;
> > > -		struct drm_vblank_crtc *vblank;
> > > -		struct intel_encoder *encoder;
> > > -
> > > -		if (!intel_crtc->active)
> > > -			continue;
> > > -
> > > -		vblank = drm_crtc_vblank_crtc(crtc);
> > > -
> > > -		if (vblank->enabled)
> > > -			return false;
> > > -
> > > -		if (!crtc_state->has_psr)
> > > -			return false;
> > > -
> > > -		for_each_encoder_on_crtc(display->drm, crtc, encoder)
> > > -			if (encoder->type != INTEL_OUTPUT_EDP ||
> > > -			    !CAN_PSR(enc_to_intel_dp(encoder)))
> > > -				return false;
> > > -	}
> > > -
> > > -	return true;
> > > -}
> > > -
> > >  static void hsw_activate_psr1(struct intel_dp *intel_dp)  {
> > >  	struct intel_display *display = to_intel_display(intel_dp); @@
> > > -1092,15 +1008,7 @@ static void hsw_activate_psr2(struct intel_dp
> > *intel_dp)
> > >  	u32 val = EDP_PSR2_ENABLE;
> > >  	u32 psr_val = 0;
> > >
> > > -	/*
> > > -	 * Wa_22019444797
> > > -	 * TODO: Disable idle frames when vblank gets enabled while
> > > -	 * PSR2 is enabled
> > > -	 */
> > > -	if (DISPLAY_VER(dev_priv) != 20 ||
> > > -	    !intel_dp->psr.is_dpkgc_configured ||
> > > -	    intel_dp->psr.is_dc5_entry_possible)
> > > -		val |=
> > EDP_PSR2_IDLE_FRAMES(psr_compute_idle_frames(intel_dp));
> > > +	val |= EDP_PSR2_IDLE_FRAMES(psr_compute_idle_frames(intel_dp));
> > >
> > >  	if (DISPLAY_VER(display) < 14 && !IS_ALDERLAKE_P(dev_priv))
> > >  		val |= EDP_SU_TRACK_ENABLE;
> > > @@ -2815,20 +2723,10 @@ void intel_psr_pre_plane_update(struct
> > intel_atomic_state *state,
> > >  	const struct intel_crtc_state *new_crtc_state =
> > >  		intel_atomic_get_new_crtc_state(state, crtc);
> > >  	struct intel_encoder *encoder;
> > > -	bool dpkgc_configured = false, dc5_entry_possible = false;
> > > -	bool wa_delayed_vblank_limit = false;
> > >
> > >  	if (!HAS_PSR(display))
> > >  		return;
> > >
> > > -	if (DISPLAY_VER(display) == 20) {
> > > -		dpkgc_configured = intel_psr_is_dpkgc_configured(display,
> > state);
> > > -		dc5_entry_possible =
> > > -			intel_psr_is_dc5_entry_possible(display, state);
> > > -		wa_delayed_vblank_limit =
> > > -
> > 	intel_psr_check_wa_delayed_vblank(&new_crtc_state-
> > >hw.adjusted_mode);
> > > -	}
> > > -
> > >  	for_each_intel_encoder_mask_with_psr(state->base.dev, encoder,
> > >  					     old_crtc_state-
> > >uapi.encoder_mask) {
> > >  		struct intel_dp *intel_dp = enc_to_intel_dp(encoder); @@ -
> > 2837,12
> > > +2735,6 @@ void intel_psr_pre_plane_update(struct intel_atomic_state
> > > *state,
> > >
> > >  		mutex_lock(&psr->lock);
> > >
> > > -		if (DISPLAY_VER(i915) == 20) {
> > > -			psr->is_dpkgc_configured = dpkgc_configured;
> > > -			psr->is_dc5_entry_possible = dc5_entry_possible;
> > > -			psr->is_wa_delayed_vblank_limit =
> > wa_delayed_vblank_limit;
> > > -		}
> > > -
> > >  		/*
> > >  		 * Reasons to disable:
> > >  		 * - PSR disabled in new state
> > > @@ -2850,7 +2742,6 @@ void intel_psr_pre_plane_update(struct
> > intel_atomic_state *state,
> > >  		 * - Changing between PSR versions
> > >  		 * - Region Early Transport changing
> > >  		 * - Display WA #1136: skl, bxt
> > > -		 * - Display WA_22019444797
> > >  		 */
> > >  		needs_to_disable |=
> > intel_crtc_needs_modeset(new_crtc_state);
> > >  		needs_to_disable |= !new_crtc_state->has_psr; @@ -2860,8
> > +2751,6 @@
> > > void intel_psr_pre_plane_update(struct intel_atomic_state *state,
> > >  			psr->su_region_et_enabled;
> > >  		needs_to_disable |= DISPLAY_VER(i915) < 11 &&
> > >  			new_crtc_state->wm_level_disabled;
> > > -		/* TODO: Disable PSR1 when vblank gets enabled while PSR1
> > is enabled */
> > > -		needs_to_disable |=
> > wa_22019444797_psr1_check(new_crtc_state, psr);
> > >
> > >  		if (psr->enabled && needs_to_disable)
> > >  			intel_psr_disable_locked(intel_dp);
> > > @@ -2902,12 +2791,6 @@ void intel_psr_post_plane_update(struct
> > intel_atomic_state *state,
> > >  		keep_disabled |= DISPLAY_VER(display) < 11 &&
> > >  			crtc_state->wm_level_disabled;
> > >
> > > -		/*
> > > -		 * Wa_22019444797
> > > -		 * TODO: Disable PSR1 when vblank gets enabled while PSR1 is
> > enabled
> > > -		 */
> > > -		keep_disabled |= wa_22019444797_psr1_check(crtc_state,
> > psr);
> > > -
> > >  		if (!psr->enabled && !keep_disabled)
> > >  			intel_psr_enable_locked(intel_dp, crtc_state);
> > >  		else if (psr->enabled && !crtc_state->wm_level_disabled)
> > > --
> > > 2.34.1
> > >

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

* ✗ CI.Patch_applied: failure for Revert "drm/i915/psr: Implement WA to help reach PC10" (rev2)
  2024-09-30  7:13 [PATCH] Revert "drm/i915/psr: Implement WA to help reach PC10" Jouni Högander
                   ` (4 preceding siblings ...)
  2024-09-30 15:21 ` Rodrigo Vivi
@ 2024-09-30 17:26 ` Patchwork
  5 siblings, 0 replies; 9+ messages in thread
From: Patchwork @ 2024-09-30 17:26 UTC (permalink / raw)
  To: Jouni Högander; +Cc: intel-xe

== Series Details ==

Series: Revert "drm/i915/psr: Implement WA to help reach PC10" (rev2)
URL   : https://patchwork.freedesktop.org/series/139275/
State : failure

== Summary ==

=== Applying kernel patches on branch 'drm-tip' with base: ===
Base commit: 4e4d7873ac76 drm-tip: 2024y-09m-30d-16h-43m-25s UTC integration manifest
=== git am output follows ===
error: patch failed: drivers/gpu/drm/i915/display/intel_display_types.h:1576
error: drivers/gpu/drm/i915/display/intel_display_types.h: patch does not apply
error: patch failed: drivers/gpu/drm/i915/display/intel_psr.c:26
error: drivers/gpu/drm/i915/display/intel_psr.c: patch does not apply
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Applying: Revert "drm/i915/psr: Implement WA to help reach PC10"
Patch failed at 0001 Revert "drm/i915/psr: Implement WA to help reach PC10"
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".



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

end of thread, other threads:[~2024-09-30 17:26 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-09-30  7:13 [PATCH] Revert "drm/i915/psr: Implement WA to help reach PC10" Jouni Högander
2024-09-30  7:20 ` ✓ CI.Patch_applied: success for " Patchwork
2024-09-30  7:20 ` ✓ CI.checkpatch: " Patchwork
2024-09-30  7:20 ` ✗ CI.KUnit: failure " Patchwork
2024-09-30 13:46 ` [PATCH] " Kandpal, Suraj
2024-09-30 15:21 ` Rodrigo Vivi
2024-09-30 15:28   ` Saarinen, Jani
2024-09-30 16:44     ` Rodrigo Vivi
2024-09-30 17:26 ` ✗ CI.Patch_applied: failure for Revert "drm/i915/psr: Implement WA to help reach PC10" (rev2) Patchwork

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.