Intel-GFX Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/5] Optimize vrr.guardband
@ 2025-10-17  5:01 Ankit Nautiyal
  2025-10-17  5:01 ` [PATCH 1/5] drm/i915/psr: Add helper to get min psr guardband Ankit Nautiyal
                   ` (5 more replies)
  0 siblings, 6 replies; 23+ messages in thread
From: Ankit Nautiyal @ 2025-10-17  5:01 UTC (permalink / raw)
  To: intel-gfx, intel-xe; +Cc: ville.syrjala, Ankit Nautiyal

Instead of setting vrr.guardband to vblank, use optimal guardband that
works for most of the cases. This will help in avoiding need of change
in guardband and fix the LRR feature that needs seamless switching to
a lower refresh rate.

Rev2:
- Drop patch to check guardband in crtc_check phase, instead check
  guardband for SDP in compute_config_late.
- Modify the helper to get the min sdp guardband if all SDPs are assumed
  to be enabled.
- Rename the helpers to get min guardband for sdp and psr.

Rev3:
- Drop the squashed patches as the dependency changes are already
  merged.
- Avoid optimized guardband for HDMI for now.
- Allow support for optmized guardband only to platforms that always have
  VRR TG active in the main patch.
- Add a separate patch for extending support for optimized guardband to
  other platforms whenever VRR TG gets enabled.

Ankit Nautiyal (5):
  drm/i915/psr: Add helper to get min psr guardband
  drm/i915/dp: Add helper to get min sdp guardband
  drm/i915/dp: Check if guardband can accommodate sdp latencies
  drm/i915/vrr: Use the min static optimized guardband
  drm/i915/vrr: Use optimized guardband whenever VRR TG is active

 drivers/gpu/drm/i915/display/intel_dp.c  | 58 ++++++++++++++++++++++
 drivers/gpu/drm/i915/display/intel_dp.h  |  2 +
 drivers/gpu/drm/i915/display/intel_psr.c | 12 +++++
 drivers/gpu/drm/i915/display/intel_psr.h |  1 +
 drivers/gpu/drm/i915/display/intel_vrr.c | 62 +++++++++++++++++++++++-
 5 files changed, 133 insertions(+), 2 deletions(-)

-- 
2.45.2


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

* [PATCH 1/5] drm/i915/psr: Add helper to get min psr guardband
  2025-10-17  5:01 [PATCH 0/5] Optimize vrr.guardband Ankit Nautiyal
@ 2025-10-17  5:01 ` Ankit Nautiyal
  2025-10-17  9:07   ` Hogander, Jouni
  2025-10-17  5:01 ` [PATCH 2/5] drm/i915/dp: Add helper to get min sdp guardband Ankit Nautiyal
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 23+ messages in thread
From: Ankit Nautiyal @ 2025-10-17  5:01 UTC (permalink / raw)
  To: intel-gfx, intel-xe; +Cc: ville.syrjala, Ankit Nautiyal

Introduce a helper to compute the max link wake latency when using
Auxless/Aux wake mechanism for PSR/Panel Replay/LOBF features.

This will be used to compute the minimum guardband so that the link wake
latencies are accounted and these features work smoothly for higher
refresh rate panels.

Bspec: 70151, 71477
Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
---
 drivers/gpu/drm/i915/display/intel_psr.c | 12 ++++++++++++
 drivers/gpu/drm/i915/display/intel_psr.h |  1 +
 2 files changed, 13 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c
index 703e5f6af04c..a8303b669853 100644
--- a/drivers/gpu/drm/i915/display/intel_psr.c
+++ b/drivers/gpu/drm/i915/display/intel_psr.c
@@ -4416,3 +4416,15 @@ void intel_psr_compute_config_late(struct intel_dp *intel_dp,
 
 	intel_psr_set_non_psr_pipes(intel_dp, crtc_state);
 }
+
+int intel_psr_min_guardband(struct intel_crtc_state *crtc_state)
+{
+	struct intel_display *display = to_intel_display(crtc_state);
+	int auxless_wake_lines = crtc_state->alpm_state.aux_less_wake_lines;
+	int wake_lines = DISPLAY_VER(display) < 20 ?
+			 psr2_block_count_lines(crtc_state->alpm_state.io_wake_lines,
+						crtc_state->alpm_state.fast_wake_lines) :
+			 crtc_state->alpm_state.io_wake_lines;
+
+	return max(auxless_wake_lines, wake_lines);
+}
diff --git a/drivers/gpu/drm/i915/display/intel_psr.h b/drivers/gpu/drm/i915/display/intel_psr.h
index b17ce312dc37..620b35928832 100644
--- a/drivers/gpu/drm/i915/display/intel_psr.h
+++ b/drivers/gpu/drm/i915/display/intel_psr.h
@@ -85,5 +85,6 @@ bool intel_psr_needs_alpm_aux_less(struct intel_dp *intel_dp,
 				   const struct intel_crtc_state *crtc_state);
 void intel_psr_compute_config_late(struct intel_dp *intel_dp,
 				   struct intel_crtc_state *crtc_state);
+int intel_psr_min_guardband(struct intel_crtc_state *crtc_state);
 
 #endif /* __INTEL_PSR_H__ */
-- 
2.45.2


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

* [PATCH 2/5] drm/i915/dp: Add helper to get min sdp guardband
  2025-10-17  5:01 [PATCH 0/5] Optimize vrr.guardband Ankit Nautiyal
  2025-10-17  5:01 ` [PATCH 1/5] drm/i915/psr: Add helper to get min psr guardband Ankit Nautiyal
@ 2025-10-17  5:01 ` Ankit Nautiyal
  2025-10-17 10:50   ` Hogander, Jouni
  2025-10-17  5:02 ` [PATCH 3/5] drm/i915/dp: Check if guardband can accommodate sdp latencies Ankit Nautiyal
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 23+ messages in thread
From: Ankit Nautiyal @ 2025-10-17  5:01 UTC (permalink / raw)
  To: intel-gfx, intel-xe; +Cc: ville.syrjala, Ankit Nautiyal

Add a helper to compute vblank time needed for transmitting specific
DisplayPort SDPs like PPS, GAMUT_METADATA, and VSC_EXT. Latency is
based on line count per packet type.

This will be used to ensure adequate guardband when features like DSC/HDR
are enabled.

Bspec: 70151
Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
---
 drivers/gpu/drm/i915/display/intel_dp.c | 36 +++++++++++++++++++++++++
 drivers/gpu/drm/i915/display/intel_dp.h |  2 ++
 2 files changed, 38 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index 7059d55687cf..3f2c319e3d6f 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -6990,3 +6990,39 @@ int intel_dp_compute_config_late(struct intel_encoder *encoder,
 
 	return 0;
 }
+
+static
+int intel_dp_get_lines_for_sdp(u32 type)
+{
+	switch (type) {
+	case DP_SDP_VSC_EXT_VESA:
+	case DP_SDP_VSC_EXT_CEA:
+		return 10;
+	case HDMI_PACKET_TYPE_GAMUT_METADATA:
+		return 8;
+	case DP_SDP_PPS:
+		return 6;
+	default:
+		break;
+	}
+
+	return 0;
+}
+
+int intel_dp_sdp_min_guardband(const struct intel_crtc_state *crtc_state,
+			       bool assume_all_enabled)
+{
+	int sdp_guardband = 0;
+
+	if (assume_all_enabled ||
+	    crtc_state->infoframes.enable &
+	    intel_hdmi_infoframe_enable(HDMI_PACKET_TYPE_GAMUT_METADATA))
+		sdp_guardband = max(sdp_guardband,
+				    intel_dp_get_lines_for_sdp(HDMI_PACKET_TYPE_GAMUT_METADATA));
+
+	if (assume_all_enabled ||
+	    crtc_state->dsc.compression_enable)
+		sdp_guardband = max(sdp_guardband, intel_dp_get_lines_for_sdp(DP_SDP_PPS));
+
+	return sdp_guardband;
+}
diff --git a/drivers/gpu/drm/i915/display/intel_dp.h b/drivers/gpu/drm/i915/display/intel_dp.h
index 281ced3a3b39..7ee5aeb28fe2 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.h
+++ b/drivers/gpu/drm/i915/display/intel_dp.h
@@ -221,5 +221,7 @@ bool intel_dp_in_hdr_mode(const struct drm_connector_state *conn_state);
 int intel_dp_compute_config_late(struct intel_encoder *encoder,
 				 struct intel_crtc_state *crtc_state,
 				 struct drm_connector_state *conn_state);
+int intel_dp_sdp_min_guardband(const struct intel_crtc_state *crtc_state,
+			       bool assume_all_enabled);
 
 #endif /* __INTEL_DP_H__ */
-- 
2.45.2


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

* [PATCH 3/5] drm/i915/dp: Check if guardband can accommodate sdp latencies
  2025-10-17  5:01 [PATCH 0/5] Optimize vrr.guardband Ankit Nautiyal
  2025-10-17  5:01 ` [PATCH 1/5] drm/i915/psr: Add helper to get min psr guardband Ankit Nautiyal
  2025-10-17  5:01 ` [PATCH 2/5] drm/i915/dp: Add helper to get min sdp guardband Ankit Nautiyal
@ 2025-10-17  5:02 ` Ankit Nautiyal
  2025-10-17 12:02   ` Ville Syrjälä
  2025-10-17  5:02 ` [PATCH 4/5] drm/i915/vrr: Use the min static optimized guardband Ankit Nautiyal
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 23+ messages in thread
From: Ankit Nautiyal @ 2025-10-17  5:02 UTC (permalink / raw)
  To: intel-gfx, intel-xe; +Cc: ville.syrjala, Ankit Nautiyal

Check if guardband is sufficient for all DP SDP latencies.
If its not, fail .compute_config_late().

Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
---
 drivers/gpu/drm/i915/display/intel_dp.c | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index 3f2c319e3d6f..8ae99cee79d4 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -93,6 +93,7 @@
 #include "intel_psr.h"
 #include "intel_quirks.h"
 #include "intel_tc.h"
+#include "intel_vblank.h"
 #include "intel_vdsc.h"
 #include "intel_vrr.h"
 
@@ -6980,14 +6981,35 @@ void intel_dp_mst_resume(struct intel_display *display)
 	}
 }
 
+static
+int intel_dp_sdp_compute_config_late(struct intel_crtc_state *crtc_state)
+{
+	struct intel_display *display = to_intel_display(crtc_state);
+	int guardband = intel_crtc_vblank_length(crtc_state);
+	int min_sdp_guardband = intel_dp_sdp_min_guardband(crtc_state, false);
+
+	if (guardband < min_sdp_guardband) {
+		drm_dbg_kms(display->drm, "guardband %d < min sdp guardband %d\n",
+			    guardband, min_sdp_guardband);
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
 int intel_dp_compute_config_late(struct intel_encoder *encoder,
 				 struct intel_crtc_state *crtc_state,
 				 struct drm_connector_state *conn_state)
 {
 	struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
+	int ret;
 
 	intel_psr_compute_config_late(intel_dp, crtc_state);
 
+	ret = intel_dp_sdp_compute_config_late(crtc_state);
+	if (ret)
+		return ret;
+
 	return 0;
 }
 
-- 
2.45.2


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

* [PATCH 4/5] drm/i915/vrr: Use the min static optimized guardband
  2025-10-17  5:01 [PATCH 0/5] Optimize vrr.guardband Ankit Nautiyal
                   ` (2 preceding siblings ...)
  2025-10-17  5:02 ` [PATCH 3/5] drm/i915/dp: Check if guardband can accommodate sdp latencies Ankit Nautiyal
@ 2025-10-17  5:02 ` Ankit Nautiyal
  2025-10-17 12:06   ` Ville Syrjälä
  2025-10-17  5:02 ` [PATCH 5/5] drm/i915/vrr: Use optimized guardband whenever VRR TG is active Ankit Nautiyal
  2025-10-17  6:10 ` ✓ i915.CI.BAT: success for Optimize vrr.guardband (rev3) Patchwork
  5 siblings, 1 reply; 23+ messages in thread
From: Ankit Nautiyal @ 2025-10-17  5:02 UTC (permalink / raw)
  To: intel-gfx, intel-xe; +Cc: ville.syrjala, Ankit Nautiyal

In the current VRR implementation, vrr.vmin and vrr.guardband are set such
that they do not need to change when switching from fixed refresh rate to
variable refresh rate. Specifically, vrr.guardband is always set to match
the vblank length. This approach works for most cases, but not for LRR,
where the guardband would need to change while the VRR timing generator is
still active.

With the VRR TG always active, live updates to guardband are unsafe and not
recommended. To ensure hardware safety, guardband was moved out of the
!fastset block, meaning any change now requires a full modeset.
This breaks seamless LRR switching, which was previously supported.

Since the problem arises from guardband being matched to the vblank length,
solution is to use a minimal, sufficient static value, instead. So we use a
static guardband defined during mode-set that fits within the smallest
expected vblank and remains unchanged in case of features like LRR where
vtotal changes. To compute this minimum guardband we take into account
latencies/delays due to different features as mentioned in the Bspec.

Introduce a helper to compute the minimal sufficient guardband.
On platforms where the VRR timing generator is always ON, we optimize the
guardband regardless of whether the display is operating in fixed or
variable refresh rate mode.

v2:
- Use max of sagv latency and skl_wm_latency(1) for PM delay
  computation. (Ville)
- Avoid guardband optimization for HDMI for now. (Ville)
- Allow guardband optimization only for platforms with
  intel_vrr_always_use_vrr_tg = true. (Ville)
- Add comments for PM delay and a #TODO note for HDMI.

Bspec: 70151
Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
---
 drivers/gpu/drm/i915/display/intel_vrr.c | 62 +++++++++++++++++++++++-
 1 file changed, 60 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_vrr.c b/drivers/gpu/drm/i915/display/intel_vrr.c
index 597008a6c744..cd7bed358984 100644
--- a/drivers/gpu/drm/i915/display/intel_vrr.c
+++ b/drivers/gpu/drm/i915/display/intel_vrr.c
@@ -6,12 +6,16 @@
 
 #include <drm/drm_print.h>
 
+#include "intel_crtc.h"
 #include "intel_de.h"
 #include "intel_display_regs.h"
 #include "intel_display_types.h"
 #include "intel_dp.h"
+#include "intel_psr.h"
 #include "intel_vrr.h"
 #include "intel_vrr_regs.h"
+#include "skl_prefill.h"
+#include "skl_watermark.h"
 
 #define FIXED_POINT_PRECISION		100
 #define CMRR_PRECISION_TOLERANCE	10
@@ -433,17 +437,71 @@ intel_vrr_max_guardband(struct intel_crtc_state *crtc_state)
 		   intel_vrr_max_vblank_guardband(crtc_state));
 }
 
