Re: [PATCH v8 07/27] drm/i915/display: Handle vlv dsi workaround in scanline_in_safe_range too

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Moving the VLV/CHV DSI scanline workaround into a sleeping wait condition causes missed vblanks and 1ms timeouts.
--

commit 73c9aa3755ab5a555f9a46d12d2906f90594656d
Author: Maarten Lankhorst <dev@lankhorst.se>

drm/i915/display: Handle vlv dsi workaround in scanline_in_safe_range too

This commit moves the VLV/CHV DSI scanline workaround from a busy-wait loop
in the intel_vblank_evade function into the scanline_in_safe_range helper.
The goal is to make the vblank evasion more deterministic by reducing the
time spent looping with interrupts disabled.

> diff --git a/drivers/gpu/drm/i915/display/intel_vblank.c b/drivers/gpu/drm/i915/display/intel_vblank.c
> index ca08059e088ea..6f1fa952b5391 100644
> --- a/drivers/gpu/drm/i915/display/intel_vblank.c
> +++ b/drivers/gpu/drm/i915/display/intel_vblank.c
> @@ -739,6 +739,24 @@ static bool scanline_in_safe_range(struct intel_vblank_evade_ctx *evade, int *sc
>  	else
>  		*scanline = __intel_get_crtc_scanline(evade->crtc);
>  
> +	/*
> +	 * On VLV/CHV DSI the scanline counter would appear to

[ ... ]

> +	 * FIXME figure out if BXT+ DSI suffers from this as well
> +	 */
> +	if (evade->need_vlv_dsi_wa && *scanline == evade->vblank_start)
> +		return false;
> +

[Severity: High]
Does moving this condition into scanline_in_safe_range() cause a stall during
atomic commits?

Because scanline_in_safe_range() is used as the condition for sleeping in
intel_vblank_evade():

intel_vblank_evade() {
    ...
    timeout = wait_event_timeout(*wq,
        scanline_in_safe_range(evade, &scanline, true), timeout);
    ...
}

When the vblank interrupt fires and wakes the waitqueue, the scanline counter
will still read as evade->vblank_start, since the hardware leaves it at that
value for a full scanline duration.

Since scanline equals evade->vblank_start, scanline_in_safe_range() returns
false, causing wait_event_timeout() to put the thread back to sleep.

As no further vblank interrupts will fire for this frame, will the thread
sleep for the entire 1ms timeout, leading to a regression with missed vblanks
and dropped frames?

>  	return *scanline < evade->min || *scanline > evade->max;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260615201846.307297-1-dev@lankhorst.se?part=7