Re: [PATCH 6/9] drm/i915/display: Use set context latency in evasion logic

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

On Mon, Sep 22, 2025 at 01:18:06PM +0300, Ville Syrjälä wrote:
> On Sun, Sep 21, 2025 at 10:05:32AM +0530, Ankit Nautiyal wrote:
> > Currently we use difference between vactive and vblank delay to
> > implicitly wait for SCL lines.
> > 
> > Remove the function intel_mode_vblank_delay as we can simply use
> > the set context latency instead.
> > 
> > Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_dsb.c    | 4 ++--
> >  drivers/gpu/drm/i915/display/intel_vblank.c | 7 +------
> >  drivers/gpu/drm/i915/display/intel_vblank.h | 1 -
> >  3 files changed, 3 insertions(+), 9 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_dsb.c b/drivers/gpu/drm/i915/display/intel_dsb.c
> > index ca31e928ecb0..dfe928aefdcd 100644
> > --- a/drivers/gpu/drm/i915/display/intel_dsb.c
> > +++ b/drivers/gpu/drm/i915/display/intel_dsb.c
> > @@ -130,7 +130,7 @@ static int dsb_vblank_delay(struct intel_atomic_state *state,
> >  		 */
> >  		return intel_vrr_scl_delay(crtc_state) + 1;
> >  	else
> > -		return intel_mode_vblank_delay(&crtc_state->hw.adjusted_mode);
> > +		return crtc_state->set_context_latency;
> 
> I think we want to leave all the non-VRR cases to use
> intel_mode_vblank_delay(). Otherwise when running with fixed
> refresh rate we won't account for a reduced guardband.
> 
> And for the cases where the fixed refresh rate is handle by the legacy
> timing generator we actually need a slightly different delay for the
> legacy and VRR timing generators on TGL (due to
> intel_vrr_extra_vblank_delay() only affecting the VRR timing generator).

Just to elaborate on this, I am thinking that adjusted_mode.crtc_vblank_start
should *always* match the delayed vblank for the fixed refresh rate timings.

So I am envisioning the following rules:

always_use_vrr_tg():
	crtc_vblank_start should reflect the undelayed vblank
	for the VRR TG fixed refresh rate case (ie. fixed_rr_vtotal - guardband).
	This should in fact be the same for both the VRR timings and fixed
	RR timings because the vmin and guardband should be the same for both.

!always_use_vrr_tg()
	crtc_vblank_start should reflect the undelayed vblank
	for the legacy TG (ie. vactive + SCL). The VRR timing
	generator's vblank can be different here due to reduced
	guardband.

This is rather important when we're doing a full modeset and userspace
has already requested vrr.enable=true. The actual modeset part will be
excuted while still running with the fixed refresh rate timings (either
using VRR TG or legacy TG depending on always_use_vrr_tg()). So the
vblank evasion prior to commit_arm() will need to know the correct
position of the delayed vblank for the fixed RR timings. We will then
switch over to the VRR timings (and possibly to the other timing
generator) during the actul commit.

This also means that intel_mode_vblank_delay() will always give us
the total delay betweern the undelayed vblank and delayed vblank for
the fixed RR timings. And this is exactly what we want
for eg. intel_dsb_wait_vblank_delay() since we will have configured
DSB_CHICKEN to use the undelayed vblank (as opposed to safe window)
and thus intel_dsb_wait_vblanks()/DSB_WAIT_FOR_VBLANK will wait for
the undelayed vblank.

> 
> >  }
> >  
> >  static int dsb_vtotal(struct intel_atomic_state *state,
> > @@ -733,7 +733,7 @@ void intel_dsb_vblank_evade(struct intel_atomic_state *state,
> >  		start = end - vblank_delay - latency;
> >  		intel_dsb_wait_scanline_out(state, dsb, start, end);
> >  	} else {
> > -		int vblank_delay = intel_mode_vblank_delay(&crtc_state->hw.adjusted_mode);
> > +		int vblank_delay = crtc_state->set_context_latency;
> >  
> >  		end = intel_mode_vblank_start(&crtc_state->hw.adjusted_mode);
> >  		start = end - vblank_delay - latency;
> > diff --git a/drivers/gpu/drm/i915/display/intel_vblank.c b/drivers/gpu/drm/i915/display/intel_vblank.c
> > index 9441b7bacd27..8c4cb6913ef9 100644
> > --- a/drivers/gpu/drm/i915/display/intel_vblank.c
> > +++ b/drivers/gpu/drm/i915/display/intel_vblank.c
> > @@ -619,11 +619,6 @@ int intel_mode_vtotal(const struct drm_display_mode *mode)
> >  	return vtotal;
> >  }
> >  
> > -int intel_mode_vblank_delay(const struct drm_display_mode *mode)
> > -{
> > -	return intel_mode_vblank_start(mode) - intel_mode_vdisplay(mode);
> > -}
> > -
> >  static const struct intel_crtc_state *
> >  pre_commit_crtc_state(const struct intel_crtc_state *old_crtc_state,
> >  		      const struct intel_crtc_state *new_crtc_state)
> > @@ -685,7 +680,7 @@ void intel_vblank_evade_init(const struct intel_crtc_state *old_crtc_state,
> >  	} else {
> >  		evade->vblank_start = intel_mode_vblank_start(adjusted_mode);
> >  
> > -		vblank_delay = intel_mode_vblank_delay(adjusted_mode);
> > +		vblank_delay = crtc_state->set_context_latency;
> >  	}
> >  
> >  	/* FIXME needs to be calibrated sensibly */
> > diff --git a/drivers/gpu/drm/i915/display/intel_vblank.h b/drivers/gpu/drm/i915/display/intel_vblank.h
> > index 21fbb08d61d5..0fd6f7aeffd4 100644
> > --- a/drivers/gpu/drm/i915/display/intel_vblank.h
> > +++ b/drivers/gpu/drm/i915/display/intel_vblank.h
> > @@ -25,7 +25,6 @@ int intel_mode_vdisplay(const struct drm_display_mode *mode);
> >  int intel_mode_vblank_start(const struct drm_display_mode *mode);
> >  int intel_mode_vblank_end(const struct drm_display_mode *mode);
> >  int intel_mode_vtotal(const struct drm_display_mode *mode);
> > -int intel_mode_vblank_delay(const struct drm_display_mode *mode);
> >  
> >  void intel_vblank_evade_init(const struct intel_crtc_state *old_crtc_state,
> >  			     const struct intel_crtc_state *new_crtc_state,
> > -- 
> > 2.45.2
> 
> -- 
> Ville Syrjälä
> Intel

-- 
Ville Syrjälä
Intel