On Mon, Sep 22, 2025 at 02:19:39PM +0300, Ville Syrjälä wrote:
> 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 <[email protected]>
> > > ---
> > > 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
Ugh. *delayed vblank* of course
> 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
ditto
> 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
--
Ville Syrjälä
Intel
