On 9/24/2025 5:34 PM, Ville Syrjälä wrote:
On Wed, Sep 24, 2025 at 04:21:28PM +0530, Ankit Nautiyal wrote:
The maximum guardband value is constrained by two factors:
- The actual vblank length minus set context latency (SCL)
- The hardware register field width:
- 8 bits for ICL/TGL (VRR_CTL_PIPELINE_FULL_MASK -> max 255)
- 16 bits for ADL+ (XELPD_VRR_CTL_VRR_GUARDBAND_MASK -> max 65535)
Remove the #FIXME and clamp the guardband to the maximum allowed value.
v2:
- Use REG_FIELD_MAX(). (Ville)
- Separate out functions for intel_vrr_max_guardband(),
intel_vrr_max_vblank_guardband(). (Ville)
v3:
- Fix Typo: Add the missing adjusted_mode->crtc_vdisplay in guardband
computation. (Ville)
- Refactor intel_vrr_max_hw_guardband() and use else for consistency.
(Ville)
Signed-off-by: Ankit Nautiyal <[email protected]>
Reviewed-by: Ville Syrjälä <[email protected]>
---
drivers/gpu/drm/i915/display/intel_vrr.c | 49 ++++++++++++++++++------
1 file changed, 37 insertions(+), 12 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_vrr.c
b/drivers/gpu/drm/i915/display/intel_vrr.c
index 26c5c32a9a58..e29b4050a9df 100644
--- a/drivers/gpu/drm/i915/display/intel_vrr.c
+++ b/drivers/gpu/drm/i915/display/intel_vrr.c
@@ -409,6 +409,40 @@ intel_vrr_compute_config(struct intel_crtc_state
*crtc_state,
}
}
+static int
+intel_vrr_max_hw_guardband(const struct intel_crtc_state *crtc_state)
+{
+ struct intel_display *display = to_intel_display(crtc_state);
+ int max_pipeline_full = REG_FIELD_MAX(VRR_CTL_PIPELINE_FULL_MASK);
+ int max_guardband;
+
+ if (DISPLAY_VER(display) >= 13)
+ max_guardband = REG_FIELD_MAX(XELPD_VRR_CTL_VRR_GUARDBAND_MASK);
+ else
+ max_guardband = intel_vrr_pipeline_full_to_guardband(crtc_state,
+
max_pipeline_full);
+ return max_guardband;
The 'max_guardband' variable looks useless here, could just return
directly from both sides of the if-else.
'max_pipeline_full' is perhaps redundant too, but I suppose the
line would get pretty long without it. So maybe it makes sense to keep
it.
Yeah the line was getting pretty long, so added max_pipeline_full.
Can drop max_guardband though.
Regards,
Ankit
+}
+
+static int
+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);
+}
+
+static int
+intel_vrr_max_guardband(struct intel_crtc_state *crtc_state)
+{
+ return min(intel_vrr_max_hw_guardband(crtc_state),
+ intel_vrr_max_vblank_guardband(crtc_state));
+}
+
void intel_vrr_compute_config_late(struct intel_crtc_state *crtc_state)
{
struct intel_display *display = to_intel_display(crtc_state);
@@ -417,22 +451,13 @@ void intel_vrr_compute_config_late(struct
intel_crtc_state *crtc_state)
if (!intel_vrr_possible(crtc_state))
return;
- crtc_state->vrr.guardband =
- crtc_state->vrr.vmin -
- adjusted_mode->crtc_vdisplay -
- crtc_state->set_context_latency -
- intel_vrr_extra_vblank_delay(display);
-
- if (DISPLAY_VER(display) < 13) {
- /* FIXME handle the limit in a proper way */
- crtc_state->vrr.guardband =
- min(crtc_state->vrr.guardband,
- intel_vrr_pipeline_full_to_guardband(crtc_state,
255));
+ crtc_state->vrr.guardband = min(crtc_state->vrr.vmin -
adjusted_mode->crtc_vdisplay,
+ intel_vrr_max_guardband(crtc_state));
+ if (DISPLAY_VER(display) < 13)
crtc_state->vrr.pipeline_full =
intel_vrr_guardband_to_pipeline_full(crtc_state,
crtc_state->vrr.guardband);
- }
}
static u32 trans_vrr_ctl(const struct intel_crtc_state *crtc_state)
--
2.45.2
