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. [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 772024f1c344e..75f334fe25aaa 100644 --- a/drivers/gpu/drm/i915/display/intel_vblank.c +++ b/drivers/gpu/drm/i915/display/intel_vblank.c @@ -317,6 +317,20 @@ static void intel_vblank_section_exit(struct intel_display *display) struct drm_i915_private *i915 = to_i915(display->drm); spin_unlock(&i915->uncore.lock); } + +static void intel_vblank_section_enter_irqf(struct intel_display *display, unsigned long *flags) + __acquires(i915->uncore.lock) +{ + struct drm_i915_private *i915 = to_i915(display->drm); + spin_lock_irqsave(&i915->uncore.lock, *flags); +} + +static void intel_vblank_section_exit_irqf(struct intel_display *display, unsigned long flags) + __releases(i915->uncore.lock) +{ + struct drm_i915_private *i915 = to_i915(display->drm); + spin_unlock_irqrestore(&i915->uncore.lock, flags); +} #else static void intel_vblank_section_enter(struct intel_display *display) { @@ -325,6 +339,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, @@ -361,10 +386,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) @@ -428,10 +453,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 @@ -469,13 +494,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
