> -----Original Message-----
> From: Intel-gfx <[email protected]> On Behalf Of Maarten
> Lankhorst
> Sent: Wednesday, January 21, 2026 7:23 PM
> To: [email protected]; [email protected]
> Cc: Mike Galbraith <[email protected]>; Mario Kleiner
> <[email protected]>; Thomas Gleixner <[email protected]>; Sebastian
> Andrzej Siewior <[email protected]>; Maarten Lankhorst
> <[email protected]>
> Subject: [i915-rt v5 10/21] drm/i915: Use preempt_disable/enable_rt() where
> recommended
>
> From: Mike Galbraith <[email protected]>
>
> Mario Kleiner suggest in commit
> ad3543ede630f ("drm/intel: Push get_scanout_position() timestamping into kms
> driver.")
>
> a spots where preemption should be disabled on PREEMPT_RT. The difference is
> that on PREEMPT_RT the intel_uncore::lock disables neither preemption nor
> interrupts and so region remains preemptible.
>
> The area covers only register reads and writes. The part that worries me
> is:
> - __intel_get_crtc_scanline() the worst case is 100us if no match is
> found.
>
> - intel_crtc_scanlines_since_frame_timestamp() not sure how long this
> may take in the worst case.
>
> It was in the RT queue for a while and nobody complained.
> Disable preemption on PREEPMPT_RT during timestamping.
Looks Good to me.
Reviewed-by: Uma Shankar <[email protected]>
> [bigeasy: patch description.]
>
> Cc: Mario Kleiner <[email protected]>
> Signed-off-by: Mike Galbraith <[email protected]>
> Signed-off-by: Thomas Gleixner <[email protected]>
> Signed-off-by: Sebastian Andrzej Siewior <[email protected]>
> Signed-off-by: Maarten Lankhorst <[email protected]>
> ---
> drivers/gpu/drm/i915/display/intel_vblank.c | 43 ++++++++++++++++-----
> 1 file changed, 33 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_vblank.c
> b/drivers/gpu/drm/i915/display/intel_vblank.c
> index 6bc784563a7c1..e204c260b9aef 100644
> --- a/drivers/gpu/drm/i915/display/intel_vblank.c
> +++ b/drivers/gpu/drm/i915/display/intel_vblank.c
> @@ -316,6 +316,20 @@ static void intel_vblank_section_exit(struct
> intel_display
> *display)
> struct intel_uncore *uncore = to_intel_uncore(display->drm);
> spin_unlock(&uncore->lock);
> }
> +
> +static void intel_vblank_section_enter_irqf(struct intel_display *display,
> unsigned
> long *flags)
> + __acquires(i915->uncore.lock)
> +{
> + struct intel_uncore *uncore = to_intel_uncore(display->drm);
> + spin_lock_irqsave(&uncore->lock, *flags); }
> +
> +static void intel_vblank_section_exit_irqf(struct intel_display *display,
> unsigned
> long flags)
> + __releases(i915->uncore.lock)
> +{
> + struct intel_uncore *uncore = to_intel_uncore(display->drm);
> + spin_unlock_irqrestore(&uncore->lock, flags); }
> #else
> static void intel_vblank_section_enter(struct intel_display *display) { @@
> -324,6
> +338,17 @@ static void intel_vblank_section_enter(struct intel_display
> *display)
> static void intel_vblank_section_exit(struct intel_display *display) { }
> +
> +static void intel_vblank_section_enter_irqf(struct intel_display
> +*display, unsigned long *flags) {
> + *flags = 0;
> +}
> +
> +static void intel_vblank_section_exit_irqf(struct intel_display
> +*display, unsigned long flags) {
> + if (flags)
> + return;
> +}
> #endif
>
> static bool i915_get_crtc_scanoutpos(struct drm_crtc *_crtc, @@ -360,10
> +385,10 @@ static bool i915_get_crtc_scanoutpos(struct drm_crtc *_crtc,
> * timing critical raw register reads, potentially with
> * preemption disabled, so the following code must not block.
> */
> - local_irq_save(irqflags);
> - intel_vblank_section_enter(display);
> + intel_vblank_section_enter_irqf(display, &irqflags);
>
> - /* preempt_disable_rt() should go right here in PREEMPT_RT patchset. */
> + if (IS_ENABLED(CONFIG_PREEMPT_RT))
> + preempt_disable();
>
> /* Get optional system timestamp before query. */
> if (stime)
> @@ -427,10 +452,10 @@ static bool i915_get_crtc_scanoutpos(struct drm_crtc
> *_crtc,
> if (etime)
> *etime = ktime_get();
>
> - /* preempt_enable_rt() should go right here in PREEMPT_RT patchset. */
> + if (IS_ENABLED(CONFIG_PREEMPT_RT))
> + preempt_enable();
>
> - intel_vblank_section_exit(display);
> - local_irq_restore(irqflags);
> + intel_vblank_section_exit_irqf(display, irqflags);
>
> /*
> * While in vblank, position will be negative @@ -468,13 +493,11 @@ int
> intel_get_crtc_scanline(struct intel_crtc *crtc)
> unsigned long irqflags;
> int position;
>
> - local_irq_save(irqflags);
> - intel_vblank_section_enter(display);
> + intel_vblank_section_enter_irqf(display, &irqflags);
>
> position = __intel_get_crtc_scanline(crtc);
>
> - intel_vblank_section_exit(display);
> - local_irq_restore(irqflags);
> + intel_vblank_section_exit_irqf(display, irqflags);
>
> return position;
> }
> --
> 2.51.0
