[PATCH] drm/i915/display: Ensure enough lines between delayed VBlank and VBlank

[Hogander, Jouni]

On Sun, 2025-04-20 at 15:50 +0000, Murthy, Arun R wrote:
> > -----Original Message-----
> > From: Intel-gfx <intel-gfx-bounces at lists.freedesktop.org> On Behalf
> > Of Jouni
> > Högander
> > Sent: Thursday, April 17, 2025 11:54 PM
> > To: intel-gfx at lists.freedesktop.org; intel-xe at lists.freedesktop.org
> > Cc: Hogander, Jouni <jouni.hogander at intel.com>
> > 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 <jouni.hogander at intel.com>
> > ---
> >  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;

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
>