Re: [PATCH 3/9] drm/i915/display: Use VBLANK_START to get the vblank delay for TGL

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

On Mon, Sep 22, 2025 at 03:50:17PM +0530, Nautiyal, Ankit K wrote:
> 
> On 9/22/2025 3:37 PM, Ville Syrjälä wrote:
> > On Sun, Sep 21, 2025 at 10:05:29AM +0530, Ankit Nautiyal wrote:
> >> For TGL the TRANS_SET_CONTEXT_LATENCY doesn't exist to account for SCL.
> >> However, the VBLANK_START-VACTIVE diffence plays an identical role here ie.
> >> it can be used to create the SCL window ahead of the undelayed vblank.
> >>
> >> While readback there is no specific register to read out the SCL, so use
> >> the same helper intel_crtc_compute_scl() and reduce the vblank_start by
> >> this amount.
> >>
> >> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
> >> ---
> >>   drivers/gpu/drm/i915/display/intel_display.c | 10 ++++++++++
> >>   1 file changed, 10 insertions(+)
> >>
> >> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> >> index 5a7794387ea2..455bbebb50a5 100644
> >> --- a/drivers/gpu/drm/i915/display/intel_display.c
> >> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> >> @@ -2822,6 +2822,16 @@ static void intel_get_transcoder_timings(struct intel_crtc *crtc,
> >>   		adjusted_mode->crtc_vblank_end += 1;
> >>   	}
> >>   
> >> +	if (DISPLAY_VER(display) == 12) {
> >> +		/*
> >> +		 * There is no specific register for SCL for TGL.
> >> +		 * Derive the value from the helper intel_crtc_set_context_latency().
> >> +		 * crtc_vblank_start needs to be reduced with this amount.
> >> +		 */
> >> +		pipe_config->set_context_latency = intel_crtc_set_context_latency(pipe_config);
> > That thing *computes* the SCL, it doesn't teel us what the
> > current hw value is.
> >
> >> +		adjusted_mode->crtc_vblank_start -= pipe_config->set_context_latency;
> >> +	}
> >> +
> >>   	if (DISPLAY_VER(display) >= 13 && !transcoder_is_dsi(cpu_transcoder)) {
> >>   		pipe_config->set_context_latency =
> >>   			intel_de_read(display,
> > I think this whole thing needs to look something like this:
> >
> > if (DISPLAY_VER >= 13 && !dsi) {
> > 	set_context_latency = read(TRANS_SET_CONTEXT_LATENCY);
> > 	crtc_vblank_start = crtc_vdisplay + set_set_context_latency;
> > } else if (DISPLAY_VER >= 12) {
> > 	set_context_latency = crtc_vblank_start - crtc_vdisplay;
> 
> I was wondering if we optimize the guardband for TGL, this will be a 
> problem. It will make set_context_latency huge for TGL.
> 
> Or shall we drop the guardband optimization changes for TGL?

We'll anyway clamp the guardband to the hw/vblank max.

So the initial value we can calculate any which way we want (as long
as we don't calculate it to be too small since that will either make
the commit fail, or will burn away power needleslly).

> 
> 
> > }
> >
> > That'll also cover DSI (not that the DSI code itself is actually
> > ready for SCL...).
> >
> > And I think you need to squash this into the set_context_latency
> > patch. Otherwise we'll get state checker mismatches on TGL in
> > between.
> 
> Yes right, I just wanted to get comments specific to this patch :) .
> 
> This change does belong to the previous patch.
> 
> 
> Regards,
> 
> Ankit
> 
> 
> >

-- 
Ville Syrjälä
Intel