On Tue, Sep 23, 2025 at 06:40:43PM +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: Address comments from Ville:
> - Use REG_FIELD_MAX()
> - Separate out functions for intel_vrr_max_guardband(),
> intel_vrr_max_vblank_guardband().
>
> Signed-off-by: Ankit Nautiyal <[email protected]>
> ---
> drivers/gpu/drm/i915/display/intel_vrr.c | 44 +++++++++++++++++-------
> 1 file changed, 32 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_vrr.c
> b/drivers/gpu/drm/i915/display/intel_vrr.c
> index 8f851d3a3f44..f37076575bfe 100644
> --- a/drivers/gpu/drm/i915/display/intel_vrr.c
> +++ b/drivers/gpu/drm/i915/display/intel_vrr.c
> @@ -409,6 +409,35 @@ 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);
> +
> + if (DISPLAY_VER(display) >= 13)
> + return REG_FIELD_MAX(XELPD_VRR_CTL_VRR_GUARDBAND_MASK);
> +
I'd put 'else' here for consistency with other stuff.
> + return intel_vrr_pipeline_full_to_guardband(crtc_state,
> +
> REG_FIELD_MAX(VRR_CTL_PIPELINE_FULL_MASK));
> +}
> +
> +static int
> +intel_vrr_max_vblank_guardband(const struct intel_crtc_state *crtc_state)
> +{
> + struct intel_display *display = to_intel_display(crtc_state);
> +
> + return crtc_state->vrr.vmin -
Missing a -crtc_vdisplay here.
what those
Reviewed-by: Ville Syrjälä <[email protected]>
> + 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 +446,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->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
--
Ville Syrjälä
Intel
