Attachment 94883 Details for Bug 75502 – Reinstate the periodic pageflip stall detector

[patch] Reinstate the periodic pageflip stall detector

0001-drm-i915-Check-for-a-stalled-page-flip-after-each-vb.patch (text/plain), 11.38 KB, created by Chris Wilson on 2014-02-28 12:39:33 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Chris Wilson

Created: 2014-02-28 12:39:33 UTC

Size: 11.38 KB

patch

obsolete

>From 2d878a38c2441c0b9c017573f590bc9d28400d2d Mon Sep 17 00:00:00 2001
>From: Chris Wilson <chris@chris-wilson.co.uk>
>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 <simon.farnsworth@onelan.co.uk>
>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 <simon@farnz.org.uk>
>Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=75502
>Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
>---
> 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
>

Actions: View | Splinter Review

Attachments on bug 75502: 94724 | 94763 | 94883 | 95088 | 95792 | 95793 | 97218 | 97221 | 98285 | 98614

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.