Re: [PATCH 1/2] drm/i915/dsb: wait one scanline before delayed-vblank DSL wait

["Nautiyal, Ankit K" <ankit.k.nautiyal@intel.com>]

On 6/5/2026 8:12 PM, Ville Syrjälä wrote:
> On Mon, May 25, 2026 at 02:41:58PM +0530, Ankit Nautiyal wrote:
>> In intel_dsb_wait_for_delayed_vblank() the VRR path issues a
>> WAIT_DSL_OUT(safe_window_start, vmin_safe_window_end) followed by a
>> WAIT_USEC for SCL+1 scanlines to land on the delayed vblank.
>>
>> Experimentally, when PIPEDSL is exactly at safe_window_start as the
>> WAIT_DSL_OUT is reached, the wait is skipped and the DSB falls
>> through immediately. The following WAIT_USEC then runs from too
>> early a reference and the DSB interrupt (flip-done event) fires
>> roughly one frame ahead of the delayed vblank.
>>
>> Wait one scanline first so PIPEDSL has moved into the window before
>> WAIT_DSL_OUT runs.
>>
>> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
>> ---
>>   drivers/gpu/drm/i915/display/intel_dsb.c | 8 ++++++++
>>   1 file changed, 8 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/display/intel_dsb.c b/drivers/gpu/drm/i915/display/intel_dsb.c
>> index fec8a56e21ea..dae8ea33d069 100644
>> --- a/drivers/gpu/drm/i915/display/intel_dsb.c
>> +++ b/drivers/gpu/drm/i915/display/intel_dsb.c
>> @@ -902,7 +902,15 @@ void intel_dsb_wait_for_delayed_vblank(struct intel_atomic_state *state,
>>   		 * the hardware itself guarantees that we're SCL lines
>>   		 * away from the delayed vblank, and we won't be inside
>>   		 * the vmin safe window so this extra wait does nothing.
>> +		 *
>> +		 * Experimentally, when PIPEDSL is exactly at
>> +		 * safe_window_start as the next WAIT_DSL_OUT is reached,
>> +		 * the wait is skipped immediately.
> That's an interesting find. I was a bit sceptical at first, but after
> poking at this locally it does appear to true.
>
> It looks like the DSBs view of the scanline counter is somehow delayed
> a bit, rather than the vblank/safe window wait terminating early.
> If I configure the undelayed and delayed vblanks to coincide then
> I can't sneak a PLANE_SURF write to latch on that frame anymore
> after the "wait vblank" instruction. So the delayed vblank
> apparently happened at the same time the vblank/safe window wait
> terminated.
>
> Not sure what's causing the delay, or how long it exactly is. I was
> able to get past it with ~5 NOOPs in some cases, but then reducing
> the dotclock made it so that 5 NOOPs were no longer enough. And speeding
> up the DSB by pushing CDCLK higher also required more NOOPs. So the
> delay does seem to be related to the dotclock/timings in some way.
>
> The "wait scanline" instruction is also affected.
>
> And for good measure I also tried a poll instruction on the
> PIPEDSL register, and that also sees the stale value. I was kinda
> hoping the read from that register would somehow force things
> into sync, but doesn't look like that is the case. Depending on
> the dotclock and CDCLK I may need to poll multiple times to
> get the up to date value.
>
> So far I've not been able to observe this delay on the CPU side
> using intel_display_poller.
>
>> +		 *
>> +		 * Wait one scanline first so PIPEDSL has moved into the
>> +		 * window before WAIT_DSL_OUT runs.
>>   		 */
>> +		intel_dsb_wait_usec(dsb, intel_scanlines_to_usecs(adjusted_mode, 1));
> Hmm. Instead of that I think we could just use safe_window_start-1 as
> the start scanline for the wait.
>
> Since we know there a preceding wait for vblank/safe window I don't
> think we should ever get here on that specific scanline for real.
> So if that scanline is observed then it'll always be the post-wait
> stale value.
>
> I also had a quick ponder about all the other scanline OUT waits
> we do elsewhere, and I think those are probably fine because
> they're rather concerned with the end of the scanline window,
> not the start. Seeing a slightly stale value at the end of the
> window just means we wait a little bit longer than strictly
> necessary.
>
> I suppose for a scanline IN wait the problem would be reversed,
> ie. the end of the scanline window would be where the stale value
> could bite us. Currently we don't do any of those so not a problem.


Many thanks for digging into this and testing this thoroughly.

The stale view of DSB about scanline counter explains the behaviour.

I agree the boundary adjustment approach is cleaner from a semantic 
point of view, and I will change this in next revision.

Thanks & Regards,

Ankit

>
>>   		intel_dsb_wait_scanline_out(state, dsb,
>>   					    intel_vrr_safe_window_start(crtc_state),
>>   					    intel_vrr_vmin_safe_window_end(crtc_state));
>> -- 
>> 2.45.2