> -----Original Message-----
> From: Hogander, Jouni <[email protected]>
> Sent: Tuesday, April 22, 2025 10:16 AM
> To: Murthy, Arun R <[email protected]>; [email protected];
> [email protected]
> Subject: Re: [PATCH] drm/i915/display: Ensure enough lines between delayed
> VBlank and VBlank
>
> On Sun, 2025-04-20 at 15:50 +0000, Murthy, Arun R wrote:
> > > -----Original Message-----
> > > From: Intel-gfx <[email protected]> On Behalf
> > > Of Jouni Högander
> > > Sent: Thursday, April 17, 2025 11:54 PM
> > > To: [email protected]; [email protected]
> > > Cc: Hogander, Jouni <[email protected]>
> > > Subject: [PATCH] drm/i915/display: Ensure enough lines between
> > > delayed VBlank and VBlank
> > >
> > > To deterministically capture the transition of the state machine
> > > going from SRDOFFACK to IDLE, the delayed V. Blank should be at
> > > least one line after the non-delayed V. Blank.
> > >
> > > Ensure this by following instructions from Bspec.
> > >
> > > Bspec: 69897
> > > Signed-off-by: Jouni Högander <[email protected]>
> > > ---
> > > drivers/gpu/drm/i915/display/intel_display.c | 11 ++++++++++-
> > > drivers/gpu/drm/i915/display/intel_vrr.c | 18
> > > ++++++++++++++++--
> > > 2 files changed, 26 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> > > b/drivers/gpu/drm/i915/display/intel_display.c
> > > index db524d01e574d..94156efa5aa93 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_display.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_display.c
> > > @@ -2747,9 +2747,18 @@ static void
> > > intel_set_transcoder_timings_lrr(const
> > > struct intel_crtc_state *crtc
> > > }
> > >
> > > if (DISPLAY_VER(display) >= 13) {
> > Changes looks good. But per Bspec 69985 looks like this change is not
> > applicable for Xe3+
>
> How about if I change it like this:
>
> int min_lat = intel_vrr_always_use_vrr_tg(display) || crtc_state-
> >vrr.enable ? 1 : 0;
>
The func intel_vrr_always_use_vrr_tg return true if display >=30, so should it
be !intel_vrr_always_use_vrr_tg()
Thanks and Regards,
Arun R Murthy
--------------------
> also guardband could be:
>
> if (intel_vrr_always_use_vrr_tg(display) || crtc_state->vrr.enable)
> guardband = max(crtc_state->vrr.vmin - adjusted_mode-
> >crtc_vblank_start, crtc_state->vrr.vmax - adjusted_mode->crtc_vdisplay
> - 1);
> else
> guardband = crtc_state->vrr.vmin - adjusted_mode-
> >crtc_vblank_start;
>
> What do you think?
>
> BR,
>
> Jouni Högander
>
> >
> > Thanks and Regards,
> > Arun R Murthy
> > -------------------
> > > + /*
> > > + * Comment on SRD_STATUS register in Bspec:
> > > + *
> > > + * To deterministically capture the transition of
> > > the state
> > > + * machine going from SRDOFFACK to IDLE, the
> > > delayed V. Blank
> > > + * should be at least one line after the non-
> > > delayed V. Blank.
> > > + *
> > > + * Legacy TG: TRANS_SET_CONTEXT_LATENCY > 0
> > > + */
> > > intel_de_write(display,
> > > TRANS_SET_CONTEXT_LATENCY(display,
> > > cpu_transcoder),
> > > - crtc_vblank_start - crtc_vdisplay);
> > > + max(crtc_vblank_start -
> > > crtc_vdisplay, 1));
> > >
> > > /*
> > > * VBLANK_START not used by hw, just clear it diff --git
> > > a/drivers/gpu/drm/i915/display/intel_vrr.c
> > > b/drivers/gpu/drm/i915/display/intel_vrr.c
> > > index c6565baf815a1..3a27ded45ee04 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_vrr.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_vrr.c
> > > @@ -422,8 +422,22 @@ void intel_vrr_compute_config_late(struct
> > > intel_crtc_state *crtc_state)
> > > return;
> > >
> > > if (DISPLAY_VER(display) >= 13) {
> > > - crtc_state->vrr.guardband =
> > > - crtc_state->vrr.vmin - adjusted_mode-
> > > > crtc_vblank_start;
> > > + /*
> > > + * Comment on SRD_STATUS register in Bspec:
> > > + *
> > > + * To deterministically capture the transition of
> > > the state
> > > + * machine going from SRDOFFACK to IDLE, the
> > > delayed V. Blank
> > > + * should be at least one line after the non-
> > > delayed V. Blank.
> > > + * This can be done by ensuring the VRR Guardband
> > > programming is
> > > + * less than the non-delayed V. Blank.
> > > + *
> > > + * TRANS_VRR_CTL[ VRR Guardband ] <
> > > (TRANS_VRR_VMAX[
> > > VRR Vmax ]
> > > + * - TRANS_VTOTAL[ Vertical Active ])
> > > + */
> > > + crtc_state->vrr.guardband = min(crtc_state-
> > > >vrr.vmin -
> > > + adjusted_mode-
> > > > crtc_vblank_start,
> > > + crtc_state-
> > > >vrr.vmax -
> > > + adjusted_mode-
> > > >crtc_vdisplay
> > > - 1);
> > > } else {
> > > /* hardware imposes one extra scanline somewhere */
> > > crtc_state->vrr.pipeline_full =
> > > --
> > > 2.43.0
> >
