From 99c6388ba1fb898337cacd32562136fe21ee856c Mon Sep 17 00:00:00 2001 From: Chris Wilson Date: Wed, 26 Feb 2014 10:01:29 +0000 Subject: [PATCH] drm/i915: Check that the pageflip is truly stuck before declaring so 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. In the past we used similar code to paper over races in our IRQ handling for pageflips on gen3. However, we now have similar reports that Sandybridge stops processing pageflips. In this case, the hardware generates a single interrupt for the completion of the mmio write to the DSPSURF and so we should not be loosing it due to a race. However, if userspace tries to flip again (before receiving the event notification from the last flip, for example due to a change in display servers) and the old flip is still marked as active, check the hardware state to see if it did update first. If the pageflip completed in hardware, we can simply update our bookkeeping and continue on with the new flip. 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 References: https://bugs.freedesktop.org/show_bug.cgi?id=75502 Signed-off-by: Chris Wilson --- drivers/gpu/drm/i915/i915_debugfs.c | 34 +++++++++++++--- drivers/gpu/drm/i915/intel_display.c | 72 ++++++++++++++++++++++++++++++++-- drivers/gpu/drm/i915/intel_drv.h | 1 + 3 files changed, 97 insertions(+), 10 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c index 52cf0b6..563581f 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 = (struct drm_info_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,6 +603,8 @@ 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->vblank, atomic_read(&dev->vblank[crtc->pipe].count)); if (work->enable_stall_check) seq_puts(m, "Stall check enabled, "); else @@ -610,15 +613,34 @@ static int i915_gem_pageflip_info(struct seq_file *m, void *data) 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)); + seq_printf(m, "Old framebuffer gtt_offset 0x%08lx\n", + i915_gem_obj_ggtt_offset(obj)); } 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 gtt_offset 0x%08lx\n", + i915_gem_obj_ggtt_offset(obj)); + + if (INTEL_INFO(dev)->gen >= 4) { + int dspsurf = DSPSURF(crtc->plane); + int x = crtc->base.x, y = crtc->base.y; + + complete = I915_HI_DISPBASE(I915_READ(dspsurf)) == + i915_gem_obj_ggtt_offset(obj) + + intel_gen4_compute_page_offset(&x, &y, obj->tiling_mode, + crtc->base.fb->bits_per_pixel / 8, + crtc->base.fb->pitches[0]); + } else { + int dspaddr = DSPADDR(crtc->plane); + complete = I915_READ(dspaddr) == + (i915_gem_obj_ggtt_offset(obj) + + crtc->base.y * crtc->base.fb->pitches[0] + + crtc->base.x * crtc->base.fb->bits_per_pixel/8); + } + + seq_printf(m, "MMIO update completed? %d\n", complete); } } spin_unlock_irqrestore(&dev->event_lock, flags); diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index e1992d8..c3a955b 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -8413,6 +8413,7 @@ inline static 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->vblank = atomic_read(&intel_crtc->base.dev->vblank[intel_crtc->pipe].count); /* and that it is marked active as soon as the irq could fire. */ smp_wmb(); } @@ -8701,6 +8702,46 @@ 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 (intel_crtc->unpin_work->vblank == atomic_read(&intel_crtc->base.dev->vblank[intel_crtc->pipe].count)) + 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); + int x = crtc->x, y = crtc->y; + + stall_detected = I915_HI_DISPBASE(I915_READ(dspsurf)) == + i915_gem_obj_ggtt_offset(obj) + + intel_gen4_compute_page_offset(&x, &y, obj->tiling_mode, + crtc->fb->bits_per_pixel / 8, + crtc->fb->pitches[0]); + } 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); + } + + return stall_detected; +} + static int intel_crtc_page_flip(struct drm_crtc *crtc, struct drm_framebuffer *fb, struct drm_pending_vblank_event *event, @@ -8747,12 +8788,35 @@ 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_vblank_put(dev, intel_crtc->pipe); - DRM_DEBUG_DRIVER("flip queue: crtc already busy\n"); - return -EBUSY; + 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); + } 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); diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index e5e1a5c..e3dfd08 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -562,6 +562,7 @@ struct intel_unpin_work { #define INTEL_FLIP_PENDING 1 #define INTEL_FLIP_COMPLETE 2 bool enable_stall_check; + int vblank; }; struct intel_set_config { -- 1.7.9.5