From 2d878a38c2441c0b9c017573f590bc9d28400d2d Mon Sep 17 00:00:00 2001 From: Chris Wilson Date: Fri, 28 Feb 2014 12:32:34 +0000 Subject: [PATCH] drm/i915: Check for a stalled page flip after each vblank Long ago, back in the racy haydays of 915gm interrupt handling, page flips would occasionally go astray and leave the hardware stuck, and the display not updating. This annoyed people who relied on their systems being able to display continuously updating information 24/7, and so some code to detect when the driver missed the page flip completion signal was added. Until recently, it was presumed that the interrupt handling was now flawless, but once again Simon Farnsworth has found a system whose display will stall. Reinstate the pageflip stall detection, which works by checking to see if the hardware has been updated to the new framebuffer address following each vblank. If the hardware is scanning out from the new framebuffer, but we still think the flip is pending, then we kick our driver into submision. This is a continuation of the effort started with commit 4e5359cd053bfb7d8dabe4a63624a5726848ffbc Author: Simon Farnsworth Date: Wed Sep 1 17:47:52 2010 +0100 drm/i915: Avoid pageflipping freeze when we miss the flip prepare interrupt Reported-by: Simon Farnsworth Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=75502 Signed-off-by: Chris Wilson --- drivers/gpu/drm/i915/i915_irq.c | 94 ++++++++++-------------------------- drivers/gpu/drm/i915/intel_display.c | 70 +++++++++++++++++---------- drivers/gpu/drm/i915/intel_drv.h | 1 + 3 files changed, 71 insertions(+), 94 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index e215e9a83a81..6f0be76b318d 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -1614,14 +1614,15 @@ static void valleyview_pipestat_irq_handler(struct drm_device *dev, u32 iir) spin_unlock(&dev_priv->irq_lock); for_each_pipe(pipe) { - if (pipe_stats[pipe] & PIPE_START_VBLANK_INTERRUPT_STATUS) - drm_handle_vblank(dev, pipe); - if (pipe_stats[pipe] & PLANE_FLIP_DONE_INT_STATUS_VLV) { intel_prepare_page_flip(dev, pipe); intel_finish_page_flip(dev, pipe); } + if (pipe_stats[pipe] & PIPE_START_VBLANK_INTERRUPT_STATUS && + drm_handle_vblank(dev, pipe)) + intel_check_page_flip(dev, pipe); + if (pipe_stats[pipe] & PIPE_CRC_DONE_INTERRUPT_STATUS) i9xx_pipe_crc_irq_handler(dev, pipe); @@ -1841,8 +1842,15 @@ static void ilk_display_irq_handler(struct drm_device *dev, u32 de_iir) DRM_ERROR("Poison interrupt\n"); for_each_pipe(pipe) { - if (de_iir & DE_PIPE_VBLANK(pipe)) - drm_handle_vblank(dev, pipe); + /* plane/pipes map 1:1 on ilk+ */ + if (de_iir & DE_PLANE_FLIP_DONE(pipe)) { + intel_prepare_page_flip(dev, pipe); + intel_finish_page_flip_plane(dev, pipe); + } + + if (de_iir & DE_PIPE_VBLANK(pipe) && + drm_handle_vblank(dev, pipe)) + intel_check_page_flip(dev, pipe); if (de_iir & DE_PIPE_FIFO_UNDERRUN(pipe)) if (intel_set_cpu_fifo_underrun_reporting(dev, pipe, false)) @@ -1851,12 +1859,6 @@ static void ilk_display_irq_handler(struct drm_device *dev, u32 de_iir) if (de_iir & DE_PIPE_CRC_DONE(pipe)) i9xx_pipe_crc_irq_handler(dev, pipe); - - /* plane/pipes map 1:1 on ilk+ */ - if (de_iir & DE_PLANE_FLIP_DONE(pipe)) { - intel_prepare_page_flip(dev, pipe); - intel_finish_page_flip_plane(dev, pipe); - } } /* check event from PCH */ @@ -1879,7 +1881,7 @@ static void ilk_display_irq_handler(struct drm_device *dev, u32 de_iir) static void ivb_display_irq_handler(struct drm_device *dev, u32 de_iir) { struct drm_i915_private *dev_priv = dev->dev_private; - enum pipe i; + enum pipe pipe; if (de_iir & DE_ERR_INT_IVB) ivb_err_int_handler(dev); @@ -1890,15 +1892,16 @@ static void ivb_display_irq_handler(struct drm_device *dev, u32 de_iir) if (de_iir & DE_GSE_IVB) intel_opregion_asle_intr(dev); - for_each_pipe(i) { - if (de_iir & (DE_PIPE_VBLANK_IVB(i))) - drm_handle_vblank(dev, i); - + for_each_pipe(pipe) { /* plane/pipes map 1:1 on ilk+ */ - if (de_iir & DE_PLANE_FLIP_DONE_IVB(i)) { - intel_prepare_page_flip(dev, i); - intel_finish_page_flip_plane(dev, i); + if (de_iir & DE_PLANE_FLIP_DONE_IVB(pipe)) { + intel_prepare_page_flip(dev, pipe); + intel_finish_page_flip_plane(dev, pipe); } + + if (de_iir & (DE_PIPE_VBLANK_IVB(pipe)) && + drm_handle_vblank(dev, pipe)) + intel_check_page_flip(dev, pipe); } /* check event from PCH */ @@ -2027,14 +2030,15 @@ static irqreturn_t gen8_irq_handler(int irq, void *arg) continue; pipe_iir = I915_READ(GEN8_DE_PIPE_IIR(pipe)); - if (pipe_iir & GEN8_PIPE_VBLANK) - drm_handle_vblank(dev, pipe); - if (pipe_iir & GEN8_PIPE_FLIP_DONE) { intel_prepare_page_flip(dev, pipe); intel_finish_page_flip_plane(dev, pipe); } + if (pipe_iir & GEN8_PIPE_VBLANK && + drm_handle_vblank(dev, pipe)) + intel_check_page_flip(dev, pipe); + if (pipe_iir & GEN8_PIPE_CDCLK_CRC_DONE) hsw_pipe_crc_irq_handler(dev, pipe); @@ -2320,52 +2324,6 @@ void i915_handle_error(struct drm_device *dev, bool wedged) schedule_work(&dev_priv->gpu_error.work); } -static void __always_unused i915_pageflip_stall_check(struct drm_device *dev, int pipe) -{ - drm_i915_private_t *dev_priv = dev->dev_private; - struct drm_crtc *crtc = dev_priv->pipe_to_crtc_mapping[pipe]; - struct intel_crtc *intel_crtc = to_intel_crtc(crtc); - struct drm_i915_gem_object *obj; - struct intel_unpin_work *work; - unsigned long flags; - bool stall_detected; - - /* Ignore early vblank irqs */ - if (intel_crtc == NULL) - return; - - spin_lock_irqsave(&dev->event_lock, flags); - work = intel_crtc->unpin_work; - - if (work == NULL || - atomic_read(&work->pending) >= INTEL_FLIP_COMPLETE || - !work->enable_stall_check) { - /* Either the pending flip IRQ arrived, or we're too early. Don't check */ - spin_unlock_irqrestore(&dev->event_lock, flags); - return; - } - - /* Potential stall - if we see that the flip has happened, assume a missed interrupt */ - obj = work->pending_flip_obj; - if (INTEL_INFO(dev)->gen >= 4) { - int dspsurf = DSPSURF(intel_crtc->plane); - stall_detected = I915_HI_DISPBASE(I915_READ(dspsurf)) == - i915_gem_obj_ggtt_offset(obj); - } else { - int dspaddr = DSPADDR(intel_crtc->plane); - stall_detected = I915_READ(dspaddr) == (i915_gem_obj_ggtt_offset(obj) + - crtc->y * crtc->fb->pitches[0] + - crtc->x * crtc->fb->bits_per_pixel/8); - } - - spin_unlock_irqrestore(&dev->event_lock, flags); - - if (stall_detected) { - DRM_DEBUG_DRIVER("Pageflip stall detected\n"); - intel_prepare_page_flip(dev, intel_crtc->plane); - } -} - /* Called from drm generic code, passed 'crtc' which * we use as a pipe index */ diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index c3a955b5f14b..a2cc06969c5f 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -8333,6 +8333,22 @@ static void intel_unpin_work_fn(struct work_struct *__work) kfree(work); } +static void page_flip_completed(struct drm_i915_private *dev_priv, + struct intel_crtc *intel_crtc, + struct intel_unpin_work *work) +{ + if (work->event) + drm_send_vblank_event(dev_priv->dev, + intel_crtc->pipe, + work->event); + + wake_up_all(&dev_priv->pending_flip_queue); + queue_work(dev_priv->wq, &work->work); + + trace_i915_flip_complete(intel_crtc->plane, + work->pending_flip_obj); +} + static void do_intel_finish_page_flip(struct drm_device *dev, struct drm_crtc *crtc) { @@ -8358,21 +8374,12 @@ static void do_intel_finish_page_flip(struct drm_device *dev, /* and that the unpin work is consistent wrt ->pending. */ smp_rmb(); - intel_crtc->unpin_work = NULL; - if (work->event) - drm_send_vblank_event(dev, intel_crtc->pipe, work->event); - + page_flip_completed(dev_priv, intel_crtc, work); drm_vblank_put(dev, intel_crtc->pipe); spin_unlock_irqrestore(&dev->event_lock, flags); - - wake_up_all(&dev_priv->pending_flip_queue); - - queue_work(dev_priv->wq, &work->work); - - trace_i915_flip_complete(intel_crtc->plane, work->pending_flip_obj); } void intel_finish_page_flip(struct drm_device *dev, int pipe) @@ -8702,9 +8709,9 @@ static int intel_default_queue_flip(struct drm_device *dev, return -ENODEV; } -static bool intel_pageflip_stall_check(struct drm_device *dev, - struct drm_crtc *crtc, - struct intel_unpin_work *work) +static bool __intel_pageflip_stall_check(struct drm_device *dev, + struct drm_crtc *crtc, + struct intel_unpin_work *work) { struct drm_i915_private *dev_priv = dev->dev_private; struct intel_crtc *intel_crtc = to_intel_crtc(crtc); @@ -8742,6 +8749,27 @@ static bool intel_pageflip_stall_check(struct drm_device *dev, return stall_detected; } +void intel_check_page_flip(struct drm_device *dev, int pipe) +{ + struct drm_i915_private *dev_priv = dev->dev_private; + struct drm_crtc *crtc = dev_priv->pipe_to_crtc_mapping[pipe]; + struct intel_crtc *intel_crtc = to_intel_crtc(crtc); + unsigned long flags; + + if (crtc == NULL) + return; + + spin_lock_irqsave(&dev->event_lock, flags); + if (intel_crtc->unpin_work && + __intel_pageflip_stall_check(dev, crtc, intel_crtc->unpin_work)) { + WARN_ONCE(1, "Kicking stuck page flip\n"); + drm_vblank_put(dev, intel_crtc->pipe); + page_flip_completed(dev_priv, intel_crtc, intel_crtc->unpin_work); + intel_crtc->unpin_work = NULL; + } + spin_unlock_irqrestore(&dev->event_lock, flags); +} + static int intel_crtc_page_flip(struct drm_crtc *crtc, struct drm_framebuffer *fb, struct drm_pending_vblank_event *event, @@ -8793,23 +8821,13 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc, /* Before declaring the flip queue wedged, check if * the hardware completed the operation behind our backs. */ - stall = intel_pageflip_stall_check(dev, crtc, - intel_crtc->unpin_work); + stall = __intel_pageflip_stall_check(dev, crtc, + intel_crtc->unpin_work); drm_vblank_put(dev, intel_crtc->pipe); if (stall) { DRM_DEBUG_DRIVER("flip queue: previous flip completed, continuing\n"); - - if (intel_crtc->unpin_work->event) - drm_send_vblank_event(dev, - intel_crtc->pipe, - intel_crtc->unpin_work->event); - - wake_up_all(&dev_priv->pending_flip_queue); - queue_work(dev_priv->wq, &work->work); - - trace_i915_flip_complete(intel_crtc->plane, - intel_crtc->unpin_work->pending_flip_obj); + page_flip_completed(dev_priv, intel_crtc, intel_crtc->unpin_work); } else { DRM_DEBUG_DRIVER("flip queue: crtc already busy\n"); spin_unlock_irqrestore(&dev->event_lock, flags); diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index e3dfd08de766..8777a1a54822 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -700,6 +700,7 @@ __intel_framebuffer_create(struct drm_device *dev, void intel_prepare_page_flip(struct drm_device *dev, int plane); void intel_finish_page_flip(struct drm_device *dev, int pipe); void intel_finish_page_flip_plane(struct drm_device *dev, int plane); +void intel_check_page_flip(struct drm_device *dev, int pipe); struct intel_shared_dpll *intel_crtc_to_shared_dpll(struct intel_crtc *crtc); void assert_shared_dpll(struct drm_i915_private *dev_priv, struct intel_shared_dpll *pll, -- 1.9.0