Re: [PATCH 5/9] drm/i915/vrr: s/intel_vrr_vblank_delay/intel_vrr_scl_delay

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

On Wed, Sep 24, 2025 at 03:03:39PM +0530, Nautiyal, Ankit K wrote:
> 
> On 9/23/2025 7:43 PM, Ville Syrjälä wrote:
> > On Tue, Sep 23, 2025 at 06:40:39PM +0530, Ankit Nautiyal wrote:
> >> The helper intel_vrr_vblank_delay() is used to account for scl lines
> >> + extra_vblank_delay (for ICL/TGL case) for:
> >> - evasion logic for vrr case
> >> - to wait for SCL+ lines after send push operation.
> >>
> >> Rename the helper to intel_vrr_scl_delay since we are interested in the
> >> SCL+ lines for the VRR cases.
> >>
> >> 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 | 2 +-
> >>   drivers/gpu/drm/i915/display/intel_vrr.c    | 2 +-
> >>   drivers/gpu/drm/i915/display/intel_vrr.h    | 2 +-
> >>   4 files changed, 5 insertions(+), 5 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/i915/display/intel_dsb.c b/drivers/gpu/drm/i915/display/intel_dsb.c
> >> index dee44d45b668..ca31e928ecb0 100644
> >> --- a/drivers/gpu/drm/i915/display/intel_dsb.c
> >> +++ b/drivers/gpu/drm/i915/display/intel_dsb.c
> >> @@ -128,7 +128,7 @@ static int dsb_vblank_delay(struct intel_atomic_state *state,
> >>   		 * scanline until the delayed vblank occurs after
> >>   		 * TRANS_PUSH has been written.
> >>   		 */
> >> -		return intel_vrr_vblank_delay(crtc_state) + 1;
> >> +		return intel_vrr_scl_delay(crtc_state) + 1;
> > I'd skip this renaming for now. I think after you've added the
> > safe window scanline wait you can replace all of these with
> > crtc_state->set_context_latency.
> 
> Hmm alright. I will drop this patch.
> 
> But the intel_vrr_vblank_delay() is now just 
> crtc_state->set_context_latency + intel_vrr_extra_vblank_delay().
> 
> Do you mean we don't need intel_vrr_extra_vblank_delay()?
> 
> Perhaps you are right, with the wait for vmin safe window to end, will 
> leave only SCL lines before delayed vblank.
> 
> So the one extra scanline which gets inserted for ICL/TGL will be 
> counted in the wait for safe window.

Exactly. That icl/tgl quirk is functionally identical to
just reducing the guardband by one line on ADL+, and thus
both will be covered by the safe window wait.

> 
> 
> >
> >>   	else
> >>   		return intel_mode_vblank_delay(&crtc_state->hw.adjusted_mode);
> >>   }
> >> @@ -723,7 +723,7 @@ void intel_dsb_vblank_evade(struct intel_atomic_state *state,
> >>   		intel_dsb_emit_wait_dsl(dsb, DSB_OPCODE_WAIT_DSL_OUT, 0, 0);
> >>   
> >>   	if (pre_commit_is_vrr_active(state, crtc)) {
> >> -		int vblank_delay = intel_vrr_vblank_delay(crtc_state);
> >> +		int vblank_delay = intel_vrr_scl_delay(crtc_state);
> >>   
> >>   		end = intel_vrr_vmin_vblank_start(crtc_state);
> >>   		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 c15234c1d96e..9441b7bacd27 100644
> >> --- a/drivers/gpu/drm/i915/display/intel_vblank.c
> >> +++ b/drivers/gpu/drm/i915/display/intel_vblank.c
> >> @@ -681,7 +681,7 @@ void intel_vblank_evade_init(const struct intel_crtc_state *old_crtc_state,
> >>   		else
> >>   			evade->vblank_start = intel_vrr_vmax_vblank_start(crtc_state);
> >>   
> >> -		vblank_delay = intel_vrr_vblank_delay(crtc_state);
> >> +		vblank_delay = intel_vrr_scl_delay(crtc_state);
> > I was pondering about this case especially, but I *think* it should
> > also be changed to crtc_state->set_context_latency. We don't want to
> > perform the commit while in the SCL here because then we're not in
> > the safe window and the DSB we use for LUT updates wouldn't start
> > until the next safe window starts (== next frame's vactive), whereas
> > the double buffered registers would latch already in the upcoming
> > delayed vblank.
> >
> > But performing the commit while we're between undelayed vblank
> > and SCL start should be fine since that is part of the safe
> > window. So we don't need to evade the actual undelayed vblank
> > when in VRR mode.
> >
> > The only exception here would be the LRR and M/N cases since those
> > perhaps still need to evade the undlayed vblank proper. But we always
> > drop out of VRR mode for those types of updates so they won't be
> > taking this codepath anyway.
> 
> Hmm ok so replacing intel_vrr_vblank_delay with 
> crtc_state->set_context_latency will work for both:
> 
> -the wait before push clear and
> 
> -the evasion case
> 
> So will add a last patch to just use crtc_state->set_context wherever we 
> are using intel_vrr_vblank_delay then.

ack

-- 
Ville Syrjälä
Intel