[PATCH 1/3] drm/i915: Check for a stalled page flip after each vblank

[PATCH 1/3] drm/i915: Check for a stalled page flip after each vblank

[Chris Wilson]

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

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.

v2: Cleanup, refactor, and rename

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>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c  |   23 +++---
 drivers/gpu/drm/i915/i915_irq.c      |   85 +++++++---------------
 drivers/gpu/drm/i915/intel_display.c |  130 ++++++++++++++++++++++++++++------
 drivers/gpu/drm/i915/intel_drv.h     |    2 +
 4 files changed, 147 insertions(+), 93 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index e8ea1ef..a5f0a26 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;
 
@@ -595,6 +596,8 @@ static int i915_gem_pageflip_info(struct seq_file *m, void *data)
 			seq_printf(m, "No flip due on pipe %c (plane %c)\n",
 				   pipe, plane);
 		} else {
+			u32 addr;
+
 			if (atomic_read(&work->pending) < INTEL_FLIP_COMPLETE) {
 				seq_printf(m, "Flip queued on pipe %c (plane %c)\n",
 					   pipe, plane);
@@ -602,23 +605,23 @@ 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));
-			}
+			if (INTEL_INFO(dev)->gen >= 4)
+				addr = I915_HI_DISPBASE(I915_READ(DSPSURF(crtc->plane)));
+			else
+				addr = I915_READ(DSPADDR(crtc->plane));
+			seq_printf(m, "Current scanout address 0x%08x\n", 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));
+				seq_printf(m, "New framebuffer address 0x%08lx\n", (long)work->gtt_offset);
+				seq_printf(m, "MMIO update completed? %d\n",  addr == work->gtt_offset);
 			}
 		}
 		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 229e1f1..c3db85f 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 b5cbb28..b87b4ce 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3330,6 +3330,29 @@ bool intel_has_pending_fb_unpin(struct drm_device *dev)
 	return false;
 }
 
+static void page_flip_completed(struct intel_crtc *intel_crtc)
+{
+	struct drm_i915_private *dev_priv = to_i915(intel_crtc->base.dev);
+	struct intel_unpin_work *work = intel_crtc->unpin_work;
+
+	/* ensure that the unpin work is consistent wrt ->pending. */
+	smp_rmb();
+	intel_crtc->unpin_work = NULL;
+
+	if (work->event)
+		drm_send_vblank_event(intel_crtc->base.dev,
+				      intel_crtc->pipe,
+				      work->event);
+
+	drm_crtc_vblank_put(&intel_crtc->base);
+
+	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;
@@ -8895,7 +8918,6 @@ static void intel_unpin_work_fn(struct work_struct *__work)
 static void do_intel_finish_page_flip(struct drm_device *dev,
 				      struct drm_crtc *crtc)
 {
-	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 	struct intel_unpin_work *work;
 	unsigned long flags;
@@ -8915,23 +8937,9 @@ static void do_intel_finish_page_flip(struct drm_device *dev,
 		return;
 	}
 
-	/* 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);
-
-	drm_crtc_vblank_put(crtc);
+	page_flip_completed(intel_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)
@@ -9009,11 +9017,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();
 }
@@ -9265,6 +9279,69 @@ static int intel_default_queue_flip(struct drm_device *dev,
 	return -ENODEV;
 }
 
+static bool i915_gem_object_is_idle(struct drm_i915_gem_object *obj,
+				    bool readonly)
+{
+	struct intel_engine_cs *ring = obj->ring;
+	u32 seqno = readonly ? obj->last_write_seqno : obj->last_read_seqno;
+
+	if (ring == NULL || seqno == 0)
+		return true;
+
+	return i915_seqno_passed(ring->get_seqno(ring, true), seqno);
+}
+
+static bool __intel_pageflip_stall_check(struct drm_device *dev,
+					 struct drm_crtc *crtc)
+{
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+	struct intel_unpin_work *work = intel_crtc->unpin_work;
+	u32 addr;
+
+	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;
+
+	if (!i915_gem_object_is_idle(work->pending_flip_obj, true))
+		return false;
+
+	/* Potential stall - if we see that the flip has happened, assume a missed interrupt */
+	if (INTEL_INFO(dev)->gen >= 4)
+		addr = I915_HI_DISPBASE(I915_READ(DSPSURF(intel_crtc->plane)));
+	else
+		addr = I915_READ(DSPADDR(intel_crtc->plane));
+
+	/* There is a potential issue here with a false positive after a flip
+	 * to the same address.
+	 */
+	return addr == work->gtt_offset;
+}
+
+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)) {
+		WARN_ONCE(1, "Kicking stuck page flip: queued at %d, now %d\n",
+			 intel_crtc->unpin_work->sbc, crtc_sbc(intel_crtc));
+		page_flip_completed(intel_crtc);
+	}
+	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,
@@ -9312,12 +9389,20 @@ 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);
-		drm_crtc_vblank_put(crtc);
+		/* Before declaring the flip queue wedged, check if
+		 * the hardware completed the operation behind our backs.
+		 */
+		if (__intel_pageflip_stall_check(dev, crtc)) {
+			DRM_DEBUG_DRIVER("flip queue: previous flip completed, continuing\n");
+			page_flip_completed(intel_crtc);
+		} else {
+			DRM_DEBUG_DRIVER("flip queue: crtc already busy\n");
+			spin_unlock_irqrestore(&dev->event_lock, flags);
 
-		DRM_DEBUG_DRIVER("flip queue: crtc already busy\n");
-		return -EBUSY;
+			drm_crtc_vblank_put(crtc);
+			kfree(work);
+			return -EBUSY;
+		}
 	}
 	intel_crtc->unpin_work = work;
 	spin_unlock_irqrestore(&dev->event_lock, flags);
@@ -9337,8 +9422,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);
 
@@ -9361,6 +9444,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 78d4124..ac902ad 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 {
@@ -763,6 +764,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.7.9.5

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 2/3] drm/i915: Decouple the stuck pageflip on modeset

[Chris Wilson]

If we successfully confuse the hardware, and cause it to drop a queued
pageflip, we wait for 60s and issue a warning before continuing on with
the modeset. However, this leaves the pending pageflip still stuck
indefinitely. Pretend to userspace that it does complete, and let us
start afresh following the modeset.

v2: Rebase after refactor

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c |   16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index b87b4ce..9ecc6bf 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3363,9 +3363,19 @@ void intel_crtc_wait_for_pending_flips(struct drm_crtc *crtc)
 
 	WARN_ON(waitqueue_active(&dev_priv->pending_flip_queue));
 
-	WARN_ON(wait_event_timeout(dev_priv->pending_flip_queue,
-				   !intel_crtc_has_pending_flip(crtc),
-				   60*HZ) == 0);
+	if (WARN_ON(wait_event_timeout(dev_priv->pending_flip_queue,
+				       !intel_crtc_has_pending_flip(crtc),
+				       60*HZ) == 0)) {
+		struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+		unsigned long flags;
+
+		spin_lock_irqsave(&dev->event_lock, flags);
+		if (intel_crtc->unpin_work) {
+			WARN_ONCE(1, "Removing stuck page flip\n");
+			page_flip_completed(intel_crtc);
+		}
+		spin_unlock_irqrestore(&dev->event_lock, flags);
+	}
 
 	mutex_lock(&dev->struct_mutex);
 	intel_finish_fb(crtc->primary->fb);
