[Intel-gfx] [PATCH 1/3] drm/i915: Check for a stalled page flip after each vblank
Chris Wilson
chris at chris-wilson.co.uk
Fri May 30 18:16:34 CEST 2014
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 <simon.farnsworth at onelan.co.uk>
Date: Wed Sep 1 17:47:52 2010 +0100
drm/i915: Avoid pageflipping freeze when we miss the flip prepare interrupt
This now includes a belt-and-braces approach to make sure the driver
(or the hardware) doesn't miss an interrupt and cause us to stop
updating the display should the unthinkable happen and the pageflip fail - i.e.
that the user is able to continue submitting flips.
Reported-by: Simon Farnsworth <simon at farnz.org.uk>
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=75502
Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
---
drivers/gpu/drm/i915/i915_debugfs.c | 33 ++++++++---
drivers/gpu/drm/i915/i915_irq.c | 85 ++++++++-------------------
drivers/gpu/drm/i915/intel_display.c | 109 ++++++++++++++++++++++++++++++-----
drivers/gpu/drm/i915/intel_drv.h | 2 +
4 files changed, 144 insertions(+), 85 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 0b063c03d188..a05a33ab4b33 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -581,6 +581,7 @@ static int i915_gem_pageflip_info(struct seq_file *m, void *data)
{
struct drm_info_node *node = m->private;
struct drm_device *dev = node->minor->dev;
+ struct drm_i915_private *dev_priv = dev->dev_private;
unsigned long flags;
struct intel_crtc *crtc;
@@ -602,23 +603,37 @@ static int i915_gem_pageflip_info(struct seq_file *m, void *data)
seq_printf(m, "Flip pending (waiting for vsync) on pipe %c (plane %c)\n",
pipe, plane);
}
+ seq_printf(m, "Flip queued on frame %d, now %d\n",
+ work->sbc, atomic_read(&dev->vblank[crtc->pipe].count));
if (work->enable_stall_check)
seq_puts(m, "Stall check enabled, ");
else
seq_puts(m, "Stall check waiting for page flip ioctl, ");
seq_printf(m, "%d prepares\n", atomic_read(&work->pending));
- if (work->old_fb_obj) {
- struct drm_i915_gem_object *obj = work->old_fb_obj;
- if (obj)
- seq_printf(m, "Old framebuffer gtt_offset 0x%08lx\n",
- i915_gem_obj_ggtt_offset(obj));
+ {
+ u32 addr;
+
+ if (INTEL_INFO(dev)->gen >= 4)
+ addr = DSPSURF(crtc->plane);
+ else
+ addr = DSPADDR(crtc->plane);
+
+ seq_printf(m, "Current scanout address 0x%08x\n",
+ I915_READ(addr));
}
+
if (work->pending_flip_obj) {
- struct drm_i915_gem_object *obj = work->pending_flip_obj;
- if (obj)
- seq_printf(m, "New framebuffer gtt_offset 0x%08lx\n",
- i915_gem_obj_ggtt_offset(obj));
+ bool complete;
+
+ seq_printf(m, "New framebuffer address 0x%08lx\n", (long)work->gtt_offset);
+
+ if (INTEL_INFO(dev)->gen >= 4)
+ complete = I915_HI_DISPBASE(I915_READ(DSPSURF(crtc->plane))) == work->gtt_offset;
+ else
+ complete = I915_READ(DSPADDR(crtc->plane)) == work->gtt_offset;
+
+ seq_printf(m, "MMIO update completed? %d\n", complete);
}
}
spin_unlock_irqrestore(&dev->event_lock, flags);
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index bce9afbc34bb..3be22794b53c 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -1802,8 +1802,9 @@ 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)
- intel_pipe_handle_vblank(dev, pipe);
+ if (pipe_stats[pipe] & PIPE_START_VBLANK_INTERRUPT_STATUS &&
+ intel_pipe_handle_vblank(dev, pipe))
+ intel_check_page_flip(dev, pipe);
if (pipe_stats[pipe] & PLANE_FLIP_DONE_INT_STATUS_VLV) {
intel_prepare_page_flip(dev, pipe);
@@ -2079,8 +2080,9 @@ 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))
- intel_pipe_handle_vblank(dev, pipe);
+ if (de_iir & DE_PIPE_VBLANK(pipe) &&
+ intel_pipe_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))
@@ -2129,8 +2131,9 @@ static void ivb_display_irq_handler(struct drm_device *dev, u32 de_iir)
intel_opregion_asle_intr(dev);
for_each_pipe(pipe) {
- if (de_iir & (DE_PIPE_VBLANK_IVB(pipe)))
- intel_pipe_handle_vblank(dev, pipe);
+ if (de_iir & (DE_PIPE_VBLANK_IVB(pipe)) &&
+ intel_pipe_handle_vblank(dev, pipe))
+ intel_check_page_flip(dev, pipe);
/* plane/pipes map 1:1 on ilk+ */
if (de_iir & DE_PLANE_FLIP_DONE_IVB(pipe)) {
@@ -2265,8 +2268,10 @@ 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)
- intel_pipe_handle_vblank(dev, pipe);
+
+ if (pipe_iir & GEN8_PIPE_VBLANK &&
+ intel_pipe_handle_vblank(dev, pipe))
+ intel_check_page_flip(dev, pipe);
if (pipe_iir & GEN8_PIPE_PRIMARY_FLIP_DONE) {
intel_prepare_page_flip(dev, pipe);
@@ -2575,52 +2580,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)
-{
- 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);
- 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->primary->fb->pitches[0] +
- crtc->x * crtc->primary->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
*/
@@ -3726,7 +3685,7 @@ static bool i8xx_handle_vblank(struct drm_device *dev,
return false;
if ((iir & flip_pending) == 0)
- return false;
+ goto check_page_flip;
intel_prepare_page_flip(dev, plane);
@@ -3737,11 +3696,14 @@ static bool i8xx_handle_vblank(struct drm_device *dev,
* an interrupt per se, we watch for the change at vblank.
*/
if (I915_READ16(ISR) & flip_pending)
- return false;
+ goto check_page_flip;
intel_finish_page_flip(dev, pipe);
-
return true;
+
+check_page_flip:
+ intel_check_page_flip(dev, pipe);
+ return false;
}
static irqreturn_t i8xx_irq_handler(int irq, void *arg)
@@ -3911,7 +3873,7 @@ static bool i915_handle_vblank(struct drm_device *dev,
return false;
if ((iir & flip_pending) == 0)
- return false;
+ goto check_page_flip;
intel_prepare_page_flip(dev, plane);
@@ -3922,11 +3884,14 @@ static bool i915_handle_vblank(struct drm_device *dev,
* an interrupt per se, we watch for the change at vblank.
*/
if (I915_READ(ISR) & flip_pending)
- return false;
+ goto check_page_flip;
intel_finish_page_flip(dev, pipe);
-
return true;
+
+check_page_flip:
+ intel_check_page_flip(dev, pipe);
+ return false;
}
static irqreturn_t i915_irq_handler(int irq, void *arg)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 318747b444c6..764b277e5937 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3303,6 +3303,22 @@ bool intel_has_pending_fb_unpin(struct drm_device *dev)
return false;
}
+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);
+}
+
void intel_crtc_wait_for_pending_flips(struct drm_crtc *crtc)
{
struct drm_device *dev = crtc->dev;
@@ -8848,21 +8864,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_crtc_vblank_put(crtc);
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)
@@ -8940,11 +8947,17 @@ void intel_prepare_page_flip(struct drm_device *dev, int plane)
spin_unlock_irqrestore(&dev->event_lock, flags);
}
+static inline int crtc_sbc(struct intel_crtc *crtc)
+{
+ return atomic_read(&crtc->base.dev->vblank[crtc->pipe].count);
+}
+
static inline void intel_mark_page_flip_active(struct intel_crtc *intel_crtc)
{
/* Ensure that the work item is consistent when activating it ... */
smp_wmb();
atomic_set(&intel_crtc->unpin_work->pending, INTEL_FLIP_PENDING);
+ intel_crtc->unpin_work->sbc = crtc_sbc(intel_crtc);
/* and that it is marked active as soon as the irq could fire. */
smp_wmb();
}
@@ -9196,6 +9209,58 @@ 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)
+{
+ struct drm_i915_private *dev_priv = dev->dev_private;
+ struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+ struct drm_i915_gem_object *obj;
+ bool stall_detected;
+
+ if (atomic_read(&work->pending) >= INTEL_FLIP_COMPLETE)
+ return true;
+
+ if (!work->enable_stall_check)
+ return false;
+
+ if ((crtc_sbc(intel_crtc) - intel_crtc->unpin_work->sbc) < 3)
+ return false;
+
+ /* 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)) == work->gtt_offset;
+ } else {
+ int dspaddr = DSPADDR(intel_crtc->plane);
+ stall_detected = I915_READ(dspaddr) == work->gtt_offset;
+ }
+
+ 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,
@@ -9243,12 +9308,25 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
/* We borrow the event spin lock for protecting unpin_work */
spin_lock_irqsave(&dev->event_lock, flags);
if (intel_crtc->unpin_work) {
- spin_unlock_irqrestore(&dev->event_lock, flags);
- kfree(work);
+ bool stall;
+
+ /* 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);
drm_crtc_vblank_put(crtc);
- DRM_DEBUG_DRIVER("flip queue: crtc already busy\n");
- return -EBUSY;
+ if (stall) {
+ DRM_DEBUG_DRIVER("flip queue: previous flip completed, continuing\n");
+ 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);
+
+ kfree(work);
+ return -EBUSY;
+ }
}
intel_crtc->unpin_work = work;
spin_unlock_irqrestore(&dev->event_lock, flags);
@@ -9268,8 +9346,6 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
work->pending_flip_obj = obj;
- work->enable_stall_check = true;
-
atomic_inc(&intel_crtc->unpin_work_count);
intel_crtc->reset_counter = atomic_read(&dev_priv->gpu_error.reset_counter);
@@ -9292,6 +9368,7 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
work->gtt_offset =
i915_gem_obj_ggtt_offset(obj) + intel_crtc->dspaddr_offset;
+ work->enable_stall_check = true;
ret = dev_priv->display.queue_flip(dev, crtc, fb, obj, ring, page_flip_flags);
if (ret)
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index c597b0da93b6..b17c295fe967 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -619,6 +619,7 @@ struct intel_unpin_work {
u32 flip_count;
u32 gtt_offset;
bool enable_stall_check;
+ int sbc;
};
struct intel_set_config {
@@ -761,6 +762,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,
--
2.0.0.rc4
More information about the Intel-gfx
mailing list