+static
+int intel_vrr_compute_optimized_guardband(struct intel_crtc_state *crtc_state)
+{
+	struct intel_display *display = to_intel_display(crtc_state);
+	struct skl_prefill_ctx prefill_ctx;
+	int prefill_min_guardband;
+	int prefill_latency_us;
+	int guardband = 0;
+
+	skl_prefill_init_worst(&prefill_ctx, crtc_state);
+
+	/*
+	 * The SoC power controller runs SAGV mutually exclusive with package C states,
+	 * so the max of package C and SAGV latencies is used to compute the min prefill guardband.
+	 * PM delay = max(sagv_latency, pkgc_max_latency (highest enabled wm level 1 and up))
+	 */
+	prefill_latency_us = max(display->sagv.block_time_us,
+				 skl_watermark_max_latency(display, 1));
+	prefill_min_guardband =
+		skl_prefill_min_guardband(&prefill_ctx,
+					  crtc_state,
+					  prefill_latency_us);
+
+	if (intel_crtc_has_dp_encoder(crtc_state)) {
+		guardband = max(guardband, intel_psr_min_guardband(crtc_state));
+		guardband = max(guardband, intel_dp_sdp_min_guardband(crtc_state, true));
+	}
+
+	guardband = max(guardband, prefill_min_guardband);
+
+	return guardband;
+}
+
+static bool intel_vrr_use_optimized_guardband(const struct intel_crtc_state *crtc_state)
+{
+	struct intel_display *display = to_intel_display(crtc_state);
+
+	/*
+	 * #TODO: Enable optimized guardband for HDMI
+	 * For HDMI lot of infoframes are transmitted a line or two after vsync.
+	 * Since with optimized guardband the double bufferring point is at delayed vblank,
+	 * we need to ensure that vsync happens after delayed vblank for the HDMI case.
+	 */
+	if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_HDMI))
+		return false;
+
+	return intel_vrr_always_use_vrr_tg(display);
+}
+
 void intel_vrr_compute_guardband(struct intel_crtc_state *crtc_state)
 {
 	struct intel_display *display = to_intel_display(crtc_state);
 	struct drm_display_mode *adjusted_mode = &crtc_state->hw.adjusted_mode;
 	struct drm_display_mode *pipe_mode = &crtc_state->hw.pipe_mode;
+	int guardband;
 
 	if (!intel_vrr_possible(crtc_state))
 		return;
 
-	crtc_state->vrr.guardband = min(crtc_state->vrr.vmin - adjusted_mode->crtc_vdisplay,
-					intel_vrr_max_guardband(crtc_state));
+	if (intel_vrr_use_optimized_guardband(crtc_state))
+		guardband = intel_vrr_compute_optimized_guardband(crtc_state);
+	else
+		guardband = crtc_state->vrr.vmin - adjusted_mode->crtc_vdisplay;
+
+	crtc_state->vrr.guardband = min(guardband, intel_vrr_max_guardband(crtc_state));
 
 	if (intel_vrr_always_use_vrr_tg(display)) {
 		adjusted_mode->crtc_vblank_start  =
-- 
2.45.2


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

* [PATCH 5/5] drm/i915/vrr: Use optimized guardband whenever VRR TG is active
  2025-10-17  5:01 [PATCH 0/5] Optimize vrr.guardband Ankit Nautiyal
                   ` (3 preceding siblings ...)
  2025-10-17  5:02 ` [PATCH 4/5] drm/i915/vrr: Use the min static optimized guardband Ankit Nautiyal
@ 2025-10-17  5:02 ` Ankit Nautiyal
  2025-10-17 12:13   ` Ville Syrjälä
  2025-10-17  6:10 ` ✓ i915.CI.BAT: success for Optimize vrr.guardband (rev3) Patchwork
  5 siblings, 1 reply; 23+ messages in thread
From: Ankit Nautiyal @ 2025-10-17  5:02 UTC (permalink / raw)
  To: intel-gfx, intel-xe; +Cc: ville.syrjala, Ankit Nautiyal

Currently the guardband is optimized only for platforms where the
VRR timing generator is always ON.

Extend the usage of optimized guardband to other platforms only when the
VRR is enabled.

Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
---
 drivers/gpu/drm/i915/display/intel_vrr.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/display/intel_vrr.c b/drivers/gpu/drm/i915/display/intel_vrr.c
index cd7bed358984..eb5aa0d7fc49 100644
--- a/drivers/gpu/drm/i915/display/intel_vrr.c
+++ b/drivers/gpu/drm/i915/display/intel_vrr.c
@@ -483,7 +483,7 @@ static bool intel_vrr_use_optimized_guardband(const struct intel_crtc_state *crt
 	if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_HDMI))
 		return false;
 
-	return intel_vrr_always_use_vrr_tg(display);
+	return intel_vrr_always_use_vrr_tg(display) || crtc_state->vrr.enable;
 }
 
 void intel_vrr_compute_guardband(struct intel_crtc_state *crtc_state)
-- 
2.45.2


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

* ✓ i915.CI.BAT: success for Optimize vrr.guardband (rev3)
  2025-10-17  5:01 [PATCH 0/5] Optimize vrr.guardband Ankit Nautiyal
                   ` (4 preceding siblings ...)
  2025-10-17  5:02 ` [PATCH 5/5] drm/i915/vrr: Use optimized guardband whenever VRR TG is active Ankit Nautiyal
@ 2025-10-17  6:10 ` Patchwork
  5 siblings, 0 replies; 23+ messages in thread
From: Patchwork @ 2025-10-17  6:10 UTC (permalink / raw)
  To: Ankit Nautiyal; +Cc: intel-gfx

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

== Series Details ==

Series: Optimize vrr.guardband (rev3)
URL   : https://patchwork.freedesktop.org/series/155980/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_17379 -> Patchwork_155980v3
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

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

Participating hosts (41 -> 40)
------------------------------

  Missing    (1): fi-snb-2520m 

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

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

### IGT changes ###

#### Issues hit ####

  * igt@i915_module_load@load:
    - bat-mtlp-9:         [PASS][1] -> [DMESG-WARN][2] ([i915#13494])
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_17379/bat-mtlp-9/igt@i915_module_load@load.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_155980v3/bat-mtlp-9/igt@i915_module_load@load.html

  * igt@i915_selftest@live:
    - bat-twl-1:          [PASS][3] -> [ABORT][4] ([i915#14365]) +1 other test abort
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_17379/bat-twl-1/igt@i915_selftest@live.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_155980v3/bat-twl-1/igt@i915_selftest@live.html

  * igt@i915_selftest@live@workarounds:
    - bat-mtlp-9:         [PASS][5] -> [DMESG-FAIL][6] ([i915#12061]) +1 other test dmesg-fail
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_17379/bat-mtlp-9/igt@i915_selftest@live@workarounds.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_155980v3/bat-mtlp-9/igt@i915_selftest@live@workarounds.html

  * igt@runner@aborted:
    - fi-bsw-nick:        NOTRUN -> [FAIL][7] ([i915#15124])
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_155980v3/fi-bsw-nick/igt@runner@aborted.html

  
#### Possible fixes ####

  * igt@i915_selftest@live:
    - bat-mtlp-8:         [DMESG-FAIL][8] ([i915#12061]) -> [PASS][9] +1 other test pass
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_17379/bat-mtlp-8/igt@i915_selftest@live.html
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_155980v3/bat-mtlp-8/igt@i915_selftest@live.html
    - fi-skl-6600u:       [INCOMPLETE][10] ([i915#15157]) -> [PASS][11] +1 other test pass
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_17379/fi-skl-6600u/igt@i915_selftest@live.html
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_155980v3/fi-skl-6600u/igt@i915_selftest@live.html

  * igt@i915_selftest@live@workarounds:
    - bat-dg2-9:          [DMESG-FAIL][12] ([i915#12061]) -> [PASS][13] +1 other test pass
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_17379/bat-dg2-9/igt@i915_selftest@live@workarounds.html
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_155980v3/bat-dg2-9/igt@i915_selftest@live@workarounds.html

  
  [i915#12061]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/12061
  [i915#13494]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/13494
  [i915#14365]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/14365
  [i915#15124]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/15124
  [i915#15157]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/15157


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

  * Linux: CI_DRM_17379 -> Patchwork_155980v3

  CI-20190529: 20190529
  CI_DRM_17379: b4f191dddaee9ae1871344e3dabcc40f2f14d2cb @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_8589: 8589
  Patchwork_155980v3: b4f191dddaee9ae1871344e3dabcc40f2f14d2cb @ git://anongit.freedesktop.org/gfx-ci/linux

== Logs ==

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

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

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

* Re: [PATCH 1/5] drm/i915/psr: Add helper to get min psr guardband
  2025-10-17  5:01 ` [PATCH 1/5] drm/i915/psr: Add helper to get min psr guardband Ankit Nautiyal
@ 2025-10-17  9:07   ` Hogander, Jouni
  2025-10-17  9:30     ` Hogander, Jouni
  2025-10-17  9:37     ` Nautiyal, Ankit K
  0 siblings, 2 replies; 23+ messages in thread
From: Hogander, Jouni @ 2025-10-17  9:07 UTC (permalink / raw)
  To: intel-xe@lists.freedesktop.org, Nautiyal, Ankit K,
	intel-gfx@lists.freedesktop.org
  Cc: ville.syrjala@linux.intel.com

On Fri, 2025-10-17 at 10:31 +0530, Ankit Nautiyal wrote:
> Introduce a helper to compute the max link wake latency when using
> Auxless/Aux wake mechanism for PSR/Panel Replay/LOBF features.
> 
> This will be used to compute the minimum guardband so that the link
> wake
> latencies are accounted and these features work smoothly for higher
> refresh rate panels.
> 
> Bspec: 70151, 71477
> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_psr.c | 12 ++++++++++++
>  drivers/gpu/drm/i915/display/intel_psr.h |  1 +
>  2 files changed, 13 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_psr.c
> b/drivers/gpu/drm/i915/display/intel_psr.c
> index 703e5f6af04c..a8303b669853 100644
> --- a/drivers/gpu/drm/i915/display/intel_psr.c
> +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> @@ -4416,3 +4416,15 @@ void intel_psr_compute_config_late(struct
> intel_dp *intel_dp,
>  
>  	intel_psr_set_non_psr_pipes(intel_dp, crtc_state);
>  }
> +
> +int intel_psr_min_guardband(struct intel_crtc_state *crtc_state)
> +{
> +	struct intel_display *display =
> to_intel_display(crtc_state);
> +	int auxless_wake_lines = crtc_state-
> >alpm_state.aux_less_wake_lines;
> +	int wake_lines = DISPLAY_VER(display) < 20 ?
> +			 psr2_block_count_lines(crtc_state-
> >alpm_state.io_wake_lines,
> +						crtc_state-
> >alpm_state.fast_wake_lines) :
> +			 crtc_state->alpm_state.io_wake_lines;
> +
> +	return max(auxless_wake_lines, wake_lines);

hmm, now if you add:

if (crtc_state->req_psr2_sdp_prior_scanline)
		psr_min_guardband++;

Whatever is the PSR mode it can be enabled what comes to vblank
restrictions and you can drop psr_compute_config_late?

BR,

Jouni Högander

> +}
> diff --git a/drivers/gpu/drm/i915/display/intel_psr.h
> b/drivers/gpu/drm/i915/display/intel_psr.h
> index b17ce312dc37..620b35928832 100644
> --- a/drivers/gpu/drm/i915/display/intel_psr.h
> +++ b/drivers/gpu/drm/i915/display/intel_psr.h
> @@ -85,5 +85,6 @@ bool intel_psr_needs_alpm_aux_less(struct intel_dp
> *intel_dp,
>  				   const struct intel_crtc_state
> *crtc_state);
>  void intel_psr_compute_config_late(struct intel_dp *intel_dp,
>  				   struct intel_crtc_state
> *crtc_state);
> +int intel_psr_min_guardband(struct intel_crtc_state *crtc_state);
>  
>  #endif /* __INTEL_PSR_H__ */


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

* Re: [PATCH 1/5] drm/i915/psr: Add helper to get min psr guardband
  2025-10-17  9:07   ` Hogander, Jouni
@ 2025-10-17  9:30     ` Hogander, Jouni
  2025-10-17  9:41       ` Nautiyal, Ankit K
  2025-10-17  9:37     ` Nautiyal, Ankit K
  1 sibling, 1 reply; 23+ messages in thread
From: Hogander, Jouni @ 2025-10-17  9:30 UTC (permalink / raw)
  To: intel-xe@lists.freedesktop.org, Nautiyal, Ankit K,
	intel-gfx@lists.freedesktop.org
  Cc: ville.syrjala@linux.intel.com

On Fri, 2025-10-17 at 09:07 +0000, Hogander, Jouni wrote:
> On Fri, 2025-10-17 at 10:31 +0530, Ankit Nautiyal wrote:
> > Introduce a helper to compute the max link wake latency when using
> > Auxless/Aux wake mechanism for PSR/Panel Replay/LOBF features.
> > 
> > This will be used to compute the minimum guardband so that the link
> > wake
> > latencies are accounted and these features work smoothly for higher
> > refresh rate panels.
> > 
> > Bspec: 70151, 71477
> > Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_psr.c | 12 ++++++++++++
> >  drivers/gpu/drm/i915/display/intel_psr.h |  1 +
> >  2 files changed, 13 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_psr.c
> > b/drivers/gpu/drm/i915/display/intel_psr.c
> > index 703e5f6af04c..a8303b669853 100644
> > --- a/drivers/gpu/drm/i915/display/intel_psr.c
> > +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> > @@ -4416,3 +4416,15 @@ void intel_psr_compute_config_late(struct
> > intel_dp *intel_dp,
> >  
> >  	intel_psr_set_non_psr_pipes(intel_dp, crtc_state);
> >  }
> > +
> > +int intel_psr_min_guardband(struct intel_crtc_state *crtc_state)
> > +{
> > +	struct intel_display *display =
> > to_intel_display(crtc_state);
> > +	int auxless_wake_lines = crtc_state-
> > > alpm_state.aux_less_wake_lines;
> > +	int wake_lines = DISPLAY_VER(display) < 20 ?
> > +			 psr2_block_count_lines(crtc_state-
> > > alpm_state.io_wake_lines,
> > +						crtc_state-
> > > alpm_state.fast_wake_lines) :
> > +			 crtc_state->alpm_state.io_wake_lines;
> > +
> > +	return max(auxless_wake_lines, wake_lines);
> 
> hmm, now if you add:
> 
> if (crtc_state->req_psr2_sdp_prior_scanline)
> 		psr_min_guardband++;
> 
> Whatever is the PSR mode it can be enabled what comes to vblank
> restrictions and you can drop psr_compute_config_late?

also this should be added to remove psr_compute_config_late:

        psr_min_guardband += _intel_psr_min_set_context_latency(crtc_state,
						     crtc_state->has_panel_replay,
						     crtc_state->has_sel_update);

BR,

Jouni Högander

> 
> BR,
> 
> Jouni Högander
> 
> > +}
> > diff --git a/drivers/gpu/drm/i915/display/intel_psr.h
> > b/drivers/gpu/drm/i915/display/intel_psr.h
> > index b17ce312dc37..620b35928832 100644
> > --- a/drivers/gpu/drm/i915/display/intel_psr.h
> > +++ b/drivers/gpu/drm/i915/display/intel_psr.h
> > @@ -85,5 +85,6 @@ bool intel_psr_needs_alpm_aux_less(struct
> > intel_dp
> > *intel_dp,
> >  				   const struct intel_crtc_state
> > *crtc_state);
> >  void intel_psr_compute_config_late(struct intel_dp *intel_dp,
> >  				   struct intel_crtc_state
> > *crtc_state);
> > +int intel_psr_min_guardband(struct intel_crtc_state *crtc_state);
> >  
> >  #endif /* __INTEL_PSR_H__ */
> 


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

* Re: [PATCH 1/5] drm/i915/psr: Add helper to get min psr guardband
  2025-10-17  9:07   ` Hogander, Jouni
  2025-10-17  9:30     ` Hogander, Jouni
@ 2025-10-17  9:37     ` Nautiyal, Ankit K
  2025-10-17  9:58       ` Hogander, Jouni
  1 sibling, 1 reply; 23+ messages in thread
From: Nautiyal, Ankit K @ 2025-10-17  9:37 UTC (permalink / raw)
  To: Hogander, Jouni, intel-xe@lists.freedesktop.org,
	intel-gfx@lists.freedesktop.org
  Cc: ville.syrjala@linux.intel.com


On 10/17/2025 2:37 PM, Hogander, Jouni wrote:
> On Fri, 2025-10-17 at 10:31 +0530, Ankit Nautiyal wrote:
>> Introduce a helper to compute the max link wake latency when using
>> Auxless/Aux wake mechanism for PSR/Panel Replay/LOBF features.
>>
>> This will be used to compute the minimum guardband so that the link
>> wake
>> latencies are accounted and these features work smoothly for higher
>> refresh rate panels.
>>
>> Bspec: 70151, 71477
>> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
>> ---
>>   drivers/gpu/drm/i915/display/intel_psr.c | 12 ++++++++++++
>>   drivers/gpu/drm/i915/display/intel_psr.h |  1 +
>>   2 files changed, 13 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/display/intel_psr.c
>> b/drivers/gpu/drm/i915/display/intel_psr.c
>> index 703e5f6af04c..a8303b669853 100644
>> --- a/drivers/gpu/drm/i915/display/intel_psr.c
>> +++ b/drivers/gpu/drm/i915/display/intel_psr.c
>> @@ -4416,3 +4416,15 @@ void intel_psr_compute_config_late(struct
>> intel_dp *intel_dp,
>>   
>>   	intel_psr_set_non_psr_pipes(intel_dp, crtc_state);
>>   }
>> +
>> +int intel_psr_min_guardband(struct intel_crtc_state *crtc_state)
>> +{
>> +	struct intel_display *display =
>> to_intel_display(crtc_state);
>> +	int auxless_wake_lines = crtc_state-
>>> alpm_state.aux_less_wake_lines;
>> +	int wake_lines = DISPLAY_VER(display) < 20 ?
>> +			 psr2_block_count_lines(crtc_state-
>>> alpm_state.io_wake_lines,
>> +						crtc_state-
>>> alpm_state.fast_wake_lines) :
>> +			 crtc_state->alpm_state.io_wake_lines;
>> +
>> +	return max(auxless_wake_lines, wake_lines);
> hmm, now if you add:
>
> if (crtc_state->req_psr2_sdp_prior_scanline)
> 		psr_min_guardband++;

I did not get this part. Do we need to account for 1 more wakelines if 
this flag is set?

What we want to do is to check for min guardband for 
panel_replay/sel_update based on the required wakelines.

Whether we can use the auxless_wake_lines and wake_lines as computed 
above to estimate the max wakelines or instead we should use functions 
from alpm.c :

io_buffer_wake_time() and _lnl_compute_aux_less_wake_time() to get the 
worst case wakelines.


>
> Whatever is the PSR mode it can be enabled what comes to vblank
> restrictions and you can drop psr_compute_config_late?


I think we cannot drop the psr_compute_config_late as it checks whether 
the actual guardband is enough for PSR features.

intel_psr_min_guardband() is used along with other features to have an estimate on the guardband that works for all cases, without a need to change the guardband.
It is bounded by the vblank length available

Regards,

Ankit

>
> BR,
>
> Jouni Högander
>
>> +}
>> diff --git a/drivers/gpu/drm/i915/display/intel_psr.h
>> b/drivers/gpu/drm/i915/display/intel_psr.h
>> index b17ce312dc37..620b35928832 100644
>> --- a/drivers/gpu/drm/i915/display/intel_psr.h
>> +++ b/drivers/gpu/drm/i915/display/intel_psr.h
>> @@ -85,5 +85,6 @@ bool intel_psr_needs_alpm_aux_less(struct intel_dp
>> *intel_dp,
>>   				   const struct intel_crtc_state
>> *crtc_state);
>>   void intel_psr_compute_config_late(struct intel_dp *intel_dp,
>>   				   struct intel_crtc_state
>> *crtc_state);
>> +int intel_psr_min_guardband(struct intel_crtc_state *crtc_state);
>>   
>>   #endif /* __INTEL_PSR_H__ */

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

* Re: [PATCH 1/5] drm/i915/psr: Add helper to get min psr guardband
  2025-10-17  9:30     ` Hogander, Jouni
@ 2025-10-17  9:41       ` Nautiyal, Ankit K
  0 siblings, 0 replies; 23+ messages in thread
From: Nautiyal, Ankit K @ 2025-10-17  9:41 UTC (permalink / raw)
  To: Hogander, Jouni, intel-xe@lists.freedesktop.org,
	intel-gfx@lists.freedesktop.org
  Cc: ville.syrjala@linux.intel.com


On 10/17/2025 3:00 PM, Hogander, Jouni wrote:
> On Fri, 2025-10-17 at 09:07 +0000, Hogander, Jouni wrote:
>> On Fri, 2025-10-17 at 10:31 +0530, Ankit Nautiyal wrote:
>>> Introduce a helper to compute the max link wake latency when using
>>> Auxless/Aux wake mechanism for PSR/Panel Replay/LOBF features.
>>>
>>> This will be used to compute the minimum guardband so that the link
>>> wake
>>> latencies are accounted and these features work smoothly for higher
>>> refresh rate panels.
>>>
>>> Bspec: 70151, 71477
>>> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
>>> ---
>>>   drivers/gpu/drm/i915/display/intel_psr.c | 12 ++++++++++++
>>>   drivers/gpu/drm/i915/display/intel_psr.h |  1 +
>>>   2 files changed, 13 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/i915/display/intel_psr.c
>>> b/drivers/gpu/drm/i915/display/intel_psr.c
>>> index 703e5f6af04c..a8303b669853 100644
>>> --- a/drivers/gpu/drm/i915/display/intel_psr.c
>>> +++ b/drivers/gpu/drm/i915/display/intel_psr.c
>>> @@ -4416,3 +4416,15 @@ void intel_psr_compute_config_late(struct
>>> intel_dp *intel_dp,
>>>   
>>>   	intel_psr_set_non_psr_pipes(intel_dp, crtc_state);
>>>   }
>>> +
>>> +int intel_psr_min_guardband(struct intel_crtc_state *crtc_state)
>>> +{
>>> +	struct intel_display *display =
>>> to_intel_display(crtc_state);
>>> +	int auxless_wake_lines = crtc_state-
>>>> alpm_state.aux_less_wake_lines;
>>> +	int wake_lines = DISPLAY_VER(display) < 20 ?
>>> +			 psr2_block_count_lines(crtc_state-
>>>> alpm_state.io_wake_lines,
>>> +						crtc_state-
>>>> alpm_state.fast_wake_lines) :
>>> +			 crtc_state->alpm_state.io_wake_lines;
>>> +
>>> +	return max(auxless_wake_lines, wake_lines);
>> hmm, now if you add:
>>
>> if (crtc_state->req_psr2_sdp_prior_scanline)
>> 		psr_min_guardband++;
>>
>> Whatever is the PSR mode it can be enabled what comes to vblank
>> restrictions and you can drop psr_compute_config_late?
> also this should be added to remove psr_compute_config_late:
>
>          psr_min_guardband += _intel_psr_min_set_context_latency(crtc_state,
> 						     crtc_state->has_panel_replay,
> 						     crtc_state->has_sel_update);

No we dont need to use scl here.

We compare the actual optimized guardband based on different features 
and then take min(optimized_guardband, intel_vrr_max_vblank_guardband())

intel_vrr_max_vblank_guardband() takes into account if there are SCL 
lines, so those are subtracted.

Regards,

Ankit

>
> BR,
>
> Jouni Högander
>
>> BR,
>>
>> Jouni Högander
>>
>>> +}
>>> diff --git a/drivers/gpu/drm/i915/display/intel_psr.h
>>> b/drivers/gpu/drm/i915/display/intel_psr.h
>>> index b17ce312dc37..620b35928832 100644
>>> --- a/drivers/gpu/drm/i915/display/intel_psr.h
>>> +++ b/drivers/gpu/drm/i915/display/intel_psr.h
>>> @@ -85,5 +85,6 @@ bool intel_psr_needs_alpm_aux_less(struct
>>> intel_dp
>>> *intel_dp,
>>>   				   const struct intel_crtc_state
>>> *crtc_state);
>>>   void intel_psr_compute_config_late(struct intel_dp *intel_dp,
>>>   				   struct intel_crtc_state
>>> *crtc_state);
>>> +int intel_psr_min_guardband(struct intel_crtc_state *crtc_state);
>>>   
>>>   #endif /* __INTEL_PSR_H__ */

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

* Re: [PATCH 1/5] drm/i915/psr: Add helper to get min psr guardband
  2025-10-17  9:37     ` Nautiyal, Ankit K
@ 2025-10-17  9:58       ` Hogander, Jouni
  2025-10-17 10:15         ` Hogander, Jouni
  0 siblings, 1 reply; 23+ messages in thread
From: Hogander, Jouni @ 2025-10-17  9:58 UTC (permalink / raw)
  To: intel-xe@lists.freedesktop.org, Nautiyal, Ankit K,
	intel-gfx@lists.freedesktop.org
  Cc: ville.syrjala@linux.intel.com

On Fri, 2025-10-17 at 15:07 +0530, Nautiyal, Ankit K wrote:
> 
> On 10/17/2025 2:37 PM, Hogander, Jouni wrote:
> > On Fri, 2025-10-17 at 10:31 +0530, Ankit Nautiyal wrote:
> > > Introduce a helper to compute the max link wake latency when
> > > using
> > > Auxless/Aux wake mechanism for PSR/Panel Replay/LOBF features.
> > > 
> > > This will be used to compute the minimum guardband so that the
> > > link
> > > wake
> > > latencies are accounted and these features work smoothly for
> > > higher
> > > refresh rate panels.
> > > 
> > > Bspec: 70151, 71477
> > > Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
> > > ---
> > >   drivers/gpu/drm/i915/display/intel_psr.c | 12 ++++++++++++
> > >   drivers/gpu/drm/i915/display/intel_psr.h |  1 +
> > >   2 files changed, 13 insertions(+)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/display/intel_psr.c
> > > b/drivers/gpu/drm/i915/display/intel_psr.c
> > > index 703e5f6af04c..a8303b669853 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_psr.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> > > @@ -4416,3 +4416,15 @@ void intel_psr_compute_config_late(struct
> > > intel_dp *intel_dp,
> > >   
> > >   	intel_psr_set_non_psr_pipes(intel_dp, crtc_state);
> > >   }
> > > +
> > > +int intel_psr_min_guardband(struct intel_crtc_state *crtc_state)
> > > +{
> > > +	struct intel_display *display =
> > > to_intel_display(crtc_state);
> > > +	int auxless_wake_lines = crtc_state-
> > > > alpm_state.aux_less_wake_lines;
> > > +	int wake_lines = DISPLAY_VER(display) < 20 ?
> > > +			 psr2_block_count_lines(crtc_state-
> > > > alpm_state.io_wake_lines,
> > > +						crtc_state-
> > > > alpm_state.fast_wake_lines) :
> > > +			 crtc_state->alpm_state.io_wake_lines;
> > > +
> > > +	return max(auxless_wake_lines, wake_lines);
> > hmm, now if you add:
> > 
> > if (crtc_state->req_psr2_sdp_prior_scanline)
> > 		psr_min_guardband++;
> 
> I did not get this part. Do we need to account for 1 more wakelines
> if 
> this flag is set?

If you look at how that flag affects vblank check in 
intel_psr_compute_config_late:

...
static bool _wake_lines_fit_into_vblank(const struct intel_crtc_state *crtc_state,
					int vblank,
					int wake_lines)
{
	if (crtc_state->req_psr2_sdp_prior_scanline)
		vblank -= 1;
...

So to take that into account when calculating minimum guardband needed
by PSR you need to increase by one. Same goes with SCL:

...
	int scl = _intel_psr_min_set_context_latency(crtc_state,
						     needs_panel_replay,
						     needs_sel_update);
	vblank -= scl;
...

You are already partially taking into account PSR needs when
calculating optimized guardband except these two lines which are needed
conditionally.

Also intel_psr_compute_config is run at this point -> you know which
one to use: auxless wake time or aux wake time. no need to use max()
with them. Just choose the one which is relevant.

With these changes you could drop intel_psr_compute_config_late as
vblank would be long enough for PSR mode computed by
intel_psr_compute_config, no?

BR,

Jouni Högander


> 
> What we want to do is to check for min guardband for 
> panel_replay/sel_update based on the required wakelines.
> 
> Whether we can use the auxless_wake_lines and wake_lines as computed 
> above to estimate the max wakelines or instead we should use
> functions 
> from alpm.c :
> 
> io_buffer_wake_time() and _lnl_compute_aux_less_wake_time() to get
> the 
> worst case wakelines.
> 
> 
> > 
> > Whatever is the PSR mode it can be enabled what comes to vblank
> > restrictions and you can drop psr_compute_config_late?
> 
> 
> I think we cannot drop the psr_compute_config_late as it checks
> whether 
> the actual guardband is enough for PSR features.
> 
> intel_psr_min_guardband() is used along with other features to have
> an estimate on the guardband that works for all cases, without a need
> to change the guardband.
> It is bounded by the vblank length available
> 
> Regards,
> 
> Ankit
> 
> > 
> > BR,
> > 
> > Jouni Högander
> > 
> > > +}
> > > diff --git a/drivers/gpu/drm/i915/display/intel_psr.h
> > > b/drivers/gpu/drm/i915/display/intel_psr.h
> > > index b17ce312dc37..620b35928832 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_psr.h
> > > +++ b/drivers/gpu/drm/i915/display/intel_psr.h
> > > @@ -85,5 +85,6 @@ bool intel_psr_needs_alpm_aux_less(struct
> > > intel_dp
> > > *intel_dp,
> > >   				   const struct intel_crtc_state
> > > *crtc_state);
> > >   void intel_psr_compute_config_late(struct intel_dp *intel_dp,
> > >   				   struct intel_crtc_state
> > > *crtc_state);
> > > +int intel_psr_min_guardband(struct intel_crtc_state
> > > *crtc_state);
> > >   
> > >   #endif /* __INTEL_PSR_H__ */


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

* Re: [PATCH 1/5] drm/i915/psr: Add helper to get min psr guardband
  2025-10-17  9:58       ` Hogander, Jouni
@ 2025-10-17 10:15         ` Hogander, Jouni
  2025-10-17 10:30           ` Hogander, Jouni
  2025-10-17 11:11           ` Nautiyal, Ankit K
  0 siblings, 2 replies; 23+ messages in thread
From: Hogander, Jouni @ 2025-10-17 10:15 UTC (permalink / raw)
  To: intel-xe@lists.freedesktop.org, Nautiyal, Ankit K,
	intel-gfx@lists.freedesktop.org
  Cc: ville.syrjala@linux.intel.com

On Fri, 2025-10-17 at 12:58 +0300, Hogander, Jouni wrote:
> On Fri, 2025-10-17 at 15:07 +0530, Nautiyal, Ankit K wrote:
> > 
> > On 10/17/2025 2:37 PM, Hogander, Jouni wrote:
> > > On Fri, 2025-10-17 at 10:31 +0530, Ankit Nautiyal wrote:
> > > > Introduce a helper to compute the max link wake latency when
> > > > using
> > > > Auxless/Aux wake mechanism for PSR/Panel Replay/LOBF features.
> > > > 
> > > > This will be used to compute the minimum guardband so that the
> > > > link
> > > > wake
> > > > latencies are accounted and these features work smoothly for
> > > > higher
> > > > refresh rate panels.
> > > > 
> > > > Bspec: 70151, 71477
> > > > Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
> > > > ---
> > > >   drivers/gpu/drm/i915/display/intel_psr.c | 12 ++++++++++++
> > > >   drivers/gpu/drm/i915/display/intel_psr.h |  1 +
> > > >   2 files changed, 13 insertions(+)
> > > > 
> > > > diff --git a/drivers/gpu/drm/i915/display/intel_psr.c
> > > > b/drivers/gpu/drm/i915/display/intel_psr.c
> > > > index 703e5f6af04c..a8303b669853 100644
> > > > --- a/drivers/gpu/drm/i915/display/intel_psr.c
> > > > +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> > > > @@ -4416,3 +4416,15 @@ void
> > > > intel_psr_compute_config_late(struct
> > > > intel_dp *intel_dp,
> > > >   
> > > >   	intel_psr_set_non_psr_pipes(intel_dp, crtc_state);
> > > >   }
> > > > +
> > > > +int intel_psr_min_guardband(struct intel_crtc_state
> > > > *crtc_state)
> > > > +{
> > > > +	struct intel_display *display =
> > > > to_intel_display(crtc_state);
> > > > +	int auxless_wake_lines = crtc_state-
> > > > > alpm_state.aux_less_wake_lines;
> > > > +	int wake_lines = DISPLAY_VER(display) < 20 ?
> > > > +			 psr2_block_count_lines(crtc_state-
> > > > > alpm_state.io_wake_lines,
> > > > +						crtc_state-
> > > > > alpm_state.fast_wake_lines) :
> > > > +			 crtc_state->alpm_state.io_wake_lines;
> > > > +
> > > > +	return max(auxless_wake_lines, wake_lines);
> > > hmm, now if you add:
> > > 
> > > if (crtc_state->req_psr2_sdp_prior_scanline)
> > > 		psr_min_guardband++;
> > 
> > I did not get this part. Do we need to account for 1 more wakelines
> > if 
> > this flag is set?
> 
> If you look at how that flag affects vblank check in 
> intel_psr_compute_config_late:
> 
> ...
> static bool _wake_lines_fit_into_vblank(const struct intel_crtc_state
> *crtc_state,
> 					int vblank,
> 					int wake_lines)
> {
> 	if (crtc_state->req_psr2_sdp_prior_scanline)
> 		vblank -= 1;
> ...
> 
> So to take that into account when calculating minimum guardband
> needed
> by PSR you need to increase by one. Same goes with SCL:
> 
> ...
> 	int scl = _intel_psr_min_set_context_latency(crtc_state,
> 						    
> needs_panel_replay,
> 						    
> needs_sel_update);
> 	vblank -= scl;
> ...
> 
> You are already partially taking into account PSR needs when
> calculating optimized guardband except these two lines which are
> needed
> conditionally.
> 
> Also intel_psr_compute_config is run at this point -> you know which
> one to use: auxless wake time or aux wake time. no need to use max()
> with them. Just choose the one which is relevant.
> 
> With these changes you could drop intel_psr_compute_config_late as
> vblank would be long enough for PSR mode computed by
> intel_psr_compute_config, no?

Ok, noticed now this in the last patch:

...
crtc_state->vrr.guardband = min(guardband, intel_vrr_max_guardband(crtc_state));
...

So if we need to fall back to intel_vrr_max_guardband we need to have
that intel_psr_compute_config_late.

Anyways I think you need to take into account that
req_psr2_sdp_prior_scanline and _intel_psr_min_set_context_latency in
intel_psr_min_guardband.

BR,

Jouni Högander
> 
> BR,
> 
> Jouni Högander
> 
> 
> > 
> > What we want to do is to check for min guardband for 
> > panel_replay/sel_update based on the required wakelines.
> > 
> > Whether we can use the auxless_wake_lines and wake_lines as
> > computed 
> > above to estimate the max wakelines or instead we should use
> > functions 
> > from alpm.c :
> > 
> > io_buffer_wake_time() and _lnl_compute_aux_less_wake_time() to get
> > the 
> > worst case wakelines.
> > 
> > 
> > > 
> > > Whatever is the PSR mode it can be enabled what comes to vblank
> > > restrictions and you can drop psr_compute_config_late?
> > 
> > 
> > I think we cannot drop the psr_compute_config_late as it checks
> > whether 
> > the actual guardband is enough for PSR features.
> > 
> > intel_psr_min_guardband() is used along with other features to have
> > an estimate on the guardband that works for all cases, without a
> > need
> > to change the guardband.
> > It is bounded by the vblank length available
> > 
> > Regards,
> > 
> > Ankit
> > 
> > > 
> > > BR,
> > > 
> > > Jouni Högander
> > > 
> > > > +}
> > > > diff --git a/drivers/gpu/drm/i915/display/intel_psr.h
> > > > b/drivers/gpu/drm/i915/display/intel_psr.h
> > > > index b17ce312dc37..620b35928832 100644
> > > > --- a/drivers/gpu/drm/i915/display/intel_psr.h
> > > > +++ b/drivers/gpu/drm/i915/display/intel_psr.h
> > > > @@ -85,5 +85,6 @@ bool intel_psr_needs_alpm_aux_less(struct
> > > > intel_dp
> > > > *intel_dp,
> > > >   				   const struct
> > > > intel_crtc_state
> > > > *crtc_state);
> > > >   void intel_psr_compute_config_late(struct intel_dp *intel_dp,
> > > >   				   struct intel_crtc_state
> > > > *crtc_state);
> > > > +int intel_psr_min_guardband(struct intel_crtc_state
> > > > *crtc_state);
> > > >   
> > > >   #endif /* __INTEL_PSR_H__ */
> 


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

* Re: [PATCH 1/5] drm/i915/psr: Add helper to get min psr guardband
  2025-10-17 10:15         ` Hogander, Jouni
@ 2025-10-17 10:30           ` Hogander, Jouni
  2025-10-17 11:11           ` Nautiyal, Ankit K
  1 sibling, 0 replies; 23+ messages in thread
From: Hogander, Jouni @ 2025-10-17 10:30 UTC (permalink / raw)
  To: intel-xe@lists.freedesktop.org, Nautiyal, Ankit K,
	intel-gfx@lists.freedesktop.org
  Cc: ville.syrjala@linux.intel.com

On Fri, 2025-10-17 at 13:15 +0300, Hogander, Jouni wrote:
> On Fri, 2025-10-17 at 12:58 +0300, Hogander, Jouni wrote:
> > On Fri, 2025-10-17 at 15:07 +0530, Nautiyal, Ankit K wrote:
> > > 
> > > On 10/17/2025 2:37 PM, Hogander, Jouni wrote:
> > > > On Fri, 2025-10-17 at 10:31 +0530, Ankit Nautiyal wrote:
> > > > > Introduce a helper to compute the max link wake latency when
> > > > > using
> > > > > Auxless/Aux wake mechanism for PSR/Panel Replay/LOBF
> > > > > features.
> > > > > 
> > > > > This will be used to compute the minimum guardband so that
> > > > > the
> > > > > link
> > > > > wake
> > > > > latencies are accounted and these features work smoothly for
> > > > > higher
> > > > > refresh rate panels.
> > > > > 
> > > > > Bspec: 70151, 71477
> > > > > Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
> > > > > ---
> > > > >   drivers/gpu/drm/i915/display/intel_psr.c | 12 ++++++++++++
> > > > >   drivers/gpu/drm/i915/display/intel_psr.h |  1 +
> > > > >   2 files changed, 13 insertions(+)
> > > > > 
> > > > > diff --git a/drivers/gpu/drm/i915/display/intel_psr.c
> > > > > b/drivers/gpu/drm/i915/display/intel_psr.c
> > > > > index 703e5f6af04c..a8303b669853 100644
> > > > > --- a/drivers/gpu/drm/i915/display/intel_psr.c
> > > > > +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> > > > > @@ -4416,3 +4416,15 @@ void
> > > > > intel_psr_compute_config_late(struct
> > > > > intel_dp *intel_dp,
> > > > >   
> > > > >   	intel_psr_set_non_psr_pipes(intel_dp, crtc_state);
> > > > >   }
> > > > > +
> > > > > +int intel_psr_min_guardband(struct intel_crtc_state
> > > > > *crtc_state)
> > > > > +{
> > > > > +	struct intel_display *display =
> > > > > to_intel_display(crtc_state);
> > > > > +	int auxless_wake_lines = crtc_state-
> > > > > > alpm_state.aux_less_wake_lines;
> > > > > +	int wake_lines = DISPLAY_VER(display) < 20 ?
> > > > > +			 psr2_block_count_lines(crtc_state-
> > > > > > alpm_state.io_wake_lines,
> > > > > +						crtc_state-
> > > > > > alpm_state.fast_wake_lines) :
> > > > > +			 crtc_state-
> > > > > >alpm_state.io_wake_lines;
> > > > > +
> > > > > +	return max(auxless_wake_lines, wake_lines);
> > > > hmm, now if you add:
> > > > 
> > > > if (crtc_state->req_psr2_sdp_prior_scanline)
> > > > 		psr_min_guardband++;
> > > 
> > > I did not get this part. Do we need to account for 1 more
> > > wakelines
> > > if 
> > > this flag is set?
> > 
> > If you look at how that flag affects vblank check in 
> > intel_psr_compute_config_late:
> > 
> > ...
> > static bool _wake_lines_fit_into_vblank(const struct
> > intel_crtc_state
> > *crtc_state,
> > 					int vblank,
> > 					int wake_lines)
> > {
> > 	if (crtc_state->req_psr2_sdp_prior_scanline)
> > 		vblank -= 1;
> > ...
> > 
> > So to take that into account when calculating minimum guardband
> > needed
> > by PSR you need to increase by one. Same goes with SCL:
> > 
> > ...
> > 	int scl = _intel_psr_min_set_context_latency(crtc_state,
> > 						    
> > needs_panel_replay,
> > 						    
> > needs_sel_update);
> > 	vblank -= scl;
> > ...
> > 
> > You are already partially taking into account PSR needs when
> > calculating optimized guardband except these two lines which are
> > needed
> > conditionally.
> > 
> > Also intel_psr_compute_config is run at this point -> you know
> > which
> > one to use: auxless wake time or aux wake time. no need to use
> > max()
> > with them. Just choose the one which is relevant.
> > 
> > With these changes you could drop intel_psr_compute_config_late as
> > vblank would be long enough for PSR mode computed by
> > intel_psr_compute_config, no?
> 
> Ok, noticed now this in the last patch:
> 
> ...
> crtc_state->vrr.guardband = min(guardband,
> intel_vrr_max_guardband(crtc_state));
> ...
> 
> So if we need to fall back to intel_vrr_max_guardband we need to have
> that intel_psr_compute_config_late.
> 
> Anyways I think you need to take into account that
> req_psr2_sdp_prior_scanline and _intel_psr_min_set_context_latency in
> intel_psr_min_guardband.

Also you can use auxless wake time for Panel Replay with ALPM and aux
wake time for PSR2 because only following changes are possible in
intel_psr_compute_config_late:

PSR2 (aux wake time) -> PSR1 -> guardband length doesn't matter
Panel Replay (auxless wake time) -> disabled -> guardband length
doesn't matter

BR,

Jouni Högander

> 
> BR,
> 
> Jouni Högander
> > 
> > BR,
> > 
> > Jouni Högander
> > 
> > 
> > > 
> > > What we want to do is to check for min guardband for 
> > > panel_replay/sel_update based on the required wakelines.
> > > 
> > > Whether we can use the auxless_wake_lines and wake_lines as
> > > computed 
> > > above to estimate the max wakelines or instead we should use
> > > functions 
> > > from alpm.c :
> > > 
> > > io_buffer_wake_time() and _lnl_compute_aux_less_wake_time() to
> > > get
> > > the 
> > > worst case wakelines.
> > > 
> > > 
> > > > 
> > > > Whatever is the PSR mode it can be enabled what comes to vblank
> > > > restrictions and you can drop psr_compute_config_late?
> > > 
> > > 
> > > I think we cannot drop the psr_compute_config_late as it checks
> > > whether 
> > > the actual guardband is enough for PSR features.
> > > 
> > > intel_psr_min_guardband() is used along with other features to
> > > have
> > > an estimate on the guardband that works for all cases, without a
> > > need
> > > to change the guardband.
> > > It is bounded by the vblank length available
> > > 
> > > Regards,
> > > 
> > > Ankit
> > > 
> > > > 
> > > > BR,
> > > > 
> > > > Jouni Högander
> > > > 
> > > > > +}
> > > > > diff --git a/drivers/gpu/drm/i915/display/intel_psr.h
> > > > > b/drivers/gpu/drm/i915/display/intel_psr.h
> > > > > index b17ce312dc37..620b35928832 100644
> > > > > --- a/drivers/gpu/drm/i915/display/intel_psr.h
> > > > > +++ b/drivers/gpu/drm/i915/display/intel_psr.h
> > > > > @@ -85,5 +85,6 @@ bool intel_psr_needs_alpm_aux_less(struct
> > > > > intel_dp
> > > > > *intel_dp,
> > > > >   				   const struct
> > > > > intel_crtc_state
> > > > > *crtc_state);
> > > > >   void intel_psr_compute_config_late(struct intel_dp
> > > > > *intel_dp,
> > > > >   				   struct intel_crtc_state
> > > > > *crtc_state);
> > > > > +int intel_psr_min_guardband(struct intel_crtc_state
> > > > > *crtc_state);
> > > > >   
> > > > >   #endif /* __INTEL_PSR_H__ */
> > 
> 


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

* Re: [PATCH 2/5] drm/i915/dp: Add helper to get min sdp guardband
  2025-10-17  5:01 ` [PATCH 2/5] drm/i915/dp: Add helper to get min sdp guardband Ankit Nautiyal
@ 2025-10-17 10:50   ` Hogander, Jouni
  2025-10-17 11:07     ` Nautiyal, Ankit K
  0 siblings, 1 reply; 23+ messages in thread
From: Hogander, Jouni @ 2025-10-17 10:50 UTC (permalink / raw)
  To: intel-xe@lists.freedesktop.org, Nautiyal, Ankit K,
	intel-gfx@lists.freedesktop.org
  Cc: ville.syrjala@linux.intel.com

On Fri, 2025-10-17 at 10:31 +0530, Ankit Nautiyal wrote:
> Add a helper to compute vblank time needed for transmitting specific
> DisplayPort SDPs like PPS, GAMUT_METADATA, and VSC_EXT. Latency is
> based on line count per packet type.
> 
> This will be used to ensure adequate guardband when features like
> DSC/HDR
> are enabled.
> 
> Bspec: 70151
> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_dp.c | 36
> +++++++++++++++++++++++++
>  drivers/gpu/drm/i915/display/intel_dp.h |  2 ++
>  2 files changed, 38 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
> b/drivers/gpu/drm/i915/display/intel_dp.c
> index 7059d55687cf..3f2c319e3d6f 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -6990,3 +6990,39 @@ int intel_dp_compute_config_late(struct
> intel_encoder *encoder,
>  
>  	return 0;
>  }
> +
> +static
> +int intel_dp_get_lines_for_sdp(u32 type)
> +{
> +	switch (type) {
> +	case DP_SDP_VSC_EXT_VESA:
> +	case DP_SDP_VSC_EXT_CEA:
> +		return 10;
> +	case HDMI_PACKET_TYPE_GAMUT_METADATA:
> +		return 8;
> +	case DP_SDP_PPS:
> +		return 6;

I found value 7 in the Bspec you are referring in commit message?

BR,

Jouni Högander

> +	default:
> +		break;
> +	}
> +
> +	return 0;
> +}
> +
> +int intel_dp_sdp_min_guardband(const struct intel_crtc_state
> *crtc_state,
> +			       bool assume_all_enabled)
> +{
> +	int sdp_guardband = 0;
> +
> +	if (assume_all_enabled ||
> +	    crtc_state->infoframes.enable &
> +	   
> intel_hdmi_infoframe_enable(HDMI_PACKET_TYPE_GAMUT_METADATA))
> +		sdp_guardband = max(sdp_guardband,
> +				   
> intel_dp_get_lines_for_sdp(HDMI_PACKET_TYPE_GAMUT_METADATA));
> +
> +	if (assume_all_enabled ||
> +	    crtc_state->dsc.compression_enable)
> +		sdp_guardband = max(sdp_guardband,
> intel_dp_get_lines_for_sdp(DP_SDP_PPS));
> +
> +	return sdp_guardband;
> +}
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.h
> b/drivers/gpu/drm/i915/display/intel_dp.h
> index 281ced3a3b39..7ee5aeb28fe2 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.h
> +++ b/drivers/gpu/drm/i915/display/intel_dp.h
> @@ -221,5 +221,7 @@ bool intel_dp_in_hdr_mode(const struct
> drm_connector_state *conn_state);
>  int intel_dp_compute_config_late(struct intel_encoder *encoder,
>  				 struct intel_crtc_state
> *crtc_state,
>  				 struct drm_connector_state
> *conn_state);
> +int intel_dp_sdp_min_guardband(const struct intel_crtc_state
> *crtc_state,
> +			       bool assume_all_enabled);
>  
>  #endif /* __INTEL_DP_H__ */


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

* Re: [PATCH 2/5] drm/i915/dp: Add helper to get min sdp guardband
  2025-10-17 10:50   ` Hogander, Jouni
@ 2025-10-17 11:07     ` Nautiyal, Ankit K
  0 siblings, 0 replies; 23+ messages in thread
From: Nautiyal, Ankit K @ 2025-10-17 11:07 UTC (permalink / raw)
  To: Hogander, Jouni, intel-xe@lists.freedesktop.org,
	intel-gfx@lists.freedesktop.org
  Cc: ville.syrjala@linux.intel.com


On 10/17/2025 4:20 PM, Hogander, Jouni wrote:
> On Fri, 2025-10-17 at 10:31 +0530, Ankit Nautiyal wrote:
>> Add a helper to compute vblank time needed for transmitting specific
>> DisplayPort SDPs like PPS, GAMUT_METADATA, and VSC_EXT. Latency is
>> based on line count per packet type.
>>
>> This will be used to ensure adequate guardband when features like
>> DSC/HDR
>> are enabled.
>>
>> Bspec: 70151
>> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
>> ---
>>   drivers/gpu/drm/i915/display/intel_dp.c | 36
>> +++++++++++++++++++++++++
>>   drivers/gpu/drm/i915/display/intel_dp.h |  2 ++
>>   2 files changed, 38 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
>> b/drivers/gpu/drm/i915/display/intel_dp.c
>> index 7059d55687cf..3f2c319e3d6f 100644
>> --- a/drivers/gpu/drm/i915/display/intel_dp.c
>> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
>> @@ -6990,3 +6990,39 @@ int intel_dp_compute_config_late(struct
>> intel_encoder *encoder,
>>   
>>   	return 0;
>>   }
>> +
>> +static
>> +int intel_dp_get_lines_for_sdp(u32 type)
>> +{
>> +	switch (type) {
>> +	case DP_SDP_VSC_EXT_VESA:
>> +	case DP_SDP_VSC_EXT_CEA:
>> +		return 10;
>> +	case HDMI_PACKET_TYPE_GAMUT_METADATA:
>> +		return 8;
>> +	case DP_SDP_PPS:
>> +		return 6;
> I found value 7 in the Bspec you are referring in commit message?

Hmm this indeed is 7, seems I misread the spec. Thanks for catching this!

Regards,

Ankit

>
> BR,
>
> Jouni Högander
>
>> +	default:
>> +		break;
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>> +int intel_dp_sdp_min_guardband(const struct intel_crtc_state
>> *crtc_state,
>> +			       bool assume_all_enabled)
>> +{
>> +	int sdp_guardband = 0;
>> +
>> +	if (assume_all_enabled ||
>> +	    crtc_state->infoframes.enable &
>> +	
>> intel_hdmi_infoframe_enable(HDMI_PACKET_TYPE_GAMUT_METADATA))
>> +		sdp_guardband = max(sdp_guardband,
>> +				
>> intel_dp_get_lines_for_sdp(HDMI_PACKET_TYPE_GAMUT_METADATA));
>> +
>> +	if (assume_all_enabled ||
>> +	    crtc_state->dsc.compression_enable)
>> +		sdp_guardband = max(sdp_guardband,
>> intel_dp_get_lines_for_sdp(DP_SDP_PPS));
>> +
>> +	return sdp_guardband;
>> +}
>> diff --git a/drivers/gpu/drm/i915/display/intel_dp.h
>> b/drivers/gpu/drm/i915/display/intel_dp.h
>> index 281ced3a3b39..7ee5aeb28fe2 100644
>> --- a/drivers/gpu/drm/i915/display/intel_dp.h
>> +++ b/drivers/gpu/drm/i915/display/intel_dp.h
>> @@ -221,5 +221,7 @@ bool intel_dp_in_hdr_mode(const struct
>> drm_connector_state *conn_state);
>>   int intel_dp_compute_config_late(struct intel_encoder *encoder,
>>   				 struct intel_crtc_state
>> *crtc_state,
>>   				 struct drm_connector_state
>> *conn_state);
>> +int intel_dp_sdp_min_guardband(const struct intel_crtc_state
>> *crtc_state,
>> +			       bool assume_all_enabled);
>>   
>>   #endif /* __INTEL_DP_H__ */

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

* Re: [PATCH 1/5] drm/i915/psr: Add helper to get min psr guardband
  2025-10-17 10:15         ` Hogander, Jouni
  2025-10-17 10:30           ` Hogander, Jouni
@ 2025-10-17 11:11           ` Nautiyal, Ankit K
  2025-10-17 11:30             ` Hogander, Jouni
  1 sibling, 1 reply; 23+ messages in thread
From: Nautiyal, Ankit K @ 2025-10-17 11:11 UTC (permalink / raw)
  To: Hogander, Jouni, intel-xe@lists.freedesktop.org,
	intel-gfx@lists.freedesktop.org
  Cc: ville.syrjala@linux.intel.com


On 10/17/2025 3:45 PM, Hogander, Jouni wrote:
> On Fri, 2025-10-17 at 12:58 +0300, Hogander, Jouni wrote:
>> On Fri, 2025-10-17 at 15:07 +0530, Nautiyal, Ankit K wrote:
>>> On 10/17/2025 2:37 PM, Hogander, Jouni wrote:
>>>> On Fri, 2025-10-17 at 10:31 +0530, Ankit Nautiyal wrote:
>>>>> Introduce a helper to compute the max link wake latency when
>>>>> using
>>>>> Auxless/Aux wake mechanism for PSR/Panel Replay/LOBF features.
>>>>>
>>>>> This will be used to compute the minimum guardband so that the
>>>>> link
>>>>> wake
>>>>> latencies are accounted and these features work smoothly for
>>>>> higher
>>>>> refresh rate panels.
>>>>>
>>>>> Bspec: 70151, 71477
>>>>> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
>>>>> ---
>>>>>    drivers/gpu/drm/i915/display/intel_psr.c | 12 ++++++++++++
>>>>>    drivers/gpu/drm/i915/display/intel_psr.h |  1 +
>>>>>    2 files changed, 13 insertions(+)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/i915/display/intel_psr.c
>>>>> b/drivers/gpu/drm/i915/display/intel_psr.c
>>>>> index 703e5f6af04c..a8303b669853 100644
>>>>> --- a/drivers/gpu/drm/i915/display/intel_psr.c
>>>>> +++ b/drivers/gpu/drm/i915/display/intel_psr.c
>>>>> @@ -4416,3 +4416,15 @@ void
>>>>> intel_psr_compute_config_late(struct
>>>>> intel_dp *intel_dp,
>>>>>    
>>>>>    	intel_psr_set_non_psr_pipes(intel_dp, crtc_state);
>>>>>    }
>>>>> +
>>>>> +int intel_psr_min_guardband(struct intel_crtc_state
>>>>> *crtc_state)
>>>>> +{
>>>>> +	struct intel_display *display =
>>>>> to_intel_display(crtc_state);
>>>>> +	int auxless_wake_lines = crtc_state-
>>>>>> alpm_state.aux_less_wake_lines;
>>>>> +	int wake_lines = DISPLAY_VER(display) < 20 ?
>>>>> +			 psr2_block_count_lines(crtc_state-
>>>>>> alpm_state.io_wake_lines,
>>>>> +						crtc_state-
>>>>>> alpm_state.fast_wake_lines) :
>>>>> +			 crtc_state->alpm_state.io_wake_lines;
>>>>> +
>>>>> +	return max(auxless_wake_lines, wake_lines);
>>>> hmm, now if you add:
>>>>
>>>> if (crtc_state->req_psr2_sdp_prior_scanline)
>>>> 		psr_min_guardband++;
>>> I did not get this part. Do we need to account for 1 more wakelines
>>> if
>>> this flag is set?
>> If you look at how that flag affects vblank check in
>> intel_psr_compute_config_late:
>>
>> ...
>> static bool _wake_lines_fit_into_vblank(const struct intel_crtc_state
>> *crtc_state,
>> 					int vblank,
>> 					int wake_lines)
>> {
>> 	if (crtc_state->req_psr2_sdp_prior_scanline)
>> 		vblank -= 1;
>> ...
>>
>> So to take that into account when calculating minimum guardband
>> needed
>> by PSR you need to increase by one. Same goes with SCL:
>>
>> ...
>> 	int scl = _intel_psr_min_set_context_latency(crtc_state,
>> 						
>> needs_panel_replay,
>> 						
>> needs_sel_update);
>> 	vblank -= scl;
>> ...
>>
>> You are already partially taking into account PSR needs when
>> calculating optimized guardband except these two lines which are
>> needed
>> conditionally.
>>
>> Also intel_psr_compute_config is run at this point -> you know which
>> one to use: auxless wake time or aux wake time. no need to use max()
>> with them. Just choose the one which is relevant.
>>
>> With these changes you could drop intel_psr_compute_config_late as
>> vblank would be long enough for PSR mode computed by
>> intel_psr_compute_config, no?
> Ok, noticed now this in the last patch:
>
> ...
> crtc_state->vrr.guardband = min(guardband, intel_vrr_max_guardband(crtc_state));
> ...
>
> So if we need to fall back to intel_vrr_max_guardband we need to have
> that intel_psr_compute_config_late.
>
> Anyways I think you need to take into account that
> req_psr2_sdp_prior_scanline and _intel_psr_min_set_context_latency in
> intel_psr_min_guardband.


Hmm I think you are right, we need to account for 
req_psr2_sdp_prior_scanline here.

But for SCL I still think we do not need to account in wakelines as we 
are already accounting in intel_vrr_max_guardband() which calls :

intel_vrr_max_vblank_guardband(const struct intel_crtc_state 
*crtc_state) { struct intel_display *display = 
to_intel_display(crtc_state); const struct drm_display_mode 
*adjusted_mode = &crtc_state->hw.adjusted_mode; return 
crtc_state->vrr.vmin - adjusted_mode->crtc_vdisplay - 
crtc_state->set_context_latency - intel_vrr_extra_vblank_delay(display); }

Regards,

Ankit

>
> BR,
>
> Jouni Högander
>> BR,
>>
>> Jouni Högander
>>
>>
>>> What we want to do is to check for min guardband for
>>> panel_replay/sel_update based on the required wakelines.
>>>
>>> Whether we can use the auxless_wake_lines and wake_lines as
>>> computed
>>> above to estimate the max wakelines or instead we should use
>>> functions
>>> from alpm.c :
>>>
>>> io_buffer_wake_time() and _lnl_compute_aux_less_wake_time() to get
>>> the
>>> worst case wakelines.
>>>
>>>
>>>> Whatever is the PSR mode it can be enabled what comes to vblank
>>>> restrictions and you can drop psr_compute_config_late?
>>>
>>> I think we cannot drop the psr_compute_config_late as it checks
>>> whether
>>> the actual guardband is enough for PSR features.
>>>
>>> intel_psr_min_guardband() is used along with other features to have
>>> an estimate on the guardband that works for all cases, without a
>>> need
>>> to change the guardband.
>>> It is bounded by the vblank length available
>>>
>>> Regards,
>>>
>>> Ankit
>>>
>>>> BR,
>>>>
>>>> Jouni Högander
>>>>
>>>>> +}
>>>>> diff --git a/drivers/gpu/drm/i915/display/intel_psr.h
>>>>> b/drivers/gpu/drm/i915/display/intel_psr.h
>>>>> index b17ce312dc37..620b35928832 100644
>>>>> --- a/drivers/gpu/drm/i915/display/intel_psr.h
>>>>> +++ b/drivers/gpu/drm/i915/display/intel_psr.h
>>>>> @@ -85,5 +85,6 @@ bool intel_psr_needs_alpm_aux_less(struct
>>>>> intel_dp
>>>>> *intel_dp,
>>>>>    				   const struct
>>>>> intel_crtc_state
>>>>> *crtc_state);
>>>>>    void intel_psr_compute_config_late(struct intel_dp *intel_dp,
>>>>>    				   struct intel_crtc_state
>>>>> *crtc_state);
>>>>> +int intel_psr_min_guardband(struct intel_crtc_state
>>>>> *crtc_state);
>>>>>    
>>>>>    #endif /* __INTEL_PSR_H__ */

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

* Re: [PATCH 1/5] drm/i915/psr: Add helper to get min psr guardband
  2025-10-17 11:11           ` Nautiyal, Ankit K
@ 2025-10-17 11:30             ` Hogander, Jouni
  0 siblings, 0 replies; 23+ messages in thread
From: Hogander, Jouni @ 2025-10-17 11:30 UTC (permalink / raw)
  To: intel-xe@lists.freedesktop.org, Nautiyal, Ankit K,
	intel-gfx@lists.freedesktop.org
  Cc: ville.syrjala@linux.intel.com

On Fri, 2025-10-17 at 16:41 +0530, Nautiyal, Ankit K wrote:
> 
> On 10/17/2025 3:45 PM, Hogander, Jouni wrote:
> > On Fri, 2025-10-17 at 12:58 +0300, Hogander, Jouni wrote:
> > > On Fri, 2025-10-17 at 15:07 +0530, Nautiyal, Ankit K wrote:
> > > > On 10/17/2025 2:37 PM, Hogander, Jouni wrote:
> > > > > On Fri, 2025-10-17 at 10:31 +0530, Ankit Nautiyal wrote:
> > > > > > Introduce a helper to compute the max link wake latency
> > > > > > when
> > > > > > using
> > > > > > Auxless/Aux wake mechanism for PSR/Panel Replay/LOBF
> > > > > > features.
> > > > > > 
> > > > > > This will be used to compute the minimum guardband so that
> > > > > > the
> > > > > > link
> > > > > > wake
> > > > > > latencies are accounted and these features work smoothly
> > > > > > for
> > > > > > higher
> > > > > > refresh rate panels.
> > > > > > 
> > > > > > Bspec: 70151, 71477
> > > > > > Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
> > > > > > ---
> > > > > >    drivers/gpu/drm/i915/display/intel_psr.c | 12
> > > > > > ++++++++++++
> > > > > >    drivers/gpu/drm/i915/display/intel_psr.h |  1 +
> > > > > >    2 files changed, 13 insertions(+)
> > > > > > 
> > > > > > diff --git a/drivers/gpu/drm/i915/display/intel_psr.c
> > > > > > b/drivers/gpu/drm/i915/display/intel_psr.c
> > > > > > index 703e5f6af04c..a8303b669853 100644
> > > > > > --- a/drivers/gpu/drm/i915/display/intel_psr.c
> > > > > > +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> > > > > > @@ -4416,3 +4416,15 @@ void
> > > > > > intel_psr_compute_config_late(struct
> > > > > > intel_dp *intel_dp,
> > > > > >    
> > > > > >    	intel_psr_set_non_psr_pipes(intel_dp, crtc_state);
> > > > > >    }
> > > > > > +
> > > > > > +int intel_psr_min_guardband(struct intel_crtc_state
> > > > > > *crtc_state)
> > > > > > +{
> > > > > > +	struct intel_display *display =
> > > > > > to_intel_display(crtc_state);
> > > > > > +	int auxless_wake_lines = crtc_state-
> > > > > > > alpm_state.aux_less_wake_lines;
> > > > > > +	int wake_lines = DISPLAY_VER(display) < 20 ?
> > > > > > +			
> > > > > > psr2_block_count_lines(crtc_state-
> > > > > > > alpm_state.io_wake_lines,
> > > > > > +						crtc_state
> > > > > > -
> > > > > > > alpm_state.fast_wake_lines) :
> > > > > > +			 crtc_state-
> > > > > > >alpm_state.io_wake_lines;
> > > > > > +
> > > > > > +	return max(auxless_wake_lines, wake_lines);
> > > > > hmm, now if you add:
> > > > > 
> > > > > if (crtc_state->req_psr2_sdp_prior_scanline)
> > > > > 		psr_min_guardband++;
> > > > I did not get this part. Do we need to account for 1 more
> > > > wakelines
> > > > if
> > > > this flag is set?
> > > If you look at how that flag affects vblank check in
> > > intel_psr_compute_config_late:
> > > 
> > > ...
> > > static bool _wake_lines_fit_into_vblank(const struct
> > > intel_crtc_state
> > > *crtc_state,
> > > 					int vblank,
> > > 					int wake_lines)
> > > {
> > > 	if (crtc_state->req_psr2_sdp_prior_scanline)
> > > 		vblank -= 1;
> > > ...
> > > 
> > > So to take that into account when calculating minimum guardband
> > > needed
> > > by PSR you need to increase by one. Same goes with SCL:
> > > 
> > > ...
> > > 	int scl = _intel_psr_min_set_context_latency(crtc_state,
> > > 						
> > > needs_panel_replay,
> > > 						
> > > needs_sel_update);
> > > 	vblank -= scl;
> > > ...
> > > 
> > > You are already partially taking into account PSR needs when
> > > calculating optimized guardband except these two lines which are
> > > needed
> > > conditionally.
> > > 
> > > Also intel_psr_compute_config is run at this point -> you know
> > > which
> > > one to use: auxless wake time or aux wake time. no need to use
> > > max()
> > > with them. Just choose the one which is relevant.
> > > 
> > > With these changes you could drop intel_psr_compute_config_late
> > > as
> > > vblank would be long enough for PSR mode computed by
> > > intel_psr_compute_config, no?
> > Ok, noticed now this in the last patch:
> > 
> > ...
> > crtc_state->vrr.guardband = min(guardband,
> > intel_vrr_max_guardband(crtc_state));
> > ...
> > 
> > So if we need to fall back to intel_vrr_max_guardband we need to
> > have
> > that intel_psr_compute_config_late.
> > 
> > Anyways I think you need to take into account that
> > req_psr2_sdp_prior_scanline and _intel_psr_min_set_context_latency
> > in
> > intel_psr_min_guardband.
> 
> 
> Hmm I think you are right, we need to account for 
> req_psr2_sdp_prior_scanline here.
> 
> But for SCL I still think we do not need to account in wakelines as
> we 
> are already accounting in intel_vrr_max_guardband() which calls :
> 
> intel_vrr_max_vblank_guardband(const struct intel_crtc_state 
> *crtc_state) { struct intel_display *display = 
> to_intel_display(crtc_state); const struct drm_display_mode 
> *adjusted_mode = &crtc_state->hw.adjusted_mode; return 
> crtc_state->vrr.vmin - adjusted_mode->crtc_vdisplay - 
> crtc_state->set_context_latency -
> intel_vrr_extra_vblank_delay(display); }

intel_vrr_max_guardband is used only if it's smaller than what is
computed by intel_vrr_compute_optimized_guardband. I.e. case where
intel_psr_min_guard rules used guardband it is not taken into account
unless you add it into intel_psr_min_guard.

BR,

Jouni Högander

> 
> Regards,
> 
> Ankit
> 
> > 
> > BR,
> > 
> > Jouni Högander
> > > BR,
> > > 
> > > Jouni Högander
> > > 
> > > 
> > > > What we want to do is to check for min guardband for
> > > > panel_replay/sel_update based on the required wakelines.
> > > > 
> > > > Whether we can use the auxless_wake_lines and wake_lines as
> > > > computed
> > > > above to estimate the max wakelines or instead we should use
> > > > functions
> > > > from alpm.c :
> > > > 
> > > > io_buffer_wake_time() and _lnl_compute_aux_less_wake_time() to
> > > > get
> > > > the
> > > > worst case wakelines.
> > > > 
> > > > 
> > > > > Whatever is the PSR mode it can be enabled what comes to
> > > > > vblank
> > > > > restrictions and you can drop psr_compute_config_late?
> > > > 
> > > > I think we cannot drop the psr_compute_config_late as it checks
> > > > whether
> > > > the actual guardband is enough for PSR features.
> > > > 
> > > > intel_psr_min_guardband() is used along with other features to
> > > > have
> > > > an estimate on the guardband that works for all cases, without
> > > > a
> > > > need
> > > > to change the guardband.
> > > > It is bounded by the vblank length available
> > > > 
> > > > Regards,
> > > > 
> > > > Ankit
> > > > 
> > > > > BR,
> > > > > 
> > > > > Jouni Högander
> > > > > 
> > > > > > +}
> > > > > > diff --git a/drivers/gpu/drm/i915/display/intel_psr.h
> > > > > > b/drivers/gpu/drm/i915/display/intel_psr.h
> > > > > > index b17ce312dc37..620b35928832 100644
> > > > > > --- a/drivers/gpu/drm/i915/display/intel_psr.h
> > > > > > +++ b/drivers/gpu/drm/i915/display/intel_psr.h
> > > > > > @@ -85,5 +85,6 @@ bool intel_psr_needs_alpm_aux_less(struct
> > > > > > intel_dp
> > > > > > *intel_dp,
> > > > > >    				   const struct
> > > > > > intel_crtc_state
> > > > > > *crtc_state);
> > > > > >    void intel_psr_compute_config_late(struct intel_dp
> > > > > > *intel_dp,
> > > > > >    				   struct intel_crtc_state
> > > > > > *crtc_state);
> > > > > > +int intel_psr_min_guardband(struct intel_crtc_state
> > > > > > *crtc_state);
> > > > > >    
> > > > > >    #endif /* __INTEL_PSR_H__ */


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

* Re: [PATCH 3/5] drm/i915/dp: Check if guardband can accommodate sdp latencies
  2025-10-17  5:02 ` [PATCH 3/5] drm/i915/dp: Check if guardband can accommodate sdp latencies Ankit Nautiyal
@ 2025-10-17 12:02   ` Ville Syrjälä
  0 siblings, 0 replies; 23+ messages in thread
From: Ville Syrjälä @ 2025-10-17 12:02 UTC (permalink / raw)
  To: Ankit Nautiyal; +Cc: intel-gfx, intel-xe

On Fri, Oct 17, 2025 at 10:32:00AM +0530, Ankit Nautiyal wrote:
> Check if guardband is sufficient for all DP SDP latencies.
> If its not, fail .compute_config_late().
> 
> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_dp.c | 22 ++++++++++++++++++++++
>  1 file changed, 22 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index 3f2c319e3d6f..8ae99cee79d4 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -93,6 +93,7 @@
>  #include "intel_psr.h"
>  #include "intel_quirks.h"
>  #include "intel_tc.h"
> +#include "intel_vblank.h"
>  #include "intel_vdsc.h"
>  #include "intel_vrr.h"
>  
> @@ -6980,14 +6981,35 @@ void intel_dp_mst_resume(struct intel_display *display)
>  	}
>  }
>  
> +static
> +int intel_dp_sdp_compute_config_late(struct intel_crtc_state *crtc_state)
> +{
> +	struct intel_display *display = to_intel_display(crtc_state);
> +	int guardband = intel_crtc_vblank_length(crtc_state);

In theory we might have enough legacy TG vblank but not VRR guardband
(I suppose that can only happen on ICL/TGL due to the pipeline full
limit) so I was pondering whether we might need to check both. But I
think this is fine. We would just fail when trying to enable VRR in
those cases, but without VRR enabled we can still light up th display.

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> +	int min_sdp_guardband = intel_dp_sdp_min_guardband(crtc_state, false);
> +
> +	if (guardband < min_sdp_guardband) {
> +		drm_dbg_kms(display->drm, "guardband %d < min sdp guardband %d\n",
> +			    guardband, min_sdp_guardband);
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
>  int intel_dp_compute_config_late(struct intel_encoder *encoder,
>  				 struct intel_crtc_state *crtc_state,
>  				 struct drm_connector_state *conn_state)
>  {
>  	struct intel_dp *intel_dp = enc_to_intel_dp(encoder);
> +	int ret;
>  
>  	intel_psr_compute_config_late(intel_dp, crtc_state);
>  
> +	ret = intel_dp_sdp_compute_config_late(crtc_state);
> +	if (ret)
> +		return ret;
> +
>  	return 0;
>  }
>  
> -- 
> 2.45.2

-- 
Ville Syrjälä
Intel

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

* Re: [PATCH 4/5] drm/i915/vrr: Use the min static optimized guardband
  2025-10-17  5:02 ` [PATCH 4/5] drm/i915/vrr: Use the min static optimized guardband Ankit Nautiyal
@ 2025-10-17 12:06   ` Ville Syrjälä
  0 siblings, 0 replies; 23+ messages in thread
From: Ville Syrjälä @ 2025-10-17 12:06 UTC (permalink / raw)
  To: Ankit Nautiyal; +Cc: intel-gfx, intel-xe

On Fri, Oct 17, 2025 at 10:32:01AM +0530, Ankit Nautiyal wrote:
> In the current VRR implementation, vrr.vmin and vrr.guardband are set such
> that they do not need to change when switching from fixed refresh rate to
> variable refresh rate. Specifically, vrr.guardband is always set to match
> the vblank length. This approach works for most cases, but not for LRR,
> where the guardband would need to change while the VRR timing generator is
> still active.
> 
> With the VRR TG always active, live updates to guardband are unsafe and not
> recommended. To ensure hardware safety, guardband was moved out of the
> !fastset block, meaning any change now requires a full modeset.
> This breaks seamless LRR switching, which was previously supported.
> 
> Since the problem arises from guardband being matched to the vblank length,
> solution is to use a minimal, sufficient static value, instead. So we use a
> static guardband defined during mode-set that fits within the smallest
> expected vblank and remains unchanged in case of features like LRR where
> vtotal changes. To compute this minimum guardband we take into account
> latencies/delays due to different features as mentioned in the Bspec.
> 
> Introduce a helper to compute the minimal sufficient guardband.
> On platforms where the VRR timing generator is always ON, we optimize the
> guardband regardless of whether the display is operating in fixed or
> variable refresh rate mode.
> 
> v2:
> - Use max of sagv latency and skl_wm_latency(1) for PM delay
>   computation. (Ville)
> - Avoid guardband optimization for HDMI for now. (Ville)
> - Allow guardband optimization only for platforms with
>   intel_vrr_always_use_vrr_tg = true. (Ville)
> - Add comments for PM delay and a #TODO note for HDMI.
> 
> Bspec: 70151
> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_vrr.c | 62 +++++++++++++++++++++++-
>  1 file changed, 60 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_vrr.c b/drivers/gpu/drm/i915/display/intel_vrr.c
> index 597008a6c744..cd7bed358984 100644
> --- a/drivers/gpu/drm/i915/display/intel_vrr.c
> +++ b/drivers/gpu/drm/i915/display/intel_vrr.c
> @@ -6,12 +6,16 @@
>  
>  #include <drm/drm_print.h>
>  
> +#include "intel_crtc.h"
>  #include "intel_de.h"
>  #include "intel_display_regs.h"
>  #include "intel_display_types.h"
>  #include "intel_dp.h"
> +#include "intel_psr.h"
>  #include "intel_vrr.h"
>  #include "intel_vrr_regs.h"
> +#include "skl_prefill.h"
> +#include "skl_watermark.h"
>  
>  #define FIXED_POINT_PRECISION		100
>  #define CMRR_PRECISION_TOLERANCE	10
> @@ -433,17 +437,71 @@ intel_vrr_max_guardband(struct intel_crtc_state *crtc_state)
>  		   intel_vrr_max_vblank_guardband(crtc_state));
>  }
>  
> +static
> +int intel_vrr_compute_optimized_guardband(struct intel_crtc_state *crtc_state)
> +{
> +	struct intel_display *display = to_intel_display(crtc_state);
> +	struct skl_prefill_ctx prefill_ctx;
> +	int prefill_min_guardband;
> +	int prefill_latency_us;
> +	int guardband = 0;
> +
> +	skl_prefill_init_worst(&prefill_ctx, crtc_state);
> +
> +	/*
> +	 * The SoC power controller runs SAGV mutually exclusive with package C states,
> +	 * so the max of package C and SAGV latencies is used to compute the min prefill guardband.
> +	 * PM delay = max(sagv_latency, pkgc_max_latency (highest enabled wm level 1 and up))
> +	 */
> +	prefill_latency_us = max(display->sagv.block_time_us,
> +				 skl_watermark_max_latency(display, 1));
> +	prefill_min_guardband =
> +		skl_prefill_min_guardband(&prefill_ctx,
> +					  crtc_state,
> +					  prefill_latency_us);

This could be just

guardband = skl_prefill_min_guardband(...)

and you can then get rid of the prefill_min_guardband
variable as well.

But in general lgtm
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> +
> +	if (intel_crtc_has_dp_encoder(crtc_state)) {
> +		guardband = max(guardband, intel_psr_min_guardband(crtc_state));
> +		guardband = max(guardband, intel_dp_sdp_min_guardband(crtc_state, true));
> +	}
> +
> +	guardband = max(guardband, prefill_min_guardband);
> +
> +	return guardband;
> +}
> +
> +static bool intel_vrr_use_optimized_guardband(const struct intel_crtc_state *crtc_state)
> +{
> +	struct intel_display *display = to_intel_display(crtc_state);
> +
> +	/*
> +	 * #TODO: Enable optimized guardband for HDMI
> +	 * For HDMI lot of infoframes are transmitted a line or two after vsync.
> +	 * Since with optimized guardband the double bufferring point is at delayed vblank,
> +	 * we need to ensure that vsync happens after delayed vblank for the HDMI case.
> +	 */
> +	if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_HDMI))
> +		return false;
> +
> +	return intel_vrr_always_use_vrr_tg(display);
> +}
> +
>  void intel_vrr_compute_guardband(struct intel_crtc_state *crtc_state)
>  {
>  	struct intel_display *display = to_intel_display(crtc_state);
>  	struct drm_display_mode *adjusted_mode = &crtc_state->hw.adjusted_mode;
>  	struct drm_display_mode *pipe_mode = &crtc_state->hw.pipe_mode;
> +	int guardband;
>  
>  	if (!intel_vrr_possible(crtc_state))
>  		return;
>  
> -	crtc_state->vrr.guardband = min(crtc_state->vrr.vmin - adjusted_mode->crtc_vdisplay,
> -					intel_vrr_max_guardband(crtc_state));
> +	if (intel_vrr_use_optimized_guardband(crtc_state))
> +		guardband = intel_vrr_compute_optimized_guardband(crtc_state);
> +	else
> +		guardband = crtc_state->vrr.vmin - adjusted_mode->crtc_vdisplay;
> +
> +	crtc_state->vrr.guardband = min(guardband, intel_vrr_max_guardband(crtc_state));
>  
>  	if (intel_vrr_always_use_vrr_tg(display)) {
>  		adjusted_mode->crtc_vblank_start  =
> -- 
> 2.45.2

-- 
Ville Syrjälä
Intel

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

* Re: [PATCH 5/5] drm/i915/vrr: Use optimized guardband whenever VRR TG is active
  2025-10-17  5:02 ` [PATCH 5/5] drm/i915/vrr: Use optimized guardband whenever VRR TG is active Ankit Nautiyal
@ 2025-10-17 12:13   ` Ville Syrjälä
  0 siblings, 0 replies; 23+ messages in thread
From: Ville Syrjälä @ 2025-10-17 12:13 UTC (permalink / raw)
  To: Ankit Nautiyal; +Cc: intel-gfx, intel-xe

On Fri, Oct 17, 2025 at 10:32:02AM +0530, Ankit Nautiyal wrote:
> Currently the guardband is optimized only for platforms where the
> VRR timing generator is always ON.
> 
> Extend the usage of optimized guardband to other platforms only when the
> VRR is enabled.
> 
> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_vrr.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_vrr.c b/drivers/gpu/drm/i915/display/intel_vrr.c
> index cd7bed358984..eb5aa0d7fc49 100644
> --- a/drivers/gpu/drm/i915/display/intel_vrr.c
> +++ b/drivers/gpu/drm/i915/display/intel_vrr.c
> @@ -483,7 +483,7 @@ static bool intel_vrr_use_optimized_guardband(const struct intel_crtc_state *crt
>  	if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_HDMI))
>  		return false;
>  
> -	return intel_vrr_always_use_vrr_tg(display);
> +	return intel_vrr_always_use_vrr_tg(display) || crtc_state->vrr.enable;

I was going to say this is fine, and I guess it kinda is because
intel_pipe_config_compare() will allow fastsets with a change in
guardband on !intel_vrr_always_use_vrr_tg() platforms. But I
don't think there's any real reason to compute the guardband
differently between vrr.enable==true vs. vrr.enable==false.

So I'm thinking we should just 'return true' unconditionally
here.

>  }
>  
>  void intel_vrr_compute_guardband(struct intel_crtc_state *crtc_state)
> -- 
> 2.45.2

-- 
Ville Syrjälä
Intel

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

* [PATCH 5/5] drm/i915/vrr: Use optimized guardband whenever VRR TG is active
  2025-10-17 12:34 [PATCH 0/5] Optimize vrr.guardband Ankit Nautiyal
@ 2025-10-17 12:35 ` Ankit Nautiyal
  2025-10-17 13:26   ` Ville Syrjälä
  0 siblings, 1 reply; 23+ messages in thread
From: Ankit Nautiyal @ 2025-10-17 12:35 UTC (permalink / raw)
  To: intel-gfx, intel-xe; +Cc: ville.syrjala, jouni.hogander, Ankit Nautiyal

Currently the guardband is optimized only for platforms where the
VRR timing generator is always ON.

Extend the usage of optimized guardband to all VRR supporting platforms.

v2: Drop check for `crtc_state->vrr.enable` and just return true
    unconditionally from intel_vrr_use_optimized_guardband(). (Ville)

Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
---
 drivers/gpu/drm/i915/display/intel_vrr.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_vrr.c b/drivers/gpu/drm/i915/display/intel_vrr.c
index 3da84a247193..92fb72b56f16 100644
--- a/drivers/gpu/drm/i915/display/intel_vrr.c
+++ b/drivers/gpu/drm/i915/display/intel_vrr.c
@@ -468,8 +468,6 @@ int intel_vrr_compute_optimized_guardband(struct intel_crtc_state *crtc_state)
 
 static bool intel_vrr_use_optimized_guardband(const struct intel_crtc_state *crtc_state)
 {
-	struct intel_display *display = to_intel_display(crtc_state);
-
 	/*
 	 * #TODO: Enable optimized guardband for HDMI
 	 * For HDMI lot of infoframes are transmitted a line or two after vsync.
@@ -479,7 +477,7 @@ static bool intel_vrr_use_optimized_guardband(const struct intel_crtc_state *crt
 	if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_HDMI))
 		return false;
 
-	return intel_vrr_always_use_vrr_tg(display);
+	return true;
 }
 
 void intel_vrr_compute_guardband(struct intel_crtc_state *crtc_state)
-- 
2.45.2


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

* Re: [PATCH 5/5] drm/i915/vrr: Use optimized guardband whenever VRR TG is active
  2025-10-17 12:35 ` [PATCH 5/5] drm/i915/vrr: Use optimized guardband whenever VRR TG is active Ankit Nautiyal
@ 2025-10-17 13:26   ` Ville Syrjälä
  0 siblings, 0 replies; 23+ messages in thread
From: Ville Syrjälä @ 2025-10-17 13:26 UTC (permalink / raw)
  To: Ankit Nautiyal; +Cc: intel-gfx, intel-xe, jouni.hogander

On Fri, Oct 17, 2025 at 06:05:04PM +0530, Ankit Nautiyal wrote:
> Currently the guardband is optimized only for platforms where the
> VRR timing generator is always ON.
> 
> Extend the usage of optimized guardband to all VRR supporting platforms.
> 
> v2: Drop check for `crtc_state->vrr.enable` and just return true
>     unconditionally from intel_vrr_use_optimized_guardband(). (Ville)
> 
> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> ---
>  drivers/gpu/drm/i915/display/intel_vrr.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_vrr.c b/drivers/gpu/drm/i915/display/intel_vrr.c
> index 3da84a247193..92fb72b56f16 100644
> --- a/drivers/gpu/drm/i915/display/intel_vrr.c
> +++ b/drivers/gpu/drm/i915/display/intel_vrr.c
> @@ -468,8 +468,6 @@ int intel_vrr_compute_optimized_guardband(struct intel_crtc_state *crtc_state)
>  
>  static bool intel_vrr_use_optimized_guardband(const struct intel_crtc_state *crtc_state)
>  {
> -	struct intel_display *display = to_intel_display(crtc_state);
> -
>  	/*
>  	 * #TODO: Enable optimized guardband for HDMI
>  	 * For HDMI lot of infoframes are transmitted a line or two after vsync.
> @@ -479,7 +477,7 @@ static bool intel_vrr_use_optimized_guardband(const struct intel_crtc_state *crt
>  	if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_HDMI))
>  		return false;
>  
> -	return intel_vrr_always_use_vrr_tg(display);
> +	return true;
>  }
>  
>  void intel_vrr_compute_guardband(struct intel_crtc_state *crtc_state)
> -- 
> 2.45.2

-- 
Ville Syrjälä
Intel

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

end of thread, other threads:[~2025-10-17 13:26 UTC | newest]

Thread overview: 23+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-10-17  5:01 [PATCH 0/5] Optimize vrr.guardband Ankit Nautiyal
2025-10-17  5:01 ` [PATCH 1/5] drm/i915/psr: Add helper to get min psr guardband Ankit Nautiyal
2025-10-17  9:07   ` Hogander, Jouni
2025-10-17  9:30     ` Hogander, Jouni
2025-10-17  9:41       ` Nautiyal, Ankit K
2025-10-17  9:37     ` Nautiyal, Ankit K
2025-10-17  9:58       ` Hogander, Jouni
2025-10-17 10:15         ` Hogander, Jouni
2025-10-17 10:30           ` Hogander, Jouni
2025-10-17 11:11           ` Nautiyal, Ankit K
2025-10-17 11:30             ` Hogander, Jouni
2025-10-17  5:01 ` [PATCH 2/5] drm/i915/dp: Add helper to get min sdp guardband Ankit Nautiyal
2025-10-17 10:50   ` Hogander, Jouni
2025-10-17 11:07     ` Nautiyal, Ankit K
2025-10-17  5:02 ` [PATCH 3/5] drm/i915/dp: Check if guardband can accommodate sdp latencies Ankit Nautiyal
2025-10-17 12:02   ` Ville Syrjälä
2025-10-17  5:02 ` [PATCH 4/5] drm/i915/vrr: Use the min static optimized guardband Ankit Nautiyal
2025-10-17 12:06   ` Ville Syrjälä
2025-10-17  5:02 ` [PATCH 5/5] drm/i915/vrr: Use optimized guardband whenever VRR TG is active Ankit Nautiyal
2025-10-17 12:13   ` Ville Syrjälä
2025-10-17  6:10 ` ✓ i915.CI.BAT: success for Optimize vrr.guardband (rev3) Patchwork
  -- strict thread matches above, loose matches on Subject: below --
2025-10-17 12:34 [PATCH 0/5] Optimize vrr.guardband Ankit Nautiyal
2025-10-17 12:35 ` [PATCH 5/5] drm/i915/vrr: Use optimized guardband whenever VRR TG is active Ankit Nautiyal
2025-10-17 13:26   ` Ville Syrjälä

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox