Re: [PATCH 3/8] drm/i915/vrr: s/intel_vrr_compute_config_late/intel_vrr_compute_guardband

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Mon, Oct 13, 2025 at 06:05:54PM +0530, Ankit Nautiyal wrote:
> The helper intel_vrr_compute_config_late() practically just computes the
> guardband. Rename intel_vrr_compute_config_late() to
> intel_vrr_compute_guardband().
> 
> Since we are going to compute the guardband and then move the
> vblank_start for optmizing guardband move it to
> intel_crtc_compute_config() which handles such changes.
> 
> v2: Move the function at the last after clocks, pipe_mode etc. are all
> set. (Ville)
> 
> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_display.c | 4 ++--
>  drivers/gpu/drm/i915/display/intel_vrr.c     | 2 +-
>  drivers/gpu/drm/i915/display/intel_vrr.h     | 2 +-
>  3 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index 3f725553599e..ceee5ae99c2c 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -2431,6 +2431,8 @@ static int intel_crtc_compute_config(struct intel_atomic_state *state,
>  	if (crtc_state->has_pch_encoder)
>  		return ilk_fdi_compute_config(crtc, crtc_state);
>  
> +	intel_vrr_compute_guardband(crtc_state);
> +

Yeah, this probably makes most sense here. I was pondering if there's
anything in intel_crtc_compute_config() that needs to be done after
this and perhaps there is nothing. pipe_src perhaps we could do
afterwards but I suppose it doesn't really matter which way we do
that. We can move it later if desired.

I was first thinking that with the current method we could even use
pipe_src in the prefill/guardband calculations, but we actually don't
want to do that because we don't want the guardband to depend on the
pfit configuration. But pipe_mode should be good because you'd need
a full modeset anyway to change that (if we ignore DRRS that is).

One thing we'll probably want to do is fix up the pipe_mode.crtc_vblank_start
along with adjusted_mode.crtc_vblank_start, just to be sure they'll
remain in sync. But that'll belong in the patch where we fix up
adjusted_mode itself.

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

>  	return 0;
>  }
>  
> @@ -4722,8 +4724,6 @@ intel_modeset_pipe_config_late(struct intel_atomic_state *state,
>  	struct drm_connector *connector;
>  	int i;
>  
> -	intel_vrr_compute_config_late(crtc_state);
> -
>  	for_each_new_connector_in_state(&state->base, connector,
>  					conn_state, i) {
>  		struct intel_encoder *encoder =
> diff --git a/drivers/gpu/drm/i915/display/intel_vrr.c b/drivers/gpu/drm/i915/display/intel_vrr.c
> index 4bc14b5e685f..8d71d7dc9d12 100644
> --- a/drivers/gpu/drm/i915/display/intel_vrr.c
> +++ b/drivers/gpu/drm/i915/display/intel_vrr.c
> @@ -433,7 +433,7 @@ intel_vrr_max_guardband(struct intel_crtc_state *crtc_state)
>  		   intel_vrr_max_vblank_guardband(crtc_state));
>  }
>  
> -void intel_vrr_compute_config_late(struct intel_crtc_state *crtc_state)
> +void intel_vrr_compute_guardband(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;
> diff --git a/drivers/gpu/drm/i915/display/intel_vrr.h b/drivers/gpu/drm/i915/display/intel_vrr.h
> index 7317f8730089..bc9044621635 100644
> --- a/drivers/gpu/drm/i915/display/intel_vrr.h
> +++ b/drivers/gpu/drm/i915/display/intel_vrr.h
> @@ -21,7 +21,7 @@ bool intel_vrr_possible(const struct intel_crtc_state *crtc_state);
>  void intel_vrr_check_modeset(struct intel_atomic_state *state);
>  void intel_vrr_compute_config(struct intel_crtc_state *crtc_state,
>  			      struct drm_connector_state *conn_state);
> -void intel_vrr_compute_config_late(struct intel_crtc_state *crtc_state);
> +void intel_vrr_compute_guardband(struct intel_crtc_state *crtc_state);
>  void intel_vrr_set_transcoder_timings(const struct intel_crtc_state *crtc_state);
>  void intel_vrr_enable(const struct intel_crtc_state *crtc_state);
>  void intel_vrr_send_push(struct intel_dsb *dsb,
> -- 
> 2.45.2

-- 
Ville Syrjälä
Intel