> -----Original Message-----
> From: Intel-xe <[email protected]> On Behalf Of Maarten
> Lankhorst
> Sent: Tuesday, November 4, 2025 2:06 PM
> To: [email protected]; [email protected]
> Cc: [email protected]; Maarten Lankhorst <[email protected]>;
> Mario
> Kleiner <[email protected]>; Mike Galbraith
> <[email protected]>; Thomas Gleixner <[email protected]>; Sebastian
> Andrzej Siewior <[email protected]>; Clark Williams
> <[email protected]>; Steven Rostedt <[email protected]>
> Subject: [PATCH v2 4/7] drm/i915/display: Remove locking from
> intel_vblank_evade critical section
>
> finish_wait() may take a lock, which means that it can take any amount of
> time.
> On PREEMPT-RT we should not be taking any lock after disabling preemption, so
> ensure that the completion is done before disabling interrupts.
>
> This also has the benefit of making vblank evasion more deterministic, by
> performing the final vblank check after all locking is done.
>
> Signed-off-by: Maarten Lankhorst <[email protected]>
> ---
> drivers/gpu/drm/i915/display/intel_vblank.c | 35 ++++++++++-----------
> 1 file changed, 17 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_vblank.c
> b/drivers/gpu/drm/i915/display/intel_vblank.c
> index 2b106ffa3f5f5..3628d2a1b8f38 100644
> --- a/drivers/gpu/drm/i915/display/intel_vblank.c
> +++ b/drivers/gpu/drm/i915/display/intel_vblank.c
> @@ -708,6 +708,13 @@ void intel_vblank_evade_init(const struct
> intel_crtc_state *old_crtc_state,
> evade->min -= vblank_delay;
> }
>
> +static inline int vblank_evadable(struct intel_vblank_evade_ctx *evade,
> +int *scanline) {
> + *scanline = intel_get_crtc_scanline(evade->crtc);
> +
> + return *scanline < evade->min || *scanline > evade->max; }
> +
> /* must be called with vblank interrupt already enabled! */ int
> intel_vblank_evade(struct intel_vblank_evade_ctx *evade) { @@ -715,23 +722,22
> @@ int intel_vblank_evade(struct intel_vblank_evade_ctx *evade)
> struct intel_display *display = to_intel_display(crtc);
> long timeout = msecs_to_jiffies_timeout(1);
> wait_queue_head_t *wq = drm_crtc_vblank_waitqueue(&crtc->base);
> - DEFINE_WAIT(wait);
> int scanline;
>
> if (evade->min <= 0 || evade->max <= 0)
> return 0;
>
> - for (;;) {
> - /*
> - * prepare_to_wait() has a memory barrier, which guarantees
> - * other CPUs can see the task state update by the time we
> - * read the scanline.
> - */
> - prepare_to_wait(wq, &wait, TASK_UNINTERRUPTIBLE);
> + while (!vblank_evadable(evade, &scanline)) {
> + local_irq_enable();
>
> - scanline = intel_get_crtc_scanline(crtc);
> - if (scanline < evade->min || scanline > evade->max)
> - break;
> + DEFINE_WAIT(wait);
> + while (!vblank_evadable(evade, &scanline) && timeout > 0) {
Not sure if doing the scanline check with interrupts on is ok. The scanlines
can move
if we get interrupted or what happens if we get a vblank interrupt. Looks
vulnerable to race.
I will try to check this further and get back.
Regards,
Uma Shankar
> + prepare_to_wait(wq, &wait, TASK_UNINTERRUPTIBLE);
> + timeout = schedule_timeout(timeout);
> + }
> + finish_wait(wq, &wait);
> +
> + local_irq_disable();
>
> if (!timeout) {
> drm_dbg_kms(display->drm,
> @@ -740,15 +746,8 @@ int intel_vblank_evade(struct intel_vblank_evade_ctx
> *evade)
> break;
> }
>
> - local_irq_enable();
> -
> - timeout = schedule_timeout(timeout);
> -
> - local_irq_disable();
> }
>
> - finish_wait(wq, &wait);
> -
> /*
> * On VLV/CHV DSI the scanline counter would appear to
> * increment approx. 1/3 of a scanline before start of vblank.
> --
> 2.51.0