-- 
1.7.9.5

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 3/3] drm/i915: Boost GPU frequency if we detect outstanding pageflips

[Chris Wilson]

If we hit a vblank and see that have a pageflip queue but not yet
processed, ensure that the GPU is running at maximum in order to clear
the backlog. Pageflips are only queued for the following vblank, if we
miss it, there will be a visible stutter. Boosting the GPU frequency
doesn't prevent us from missing the target vblank, but it should help
the subsequent frames hitting theirs.

v2: Reorder vblank vs flip-complete so that we only check for a missed
flip after processing the completion events, and avoid spurious boosts.

v3: Rename missed_vblank

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h      |    1 +
 drivers/gpu/drm/i915/intel_display.c |    6 ++++++
 drivers/gpu/drm/i915/intel_drv.h     |    1 +
 drivers/gpu/drm/i915/intel_pm.c      |   15 +++++++++++++++
 4 files changed, 23 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 10dd80a..33ed0c6 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -910,6 +910,7 @@ struct intel_gen6_power_mgmt {
 
 	bool enabled;
 	struct delayed_work delayed_resume_work;
+	struct work_struct boost_work;
 
 	/*
 	 * Protects RPS/RC6 register access and PCU communication.
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 9ecc6bf..aeb58fa 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -9339,6 +9339,7 @@ void intel_check_page_flip(struct drm_device *dev, int pipe)
 	struct drm_crtc *crtc = dev_priv->pipe_to_crtc_mapping[pipe];
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 	unsigned long flags;
+	bool missed_vblank;
 
 	if (crtc == NULL)
 		return;
@@ -9349,7 +9350,12 @@ void intel_check_page_flip(struct drm_device *dev, int pipe)
 			 intel_crtc->unpin_work->sbc, crtc_sbc(intel_crtc));
 		page_flip_completed(intel_crtc);
 	}
+	missed_vblank = (intel_crtc->unpin_work != NULL &&
+			 crtc_sbc(intel_crtc) - intel_crtc->unpin_work->sbc > 1);
 	spin_unlock_irqrestore(&dev->event_lock, flags);
+
+	if (missed_vblank)
+		intel_queue_rps_boost(dev);
 }
 
 static int intel_crtc_page_flip(struct drm_crtc *crtc,
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index ac902ad..75fba0d 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -966,6 +966,7 @@ void ironlake_teardown_rc6(struct drm_device *dev);
 void gen6_update_ring_freq(struct drm_device *dev);
 void gen6_rps_idle(struct drm_i915_private *dev_priv);
 void gen6_rps_boost(struct drm_i915_private *dev_priv);
+void intel_queue_rps_boost(struct drm_device *dev);
 void intel_aux_display_runtime_get(struct drm_i915_private *dev_priv);
 void intel_aux_display_runtime_put(struct drm_i915_private *dev_priv);
 void intel_runtime_pm_get(struct drm_i915_private *dev_priv);
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index b06f896..ab760e5 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -6722,6 +6722,19 @@ int vlv_freq_opcode(struct drm_i915_private *dev_priv, int val)
 	return DIV_ROUND_CLOSEST(4 * mul * val, dev_priv->mem_freq) + 0xbd - 6;
 }
 
+static void __intel_rps_boost_work(struct work_struct *work)
+{
+	gen6_rps_boost(container_of(work, struct drm_i915_private, rps.boost_work));
+}
+
+void intel_queue_rps_boost(struct drm_device *dev)
+{
+	struct drm_i915_private *dev_priv = to_i915(dev);
+
+	if (INTEL_INFO(dev)->gen >= 6)
+		queue_work(dev_priv->wq, &dev_priv->rps.boost_work);
+}
+
 void intel_pm_setup(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
@@ -6730,6 +6743,8 @@ void intel_pm_setup(struct drm_device *dev)
 
 	INIT_DELAYED_WORK(&dev_priv->rps.delayed_resume_work,
 			  intel_gen6_powersave_work);
+	INIT_WORK(&dev_priv->rps.boost_work,
+		  __intel_rps_boost_work);
 
 	dev_priv->pm.suspended = false;
 	dev_priv->pm.irqs_disabled = false;
-- 
1.7.9.5

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/3] drm/i915: Check for a stalled page flip after each vblank

[Ville Syrjälä]

On Tue, Jun 10, 2014 at 11:04:00AM +0100, Chris Wilson wrote:
> 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
> 
> 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.
> 
> v2: Cleanup, refactor, and rename
> 
> 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>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_debugfs.c  |   23 +++---
>  drivers/gpu/drm/i915/i915_irq.c      |   85 +++++++---------------
>  drivers/gpu/drm/i915/intel_display.c |  130 ++++++++++++++++++++++++++++------
>  drivers/gpu/drm/i915/intel_drv.h     |    2 +
>  4 files changed, 147 insertions(+), 93 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index e8ea1ef..a5f0a26 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;
>  
> @@ -595,6 +596,8 @@ static int i915_gem_pageflip_info(struct seq_file *m, void *data)
>  			seq_printf(m, "No flip due on pipe %c (plane %c)\n",
>  				   pipe, plane);
>  		} else {
> +			u32 addr;
> +
>  			if (atomic_read(&work->pending) < INTEL_FLIP_COMPLETE) {
>  				seq_printf(m, "Flip queued on pipe %c (plane %c)\n",
>  					   pipe, plane);
> @@ -602,23 +605,23 @@ 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));
> -			}
> +			if (INTEL_INFO(dev)->gen >= 4)
> +				addr = I915_HI_DISPBASE(I915_READ(DSPSURF(crtc->plane)));
> +			else
> +				addr = I915_READ(DSPADDR(crtc->plane));
> +			seq_printf(m, "Current scanout address 0x%08x\n", 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));
> +				seq_printf(m, "New framebuffer address 0x%08lx\n", (long)work->gtt_offset);
> +				seq_printf(m, "MMIO update completed? %d\n",  addr == work->gtt_offset);
>  			}
>  		}
>  		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 229e1f1..c3db85f 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 b5cbb28..b87b4ce 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -3330,6 +3330,29 @@ bool intel_has_pending_fb_unpin(struct drm_device *dev)
>  	return false;
>  }
>  
> +static void page_flip_completed(struct intel_crtc *intel_crtc)
> +{
> +	struct drm_i915_private *dev_priv = to_i915(intel_crtc->base.dev);
> +	struct intel_unpin_work *work = intel_crtc->unpin_work;
> +
> +	/* ensure that the unpin work is consistent wrt ->pending. */
> +	smp_rmb();
> +	intel_crtc->unpin_work = NULL;
> +
> +	if (work->event)
> +		drm_send_vblank_event(intel_crtc->base.dev,
> +				      intel_crtc->pipe,
> +				      work->event);
> +
> +	drm_crtc_vblank_put(&intel_crtc->base);
> +
> +	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;
> @@ -8895,7 +8918,6 @@ static void intel_unpin_work_fn(struct work_struct *__work)
>  static void do_intel_finish_page_flip(struct drm_device *dev,
>  				      struct drm_crtc *crtc)
>  {
> -	struct drm_i915_private *dev_priv = dev->dev_private;
>  	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
>  	struct intel_unpin_work *work;
>  	unsigned long flags;
> @@ -8915,23 +8937,9 @@ static void do_intel_finish_page_flip(struct drm_device *dev,
>  		return;
>  	}
>  
> -	/* 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);
> -
> -	drm_crtc_vblank_put(crtc);
> +	page_flip_completed(intel_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)
> @@ -9009,11 +9017,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);
> +}

Still says 'sbc' which doesn't make sense to me.

> +
>  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();
>  }
> @@ -9265,6 +9279,69 @@ static int intel_default_queue_flip(struct drm_device *dev,
>  	return -ENODEV;
>  }
>  
> +static bool i915_gem_object_is_idle(struct drm_i915_gem_object *obj,
> +				    bool readonly)
> +{
> +	struct intel_engine_cs *ring = obj->ring;
> +	u32 seqno = readonly ? obj->last_write_seqno : obj->last_read_seqno;
> +
> +	if (ring == NULL || seqno == 0)
> +		return true;
> +
> +	return i915_seqno_passed(ring->get_seqno(ring, true), seqno);
> +}
> +
> +static bool __intel_pageflip_stall_check(struct drm_device *dev,
> +					 struct drm_crtc *crtc)
> +{
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> +	struct intel_unpin_work *work = intel_crtc->unpin_work;
> +	u32 addr;
> +
> +	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;
> +
> +	if (!i915_gem_object_is_idle(work->pending_flip_obj, true))
> +		return false;

I take it this is done to mitigate my premature flip completion
concerns? Should work for the common case I suppose. Although if
someone does something like this "write,read(s),flip" it could
still complete the flip too soon. Waiting for last_read_seqno would
avoid that.

And double checking the hardware flip counter should avoid this
problem entirely. We already have it sampled in unpin_work->flip_count
for the mmio vs. cs flip race so it should be easy to check it here as
well. I suppose having both the flip counter and seqno checks should
provide the best protection for all gens.

As far as the seqno check goes, I was wondering if we should sample
the seqno when submitting the flip? I'm slightly worried how this will
work if userspace submitted a flip and already managed to pile more
rendering on top. This code would then wait for the seqno for those
later rendering operations.

> +
> +	/* Potential stall - if we see that the flip has happened, assume a missed interrupt */
> +	if (INTEL_INFO(dev)->gen >= 4)
> +		addr = I915_HI_DISPBASE(I915_READ(DSPSURF(intel_crtc->plane)));
> +	else
> +		addr = I915_READ(DSPADDR(intel_crtc->plane));
> +
> +	/* There is a potential issue here with a false positive after a flip
> +	 * to the same address.
> +	 */
> +	return addr == work->gtt_offset;
> +}
> +
> +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)) {
> +		WARN_ONCE(1, "Kicking stuck page flip: queued at %d, now %d\n",
> +			 intel_crtc->unpin_work->sbc, crtc_sbc(intel_crtc));
> +		page_flip_completed(intel_crtc);
> +	}
> +	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,
> @@ -9312,12 +9389,20 @@ 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);
> -		drm_crtc_vblank_put(crtc);
> +		/* Before declaring the flip queue wedged, check if
> +		 * the hardware completed the operation behind our backs.
> +		 */
> +		if (__intel_pageflip_stall_check(dev, crtc)) {
> +			DRM_DEBUG_DRIVER("flip queue: previous flip completed, continuing\n");
> +			page_flip_completed(intel_crtc);
> +		} else {
> +			DRM_DEBUG_DRIVER("flip queue: crtc already busy\n");
> +			spin_unlock_irqrestore(&dev->event_lock, flags);
>  
> -		DRM_DEBUG_DRIVER("flip queue: crtc already busy\n");
> -		return -EBUSY;
> +			drm_crtc_vblank_put(crtc);
> +			kfree(work);
> +			return -EBUSY;
> +		}
>  	}
>  	intel_crtc->unpin_work = work;
>  	spin_unlock_irqrestore(&dev->event_lock, flags);
> @@ -9337,8 +9422,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);
>  
> @@ -9361,6 +9444,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 78d4124..ac902ad 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 {
> @@ -763,6 +764,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.7.9.5

-- 
Ville Syrjälä
Intel OTC

Re: [PATCH 2/3] drm/i915: Decouple the stuck pageflip on modeset

[Ville Syrjälä]

On Tue, Jun 10, 2014 at 11:04:01AM +0100, Chris Wilson wrote:
> If we successfully confuse the hardware, and cause it to drop a queued
> pageflip, we wait for 60s and issue a warning before continuing on with
> the modeset. However, this leaves the pending pageflip still stuck
> indefinitely. Pretend to userspace that it does complete, and let us
> start afresh following the modeset.
> 
> v2: Rebase after refactor
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

still stands.

> ---
>  drivers/gpu/drm/i915/intel_display.c |   16 +++++++++++++---
>  1 file changed, 13 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index b87b4ce..9ecc6bf 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -3363,9 +3363,19 @@ void intel_crtc_wait_for_pending_flips(struct drm_crtc *crtc)
>  
>  	WARN_ON(waitqueue_active(&dev_priv->pending_flip_queue));
>  
> -	WARN_ON(wait_event_timeout(dev_priv->pending_flip_queue,
> -				   !intel_crtc_has_pending_flip(crtc),
> -				   60*HZ) == 0);
> +	if (WARN_ON(wait_event_timeout(dev_priv->pending_flip_queue,
> +				       !intel_crtc_has_pending_flip(crtc),
> +				       60*HZ) == 0)) {
> +		struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> +		unsigned long flags;
> +
> +		spin_lock_irqsave(&dev->event_lock, flags);
> +		if (intel_crtc->unpin_work) {
> +			WARN_ONCE(1, "Removing stuck page flip\n");
> +			page_flip_completed(intel_crtc);
> +		}
> +		spin_unlock_irqrestore(&dev->event_lock, flags);
> +	}
>  
>  	mutex_lock(&dev->struct_mutex);
>  	intel_finish_fb(crtc->primary->fb);
> -- 
> 1.7.9.5

-- 
Ville Syrjälä
Intel OTC

Re: [PATCH 1/3] drm/i915: Check for a stalled page flip after each vblank

[Chris Wilson]

On Tue, Jun 10, 2014 at 02:25:50PM +0300, Ville Syrjälä wrote:
> On Tue, Jun 10, 2014 at 11:04:00AM +0100, Chris Wilson wrote:
> > +static inline int crtc_sbc(struct intel_crtc *crtc)
> > +{
> > +	return atomic_read(&crtc->base.dev->vblank[crtc->pipe].count);
> > +}
> 
> Still says 'sbc' which doesn't make sense to me.

I just don't like the term msc. :-p
crtc_vblank_counter()?
 
> > +
> >  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();
> >  }
> > @@ -9265,6 +9279,69 @@ static int intel_default_queue_flip(struct drm_device *dev,
> >  	return -ENODEV;
> >  }
> >  
> > +static bool i915_gem_object_is_idle(struct drm_i915_gem_object *obj,
> > +				    bool readonly)
> > +{
> > +	struct intel_engine_cs *ring = obj->ring;
> > +	u32 seqno = readonly ? obj->last_write_seqno : obj->last_read_seqno;
> > +
> > +	if (ring == NULL || seqno == 0)
> > +		return true;
> > +
> > +	return i915_seqno_passed(ring->get_seqno(ring, true), seqno);
> > +}
> > +
> > +static bool __intel_pageflip_stall_check(struct drm_device *dev,
> > +					 struct drm_crtc *crtc)
> > +{
> > +	struct drm_i915_private *dev_priv = dev->dev_private;
> > +	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> > +	struct intel_unpin_work *work = intel_crtc->unpin_work;
> > +	u32 addr;
> > +
> > +	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;
> > +
> > +	if (!i915_gem_object_is_idle(work->pending_flip_obj, true))
> > +		return false;
> 
> I take it this is done to mitigate my premature flip completion
> concerns? Should work for the common case I suppose. Although if
> someone does something like this "write,read(s),flip" it could
> still complete the flip too soon. Waiting for last_read_seqno would
> avoid that.

It actually predated your concerns. I considered the person who continues
to write to the pending fb and decided that I didn't care too much for
them. What I actually wanted to do was to capture the vblank counter for
when the object was idle and then start watching for a missed flip.
Indeed, tracking when we believe the flip was queued would be better
again.
 
> And double checking the hardware flip counter should avoid this
> problem entirely. We already have it sampled in unpin_work->flip_count
> for the mmio vs. cs flip race so it should be easy to check it here as
> well. I suppose having both the flip counter and seqno checks should
> provide the best protection for all gens.

That was half the reason for waiting for that series to land first. Just
I never adapted to the framecounter code.
 
> As far as the seqno check goes, I was wondering if we should sample
> the seqno when submitting the flip? I'm slightly worried how this will
> work if userspace submitted a flip and already managed to pile more
> rendering on top. This code would then wait for the seqno for those
> later rendering operations.

Right that is how mmio flips avoid this issue.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [PATCH 3/3] drm/i915: Boost GPU frequency if we detect outstanding pageflips

[Ville Syrjälä]

On Tue, Jun 10, 2014 at 11:04:02AM +0100, Chris Wilson wrote:
> If we hit a vblank and see that have a pageflip queue but not yet
> processed, ensure that the GPU is running at maximum in order to clear
> the backlog. Pageflips are only queued for the following vblank, if we
> miss it, there will be a visible stutter. Boosting the GPU frequency
> doesn't prevent us from missing the target vblank, but it should help
> the subsequent frames hitting theirs.
> 
> v2: Reorder vblank vs flip-complete so that we only check for a missed
> flip after processing the completion events, and avoid spurious boosts.
> 
> v3: Rename missed_vblank
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h      |    1 +
>  drivers/gpu/drm/i915/intel_display.c |    6 ++++++
>  drivers/gpu/drm/i915/intel_drv.h     |    1 +
>  drivers/gpu/drm/i915/intel_pm.c      |   15 +++++++++++++++
>  4 files changed, 23 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 10dd80a..33ed0c6 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -910,6 +910,7 @@ struct intel_gen6_power_mgmt {
>  
>  	bool enabled;
>  	struct delayed_work delayed_resume_work;
> +	struct work_struct boost_work;
>  
>  	/*
>  	 * Protects RPS/RC6 register access and PCU communication.
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 9ecc6bf..aeb58fa 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -9339,6 +9339,7 @@ void intel_check_page_flip(struct drm_device *dev, int pipe)
>  	struct drm_crtc *crtc = dev_priv->pipe_to_crtc_mapping[pipe];
>  	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
>  	unsigned long flags;
> +	bool missed_vblank;
>  
>  	if (crtc == NULL)
>  		return;
> @@ -9349,7 +9350,12 @@ void intel_check_page_flip(struct drm_device *dev, int pipe)
>  			 intel_crtc->unpin_work->sbc, crtc_sbc(intel_crtc));
>  		page_flip_completed(intel_crtc);
>  	}
> +	missed_vblank = (intel_crtc->unpin_work != NULL &&
> +			 crtc_sbc(intel_crtc) - intel_crtc->unpin_work->sbc > 1);

So this will boost when we notice that we've crossed into the second
vblank since the flip was queued. I was wondering if we should try to
boost already after the first vblank. But doing that is a bit more
problematic since we process the "flip done" interrupt after the
vblank interrupt. So simply changing the check to >0 would end up
boosting every time even if the flip already happened and we're just
about to complete it when we get to processing the "flip done"
interrupt.

>  	spin_unlock_irqrestore(&dev->event_lock, flags);
> +
> +	if (missed_vblank)
> +		intel_queue_rps_boost(dev);
>  }
>  
>  static int intel_crtc_page_flip(struct drm_crtc *crtc,
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index ac902ad..75fba0d 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -966,6 +966,7 @@ void ironlake_teardown_rc6(struct drm_device *dev);
>  void gen6_update_ring_freq(struct drm_device *dev);
>  void gen6_rps_idle(struct drm_i915_private *dev_priv);
>  void gen6_rps_boost(struct drm_i915_private *dev_priv);
> +void intel_queue_rps_boost(struct drm_device *dev);
>  void intel_aux_display_runtime_get(struct drm_i915_private *dev_priv);
>  void intel_aux_display_runtime_put(struct drm_i915_private *dev_priv);
>  void intel_runtime_pm_get(struct drm_i915_private *dev_priv);
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index b06f896..ab760e5 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -6722,6 +6722,19 @@ int vlv_freq_opcode(struct drm_i915_private *dev_priv, int val)
>  	return DIV_ROUND_CLOSEST(4 * mul * val, dev_priv->mem_freq) + 0xbd - 6;
>  }
>  
> +static void __intel_rps_boost_work(struct work_struct *work)
> +{
> +	gen6_rps_boost(container_of(work, struct drm_i915_private, rps.boost_work));
> +}
> +
> +void intel_queue_rps_boost(struct drm_device *dev)
> +{
> +	struct drm_i915_private *dev_priv = to_i915(dev);
> +
> +	if (INTEL_INFO(dev)->gen >= 6)
> +		queue_work(dev_priv->wq, &dev_priv->rps.boost_work);
> +}
> +
>  void intel_pm_setup(struct drm_device *dev)
>  {
>  	struct drm_i915_private *dev_priv = dev->dev_private;
> @@ -6730,6 +6743,8 @@ void intel_pm_setup(struct drm_device *dev)
>  
>  	INIT_DELAYED_WORK(&dev_priv->rps.delayed_resume_work,
>  			  intel_gen6_powersave_work);
> +	INIT_WORK(&dev_priv->rps.boost_work,
> +		  __intel_rps_boost_work);
>  
>  	dev_priv->pm.suspended = false;
>  	dev_priv->pm.irqs_disabled = false;
> -- 
> 1.7.9.5

-- 
Ville Syrjälä
Intel OTC

Re: [PATCH 1/3] drm/i915: Check for a stalled page flip after each vblank

[Ville Syrjälä]

On Tue, Jun 10, 2014 at 12:33:48PM +0100, Chris Wilson wrote:
> On Tue, Jun 10, 2014 at 02:25:50PM +0300, Ville Syrjälä wrote:
> > On Tue, Jun 10, 2014 at 11:04:00AM +0100, Chris Wilson wrote:
> > > +static inline int crtc_sbc(struct intel_crtc *crtc)
> > > +{
> > > +	return atomic_read(&crtc->base.dev->vblank[crtc->pipe].count);
> > > +}
> > 
> > Still says 'sbc' which doesn't make sense to me.
> 
> I just don't like the term msc. :-p
> crtc_vblank_counter()?

Maybe stick it into drmP.h and call it drm_crtc_vblank_counter() or
something? Hopefully people won't confuse it with the hardware counter.

-- 
Ville Syrjälä
Intel OTC

Re: [PATCH 3/3] drm/i915: Boost GPU frequency if we detect outstanding pageflips

[Chris Wilson]

On Tue, Jun 10, 2014 at 02:41:20PM +0300, Ville Syrjälä wrote:
> On Tue, Jun 10, 2014 at 11:04:02AM +0100, Chris Wilson wrote:
> > If we hit a vblank and see that have a pageflip queue but not yet
> > processed, ensure that the GPU is running at maximum in order to clear
> > the backlog. Pageflips are only queued for the following vblank, if we
> > miss it, there will be a visible stutter. Boosting the GPU frequency
> > doesn't prevent us from missing the target vblank, but it should help
> > the subsequent frames hitting theirs.
> > 
> > v2: Reorder vblank vs flip-complete so that we only check for a missed
> > flip after processing the completion events, and avoid spurious boosts.
> > 
> > v3: Rename missed_vblank
> > 
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_drv.h      |    1 +
> >  drivers/gpu/drm/i915/intel_display.c |    6 ++++++
> >  drivers/gpu/drm/i915/intel_drv.h     |    1 +
> >  drivers/gpu/drm/i915/intel_pm.c      |   15 +++++++++++++++
> >  4 files changed, 23 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> > index 10dd80a..33ed0c6 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -910,6 +910,7 @@ struct intel_gen6_power_mgmt {
> >  
> >  	bool enabled;
> >  	struct delayed_work delayed_resume_work;
> > +	struct work_struct boost_work;
> >  
> >  	/*
> >  	 * Protects RPS/RC6 register access and PCU communication.
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index 9ecc6bf..aeb58fa 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -9339,6 +9339,7 @@ void intel_check_page_flip(struct drm_device *dev, int pipe)
> >  	struct drm_crtc *crtc = dev_priv->pipe_to_crtc_mapping[pipe];
> >  	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> >  	unsigned long flags;
> > +	bool missed_vblank;
> >  
> >  	if (crtc == NULL)
> >  		return;
> > @@ -9349,7 +9350,12 @@ void intel_check_page_flip(struct drm_device *dev, int pipe)
> >  			 intel_crtc->unpin_work->sbc, crtc_sbc(intel_crtc));
> >  		page_flip_completed(intel_crtc);
> >  	}
> > +	missed_vblank = (intel_crtc->unpin_work != NULL &&
> > +			 crtc_sbc(intel_crtc) - intel_crtc->unpin_work->sbc > 1);
> 
> So this will boost when we notice that we've crossed into the second
> vblank since the flip was queued. I was wondering if we should try to
> boost already after the first vblank. But doing that is a bit more
> problematic since we process the "flip done" interrupt after the
> vblank interrupt. So simply changing the check to >0 would end up
> boosting every time even if the flip already happened and we're just
> about to complete it when we get to processing the "flip done"
> interrupt.

Exactly. It is a little too eager if we start boosting after one missed
vblank as we process the vblank before the flip-complete.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [PATCH 3/3] drm/i915: Boost GPU frequency if we detect outstanding pageflips

[Ville Syrjälä]

On Tue, Jun 10, 2014 at 11:04:02AM +0100, Chris Wilson wrote:
> If we hit a vblank and see that have a pageflip queue but not yet
> processed, ensure that the GPU is running at maximum in order to clear
> the backlog. Pageflips are only queued for the following vblank, if we
> miss it, there will be a visible stutter. Boosting the GPU frequency
> doesn't prevent us from missing the target vblank, but it should help
> the subsequent frames hitting theirs.
> 
> v2: Reorder vblank vs flip-complete so that we only check for a missed
> flip after processing the completion events, and avoid spurious boosts.
> 
> v3: Rename missed_vblank
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>

> ---
>  drivers/gpu/drm/i915/i915_drv.h      |    1 +
>  drivers/gpu/drm/i915/intel_display.c |    6 ++++++
>  drivers/gpu/drm/i915/intel_drv.h     |    1 +
>  drivers/gpu/drm/i915/intel_pm.c      |   15 +++++++++++++++
>  4 files changed, 23 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 10dd80a..33ed0c6 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -910,6 +910,7 @@ struct intel_gen6_power_mgmt {
>  
>  	bool enabled;
>  	struct delayed_work delayed_resume_work;
> +	struct work_struct boost_work;
>  
>  	/*
>  	 * Protects RPS/RC6 register access and PCU communication.
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 9ecc6bf..aeb58fa 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -9339,6 +9339,7 @@ void intel_check_page_flip(struct drm_device *dev, int pipe)
>  	struct drm_crtc *crtc = dev_priv->pipe_to_crtc_mapping[pipe];
>  	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
>  	unsigned long flags;
> +	bool missed_vblank;
>  
>  	if (crtc == NULL)
>  		return;
> @@ -9349,7 +9350,12 @@ void intel_check_page_flip(struct drm_device *dev, int pipe)
>  			 intel_crtc->unpin_work->sbc, crtc_sbc(intel_crtc));
>  		page_flip_completed(intel_crtc);
>  	}
> +	missed_vblank = (intel_crtc->unpin_work != NULL &&
> +			 crtc_sbc(intel_crtc) - intel_crtc->unpin_work->sbc > 1);
>  	spin_unlock_irqrestore(&dev->event_lock, flags);
> +
> +	if (missed_vblank)
> +		intel_queue_rps_boost(dev);
>  }
>  
>  static int intel_crtc_page_flip(struct drm_crtc *crtc,
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index ac902ad..75fba0d 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -966,6 +966,7 @@ void ironlake_teardown_rc6(struct drm_device *dev);
>  void gen6_update_ring_freq(struct drm_device *dev);
>  void gen6_rps_idle(struct drm_i915_private *dev_priv);
>  void gen6_rps_boost(struct drm_i915_private *dev_priv);
> +void intel_queue_rps_boost(struct drm_device *dev);
>  void intel_aux_display_runtime_get(struct drm_i915_private *dev_priv);
>  void intel_aux_display_runtime_put(struct drm_i915_private *dev_priv);
>  void intel_runtime_pm_get(struct drm_i915_private *dev_priv);
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index b06f896..ab760e5 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -6722,6 +6722,19 @@ int vlv_freq_opcode(struct drm_i915_private *dev_priv, int val)
>  	return DIV_ROUND_CLOSEST(4 * mul * val, dev_priv->mem_freq) + 0xbd - 6;
>  }
>  
> +static void __intel_rps_boost_work(struct work_struct *work)
> +{
> +	gen6_rps_boost(container_of(work, struct drm_i915_private, rps.boost_work));

gen6_rps_boost() checks for rps.enabled so it doesn't matter when this
gets scheduled wrt. rps enable/disable. Check.

And we have a flush_workqueue() in unload path after the modeset
cleanup, so the work should be done by the time the workqueue gets
torn down. Check.

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> +}
> +
> +void intel_queue_rps_boost(struct drm_device *dev)
> +{
> +	struct drm_i915_private *dev_priv = to_i915(dev);
> +
> +	if (INTEL_INFO(dev)->gen >= 6)
> +		queue_work(dev_priv->wq, &dev_priv->rps.boost_work);
> +}
> +
>  void intel_pm_setup(struct drm_device *dev)
>  {
>  	struct drm_i915_private *dev_priv = dev->dev_private;
> @@ -6730,6 +6743,8 @@ void intel_pm_setup(struct drm_device *dev)
>  
>  	INIT_DELAYED_WORK(&dev_priv->rps.delayed_resume_work,
>  			  intel_gen6_powersave_work);
> +	INIT_WORK(&dev_priv->rps.boost_work,
> +		  __intel_rps_boost_work);
>  
>  	dev_priv->pm.suspended = false;
>  	dev_priv->pm.irqs_disabled = false;
> -- 
> 1.7.9.5

-- 
Ville Syrjälä
Intel OTC

Re: [PATCH 1/3] drm/i915: Check for a stalled page flip after each vblank

[Daniel Vetter]

On Tue, Jun 10, 2014 at 02:47:29PM +0300, Ville Syrjälä wrote:
> On Tue, Jun 10, 2014 at 12:33:48PM +0100, Chris Wilson wrote:
> > On Tue, Jun 10, 2014 at 02:25:50PM +0300, Ville Syrjälä wrote:
> > > On Tue, Jun 10, 2014 at 11:04:00AM +0100, Chris Wilson wrote:
> > > > +static inline int crtc_sbc(struct intel_crtc *crtc)
> > > > +{
> > > > +	return atomic_read(&crtc->base.dev->vblank[crtc->pipe].count);
> > > > +}
> > > 
> > > Still says 'sbc' which doesn't make sense to me.
> > 
> > I just don't like the term msc. :-p
> > crtc_vblank_counter()?
> 
> Maybe stick it into drmP.h and call it drm_crtc_vblank_counter() or
> something? Hopefully people won't confuse it with the hardware counter.

It's called drm_vblank_count, but a new wrapper which takes a drm_crtc *
as the argument would indeed be nice. So drm_crtc_vblank_counter(struct
drm_crtc *crtc). And please don't forget to add the kerneldoc and to pull
it into the drm docbook.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: [PATCH 3/3] drm/i915: Boost GPU frequency if we detect outstanding pageflips

[Daniel Vetter]

On Tue, Jun 10, 2014 at 01:46:56PM +0100, Chris Wilson wrote:
> If we hit a vblank and see that have a pageflip queue but not yet
> processed, ensure that the GPU is running at maximum in order to clear
> the backlog. Pageflips are only queued for the following vblank, if we
> miss it, there will be a visible stutter. Boosting the GPU frequency
> doesn't prevent us from missing the target vblank, but it should help
> the subsequent frames hitting theirs.
> 
> v2: Reorder vblank vs flip-complete so that we only check for a missed
> flip after processing the completion events, and avoid spurious boosts.
> 
> v3: Rename missed_vblank
> v4: Rebase
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>

Found one more: boost_work never gets cancelled. Probably not want we
want. I guess the best place is in the gt powersave suspend function, since
at that point interrupts are no more.

First patch from this series is merged now.
-Daniel
> ---
>  drivers/gpu/drm/i915/i915_drv.h      |  1 +
>  drivers/gpu/drm/i915/intel_display.c |  6 ++++++
>  drivers/gpu/drm/i915/intel_drv.h     |  1 +
>  drivers/gpu/drm/i915/intel_pm.c      | 15 +++++++++++++++
>  4 files changed, 23 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 10dd80a12658..33ed0c6b8a9c 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -910,6 +910,7 @@ struct intel_gen6_power_mgmt {
>  
>  	bool enabled;
>  	struct delayed_work delayed_resume_work;
> +	struct work_struct boost_work;
>  
>  	/*
>  	 * Protects RPS/RC6 register access and PCU communication.
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 5261c145e633..89a4d01fefb4 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -9326,6 +9326,7 @@ void intel_check_page_flip(struct drm_device *dev, int pipe)
>  	struct drm_crtc *crtc = dev_priv->pipe_to_crtc_mapping[pipe];
>  	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
>  	unsigned long flags;
> +	bool missed_vblank;
>  
>  	if (crtc == NULL)
>  		return;
> @@ -9336,7 +9337,12 @@ void intel_check_page_flip(struct drm_device *dev, int pipe)
>  			 intel_crtc->unpin_work->flip_queued_vblank, drm_vblank_count(dev, pipe));
>  		page_flip_completed(intel_crtc);
>  	}
> +	missed_vblank = (intel_crtc->unpin_work != NULL &&
> +			 drm_vblank_count(dev, pipe) - intel_crtc->unpin_work->flip_queued_vblank > 1);
>  	spin_unlock_irqrestore(&dev->event_lock, flags);
> +
> +	if (missed_vblank)
> +		intel_queue_rps_boost(dev);
>  }
>  
>  static int intel_crtc_page_flip(struct drm_crtc *crtc,
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 10665716eb10..2e3c65812e72 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -969,6 +969,7 @@ void ironlake_teardown_rc6(struct drm_device *dev);
>  void gen6_update_ring_freq(struct drm_device *dev);
>  void gen6_rps_idle(struct drm_i915_private *dev_priv);
>  void gen6_rps_boost(struct drm_i915_private *dev_priv);
> +void intel_queue_rps_boost(struct drm_device *dev);
>  void intel_aux_display_runtime_get(struct drm_i915_private *dev_priv);
>  void intel_aux_display_runtime_put(struct drm_i915_private *dev_priv);
>  void intel_runtime_pm_get(struct drm_i915_private *dev_priv);
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index b06f896740b5..ab760e5abc9a 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -6722,6 +6722,19 @@ int vlv_freq_opcode(struct drm_i915_private *dev_priv, int val)
>  	return DIV_ROUND_CLOSEST(4 * mul * val, dev_priv->mem_freq) + 0xbd - 6;
>  }
>  
> +static void __intel_rps_boost_work(struct work_struct *work)
> +{
> +	gen6_rps_boost(container_of(work, struct drm_i915_private, rps.boost_work));
> +}
> +
> +void intel_queue_rps_boost(struct drm_device *dev)
> +{
> +	struct drm_i915_private *dev_priv = to_i915(dev);
> +
> +	if (INTEL_INFO(dev)->gen >= 6)
> +		queue_work(dev_priv->wq, &dev_priv->rps.boost_work);
> +}
> +
>  void intel_pm_setup(struct drm_device *dev)
>  {
>  	struct drm_i915_private *dev_priv = dev->dev_private;
> @@ -6730,6 +6743,8 @@ void intel_pm_setup(struct drm_device *dev)
>  
>  	INIT_DELAYED_WORK(&dev_priv->rps.delayed_resume_work,
>  			  intel_gen6_powersave_work);
> +	INIT_WORK(&dev_priv->rps.boost_work,
> +		  __intel_rps_boost_work);
>  
>  	dev_priv->pm.suspended = false;
>  	dev_priv->pm.irqs_disabled = false;
> -- 
> 2.0.0
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

[PATCH 3/3] drm/i915: Boost GPU frequency if we detect outstanding pageflips

[Chris Wilson]

If we hit a vblank and see that have a pageflip queue but not yet
processed, ensure that the GPU is running at maximum in order to clear
the backlog. Pageflips are only queued for the following vblank, if we
miss it, there will be a visible stutter. Boosting the GPU frequency
doesn't prevent us from missing the target vblank, but it should help
the subsequent frames hitting theirs.

v2: Reorder vblank vs flip-complete so that we only check for a missed
flip after processing the completion events, and avoid spurious boosts.

v3: Rename missed_vblank
v4: Rebase

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h      |  1 +
 drivers/gpu/drm/i915/intel_display.c |  6 ++++++
 drivers/gpu/drm/i915/intel_drv.h     |  1 +
 drivers/gpu/drm/i915/intel_pm.c      | 15 +++++++++++++++
 4 files changed, 23 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 10dd80a12658..33ed0c6b8a9c 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -910,6 +910,7 @@ struct intel_gen6_power_mgmt {
 
 	bool enabled;
 	struct delayed_work delayed_resume_work;
+	struct work_struct boost_work;
 
 	/*
 	 * Protects RPS/RC6 register access and PCU communication.
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 5261c145e633..89a4d01fefb4 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -9326,6 +9326,7 @@ void intel_check_page_flip(struct drm_device *dev, int pipe)
 	struct drm_crtc *crtc = dev_priv->pipe_to_crtc_mapping[pipe];
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 	unsigned long flags;
+	bool missed_vblank;
 
 	if (crtc == NULL)
 		return;
@@ -9336,7 +9337,12 @@ void intel_check_page_flip(struct drm_device *dev, int pipe)
 			 intel_crtc->unpin_work->flip_queued_vblank, drm_vblank_count(dev, pipe));
 		page_flip_completed(intel_crtc);
 	}
+	missed_vblank = (intel_crtc->unpin_work != NULL &&
+			 drm_vblank_count(dev, pipe) - intel_crtc->unpin_work->flip_queued_vblank > 1);
 	spin_unlock_irqrestore(&dev->event_lock, flags);
+
+	if (missed_vblank)
+		intel_queue_rps_boost(dev);
 }
 
 static int intel_crtc_page_flip(struct drm_crtc *crtc,
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 10665716eb10..2e3c65812e72 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -969,6 +969,7 @@ void ironlake_teardown_rc6(struct drm_device *dev);
 void gen6_update_ring_freq(struct drm_device *dev);
 void gen6_rps_idle(struct drm_i915_private *dev_priv);
 void gen6_rps_boost(struct drm_i915_private *dev_priv);
+void intel_queue_rps_boost(struct drm_device *dev);
 void intel_aux_display_runtime_get(struct drm_i915_private *dev_priv);
 void intel_aux_display_runtime_put(struct drm_i915_private *dev_priv);
 void intel_runtime_pm_get(struct drm_i915_private *dev_priv);
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index b06f896740b5..ab760e5abc9a 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -6722,6 +6722,19 @@ int vlv_freq_opcode(struct drm_i915_private *dev_priv, int val)
 	return DIV_ROUND_CLOSEST(4 * mul * val, dev_priv->mem_freq) + 0xbd - 6;
 }
 
+static void __intel_rps_boost_work(struct work_struct *work)
+{
+	gen6_rps_boost(container_of(work, struct drm_i915_private, rps.boost_work));
+}
+
+void intel_queue_rps_boost(struct drm_device *dev)
+{
+	struct drm_i915_private *dev_priv = to_i915(dev);
+
+	if (INTEL_INFO(dev)->gen >= 6)
+		queue_work(dev_priv->wq, &dev_priv->rps.boost_work);
+}
+
 void intel_pm_setup(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
@@ -6730,6 +6743,8 @@ void intel_pm_setup(struct drm_device *dev)
 
 	INIT_DELAYED_WORK(&dev_priv->rps.delayed_resume_work,
 			  intel_gen6_powersave_work);
+	INIT_WORK(&dev_priv->rps.boost_work,
+		  __intel_rps_boost_work);
 
 	dev_priv->pm.suspended = false;
 	dev_priv->pm.irqs_disabled = false;
-- 
2.0.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 3/3] drm/i915: Boost GPU frequency if we detect outstanding pageflips

[Chris Wilson]

On Mon, Jun 02, 2014 at 10:53:57AM +0200, Daniel Vetter wrote:
> On Fri, May 30, 2014 at 05:16:36PM +0100, Chris Wilson wrote:
> > +	outstanding = (intel_crtc->unpin_work != NULL &&
> > +		       crtc_sbc(intel_crtc) - intel_crtc->unpin_work->sbc > 1);
> 
> s/outstanding/still_pending/? Since the situation is very much not
> oustanding at all ...

I changed it to
  missed_vblank = blah;
  if (missed_vblank)
     queue_boost().
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [PATCH 3/3] drm/i915: Boost GPU frequency if we detect outstanding pageflips

[Daniel Vetter]

On Fri, May 30, 2014 at 05:16:36PM +0100, Chris Wilson wrote:
> If we hit a vblank and see that have a pageflip queue but not yet
> processed, ensure that the GPU is running at maximum in order to clear
> the backlog. Pageflips are only queued for the following vblank, if we
> miss it, there will be a visible stutter. Boosting the GPU frequency
> doesn't prevent us from missing the target vblank, but it should help
> the subsequent frames hitting theirs.
> 
> v2: Reorder vblank vs flip-complete so that we only check for a missed
> flip after processing the completion events, and avoid spurious boosts.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>  drivers/gpu/drm/i915/i915_drv.h      |  1 +
>  drivers/gpu/drm/i915/intel_display.c |  6 ++++++
>  drivers/gpu/drm/i915/intel_drv.h     |  1 +
>  drivers/gpu/drm/i915/intel_pm.c      | 15 +++++++++++++++
>  4 files changed, 23 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index f9450045a532..af050e439168 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -910,6 +910,7 @@ struct intel_gen6_power_mgmt {
>  
>  	bool enabled;
>  	struct delayed_work delayed_resume_work;
> +	struct work_struct boost_work;
>  
>  	/*
>  	 * Protects RPS/RC6 register access and PCU communication.
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 54b69838e2de..3ad1529e74df 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -9258,6 +9258,7 @@ void intel_check_page_flip(struct drm_device *dev, int pipe)
>  	struct drm_crtc *crtc = dev_priv->pipe_to_crtc_mapping[pipe];
>  	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
>  	unsigned long flags;
> +	bool outstanding;
>  
>  	if (crtc == NULL)
>  		return;
> @@ -9270,7 +9271,12 @@ void intel_check_page_flip(struct drm_device *dev, int pipe)
>  		page_flip_completed(dev_priv, intel_crtc, intel_crtc->unpin_work);
>  		intel_crtc->unpin_work = NULL;
>  	}
> +	outstanding = (intel_crtc->unpin_work != NULL &&
> +		       crtc_sbc(intel_crtc) - intel_crtc->unpin_work->sbc > 1);

s/outstanding/still_pending/? Since the situation is very much not
oustanding at all ...
-Daniel

>  	spin_unlock_irqrestore(&dev->event_lock, flags);
> +
> +	if (outstanding)
> +		intel_queue_rps_boost(dev);
>  }
>  
>  static int intel_crtc_page_flip(struct drm_crtc *crtc,
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index b17c295fe967..a77f104db366 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -964,6 +964,7 @@ void ironlake_teardown_rc6(struct drm_device *dev);
>  void gen6_update_ring_freq(struct drm_device *dev);
>  void gen6_rps_idle(struct drm_i915_private *dev_priv);
>  void gen6_rps_boost(struct drm_i915_private *dev_priv);
> +void intel_queue_rps_boost(struct drm_device *dev);
>  void intel_aux_display_runtime_get(struct drm_i915_private *dev_priv);
>  void intel_aux_display_runtime_put(struct drm_i915_private *dev_priv);
>  void intel_runtime_pm_get(struct drm_i915_private *dev_priv);
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 00ab3c7a282d..dce2c268851f 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -6707,6 +6707,19 @@ int vlv_freq_opcode(struct drm_i915_private *dev_priv, int val)
>  	return DIV_ROUND_CLOSEST(4 * mul * val, dev_priv->mem_freq) + 0xbd - 6;
>  }
>  
> +static void __intel_rps_boost_work(struct work_struct *work)
> +{
> +	gen6_rps_boost(container_of(work, struct drm_i915_private, rps.boost_work));
> +}
> +
> +void intel_queue_rps_boost(struct drm_device *dev)
> +{
> +	struct drm_i915_private *dev_priv = to_i915(dev);
> +
> +	if (INTEL_INFO(dev)->gen >= 6)
> +		queue_work(dev_priv->wq, &dev_priv->rps.boost_work);
> +}
> +
>  void intel_pm_setup(struct drm_device *dev)
>  {
>  	struct drm_i915_private *dev_priv = dev->dev_private;
> @@ -6715,6 +6728,8 @@ void intel_pm_setup(struct drm_device *dev)
>  
>  	INIT_DELAYED_WORK(&dev_priv->rps.delayed_resume_work,
>  			  intel_gen6_powersave_work);
> +	INIT_WORK(&dev_priv->rps.boost_work,
> +		  __intel_rps_boost_work);
>  
>  	dev_priv->pm.suspended = false;
>  	dev_priv->pm.irqs_disabled = false;
> -- 
> 2.0.0.rc4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

[PATCH 3/3] drm/i915: Boost GPU frequency if we detect outstanding pageflips

[Chris Wilson]

If we hit a vblank and see that have a pageflip queue but not yet
processed, ensure that the GPU is running at maximum in order to clear
the backlog. Pageflips are only queued for the following vblank, if we
miss it, there will be a visible stutter. Boosting the GPU frequency
doesn't prevent us from missing the target vblank, but it should help
the subsequent frames hitting theirs.

v2: Reorder vblank vs flip-complete so that we only check for a missed
flip after processing the completion events, and avoid spurious boosts.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_drv.h      |  1 +
 drivers/gpu/drm/i915/intel_display.c |  6 ++++++
 drivers/gpu/drm/i915/intel_drv.h     |  1 +
 drivers/gpu/drm/i915/intel_pm.c      | 15 +++++++++++++++
 4 files changed, 23 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index f9450045a532..af050e439168 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -910,6 +910,7 @@ struct intel_gen6_power_mgmt {
 
 	bool enabled;
 	struct delayed_work delayed_resume_work;
+	struct work_struct boost_work;
 
 	/*
 	 * Protects RPS/RC6 register access and PCU communication.
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 54b69838e2de..3ad1529e74df 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -9258,6 +9258,7 @@ void intel_check_page_flip(struct drm_device *dev, int pipe)
 	struct drm_crtc *crtc = dev_priv->pipe_to_crtc_mapping[pipe];
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 	unsigned long flags;
+	bool outstanding;
 
 	if (crtc == NULL)
 		return;
@@ -9270,7 +9271,12 @@ void intel_check_page_flip(struct drm_device *dev, int pipe)
 		page_flip_completed(dev_priv, intel_crtc, intel_crtc->unpin_work);
 		intel_crtc->unpin_work = NULL;
 	}
+	outstanding = (intel_crtc->unpin_work != NULL &&
+		       crtc_sbc(intel_crtc) - intel_crtc->unpin_work->sbc > 1);
 	spin_unlock_irqrestore(&dev->event_lock, flags);
+
+	if (outstanding)
+		intel_queue_rps_boost(dev);
 }
 
 static int intel_crtc_page_flip(struct drm_crtc *crtc,
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index b17c295fe967..a77f104db366 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -964,6 +964,7 @@ void ironlake_teardown_rc6(struct drm_device *dev);
 void gen6_update_ring_freq(struct drm_device *dev);
 void gen6_rps_idle(struct drm_i915_private *dev_priv);
 void gen6_rps_boost(struct drm_i915_private *dev_priv);
+void intel_queue_rps_boost(struct drm_device *dev);
 void intel_aux_display_runtime_get(struct drm_i915_private *dev_priv);
 void intel_aux_display_runtime_put(struct drm_i915_private *dev_priv);
 void intel_runtime_pm_get(struct drm_i915_private *dev_priv);
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 00ab3c7a282d..dce2c268851f 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -6707,6 +6707,19 @@ int vlv_freq_opcode(struct drm_i915_private *dev_priv, int val)
 	return DIV_ROUND_CLOSEST(4 * mul * val, dev_priv->mem_freq) + 0xbd - 6;
 }
 
+static void __intel_rps_boost_work(struct work_struct *work)
+{
+	gen6_rps_boost(container_of(work, struct drm_i915_private, rps.boost_work));
+}
+
+void intel_queue_rps_boost(struct drm_device *dev)
+{
+	struct drm_i915_private *dev_priv = to_i915(dev);
+
+	if (INTEL_INFO(dev)->gen >= 6)
+		queue_work(dev_priv->wq, &dev_priv->rps.boost_work);
+}
+
 void intel_pm_setup(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
@@ -6715,6 +6728,8 @@ void intel_pm_setup(struct drm_device *dev)
 
 	INIT_DELAYED_WORK(&dev_priv->rps.delayed_resume_work,
 			  intel_gen6_powersave_work);
+	INIT_WORK(&dev_priv->rps.boost_work,
+		  __intel_rps_boost_work);
 
 	dev_priv->pm.suspended = false;
 	dev_priv->pm.irqs_disabled = false;
-- 
2.0.0.rc4