[PATCH v2 00/11] drm/i915: Rework page flip to be more atomic like, and remove cs flips.

[PATCH v2 00/11] drm/i915: Rework page flip to be more atomic like, and remove cs flips.

[Maarten Lankhorst]

This patch first adds drm_accurate_vblank_count, which needs an ack from airlied to get through dinq.

After adding support for mmio updates on all platforms support for cs flips is removed.
It's hard to test properly and makes async atomic commit harder to get right.

Maarten Lankhorst (11):
  drm/core: Add drm_accurate_vblank_count, v4.
  drm/i915: Remove stallcheck special handling.
  drm/i915: Remove intel_prepare_page_flip.
  drm/i915: Add support for detecting vblanks when hw frame counter is unavailable.
  drm/i915: Allow mmio updates on all platforms, v2.
  drm/i915: Convert flip_work to a list.
  drm/i915: Add the exclusive fence to plane_state.
  drm/i915: Rework intel_crtc_page_flip to be almost atomic, v3.
  drm/i915: Remove cs based page flip support.
  drm/i915: Remove use_mmio_flip kernel parameter.
  drm/i915: Remove queue_flip pointer.

 drivers/gpu/drm/drm_irq.c                 |   26 +
 drivers/gpu/drm/i915/i915_debugfs.c       |   93 +--
 drivers/gpu/drm/i915/i915_drv.h           |    5 -
 drivers/gpu/drm/i915/i915_irq.c           |   18 +-
 drivers/gpu/drm/i915/i915_params.c        |    5 -
 drivers/gpu/drm/i915/i915_params.h        |    1 -
 drivers/gpu/drm/i915/intel_atomic_plane.c |    1 +
 drivers/gpu/drm/i915/intel_display.c      | 1136 +++++++++--------------------
 drivers/gpu/drm/i915/intel_drv.h          |   43 +-
 drivers/gpu/drm/i915/intel_lrc.c          |    3 +-
 drivers/gpu/drm/i915/intel_sprite.c       |   16 +-
 include/drm/drmP.h                        |    1 +
 12 files changed, 443 insertions(+), 905 deletions(-)

-- 
2.1.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v2 01/11] drm/core: Add drm_accurate_vblank_count, v4.

[Maarten Lankhorst]

This function is useful for gen2 intel devices which have no frame
counter, but need a way to determine the current vblank count without
racing with the vblank interrupt handler.

intel_pipe_update_start checks if no vblank interrupt will occur
during vblank evasion, but cannot check whether the vblank handler has
run to completion. This function uses the timestamps to determine
when the last vblank has happened, and interpolates from there.

Changes since v1:
- Take vblank_time_lock and don't use drm_vblank_count_and_time.
Changes since v2:
- Don't return time of last vblank.
Changes since v3:
- Change pipe to unsigned int. (Ville)
- Remove unused documentation for tv_ret. (kbuild)

Cc: Mario Kleiner <mario.kleiner.de@gmail.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/drm_irq.c | 26 ++++++++++++++++++++++++++
 include/drm/drmP.h        |  1 +
 2 files changed, 27 insertions(+)

diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
index 3c1a6f18e71c..f1bda13562da 100644
--- a/drivers/gpu/drm/drm_irq.c
+++ b/drivers/gpu/drm/drm_irq.c
@@ -303,6 +303,32 @@ static void drm_update_vblank_count(struct drm_device *dev, unsigned int pipe,
 	store_vblank(dev, pipe, diff, &t_vblank, cur_vblank);
 }
 
+/**
+ * drm_accurate_vblank_count - retrieve the master vblank counter
+ * @crtc: which counter to retrieve
+ *
+ * This function is similar to @drm_crtc_vblank_count but this
+ * function interpolates to handle a race with vblank irq's.
+ */
+
+u32 drm_accurate_vblank_count(struct drm_crtc *crtc)
+{
+	struct drm_device *dev = crtc->dev;
+	unsigned int pipe = drm_crtc_index(crtc);
+	u32 vblank;
+	unsigned long flags;
+
+	spin_lock_irqsave(&dev->vblank_time_lock, flags);
+
+	drm_update_vblank_count(dev, pipe, 0);
+	vblank = dev->vblank[pipe].count;
+
+	spin_unlock_irqrestore(&dev->vblank_time_lock, flags);
+
+	return vblank;
+}
+EXPORT_SYMBOL(drm_accurate_vblank_count);
+
 /*
  * Disable vblank irq's on crtc, make sure that last vblank count
  * of hardware and corresponding consistent software vblank counter
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index 31483c2fef51..747c80da70e5 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -995,6 +995,7 @@ extern void drm_crtc_vblank_off(struct drm_crtc *crtc);
 extern void drm_crtc_vblank_reset(struct drm_crtc *crtc);
 extern void drm_crtc_vblank_on(struct drm_crtc *crtc);
 extern void drm_vblank_cleanup(struct drm_device *dev);
+extern u32 drm_accurate_vblank_count(struct drm_crtc *crtc);
 extern u32 drm_vblank_no_hw_counter(struct drm_device *dev, unsigned int pipe);
 
 extern int drm_calc_vbltimestamp_from_scanoutpos(struct drm_device *dev,
-- 
2.1.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v2 02/11] drm/i915: Remove stallcheck special handling.

[Maarten Lankhorst]

Re-use unpin_work->pending, but also set vblank count before
intel_mark_page_flip_active to be sure.

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c  | 11 ++++++-----
 drivers/gpu/drm/i915/intel_display.c | 31 ++++++++++++-------------------
 drivers/gpu/drm/i915/intel_drv.h     |  1 -
 3 files changed, 18 insertions(+), 25 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 9640738aabf2..df8073a2ffbe 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -582,9 +582,14 @@ 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 pending;
 			u32 addr;
 
-			if (atomic_read(&work->pending) < INTEL_FLIP_COMPLETE) {
+			pending = atomic_read(&work->pending);
+			if (pending == INTEL_FLIP_INACTIVE) {
+				seq_printf(m, "Flip ioctl preparing on pipe %c (plane %c)\n",
+					   pipe, plane);
+			} else if (pending >= INTEL_FLIP_COMPLETE) {
 				seq_printf(m, "Flip queued on pipe %c (plane %c)\n",
 					   pipe, plane);
 			} else {
@@ -606,10 +611,6 @@ static int i915_gem_pageflip_info(struct seq_file *m, void *data)
 				   work->flip_queued_vblank,
 				   work->flip_ready_vblank,
 				   drm_crtc_vblank_count(&crtc->base));
-			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 (INTEL_INFO(dev)->gen >= 4)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index f2be54a48727..618e034a7a5e 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -11415,8 +11415,6 @@ static void intel_do_mmio_flip(struct intel_mmio_flip *mmio_flip)
 	if (work == NULL)
 		return;
 
-	intel_mark_page_flip_active(work);
-
 	intel_pipe_update_start(crtc);
 
 	if (INTEL_INFO(mmio_flip->i915)->gen >= 9)
@@ -11426,6 +11424,8 @@ static void intel_do_mmio_flip(struct intel_mmio_flip *mmio_flip)
 		ilk_do_mmio_flip(crtc, work);
 
 	intel_pipe_update_end(crtc);
+
+	intel_mark_page_flip_active(work);
 }
 
 static void intel_mmio_flip_work_func(struct work_struct *work)
@@ -11492,15 +11492,11 @@ static bool __intel_pageflip_stall_check(struct drm_device *dev,
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 	struct intel_unpin_work *work = intel_crtc->unpin_work;
 	u32 addr;
+	u32 pending;
 
-	if (atomic_read(&work->pending) >= INTEL_FLIP_COMPLETE)
-		return true;
-
-	if (atomic_read(&work->pending) < INTEL_FLIP_PENDING)
-		return false;
-
-	if (!work->enable_stall_check)
-		return false;
+	pending = atomic_read(&work->pending);
+	if (pending != INTEL_FLIP_PENDING)
+		return pending == INTEL_FLIP_COMPLETE;
 
 	if (work->flip_ready_vblank == 0) {
 		if (work->flip_queued_req &&
@@ -11676,6 +11672,11 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
 	 */
 	if (!mmio_flip) {
 		ret = i915_gem_object_sync(obj, engine, &request);
+		if (!ret && !request) {
+			request = i915_gem_request_alloc(engine, NULL);
+			ret = PTR_ERR_OR_ZERO(request);
+		}
+
 		if (ret)
 			goto cleanup_pending;
 	}
@@ -11687,6 +11688,7 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
 	work->gtt_offset = intel_plane_obj_offset(to_intel_plane(primary),
 						  obj, 0);
 	work->gtt_offset += intel_crtc->dspaddr_offset;
+	work->flip_queued_vblank = drm_crtc_vblank_count(crtc);
 
 	if (mmio_flip) {
 		ret = intel_queue_mmio_flip(dev, crtc, obj);
@@ -11696,14 +11698,6 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
 		i915_gem_request_assign(&work->flip_queued_req,
 					obj->last_write_req);
 	} else {
-		if (!request) {
-			request = i915_gem_request_alloc(engine, NULL);
-			if (IS_ERR(request)) {
-				ret = PTR_ERR(request);
-				goto cleanup_unpin;
-			}
-		}
-
 		ret = dev_priv->display.queue_flip(dev, crtc, fb, obj, request,
 						   page_flip_flags);
 		if (ret)
@@ -11716,7 +11710,6 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
 		i915_add_request_no_flush(request);
 
 	work->flip_queued_vblank = drm_crtc_vblank_count(crtc);
-	work->enable_stall_check = true;
 
 	i915_gem_track_fb(intel_fb_obj(work->old_fb), obj,
 			  to_intel_plane(primary)->frontbuffer_bit);
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index e0fcfa1683cc..ce2a6d985bbe 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -952,7 +952,6 @@ struct intel_unpin_work {
 	struct drm_i915_gem_request *flip_queued_req;
 	u32 flip_queued_vblank;
 	u32 flip_ready_vblank;
-	bool enable_stall_check;
 };
 
 struct intel_load_detect_pipe {
-- 
2.1.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v2 03/11] drm/i915: Remove intel_prepare_page_flip.

[Maarten Lankhorst]

Do it in 1 step instead, use atomic_read since INTEL_FLIP_COMPLETE
is no longer useful.

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c  |  3 --
 drivers/gpu/drm/i915/i915_irq.c      | 18 ++-----
 drivers/gpu/drm/i915/intel_display.c | 96 ++++++++++++++----------------------
 drivers/gpu/drm/i915/intel_drv.h     |  2 -
 4 files changed, 40 insertions(+), 79 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index df8073a2ffbe..c3b029e7bffd 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -589,9 +589,6 @@ static int i915_gem_pageflip_info(struct seq_file *m, void *data)
 			if (pending == INTEL_FLIP_INACTIVE) {
 				seq_printf(m, "Flip ioctl preparing on pipe %c (plane %c)\n",
 					   pipe, plane);
-			} else if (pending >= INTEL_FLIP_COMPLETE) {
-				seq_printf(m, "Flip queued on pipe %c (plane %c)\n",
-					   pipe, plane);
 			} else {
 				seq_printf(m, "Flip pending (waiting for vsync) on pipe %c (plane %c)\n",
 					   pipe, plane);
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 679f08c944ef..5ed2f73a7ea9 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -1707,10 +1707,8 @@ static void valleyview_pipestat_irq_handler(struct drm_device *dev, u32 iir)
 		    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);
+		if (pipe_stats[pipe] & PLANE_FLIP_DONE_INT_STATUS_VLV)
 			intel_finish_page_flip(dev, pipe);
-		}
 
 		if (pipe_stats[pipe] & PIPE_CRC_DONE_INTERRUPT_STATUS)
 			i9xx_pipe_crc_irq_handler(dev, pipe);
@@ -2109,10 +2107,8 @@ static void ilk_display_irq_handler(struct drm_device *dev, u32 de_iir)
 			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);
+		if (de_iir & DE_PLANE_FLIP_DONE(pipe))
 			intel_finish_page_flip_plane(dev, pipe);
-		}
 	}
 
 	/* check event from PCH */
@@ -2156,10 +2152,8 @@ static void ivb_display_irq_handler(struct drm_device *dev, u32 de_iir)
 			intel_check_page_flip(dev, pipe);
 
 		/* plane/pipes map 1:1 on ilk+ */
-		if (de_iir & DE_PLANE_FLIP_DONE_IVB(pipe)) {
-			intel_prepare_page_flip(dev, pipe);
+		if (de_iir & DE_PLANE_FLIP_DONE_IVB(pipe))
 			intel_finish_page_flip_plane(dev, pipe);
-		}
 	}
 
 	/* check event from PCH */
@@ -2363,10 +2357,8 @@ gen8_de_irq_handler(struct drm_i915_private *dev_priv, u32 master_ctl)
 		else
 			flip_done &= GEN8_PIPE_PRIMARY_FLIP_DONE;
 
-		if (flip_done) {
-			intel_prepare_page_flip(dev, pipe);
+		if (flip_done)
 			intel_finish_page_flip_plane(dev, pipe);
-		}
 
 		if (iir & GEN8_PIPE_CDCLK_CRC_DONE)
 			hsw_pipe_crc_irq_handler(dev, pipe);
@@ -4024,7 +4016,6 @@ static bool i8xx_handle_vblank(struct drm_device *dev,
 	if (I915_READ16(ISR) & flip_pending)
 		goto check_page_flip;
 
-	intel_prepare_page_flip(dev, plane);
 	intel_finish_page_flip(dev, pipe);
 	return true;
 
@@ -4215,7 +4206,6 @@ static bool i915_handle_vblank(struct drm_device *dev,
 	if (I915_READ(ISR) & flip_pending)
 		goto check_page_flip;
 
-	intel_prepare_page_flip(dev, plane);
 	intel_finish_page_flip(dev, pipe);
 	return true;
 
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 618e034a7a5e..dc42335409b3 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3101,7 +3101,6 @@ static void intel_complete_page_flips(struct drm_device *dev)
 		struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 		enum plane plane = intel_crtc->plane;
 
-		intel_prepare_page_flip(dev, plane);
 		intel_finish_page_flip_plane(dev, plane);
 	}
 }
@@ -10925,53 +10924,6 @@ static void intel_unpin_work_fn(struct work_struct *__work)
 	kfree(work);
 }
 
-static void do_intel_finish_page_flip(struct drm_device *dev,
-				      struct drm_crtc *crtc)
-{
-	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
-	struct intel_unpin_work *work;
-	unsigned long flags;
-
-	/* Ignore early vblank irqs */
-	if (intel_crtc == NULL)
-		return;
-
-	/*
-	 * This is called both by irq handlers and the reset code (to complete
-	 * lost pageflips) so needs the full irqsave spinlocks.
-	 */
-	spin_lock_irqsave(&dev->event_lock, flags);
-	work = intel_crtc->unpin_work;
-
-	/* Ensure we don't miss a work->pending update ... */
-	smp_rmb();
-
-	if (work == NULL || atomic_read(&work->pending) < INTEL_FLIP_COMPLETE) {
-		spin_unlock_irqrestore(&dev->event_lock, flags);
-		return;
-	}
-
-	page_flip_completed(intel_crtc);
-
-	spin_unlock_irqrestore(&dev->event_lock, flags);
-}
-
-void intel_finish_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];
-
-	do_intel_finish_page_flip(dev, crtc);
-}
-
-void intel_finish_page_flip_plane(struct drm_device *dev, int plane)
-{
-	struct drm_i915_private *dev_priv = dev->dev_private;
-	struct drm_crtc *crtc = dev_priv->plane_to_crtc_mapping[plane];
-
-	do_intel_finish_page_flip(dev, crtc);
-}
-
 /* Is 'a' after or equal to 'b'? */
 static bool g4x_flip_count_after_eq(u32 a, u32 b)
 {
@@ -11024,28 +10976,52 @@ static bool page_flip_finished(struct intel_crtc *crtc)
 				    crtc->unpin_work->flip_count);
 }
 
-void intel_prepare_page_flip(struct drm_device *dev, int plane)
+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(dev_priv->plane_to_crtc_mapping[plane]);
+	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+	struct intel_unpin_work *work;
 	unsigned long flags;
 
+	/* Ignore early vblank irqs */
+	if (intel_crtc == NULL)
+		return;
 
 	/*
 	 * This is called both by irq handlers and the reset code (to complete
 	 * lost pageflips) so needs the full irqsave spinlocks.
-	 *
-	 * NB: An MMIO update of the plane base pointer will also
-	 * generate a page-flip completion irq, i.e. every modeset
-	 * is also accompanied by a spurious intel_prepare_page_flip().
 	 */
 	spin_lock_irqsave(&dev->event_lock, flags);
-	if (intel_crtc->unpin_work && page_flip_finished(intel_crtc))
-		atomic_inc_not_zero(&intel_crtc->unpin_work->pending);
+	work = intel_crtc->unpin_work;
+
+	if (work == NULL ||
+	    atomic_read(&work->pending) == INTEL_FLIP_INACTIVE ||
+	   !page_flip_finished(intel_crtc)) {
+		spin_unlock_irqrestore(&dev->event_lock, flags);
+		return;
+	}
+
+	page_flip_completed(intel_crtc);
+
 	spin_unlock_irqrestore(&dev->event_lock, flags);
 }
 
+void intel_finish_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];
+
+	do_intel_finish_page_flip(dev, crtc);
+}
+
+void intel_finish_page_flip_plane(struct drm_device *dev, int plane)
+{
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct drm_crtc *crtc = dev_priv->plane_to_crtc_mapping[plane];
+
+	do_intel_finish_page_flip(dev, crtc);
+}
+
 static inline void intel_mark_page_flip_active(struct intel_unpin_work *work)
 {
 	/* Ensure that the work item is consistent when activating it ... */
@@ -11495,8 +11471,8 @@ static bool __intel_pageflip_stall_check(struct drm_device *dev,
 	u32 pending;
 
 	pending = atomic_read(&work->pending);
-	if (pending != INTEL_FLIP_PENDING)
-		return pending == INTEL_FLIP_COMPLETE;
+	if (pending == INTEL_FLIP_INACTIVE)
+		return false;
 
 	if (work->flip_ready_vblank == 0) {
 		if (work->flip_queued_req &&
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index ce2a6d985bbe..a74edecfd179 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -946,7 +946,6 @@ struct intel_unpin_work {
 	atomic_t pending;
 #define INTEL_FLIP_INACTIVE	0
 #define INTEL_FLIP_PENDING	1
-#define INTEL_FLIP_COMPLETE	2
 	u32 flip_count;
 	u32 gtt_offset;
 	struct drm_i915_gem_request *flip_queued_req;
@@ -1159,7 +1158,6 @@ struct drm_framebuffer *
 __intel_framebuffer_create(struct drm_device *dev,
 			   struct drm_mode_fb_cmd2 *mode_cmd,
 			   struct drm_i915_gem_object *obj);
-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);
-- 
2.1.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v2 04/11] drm/i915: Add support for detecting vblanks when hw frame counter is unavailable.

[Maarten Lankhorst]

This uses the newly created drm_accurate_vblank_count_and_time to accurately
get a vblank count when the hw counter is unavailable.
---
 drivers/gpu/drm/i915/intel_display.c | 10 ++++++++++
 drivers/gpu/drm/i915/intel_drv.h     |  3 +++
 drivers/gpu/drm/i915/intel_sprite.c  |  8 ++------
 3 files changed, 15 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index dc42335409b3..f1a895153e64 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -13498,6 +13498,16 @@ static int intel_atomic_prepare_commit(struct drm_device *dev,
 	return ret;
 }
 
+u32 intel_crtc_get_vblank_counter(struct intel_crtc *crtc)
+{
+	struct drm_device *dev = crtc->base.dev;
+
+	if (!dev->max_vblank_count)
+		return drm_accurate_vblank_count(&crtc->base);
+
+	return dev->driver->get_vblank_counter(dev, crtc->pipe);
+}
+
 static void intel_atomic_wait_for_vblanks(struct drm_device *dev,
 					  struct drm_i915_private *dev_priv,
 					  unsigned crtc_mask)
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index a74edecfd179..a2712f6f1eb3 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1141,6 +1141,9 @@ intel_wait_for_vblank_if_active(struct drm_device *dev, int pipe)
 	if (crtc->active)
 		intel_wait_for_vblank(dev, pipe);
 }
+
+u32 intel_crtc_get_vblank_counter(struct intel_crtc *crtc);
+
 int ironlake_get_lanes_required(int target_clock, int link_bw, int bpp);
 void vlv_wait_port_ready(struct drm_i915_private *dev_priv,
 			 struct intel_digital_port *dport,
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index 0f3e2303e0e9..e2de6b0df5a8 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -80,9 +80,7 @@ static int usecs_to_scanlines(const struct drm_display_mode *adjusted_mode,
  */
 void intel_pipe_update_start(struct intel_crtc *crtc)
 {
-	struct drm_device *dev = crtc->base.dev;
 	const struct drm_display_mode *adjusted_mode = &crtc->config->base.adjusted_mode;
-	enum pipe pipe = crtc->pipe;
 	long timeout = msecs_to_jiffies_timeout(1);
 	int scanline, min, max, vblank_start;
 	wait_queue_head_t *wq = drm_crtc_vblank_waitqueue(&crtc->base);
@@ -139,8 +137,7 @@ void intel_pipe_update_start(struct intel_crtc *crtc)
 
 	crtc->debug.scanline_start = scanline;
 	crtc->debug.start_vbl_time = ktime_get();
-	crtc->debug.start_vbl_count =
-		dev->driver->get_vblank_counter(dev, pipe);
+	crtc->debug.start_vbl_count = intel_crtc_get_vblank_counter(crtc);
 
 	trace_i915_pipe_update_vblank_evaded(crtc);
 }
@@ -156,10 +153,9 @@ void intel_pipe_update_start(struct intel_crtc *crtc)
  */
 void intel_pipe_update_end(struct intel_crtc *crtc)
 {
-	struct drm_device *dev = crtc->base.dev;
 	enum pipe pipe = crtc->pipe;
 	int scanline_end = intel_get_crtc_scanline(crtc);
-	u32 end_vbl_count = dev->driver->get_vblank_counter(dev, pipe);
+	u32 end_vbl_count = intel_crtc_get_vblank_counter(crtc);
 	ktime_t end_vbl_time = ktime_get();
 
 	trace_i915_pipe_update_end(crtc, end_vbl_count, scanline_end);
-- 
2.1.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v2 05/11] drm/i915: Allow mmio updates on all platforms, v2.

[Maarten Lankhorst]

Rename intel_unpin_work to intel_flip_work and use it for mmio flips
and unpinning. Use flip_queued_req to hold the wait request in the
mmio case and allow the vblank interrupt to complete mmio work to
have mmio flips run correctly on g4 and earlier.

Changes since v1:
- Add smp_mb__after_atomic() to __intel_pageflip_stall_check,
  to match the smp_mb__before_atomic() in pipe_update_end().
- Check for cur != flip_queued_vblank in pageflip_stall_check.

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c  |   4 +-
 drivers/gpu/drm/i915/intel_display.c | 271 +++++++++++------------------------
 drivers/gpu/drm/i915/intel_drv.h     |  18 +--
 drivers/gpu/drm/i915/intel_sprite.c  |   8 +-
 4 files changed, 95 insertions(+), 206 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index c3b029e7bffd..5662cd5a1a9d 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -574,10 +574,10 @@ static int i915_gem_pageflip_info(struct seq_file *m, void *data)
 	for_each_intel_crtc(dev, crtc) {
 		const char pipe = pipe_name(crtc->pipe);
 		const char plane = plane_name(crtc->plane);
-		struct intel_unpin_work *work;
+		struct intel_flip_work *work;
 
 		spin_lock_irq(&dev->event_lock);
-		work = crtc->unpin_work;
+		work = crtc->flip_work;
 		if (work == NULL) {
 			seq_printf(m, "No flip due on pipe %c (plane %c)\n",
 				   pipe, plane);
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index f1a895153e64..b614f118b973 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -48,6 +48,11 @@
 #include <linux/reservation.h>
 #include <linux/dma-buf.h>
 
+static bool is_mmio_work(struct intel_flip_work *work)
+{
+	return work->mmio_work.func;
+}
+
 /* Primary plane formats for gen <= 3 */
 static const uint32_t i8xx_primary_formats[] = {
 	DRM_FORMAT_C8,
@@ -3285,7 +3290,7 @@ static bool intel_crtc_has_pending_flip(struct drm_crtc *crtc)
 		return false;
 
 	spin_lock_irq(&dev->event_lock);
-	pending = to_intel_crtc(crtc)->unpin_work != NULL;
+	pending = to_intel_crtc(crtc)->flip_work != NULL;
 	spin_unlock_irq(&dev->event_lock);
 
 	return pending;
@@ -3864,7 +3869,7 @@ bool intel_has_pending_fb_unpin(struct drm_device *dev)
 		if (atomic_read(&crtc->unpin_work_count) == 0)
 			continue;
 
-		if (crtc->unpin_work)
+		if (crtc->flip_work)
 			intel_wait_for_vblank(dev, crtc->pipe);
 
 		return true;
@@ -3876,11 +3881,11 @@ bool intel_has_pending_fb_unpin(struct drm_device *dev)
 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;
+	struct intel_flip_work *work = intel_crtc->flip_work;
 
-	/* ensure that the unpin work is consistent wrt ->pending. */
+	/* ensure that the flip work is consistent wrt ->pending. */
 	smp_rmb();
-	intel_crtc->unpin_work = NULL;
+	intel_crtc->flip_work = NULL;
 
 	if (work->event)
 		drm_send_vblank_event(intel_crtc->base.dev,
@@ -3890,7 +3895,7 @@ static void page_flip_completed(struct intel_crtc *intel_crtc)
 	drm_crtc_vblank_put(&intel_crtc->base);
 
 	wake_up_all(&dev_priv->pending_flip_queue);
-	queue_work(dev_priv->wq, &work->work);
+	queue_work(dev_priv->wq, &work->unpin_work);
 
 	trace_i915_flip_complete(intel_crtc->plane,
 				 work->pending_flip_obj);
@@ -3914,9 +3919,11 @@ static int intel_crtc_wait_for_pending_flips(struct drm_crtc *crtc)
 
 	if (ret == 0) {
 		struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+		struct intel_flip_work *work;
 
 		spin_lock_irq(&dev->event_lock);
-		if (intel_crtc->unpin_work) {
+		work = intel_crtc->flip_work;
+		if (work && !is_mmio_work(work)) {
 			WARN_ONCE(1, "Removing stuck page flip\n");
 			page_flip_completed(intel_crtc);
 		}
@@ -6308,7 +6315,7 @@ static void intel_crtc_disable_noatomic(struct drm_crtc *crtc)
 		return;
 
 	if (to_intel_plane_state(crtc->primary->state)->visible) {
-		WARN_ON(intel_crtc->unpin_work);
+		WARN_ON(intel_crtc->flip_work);
 
 		intel_pre_disable_primary_noatomic(crtc);
 
@@ -10881,15 +10888,16 @@ static void intel_crtc_destroy(struct drm_crtc *crtc)
 {
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 	struct drm_device *dev = crtc->dev;
-	struct intel_unpin_work *work;
+	struct intel_flip_work *work;
 
 	spin_lock_irq(&dev->event_lock);
-	work = intel_crtc->unpin_work;
-	intel_crtc->unpin_work = NULL;
+	work = intel_crtc->flip_work;
+	intel_crtc->flip_work = NULL;
 	spin_unlock_irq(&dev->event_lock);
 
 	if (work) {
-		cancel_work_sync(&work->work);
+		cancel_work_sync(&work->mmio_work);
+		cancel_work_sync(&work->unpin_work);
 		kfree(work);
 	}
 
@@ -10900,12 +10908,15 @@ static void intel_crtc_destroy(struct drm_crtc *crtc)
 
 static void intel_unpin_work_fn(struct work_struct *__work)
 {
-	struct intel_unpin_work *work =
-		container_of(__work, struct intel_unpin_work, work);
+	struct intel_flip_work *work =
+		container_of(__work, struct intel_flip_work, unpin_work);
 	struct intel_crtc *crtc = to_intel_crtc(work->crtc);
 	struct drm_device *dev = crtc->base.dev;
 	struct drm_plane *primary = crtc->base.primary;
 
+	if (is_mmio_work(work))
+		flush_work(&work->mmio_work);
+
 	mutex_lock(&dev->struct_mutex);
 	intel_unpin_fb_obj(work->old_fb, primary->state->rotation);
 	drm_gem_object_unreference(&work->pending_flip_obj->base);
@@ -10971,16 +10982,16 @@ static bool page_flip_finished(struct intel_crtc *crtc)
 	 * anyway, we don't really care.
 	 */
 	return (I915_READ(DSPSURFLIVE(crtc->plane)) & ~0xfff) ==
-		crtc->unpin_work->gtt_offset &&
+		crtc->flip_work->gtt_offset &&
 		g4x_flip_count_after_eq(I915_READ(PIPE_FLIPCOUNT_G4X(crtc->pipe)),
-				    crtc->unpin_work->flip_count);
+				    crtc->flip_work->flip_count);
 }
 
 static void do_intel_finish_page_flip(struct drm_device *dev,
 				      struct drm_crtc *crtc)
 {
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
-	struct intel_unpin_work *work;
+	struct intel_flip_work *work;
 	unsigned long flags;
 
 	/* Ignore early vblank irqs */
@@ -10992,11 +11003,11 @@ static void do_intel_finish_page_flip(struct drm_device *dev,
 	 * lost pageflips) so needs the full irqsave spinlocks.
 	 */
 	spin_lock_irqsave(&dev->event_lock, flags);
-	work = intel_crtc->unpin_work;
+	work = intel_crtc->flip_work;
 
 	if (work == NULL ||
 	    atomic_read(&work->pending) == INTEL_FLIP_INACTIVE ||
-	   !page_flip_finished(intel_crtc)) {
+	    !page_flip_finished(intel_crtc)) {
 		spin_unlock_irqrestore(&dev->event_lock, flags);
 		return;
 	}
@@ -11022,7 +11033,7 @@ void intel_finish_page_flip_plane(struct drm_device *dev, int plane)
 	do_intel_finish_page_flip(dev, crtc);
 }
 
-static inline void intel_mark_page_flip_active(struct intel_unpin_work *work)
+static inline void intel_mark_page_flip_active(struct intel_flip_work *work)
 {
 	/* Ensure that the work item is consistent when activating it ... */
 	smp_wmb();
@@ -11059,10 +11070,9 @@ static int intel_gen2_queue_flip(struct drm_device *dev,
 	intel_ring_emit(engine, MI_DISPLAY_FLIP |
 			MI_DISPLAY_FLIP_PLANE(intel_crtc->plane));
 	intel_ring_emit(engine, fb->pitches[0]);
-	intel_ring_emit(engine, intel_crtc->unpin_work->gtt_offset);
+	intel_ring_emit(engine, intel_crtc->flip_work->gtt_offset);
 	intel_ring_emit(engine, 0); /* aux display base address, unused */
 
-	intel_mark_page_flip_active(intel_crtc->unpin_work);
 	return 0;
 }
 
@@ -11091,10 +11101,9 @@ static int intel_gen3_queue_flip(struct drm_device *dev,
 	intel_ring_emit(engine, MI_DISPLAY_FLIP_I915 |
 			MI_DISPLAY_FLIP_PLANE(intel_crtc->plane));
 	intel_ring_emit(engine, fb->pitches[0]);
-	intel_ring_emit(engine, intel_crtc->unpin_work->gtt_offset);
+	intel_ring_emit(engine, intel_crtc->flip_work->gtt_offset);
 	intel_ring_emit(engine, MI_NOOP);
 
-	intel_mark_page_flip_active(intel_crtc->unpin_work);
 	return 0;
 }
 
@@ -11122,7 +11131,7 @@ static int intel_gen4_queue_flip(struct drm_device *dev,
 	intel_ring_emit(engine, MI_DISPLAY_FLIP |
 			MI_DISPLAY_FLIP_PLANE(intel_crtc->plane));
 	intel_ring_emit(engine, fb->pitches[0]);
-	intel_ring_emit(engine, intel_crtc->unpin_work->gtt_offset |
+	intel_ring_emit(engine, intel_crtc->flip_work->gtt_offset |
 			obj->tiling_mode);
 
 	/* XXX Enabling the panel-fitter across page-flip is so far
@@ -11133,7 +11142,6 @@ static int intel_gen4_queue_flip(struct drm_device *dev,
 	pipesrc = I915_READ(PIPESRC(intel_crtc->pipe)) & 0x0fff0fff;
 	intel_ring_emit(engine, pf | pipesrc);
 
-	intel_mark_page_flip_active(intel_crtc->unpin_work);
 	return 0;
 }
 
@@ -11157,7 +11165,7 @@ static int intel_gen6_queue_flip(struct drm_device *dev,
 	intel_ring_emit(engine, MI_DISPLAY_FLIP |
 			MI_DISPLAY_FLIP_PLANE(intel_crtc->plane));
 	intel_ring_emit(engine, fb->pitches[0] | obj->tiling_mode);
-	intel_ring_emit(engine, intel_crtc->unpin_work->gtt_offset);
+	intel_ring_emit(engine, intel_crtc->flip_work->gtt_offset);
 
 	/* Contrary to the suggestions in the documentation,
 	 * "Enable Panel Fitter" does not seem to be required when page
@@ -11169,7 +11177,6 @@ static int intel_gen6_queue_flip(struct drm_device *dev,
 	pipesrc = I915_READ(PIPESRC(intel_crtc->pipe)) & 0x0fff0fff;
 	intel_ring_emit(engine, pf | pipesrc);
 
-	intel_mark_page_flip_active(intel_crtc->unpin_work);
 	return 0;
 }
 
@@ -11261,10 +11268,9 @@ static int intel_gen7_queue_flip(struct drm_device *dev,
 
 	intel_ring_emit(engine, MI_DISPLAY_FLIP_I915 | plane_bit);
 	intel_ring_emit(engine, (fb->pitches[0] | obj->tiling_mode));
-	intel_ring_emit(engine, intel_crtc->unpin_work->gtt_offset);
+	intel_ring_emit(engine, intel_crtc->flip_work->gtt_offset);
 	intel_ring_emit(engine, (MI_NOOP));
 
-	intel_mark_page_flip_active(intel_crtc->unpin_work);
 	return 0;
 }
 
@@ -11282,9 +11288,6 @@ static bool use_mmio_flip(struct intel_engine_cs *engine,
 	if (engine == NULL)
 		return true;
 
-	if (INTEL_INFO(engine->dev)->gen < 5)
-		return false;
-
 	if (i915.use_mmio_flip < 0)
 		return false;
 	else if (i915.use_mmio_flip > 0)
@@ -11299,126 +11302,21 @@ static bool use_mmio_flip(struct intel_engine_cs *engine,
 		return engine != i915_gem_request_get_engine(obj->last_write_req);
 }
 
-static void skl_do_mmio_flip(struct intel_crtc *intel_crtc,
-			     unsigned int rotation,
-			     struct intel_unpin_work *work)
+static void intel_mmio_flip_work_func(struct work_struct *w)
 {
-	struct drm_device *dev = intel_crtc->base.dev;
-	struct drm_i915_private *dev_priv = dev->dev_private;
-	struct drm_framebuffer *fb = intel_crtc->base.primary->fb;
-	const enum pipe pipe = intel_crtc->pipe;
-	u32 ctl, stride, tile_height;
-
-	ctl = I915_READ(PLANE_CTL(pipe, 0));
-	ctl &= ~PLANE_CTL_TILED_MASK;
-	switch (fb->modifier[0]) {
-	case DRM_FORMAT_MOD_NONE:
-		break;
-	case I915_FORMAT_MOD_X_TILED:
-		ctl |= PLANE_CTL_TILED_X;
-		break;
-	case I915_FORMAT_MOD_Y_TILED:
-		ctl |= PLANE_CTL_TILED_Y;
-		break;
-	case I915_FORMAT_MOD_Yf_TILED:
-		ctl |= PLANE_CTL_TILED_YF;
-		break;
-	default:
-		MISSING_CASE(fb->modifier[0]);
-	}
-
-	/*
-	 * The stride is either expressed as a multiple of 64 bytes chunks for
-	 * linear buffers or in number of tiles for tiled buffers.
-	 */
-	if (intel_rotation_90_or_270(rotation)) {
-		/* stride = Surface height in tiles */
-		tile_height = intel_tile_height(dev_priv, fb->modifier[0], 0);
-		stride = DIV_ROUND_UP(fb->height, tile_height);
-	} else {
-		stride = fb->pitches[0] /
-			intel_fb_stride_alignment(dev_priv, fb->modifier[0],
-						  fb->pixel_format);
-	}
-
-	/*
-	 * Both PLANE_CTL and PLANE_STRIDE are not updated on vblank but on
-	 * PLANE_SURF updates, the update is then guaranteed to be atomic.
-	 */
-	I915_WRITE(PLANE_CTL(pipe, 0), ctl);
-	I915_WRITE(PLANE_STRIDE(pipe, 0), stride);
-
-	I915_WRITE(PLANE_SURF(pipe, 0), work->gtt_offset);
-	POSTING_READ(PLANE_SURF(pipe, 0));
-}
-
-static void ilk_do_mmio_flip(struct intel_crtc *intel_crtc,
-			     struct intel_unpin_work *work)
-{
-	struct drm_device *dev = intel_crtc->base.dev;
+	struct intel_flip_work *work =
+		container_of(w, struct intel_flip_work, mmio_work);
+	struct intel_crtc *crtc = to_intel_crtc(work->crtc);
+	struct drm_device *dev = crtc->base.dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
-	struct intel_framebuffer *intel_fb =
-		to_intel_framebuffer(intel_crtc->base.primary->fb);
-	struct drm_i915_gem_object *obj = intel_fb->obj;
-	i915_reg_t reg = DSPCNTR(intel_crtc->plane);
-	u32 dspcntr;
-
-	dspcntr = I915_READ(reg);
-
-	if (obj->tiling_mode != I915_TILING_NONE)
-		dspcntr |= DISPPLANE_TILED;
-	else
-		dspcntr &= ~DISPPLANE_TILED;
+	struct intel_plane *primary = to_intel_plane(crtc->base.primary);
+	struct drm_i915_gem_object *obj = intel_fb_obj(primary->base.state->fb);
 
-	I915_WRITE(reg, dspcntr);
-
-	I915_WRITE(DSPSURF(intel_crtc->plane), work->gtt_offset);
-	POSTING_READ(DSPSURF(intel_crtc->plane));
-}
-
-/*
- * XXX: This is the temporary way to update the plane registers until we get
- * around to using the usual plane update functions for MMIO flips
- */
-static void intel_do_mmio_flip(struct intel_mmio_flip *mmio_flip)
-{
-	struct intel_crtc *crtc = mmio_flip->crtc;
-	struct intel_unpin_work *work;
-
-	spin_lock_irq(&crtc->base.dev->event_lock);
-	work = crtc->unpin_work;
-	spin_unlock_irq(&crtc->base.dev->event_lock);
-	if (work == NULL)
-		return;
-
-	intel_pipe_update_start(crtc);
-
-	if (INTEL_INFO(mmio_flip->i915)->gen >= 9)
-		skl_do_mmio_flip(crtc, mmio_flip->rotation, work);
-	else
-		/* use_mmio_flip() retricts MMIO flips to ilk+ */
-		ilk_do_mmio_flip(crtc, work);
-
-	intel_pipe_update_end(crtc);
-
-	intel_mark_page_flip_active(work);
-}
-
-static void intel_mmio_flip_work_func(struct work_struct *work)
-{
-	struct intel_mmio_flip *mmio_flip =
-		container_of(work, struct intel_mmio_flip, work);
-	struct intel_framebuffer *intel_fb =
-		to_intel_framebuffer(mmio_flip->crtc->base.primary->fb);
-	struct drm_i915_gem_object *obj = intel_fb->obj;
-
-	if (mmio_flip->req) {
-		WARN_ON(__i915_wait_request(mmio_flip->req,
-					    mmio_flip->crtc->reset_counter,
+	if (work->flip_queued_req)
+		WARN_ON(__i915_wait_request(work->flip_queued_req,
+					    crtc->reset_counter,
 					    false, NULL,
-					    &mmio_flip->i915->rps.mmioflips));
-		i915_gem_request_unreference__unlocked(mmio_flip->req);
-	}
+					    &dev_priv->rps.mmioflips));
 
 	/* For framebuffer backed by dmabuf, wait for fence */
 	if (obj->base.dma_buf)
@@ -11426,29 +11324,11 @@ static void intel_mmio_flip_work_func(struct work_struct *work)
 							    false, false,
 							    MAX_SCHEDULE_TIMEOUT) < 0);
 
-	intel_do_mmio_flip(mmio_flip);
-	kfree(mmio_flip);
-}
-
-static int intel_queue_mmio_flip(struct drm_device *dev,
-				 struct drm_crtc *crtc,
-				 struct drm_i915_gem_object *obj)
-{
-	struct intel_mmio_flip *mmio_flip;
-
-	mmio_flip = kmalloc(sizeof(*mmio_flip), GFP_KERNEL);
-	if (mmio_flip == NULL)
-		return -ENOMEM;
-
-	mmio_flip->i915 = to_i915(dev);
-	mmio_flip->req = i915_gem_request_reference(obj->last_write_req);
-	mmio_flip->crtc = to_intel_crtc(crtc);
-	mmio_flip->rotation = crtc->primary->state->rotation;
-
-	INIT_WORK(&mmio_flip->work, intel_mmio_flip_work_func);
-	schedule_work(&mmio_flip->work);
-
-	return 0;
+	intel_pipe_update_start(crtc);
+	primary->update_plane(&primary->base,
+			      crtc->config,
+			      to_intel_plane_state(primary->base.state));
+	intel_pipe_update_end(crtc, work);
 }
 
 static int intel_default_queue_flip(struct drm_device *dev,
@@ -11466,7 +11346,7 @@ static bool __intel_pageflip_stall_check(struct drm_device *dev,
 {
 	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;
+	struct intel_flip_work *work = intel_crtc->flip_work;
 	u32 addr;
 	u32 pending;
 
@@ -11474,6 +11354,15 @@ static bool __intel_pageflip_stall_check(struct drm_device *dev,
 	if (pending == INTEL_FLIP_INACTIVE)
 		return false;
 
+	smp_mb__after_atomic();
+
+	if (is_mmio_work(work)) {
+		u32 cur = intel_crtc_get_vblank_counter(intel_crtc);
+
+		/* MMIO work completes when vblank is different from flip_queued_vblank. */
+		return cur != work->flip_queued_vblank;
+	}
+
 	if (work->flip_ready_vblank == 0) {
 		if (work->flip_queued_req &&
 		    !i915_gem_request_completed(work->flip_queued_req, true))
@@ -11504,7 +11393,7 @@ 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);
-	struct intel_unpin_work *work;
+	struct intel_flip_work *work;
 
 	WARN_ON(!in_interrupt());
 
@@ -11512,14 +11401,15 @@ void intel_check_page_flip(struct drm_device *dev, int pipe)
 		return;
 
 	spin_lock(&dev->event_lock);
-	work = intel_crtc->unpin_work;
+	work = intel_crtc->flip_work;
 	if (work != NULL && __intel_pageflip_stall_check(dev, crtc)) {
-		WARN_ONCE(1, "Kicking stuck page flip: queued at %d, now %d\n",
+		WARN_ONCE(!is_mmio_work(work),
+			  "Kicking stuck page flip: queued at %d, now %d\n",
 			 work->flip_queued_vblank, drm_vblank_count(dev, pipe));
 		page_flip_completed(intel_crtc);
 		work = NULL;
 	}
-	if (work != NULL &&
+	if (work != NULL && !is_mmio_work(work) &&
 	    drm_vblank_count(dev, pipe) - work->flip_queued_vblank > 1)
 		intel_queue_rps_boost_for_request(dev, work->flip_queued_req);
 	spin_unlock(&dev->event_lock);
@@ -11537,7 +11427,7 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 	struct drm_plane *primary = crtc->primary;
 	enum pipe pipe = intel_crtc->pipe;
-	struct intel_unpin_work *work;
+	struct intel_flip_work *work;
 	struct intel_engine_cs *engine;
 	bool mmio_flip;
 	struct drm_i915_gem_request *request = NULL;
@@ -11574,15 +11464,15 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
 	work->event = event;
 	work->crtc = crtc;
 	work->old_fb = old_fb;
-	INIT_WORK(&work->work, intel_unpin_work_fn);
+	INIT_WORK(&work->unpin_work, intel_unpin_work_fn);
 
 	ret = drm_crtc_vblank_get(crtc);
 	if (ret)
 		goto free_work;
 
-	/* We borrow the event spin lock for protecting unpin_work */
+	/* We borrow the event spin lock for protecting flip_work */
 	spin_lock_irq(&dev->event_lock);
-	if (intel_crtc->unpin_work) {
+	if (intel_crtc->flip_work) {
 		/* Before declaring the flip queue wedged, check if
 		 * the hardware completed the operation behind our backs.
 		 */
@@ -11598,7 +11488,7 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
 			return -EBUSY;
 		}
 	}
-	intel_crtc->unpin_work = work;
+	intel_crtc->flip_work = work;
 	spin_unlock_irq(&dev->event_lock);
 
 	if (atomic_read(&intel_crtc->unpin_work_count) >= 2)
@@ -11667,23 +11557,22 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
 	work->flip_queued_vblank = drm_crtc_vblank_count(crtc);
 
 	if (mmio_flip) {
-		ret = intel_queue_mmio_flip(dev, crtc, obj);
-		if (ret)
-			goto cleanup_unpin;
+		INIT_WORK(&work->mmio_work, intel_mmio_flip_work_func);
 
 		i915_gem_request_assign(&work->flip_queued_req,
 					obj->last_write_req);
+
+		schedule_work(&work->mmio_work);
 	} else {
+		i915_gem_request_assign(&work->flip_queued_req, request);
 		ret = dev_priv->display.queue_flip(dev, crtc, fb, obj, request,
 						   page_flip_flags);
 		if (ret)
 			goto cleanup_unpin;
 
-		i915_gem_request_assign(&work->flip_queued_req, request);
-	}
-
-	if (request)
+		intel_mark_page_flip_active(work);
 		i915_add_request_no_flush(request);
+	}
 
 	work->flip_queued_vblank = drm_crtc_vblank_count(crtc);
 
@@ -11713,7 +11602,7 @@ cleanup:
 	drm_framebuffer_unreference(work->old_fb);
 
 	spin_lock_irq(&dev->event_lock);
-	intel_crtc->unpin_work = NULL;
+	intel_crtc->flip_work = NULL;
 	spin_unlock_irq(&dev->event_lock);
 
 	drm_crtc_vblank_put(crtc);
@@ -14018,7 +13907,7 @@ static void intel_finish_crtc_commit(struct drm_crtc *crtc,
 {
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 
-	intel_pipe_update_end(intel_crtc);
+	intel_pipe_update_end(intel_crtc, NULL);
 }
 
 /**
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index a2712f6f1eb3..d46aff0350a0 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -592,14 +592,6 @@ struct vlv_wm_state {
 	bool cxsr;
 };
 
-struct intel_mmio_flip {
-	struct work_struct work;
-	struct drm_i915_private *i915;
-	struct drm_i915_gem_request *req;
-	struct intel_crtc *crtc;
-	unsigned int rotation;
-};
-
 struct intel_crtc {
 	struct drm_crtc base;
 	enum pipe pipe;
@@ -614,7 +606,7 @@ struct intel_crtc {
 	unsigned long enabled_power_domains;
 	bool lowfreq_avail;
 	struct intel_overlay *overlay;
-	struct intel_unpin_work *unpin_work;
+	struct intel_flip_work *flip_work;
 
 	atomic_t unpin_work_count;
 
@@ -937,8 +929,10 @@ intel_get_crtc_for_plane(struct drm_device *dev, int plane)
 	return dev_priv->plane_to_crtc_mapping[plane];
 }
 
-struct intel_unpin_work {
-	struct work_struct work;
+struct intel_flip_work {
+	struct work_struct unpin_work;
+	struct work_struct mmio_work;
+
 	struct drm_crtc *crtc;
 	struct drm_framebuffer *old_fb;
 	struct drm_i915_gem_object *pending_flip_obj;
@@ -1618,7 +1612,7 @@ int intel_plane_init(struct drm_device *dev, enum pipe pipe, int plane);
 int intel_sprite_set_colorkey(struct drm_device *dev, void *data,
 			      struct drm_file *file_priv);
 void intel_pipe_update_start(struct intel_crtc *crtc);
-void intel_pipe_update_end(struct intel_crtc *crtc);
+void intel_pipe_update_end(struct intel_crtc *crtc, struct intel_flip_work *work);
 
 /* intel_tv.c */
 void intel_tv_init(struct drm_device *dev);
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index e2de6b0df5a8..8ec7ce549835 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -151,13 +151,19 @@ void intel_pipe_update_start(struct intel_crtc *crtc)
  * re-enables interrupts and verifies the update was actually completed
  * before a vblank using the value of @start_vbl_count.
  */
-void intel_pipe_update_end(struct intel_crtc *crtc)
+void intel_pipe_update_end(struct intel_crtc *crtc, struct intel_flip_work *work)
 {
 	enum pipe pipe = crtc->pipe;
 	int scanline_end = intel_get_crtc_scanline(crtc);
 	u32 end_vbl_count = intel_crtc_get_vblank_counter(crtc);
 	ktime_t end_vbl_time = ktime_get();
 
+	if (work) {
+		work->flip_queued_vblank = end_vbl_count;
+		smp_mb__before_atomic();
+		atomic_set(&work->pending, INTEL_FLIP_PENDING);
+	}
+
 	trace_i915_pipe_update_end(crtc, end_vbl_count, scanline_end);
 
 	local_irq_enable();
-- 
2.1.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v2 06/11] drm/i915: Convert flip_work to a list.

[Maarten Lankhorst]

This will be required to allow more than 1 update in the future.

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c  |  90 ++++++++++++++++-------------
 drivers/gpu/drm/i915/i915_drv.h      |   2 +-
 drivers/gpu/drm/i915/intel_display.c | 107 +++++++++++++++++++----------------
 drivers/gpu/drm/i915/intel_drv.h     |   4 +-
 4 files changed, 114 insertions(+), 89 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 5662cd5a1a9d..bc4e97a141d5 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -559,6 +559,53 @@ static int i915_gem_gtt_info(struct seq_file *m, void *data)
 	return 0;
 }
 
+static void i915_dump_pageflip(struct seq_file *m,
+			       struct drm_i915_private *dev_priv,
+			       struct intel_crtc *crtc,
+			       struct intel_flip_work *work)
+{
+	const char pipe = pipe_name(crtc->pipe);
+	const char plane = plane_name(crtc->plane);
+	u32 pending;
+	u32 addr;
+
+	pending = atomic_read(&work->pending);
+	if (pending == INTEL_FLIP_INACTIVE) {
+		seq_printf(m, "Flip ioctl preparing on pipe %c (plane %c)\n",
+			   pipe, plane);
+	} else {
+		seq_printf(m, "Flip pending (waiting for vsync) on pipe %c (plane %c)\n",
+			   pipe, plane);
+	}
+	if (work->flip_queued_req) {
+		struct intel_engine_cs *engine = i915_gem_request_get_engine(work->flip_queued_req);
+
+		seq_printf(m, "Flip queued on %s at seqno %x, next seqno %x [current breadcrumb %x], completed? %d\n",
+			   engine->name,
+			   i915_gem_request_get_seqno(work->flip_queued_req),
+			   dev_priv->next_seqno,
+			   engine->get_seqno(engine),
+			   i915_gem_request_completed(work->flip_queued_req, true));
+	} else
+		seq_printf(m, "Flip not associated with any ring\n");
+	seq_printf(m, "Flip queued on frame %d, (was ready on frame %d), now %d\n",
+		   work->flip_queued_vblank,
+		   work->flip_ready_vblank,
+		   drm_crtc_vblank_count(&crtc->base));
+	seq_printf(m, "%d prepares\n", atomic_read(&work->pending));
+
+	if (INTEL_INFO(dev_priv)->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) {
+		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);
+	}
+}
+
 static int i915_gem_pageflip_info(struct seq_file *m, void *data)
 {
 	struct drm_info_node *node = m->private;
@@ -577,48 +624,13 @@ static int i915_gem_pageflip_info(struct seq_file *m, void *data)
 		struct intel_flip_work *work;
 
 		spin_lock_irq(&dev->event_lock);
-		work = crtc->flip_work;
-		if (work == NULL) {
+		if (list_empty(&crtc->flip_work)) {
 			seq_printf(m, "No flip due on pipe %c (plane %c)\n",
 				   pipe, plane);
 		} else {
-			u32 pending;
-			u32 addr;
-
-			pending = atomic_read(&work->pending);
-			if (pending == INTEL_FLIP_INACTIVE) {
-				seq_printf(m, "Flip ioctl preparing on pipe %c (plane %c)\n",
-					   pipe, plane);
-			} else {
-				seq_printf(m, "Flip pending (waiting for vsync) on pipe %c (plane %c)\n",
-					   pipe, plane);
-			}
-			if (work->flip_queued_req) {
-				struct intel_engine_cs *engine = i915_gem_request_get_engine(work->flip_queued_req);
-
-				seq_printf(m, "Flip queued on %s at seqno %x, next seqno %x [current breadcrumb %x], completed? %d\n",
-					   engine->name,
-					   i915_gem_request_get_seqno(work->flip_queued_req),
-					   dev_priv->next_seqno,
-					   engine->get_seqno(engine),
-					   i915_gem_request_completed(work->flip_queued_req, true));
-			} else
-				seq_printf(m, "Flip not associated with any ring\n");
-			seq_printf(m, "Flip queued on frame %d, (was ready on frame %d), now %d\n",
-				   work->flip_queued_vblank,
-				   work->flip_ready_vblank,
-				   drm_crtc_vblank_count(&crtc->base));
-			seq_printf(m, "%d prepares\n", atomic_read(&work->pending));
-
-			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) {
-				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);
+			list_for_each_entry(work, &crtc->flip_work, head); {
+				i915_dump_pageflip(m, dev_priv, crtc, work);
+				seq_puts(m, "\n");
 			}
 		}
 		spin_unlock_irq(&dev->event_lock);
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 542401659013..7927bd667a6d 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -605,7 +605,7 @@ struct drm_i915_display_funcs {
 			  struct drm_framebuffer *fb,
 			  struct drm_i915_gem_object *obj,
 			  struct drm_i915_gem_request *req,
-			  uint32_t flags);
+			  uint64_t gtt_offset);
 	void (*hpd_irq_setup)(struct drm_device *dev);
 	/* clock updates for mode set */
 	/* cursor updates */
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index b614f118b973..3d29bc6910f6 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3283,17 +3283,12 @@ static bool intel_crtc_has_pending_flip(struct drm_crtc *crtc)
 	struct drm_device *dev = crtc->dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
-	bool pending;
 
 	if (i915_reset_in_progress(&dev_priv->gpu_error) ||
 	    intel_crtc->reset_counter != atomic_read(&dev_priv->gpu_error.reset_counter))
 		return false;
 
-	spin_lock_irq(&dev->event_lock);
-	pending = to_intel_crtc(crtc)->flip_work != NULL;
-	spin_unlock_irq(&dev->event_lock);
-
-	return pending;
+	return !list_empty_careful(&to_intel_crtc(crtc)->flip_work);
 }
 
 static void intel_update_pipe_config(struct intel_crtc *crtc,
@@ -3869,7 +3864,7 @@ bool intel_has_pending_fb_unpin(struct drm_device *dev)
 		if (atomic_read(&crtc->unpin_work_count) == 0)
 			continue;
 
-		if (crtc->flip_work)
+		if (!list_empty_careful(&crtc->flip_work))
 			intel_wait_for_vblank(dev, crtc->pipe);
 
 		return true;
@@ -3878,14 +3873,11 @@ bool intel_has_pending_fb_unpin(struct drm_device *dev)
 	return false;
 }
 
-static void page_flip_completed(struct intel_crtc *intel_crtc)
+static void page_flip_completed(struct intel_crtc *intel_crtc, struct intel_flip_work *work)
 {
 	struct drm_i915_private *dev_priv = to_i915(intel_crtc->base.dev);
-	struct intel_flip_work *work = intel_crtc->flip_work;
 
-	/* ensure that the flip work is consistent wrt ->pending. */
-	smp_rmb();
-	intel_crtc->flip_work = NULL;
+	list_del_init(&work->head);
 
 	if (work->event)
 		drm_send_vblank_event(intel_crtc->base.dev,
@@ -3922,10 +3914,11 @@ static int intel_crtc_wait_for_pending_flips(struct drm_crtc *crtc)
 		struct intel_flip_work *work;
 
 		spin_lock_irq(&dev->event_lock);
-		work = intel_crtc->flip_work;
+		work = list_first_entry_or_null(&intel_crtc->flip_work,
+						struct intel_flip_work, head);
 		if (work && !is_mmio_work(work)) {
 			WARN_ONCE(1, "Removing stuck page flip\n");
-			page_flip_completed(intel_crtc);
+			page_flip_completed(intel_crtc, work);
 		}
 		spin_unlock_irq(&dev->event_lock);
 	}
@@ -6315,7 +6308,7 @@ static void intel_crtc_disable_noatomic(struct drm_crtc *crtc)
 		return;
 
 	if (to_intel_plane_state(crtc->primary->state)->visible) {
-		WARN_ON(intel_crtc->flip_work);
+		WARN_ON(list_empty(&intel_crtc->flip_work));
 
 		intel_pre_disable_primary_noatomic(crtc);
 
@@ -10889,17 +10882,24 @@ static void intel_crtc_destroy(struct drm_crtc *crtc)
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 	struct drm_device *dev = crtc->dev;
 	struct intel_flip_work *work;
+	struct list_head head;
+
+	INIT_LIST_HEAD(&head);
 
 	spin_lock_irq(&dev->event_lock);
-	work = intel_crtc->flip_work;
-	intel_crtc->flip_work = NULL;
-	spin_unlock_irq(&dev->event_lock);
+	while (!list_empty(&intel_crtc->flip_work)) {
+		work = list_first_entry(&intel_crtc->flip_work,
+					struct intel_flip_work, head);
+		list_del_init(&work->head);
+		spin_unlock_irq(&dev->event_lock);
 
-	if (work) {
 		cancel_work_sync(&work->mmio_work);
 		cancel_work_sync(&work->unpin_work);
 		kfree(work);
+
+		spin_lock_irq(&dev->event_lock);
 	}
+	spin_unlock_irq(&dev->event_lock);
 
 	drm_crtc_cleanup(crtc);
 
@@ -10941,7 +10941,8 @@ static bool g4x_flip_count_after_eq(u32 a, u32 b)
 	return !((a - b) & 0x80000000);
 }
 
-static bool page_flip_finished(struct intel_crtc *crtc)
+static bool page_flip_finished(struct intel_crtc *crtc,
+			       struct intel_flip_work *work)
 {
 	struct drm_device *dev = crtc->base.dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
@@ -10982,9 +10983,9 @@ static bool page_flip_finished(struct intel_crtc *crtc)
 	 * anyway, we don't really care.
 	 */
 	return (I915_READ(DSPSURFLIVE(crtc->plane)) & ~0xfff) ==
-		crtc->flip_work->gtt_offset &&
+		work->gtt_offset &&
 		g4x_flip_count_after_eq(I915_READ(PIPE_FLIPCOUNT_G4X(crtc->pipe)),
-				    crtc->flip_work->flip_count);
+					work->flip_count);
 }
 
 static void do_intel_finish_page_flip(struct drm_device *dev,
@@ -11003,16 +11004,18 @@ static void do_intel_finish_page_flip(struct drm_device *dev,
 	 * lost pageflips) so needs the full irqsave spinlocks.
 	 */
 	spin_lock_irqsave(&dev->event_lock, flags);
-	work = intel_crtc->flip_work;
+	work = list_first_entry_or_null(&intel_crtc->flip_work,
+					struct intel_flip_work,
+					head);
 
 	if (work == NULL ||
 	    atomic_read(&work->pending) == INTEL_FLIP_INACTIVE ||
-	    !page_flip_finished(intel_crtc)) {
+	    !page_flip_finished(intel_crtc, work)) {
 		spin_unlock_irqrestore(&dev->event_lock, flags);
 		return;
 	}
 
-	page_flip_completed(intel_crtc);
+	page_flip_completed(intel_crtc, work);
 
 	spin_unlock_irqrestore(&dev->event_lock, flags);
 }
@@ -11047,7 +11050,7 @@ static int intel_gen2_queue_flip(struct drm_device *dev,
 				 struct drm_framebuffer *fb,
 				 struct drm_i915_gem_object *obj,
 				 struct drm_i915_gem_request *req,
-				 uint32_t flags)
+				 uint64_t gtt_offset)
 {
 	struct intel_engine_cs *engine = req->engine;
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
@@ -11070,7 +11073,7 @@ static int intel_gen2_queue_flip(struct drm_device *dev,
 	intel_ring_emit(engine, MI_DISPLAY_FLIP |
 			MI_DISPLAY_FLIP_PLANE(intel_crtc->plane));
 	intel_ring_emit(engine, fb->pitches[0]);
-	intel_ring_emit(engine, intel_crtc->flip_work->gtt_offset);
+	intel_ring_emit(engine, gtt_offset);
 	intel_ring_emit(engine, 0); /* aux display base address, unused */
 
 	return 0;
@@ -11081,7 +11084,7 @@ static int intel_gen3_queue_flip(struct drm_device *dev,
 				 struct drm_framebuffer *fb,
 				 struct drm_i915_gem_object *obj,
 				 struct drm_i915_gem_request *req,
-				 uint32_t flags)
+				 uint64_t gtt_offset)
 {
 	struct intel_engine_cs *engine = req->engine;
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
@@ -11101,7 +11104,7 @@ static int intel_gen3_queue_flip(struct drm_device *dev,
 	intel_ring_emit(engine, MI_DISPLAY_FLIP_I915 |
 			MI_DISPLAY_FLIP_PLANE(intel_crtc->plane));
 	intel_ring_emit(engine, fb->pitches[0]);
-	intel_ring_emit(engine, intel_crtc->flip_work->gtt_offset);
+	intel_ring_emit(engine, gtt_offset);
 	intel_ring_emit(engine, MI_NOOP);
 
 	return 0;
@@ -11112,7 +11115,7 @@ static int intel_gen4_queue_flip(struct drm_device *dev,
 				 struct drm_framebuffer *fb,
 				 struct drm_i915_gem_object *obj,
 				 struct drm_i915_gem_request *req,
-				 uint32_t flags)
+				 uint64_t gtt_offset)
 {
 	struct intel_engine_cs *engine = req->engine;
 	struct drm_i915_private *dev_priv = dev->dev_private;
@@ -11131,8 +11134,7 @@ static int intel_gen4_queue_flip(struct drm_device *dev,
 	intel_ring_emit(engine, MI_DISPLAY_FLIP |
 			MI_DISPLAY_FLIP_PLANE(intel_crtc->plane));
 	intel_ring_emit(engine, fb->pitches[0]);
-	intel_ring_emit(engine, intel_crtc->flip_work->gtt_offset |
-			obj->tiling_mode);
+	intel_ring_emit(engine, gtt_offset | obj->tiling_mode);
 
 	/* XXX Enabling the panel-fitter across page-flip is so far
 	 * untested on non-native modes, so ignore it for now.
@@ -11150,7 +11152,7 @@ static int intel_gen6_queue_flip(struct drm_device *dev,
 				 struct drm_framebuffer *fb,
 				 struct drm_i915_gem_object *obj,
 				 struct drm_i915_gem_request *req,
-				 uint32_t flags)
+				 uint64_t gtt_offset)
 {
 	struct intel_engine_cs *engine = req->engine;
 	struct drm_i915_private *dev_priv = dev->dev_private;
@@ -11165,7 +11167,7 @@ static int intel_gen6_queue_flip(struct drm_device *dev,
 	intel_ring_emit(engine, MI_DISPLAY_FLIP |
 			MI_DISPLAY_FLIP_PLANE(intel_crtc->plane));
 	intel_ring_emit(engine, fb->pitches[0] | obj->tiling_mode);
-	intel_ring_emit(engine, intel_crtc->flip_work->gtt_offset);
+	intel_ring_emit(engine, gtt_offset);
 
 	/* Contrary to the suggestions in the documentation,
 	 * "Enable Panel Fitter" does not seem to be required when page
@@ -11185,7 +11187,7 @@ static int intel_gen7_queue_flip(struct drm_device *dev,
 				 struct drm_framebuffer *fb,
 				 struct drm_i915_gem_object *obj,
 				 struct drm_i915_gem_request *req,
-				 uint32_t flags)
+				 uint64_t gtt_offset)
 {
 	struct intel_engine_cs *engine = req->engine;
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
@@ -11268,7 +11270,7 @@ static int intel_gen7_queue_flip(struct drm_device *dev,
 
 	intel_ring_emit(engine, MI_DISPLAY_FLIP_I915 | plane_bit);
 	intel_ring_emit(engine, (fb->pitches[0] | obj->tiling_mode));
-	intel_ring_emit(engine, intel_crtc->flip_work->gtt_offset);
+	intel_ring_emit(engine, gtt_offset);
 	intel_ring_emit(engine, (MI_NOOP));
 
 	return 0;
@@ -11336,17 +11338,17 @@ static int intel_default_queue_flip(struct drm_device *dev,
 				    struct drm_framebuffer *fb,
 				    struct drm_i915_gem_object *obj,
 				    struct drm_i915_gem_request *req,
-				    uint32_t flags)
+				    uint64_t gtt_offset)
 {
 	return -ENODEV;
 }
 
 static bool __intel_pageflip_stall_check(struct drm_device *dev,
-					 struct drm_crtc *crtc)
+					 struct drm_crtc *crtc,
+					 struct intel_flip_work *work)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
-	struct intel_flip_work *work = intel_crtc->flip_work;
 	u32 addr;
 	u32 pending;
 
@@ -11401,12 +11403,14 @@ void intel_check_page_flip(struct drm_device *dev, int pipe)
 		return;
 
 	spin_lock(&dev->event_lock);
-	work = intel_crtc->flip_work;
-	if (work != NULL && __intel_pageflip_stall_check(dev, crtc)) {
+	work = list_first_entry_or_null(&intel_crtc->flip_work,
+					struct intel_flip_work, head);
+
+	if (work != NULL && __intel_pageflip_stall_check(dev, crtc, work)) {
 		WARN_ONCE(!is_mmio_work(work),
 			  "Kicking stuck page flip: queued at %d, now %d\n",
 			 work->flip_queued_vblank, drm_vblank_count(dev, pipe));
-		page_flip_completed(intel_crtc);
+		page_flip_completed(intel_crtc, work);
 		work = NULL;
 	}
 	if (work != NULL && !is_mmio_work(work) &&
@@ -11472,13 +11476,18 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
 
 	/* We borrow the event spin lock for protecting flip_work */
 	spin_lock_irq(&dev->event_lock);
-	if (intel_crtc->flip_work) {
+	if (!list_empty(&intel_crtc->flip_work)) {
+		struct intel_flip_work *old_work;
+
+		old_work = list_first_entry(&intel_crtc->flip_work,
+					    struct intel_flip_work, head);
+
 		/* Before declaring the flip queue wedged, check if
 		 * the hardware completed the operation behind our backs.
 		 */
-		if (__intel_pageflip_stall_check(dev, crtc)) {
+		if (__intel_pageflip_stall_check(dev, crtc, old_work)) {
 			DRM_DEBUG_DRIVER("flip queue: previous flip completed, continuing\n");
-			page_flip_completed(intel_crtc);
+			page_flip_completed(intel_crtc, old_work);
 		} else {
 			DRM_DEBUG_DRIVER("flip queue: crtc already busy\n");
 			spin_unlock_irq(&dev->event_lock);
@@ -11488,7 +11497,7 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
 			return -EBUSY;
 		}
 	}
-	intel_crtc->flip_work = work;
+	list_add_tail(&work->head, &intel_crtc->flip_work);
 	spin_unlock_irq(&dev->event_lock);
 
 	if (atomic_read(&intel_crtc->unpin_work_count) >= 2)
@@ -11566,7 +11575,7 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
 	} else {
 		i915_gem_request_assign(&work->flip_queued_req, request);
 		ret = dev_priv->display.queue_flip(dev, crtc, fb, obj, request,
-						   page_flip_flags);
+						   work->gtt_offset);
 		if (ret)
 			goto cleanup_unpin;
 
@@ -11602,7 +11611,7 @@ cleanup:
 	drm_framebuffer_unreference(work->old_fb);
 
 	spin_lock_irq(&dev->event_lock);
-	intel_crtc->flip_work = NULL;
+	list_del(&work->head);
 	spin_unlock_irq(&dev->event_lock);
 
 	drm_crtc_vblank_put(crtc);
@@ -14220,6 +14229,8 @@ static void intel_crtc_init(struct drm_device *dev, int pipe)
 	intel_crtc->base.state = &crtc_state->base;
 	crtc_state->base.crtc = &intel_crtc->base;
 
+	INIT_LIST_HEAD(&intel_crtc->flip_work);
+
 	/* initialize shared scalers */
 	if (INTEL_INFO(dev)->gen >= 9) {
 		if (pipe == PIPE_C)
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index d46aff0350a0..e584538ad776 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -606,7 +606,7 @@ struct intel_crtc {
 	unsigned long enabled_power_domains;
 	bool lowfreq_avail;
 	struct intel_overlay *overlay;
-	struct intel_flip_work *flip_work;
+	struct list_head flip_work;
 
 	atomic_t unpin_work_count;
 
@@ -930,6 +930,8 @@ intel_get_crtc_for_plane(struct drm_device *dev, int plane)
 }
 
 struct intel_flip_work {
+	struct list_head head;
+
 	struct work_struct unpin_work;
 	struct work_struct mmio_work;
 
-- 
2.1.0

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

[PATCH v2 07/11] drm/i915: Add the exclusive fence to plane_state.

[Maarten Lankhorst]

Set plane_state->base.fence to the dma_buf exclusive fence,
and add a wait to the mmio function. This will make it easier
to unify plane updates later on.

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_atomic_plane.c |  1 +
 drivers/gpu/drm/i915/intel_display.c      | 54 +++++++++++++++++++++++--------
 2 files changed, 42 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_atomic_plane.c b/drivers/gpu/drm/i915/intel_atomic_plane.c
index 7de7721f65bc..2ab45f16fa65 100644
--- a/drivers/gpu/drm/i915/intel_atomic_plane.c
+++ b/drivers/gpu/drm/i915/intel_atomic_plane.c
@@ -102,6 +102,7 @@ intel_plane_destroy_state(struct drm_plane *plane,
 			  struct drm_plane_state *state)
 {
 	WARN_ON(state && to_intel_plane_state(state)->wait_req);
+	WARN_ON(state && state->fence);
 	drm_atomic_helper_plane_destroy_state(plane, state);
 }
 
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 3d29bc6910f6..a9cec8ae0bc4 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -13370,6 +13370,15 @@ static int intel_atomic_prepare_commit(struct drm_device *dev,
 			struct intel_plane_state *intel_plane_state =
 				to_intel_plane_state(plane_state);
 
+			if (plane_state->fence) {
+				long lret = fence_wait(plane_state->fence, true);
+
+				if (lret < 0) {
+					ret = lret;
+					break;
+				}
+			}
+
 			if (!intel_plane_state->wait_req)
 				continue;
 
@@ -13701,6 +13710,33 @@ static const struct drm_crtc_funcs intel_crtc_funcs = {
 	.atomic_destroy_state = intel_crtc_destroy_state,
 };
 
+static struct fence *intel_get_excl_fence(struct drm_i915_gem_object *obj)
+{
+	struct reservation_object *resv;
+
+
+	if (!obj->base.dma_buf)
+		return NULL;
+
+	resv = obj->base.dma_buf->resv;
+
+	/* For framebuffer backed by dmabuf, wait for fence */
+	while (1) {
+		struct fence *fence_excl, *ret = NULL;
+
+		rcu_read_lock();
+
+		fence_excl = rcu_dereference(resv->fence_excl);
+		if (fence_excl)
+			ret = fence_get_rcu(fence_excl);
+
+		rcu_read_unlock();
+
+		if (ret == fence_excl)
+			return ret;
+	}
+}
+
 /**
  * intel_prepare_plane_fb - Prepare fb for usage on plane
  * @plane: drm plane to prepare for
@@ -13752,19 +13788,6 @@ intel_prepare_plane_fb(struct drm_plane *plane,
 			return ret;
 	}
 
-	/* For framebuffer backed by dmabuf, wait for fence */
-	if (obj && obj->base.dma_buf) {
-		long lret;
-
-		lret = reservation_object_wait_timeout_rcu(obj->base.dma_buf->resv,
-							   false, true,
-							   MAX_SCHEDULE_TIMEOUT);
-		if (lret == -ERESTARTSYS)
-			return lret;
-
-		WARN(lret < 0, "waiting returns %li\n", lret);
-	}
-
 	if (!obj) {
 		ret = 0;
 	} else if (plane->type == DRM_PLANE_TYPE_CURSOR &&
@@ -13784,6 +13807,8 @@ intel_prepare_plane_fb(struct drm_plane *plane,
 
 			i915_gem_request_assign(&plane_state->wait_req,
 						obj->last_write_req);
+
+			plane_state->base.fence = intel_get_excl_fence(obj);
 		}
 
 		i915_gem_track_fb(old_obj, obj, intel_plane->frontbuffer_bit);
@@ -13826,6 +13851,9 @@ intel_cleanup_plane_fb(struct drm_plane *plane,
 		i915_gem_track_fb(old_obj, obj, intel_plane->frontbuffer_bit);
 
 	i915_gem_request_assign(&old_intel_state->wait_req, NULL);
+
+	fence_put(old_intel_state->base.fence);
+	old_intel_state->base.fence = NULL;
 }
 
 int
-- 
2.1.0

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

[PATCH v2 08/11] drm/i915: Rework intel_crtc_page_flip to be almost atomic, v3.

[Maarten Lankhorst]

Create a work structure that will be used for all changes. This will
be used later on in the atomic commit function.

Changes since v1:
- Free old_crtc_state from unpin_work_fn properly.
Changes since v2:
- Add hunk for calling hw state verifier.
- Add missing support for color spaces.

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c  |  36 +-
 drivers/gpu/drm/i915/intel_display.c | 673 +++++++++++++++++++++--------------
 drivers/gpu/drm/i915/intel_drv.h     |  13 +-
 3 files changed, 437 insertions(+), 285 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index bc4e97a141d5..04ea4129c9f5 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -565,29 +565,43 @@ static void i915_dump_pageflip(struct seq_file *m,
 			       struct intel_flip_work *work)
 {
 	const char pipe = pipe_name(crtc->pipe);
-	const char plane = plane_name(crtc->plane);
 	u32 pending;
 	u32 addr;
+	int i;
 
 	pending = atomic_read(&work->pending);
 	if (pending == INTEL_FLIP_INACTIVE) {
 		seq_printf(m, "Flip ioctl preparing on pipe %c (plane %c)\n",
-			   pipe, plane);
+			   pipe, plane_name(crtc->plane));
 	} else {
 		seq_printf(m, "Flip pending (waiting for vsync) on pipe %c (plane %c)\n",
-			   pipe, plane);
+			   pipe, plane_name(crtc->plane));
 	}
-	if (work->flip_queued_req) {
-		struct intel_engine_cs *engine = i915_gem_request_get_engine(work->flip_queued_req);
 
-		seq_printf(m, "Flip queued on %s at seqno %x, next seqno %x [current breadcrumb %x], completed? %d\n",
+
+	for (i = 0; i < work->num_planes; i++) {
+		struct intel_plane_state *old_plane_state = work->old_plane_state[i];
+		struct drm_plane *plane = old_plane_state->base.plane;
+		struct drm_i915_gem_request *req = old_plane_state->wait_req;
+		struct intel_engine_cs *engine;
+
+		seq_printf(m, "[PLANE:%i] part of flip.\n", plane->base.id);
+
+		if (!req) {
+			seq_printf(m, "Plane not associated with any engine\n");
+			continue;
+		}
+
+		engine = i915_gem_request_get_engine(req);
+
+		seq_printf(m, "Plane blocked on %s at seqno %x, next seqno %x [current breadcrumb %x], completed? %d\n",
 			   engine->name,
-			   i915_gem_request_get_seqno(work->flip_queued_req),
+			   i915_gem_request_get_seqno(req),
 			   dev_priv->next_seqno,
 			   engine->get_seqno(engine),
-			   i915_gem_request_completed(work->flip_queued_req, true));
-	} else
-		seq_printf(m, "Flip not associated with any ring\n");
+			   i915_gem_request_completed(req, true));
+	}
+
 	seq_printf(m, "Flip queued on frame %d, (was ready on frame %d), now %d\n",
 		   work->flip_queued_vblank,
 		   work->flip_ready_vblank,
@@ -600,7 +614,7 @@ static void i915_dump_pageflip(struct seq_file *m,
 		addr = I915_READ(DSPADDR(crtc->plane));
 	seq_printf(m, "Current scanout address 0x%08x\n", addr);
 
-	if (work->pending_flip_obj) {
+	if (work->flip_queued_req) {
 		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);
 	}
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index a9cec8ae0bc4..2a8392ada270 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -50,7 +50,7 @@
 
 static bool is_mmio_work(struct intel_flip_work *work)
 {
-	return work->mmio_work.func;
+	return !work->flip_queued_req;
 }
 
 /* Primary plane formats for gen <= 3 */
@@ -122,6 +122,9 @@ static void ironlake_pfit_disable(struct intel_crtc *crtc, bool force);
 static void ironlake_pfit_enable(struct intel_crtc *crtc);
 static void intel_modeset_setup_hw_state(struct drm_device *dev);
 static void intel_pre_disable_primary_noatomic(struct drm_crtc *crtc);
+static void intel_modeset_verify_crtc(struct drm_crtc *crtc,
+				      struct drm_crtc_state *old_state,
+				      struct drm_crtc_state *new_state);
 
 typedef struct {
 	int	min, max;
@@ -2512,20 +2515,6 @@ out_unref_obj:
 	return false;
 }
 
-/* Update plane->state->fb to match plane->fb after driver-internal updates */
-static void
-update_state_fb(struct drm_plane *plane)
-{
-	if (plane->fb == plane->state->fb)
-		return;
-
-	if (plane->state->fb)
-		drm_framebuffer_unreference(plane->state->fb);
-	plane->state->fb = plane->fb;
-	if (plane->state->fb)
-		drm_framebuffer_reference(plane->state->fb);
-}
-
 static void
 intel_find_initial_plane_obj(struct intel_crtc *intel_crtc,
 			     struct intel_initial_plane_config *plane_config)
@@ -3876,8 +3865,8 @@ bool intel_has_pending_fb_unpin(struct drm_device *dev)
 static void page_flip_completed(struct intel_crtc *intel_crtc, struct intel_flip_work *work)
 {
 	struct drm_i915_private *dev_priv = to_i915(intel_crtc->base.dev);
-
-	list_del_init(&work->head);
+	struct drm_plane_state *new_plane_state;
+	struct drm_plane *primary = intel_crtc->base.primary;
 
 	if (work->event)
 		drm_send_vblank_event(intel_crtc->base.dev,
@@ -3886,11 +3875,19 @@ static void page_flip_completed(struct intel_crtc *intel_crtc, struct intel_flip
 
 	drm_crtc_vblank_put(&intel_crtc->base);
 
-	wake_up_all(&dev_priv->pending_flip_queue);
-	queue_work(dev_priv->wq, &work->unpin_work);
+	new_plane_state = &work->old_plane_state[0]->base;
+	if (work->num_planes >= 1 &&
+	    new_plane_state->plane == primary &&
+	    new_plane_state->fb)
+		trace_i915_flip_complete(intel_crtc->plane,
+					 intel_fb_obj(new_plane_state->fb));
 
-	trace_i915_flip_complete(intel_crtc->plane,
-				 work->pending_flip_obj);
+	if (work->can_async_unpin) {
+		list_del_init(&work->head);
+		wake_up_all(&dev_priv->pending_flip_queue);
+	}
+
+	queue_work(dev_priv->wq, &work->unpin_work);
 }
 
 static int intel_crtc_wait_for_pending_flips(struct drm_crtc *crtc)
@@ -3916,7 +3913,9 @@ static int intel_crtc_wait_for_pending_flips(struct drm_crtc *crtc)
 		spin_lock_irq(&dev->event_lock);
 		work = list_first_entry_or_null(&intel_crtc->flip_work,
 						struct intel_flip_work, head);
-		if (work && !is_mmio_work(work)) {
+
+		if (work && !is_mmio_work(work) &&
+		    !work_busy(&work->unpin_work)) {
 			WARN_ONCE(1, "Removing stuck page flip\n");
 			page_flip_completed(intel_crtc, work);
 		}
@@ -10906,31 +10905,112 @@ static void intel_crtc_destroy(struct drm_crtc *crtc)
 	kfree(intel_crtc);
 }
 
+static void intel_crtc_post_flip_update(struct intel_flip_work *work,
+					struct drm_crtc *crtc)
+{
+	struct intel_crtc_state *crtc_state = work->new_crtc_state;
+	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+
+	if (crtc_state->disable_cxsr)
+		intel_crtc->wm.cxsr_allowed = true;
+
+	if (crtc_state->update_wm_post && crtc_state->base.active)
+		intel_update_watermarks(crtc);
+
+	if (work->num_planes > 0 &&
+	    work->old_plane_state[0]->base.plane == crtc->primary) {
+		struct intel_plane_state *plane_state =
+			work->new_plane_state[0];
+
+		if (plane_state->visible &&
+		    (needs_modeset(&crtc_state->base) ||
+		     !work->old_plane_state[0]->visible))
+			intel_post_enable_primary(crtc);
+	}
+}
+
 static void intel_unpin_work_fn(struct work_struct *__work)
 {
 	struct intel_flip_work *work =
 		container_of(__work, struct intel_flip_work, unpin_work);
-	struct intel_crtc *crtc = to_intel_crtc(work->crtc);
-	struct drm_device *dev = crtc->base.dev;
-	struct drm_plane *primary = crtc->base.primary;
+	struct drm_crtc *crtc = work->old_crtc_state->base.crtc;
+	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+	struct drm_device *dev = crtc->dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	int i;
 
-	if (is_mmio_work(work))
-		flush_work(&work->mmio_work);
+	if (work->fb_bits)
+		intel_frontbuffer_flip_complete(dev, work->fb_bits);
 
-	mutex_lock(&dev->struct_mutex);
-	intel_unpin_fb_obj(work->old_fb, primary->state->rotation);
-	drm_gem_object_unreference(&work->pending_flip_obj->base);
+	/*
+	 * Unless work->can_async_unpin is false, there's no way to ensure
+	 * that work->new_crtc_state contains valid memory during unpin
+	 * because intel_atomic_commit may free it before this runs.
+	 */
+	if (!work->can_async_unpin)
+		intel_crtc_post_flip_update(work, crtc);
+
+	if (work->fb_bits & to_intel_plane(crtc->primary)->frontbuffer_bit)
+		intel_fbc_post_update(intel_crtc);
+
+	if (work->put_power_domains)
+		modeset_put_power_domains(dev_priv, work->put_power_domains);
+
+	/* Make sure mmio work is completely finished before freeing all state here. */
+	flush_work(&work->mmio_work);
+
+	if (!work->can_async_unpin)
+		/* This must be called before work is unpinned for serialization. */
+		intel_modeset_verify_crtc(crtc, &work->old_crtc_state->base,
+					  &work->new_crtc_state->base);
+
+	if (!work->can_async_unpin || !list_empty(&work->head)) {
+		spin_lock_irq(&dev->event_lock);
+		WARN(list_empty(&work->head) != work->can_async_unpin,
+		     "[CRTC:%i] Pin work %p async %i with %i planes, active %i -> %i ms %i\n",
+		     crtc->base.id, work, work->can_async_unpin, work->num_planes,
+		     work->old_crtc_state->base.active, work->new_crtc_state->base.active,
+		     needs_modeset(&work->new_crtc_state->base));
+
+		if (!list_empty(&work->head))
+			list_del(&work->head);
+
+		wake_up_all(&dev_priv->pending_flip_queue);
+		spin_unlock_irq(&dev->event_lock);
+	}
+
+	intel_crtc_destroy_state(crtc, &work->old_crtc_state->base);
 
 	if (work->flip_queued_req)
-		i915_gem_request_assign(&work->flip_queued_req, NULL);
-	mutex_unlock(&dev->struct_mutex);
+		i915_gem_request_unreference__unlocked(work->flip_queued_req);
+
+	for (i = 0; i < work->num_planes; i++) {
+		struct intel_plane_state *old_plane_state =
+			work->old_plane_state[i];
+		struct drm_framebuffer *old_fb = old_plane_state->base.fb;
+		struct drm_plane *plane = old_plane_state->base.plane;
+		struct drm_i915_gem_request *req;
+
+		req = old_plane_state->wait_req;
+		old_plane_state->wait_req = NULL;
+		i915_gem_request_unreference__unlocked(req);
+
+		fence_put(old_plane_state->base.fence);
+		old_plane_state->base.fence = NULL;
+
+		if (old_fb &&
+		    (plane->type != DRM_PLANE_TYPE_CURSOR ||
+		     !INTEL_INFO(dev_priv)->cursor_needs_physical)) {
+			mutex_lock(&dev->struct_mutex);
+			intel_unpin_fb_obj(old_fb, old_plane_state->base.rotation);
+			mutex_unlock(&dev->struct_mutex);
+		}
 
-	intel_frontbuffer_flip_complete(dev, to_intel_plane(primary)->frontbuffer_bit);
-	intel_fbc_post_update(crtc);
-	drm_framebuffer_unreference(work->old_fb);
+		intel_plane_destroy_state(plane, &old_plane_state->base);
+	}
 
-	BUG_ON(atomic_read(&crtc->unpin_work_count) == 0);
-	atomic_dec(&crtc->unpin_work_count);
+	if (!WARN_ON(atomic_read(&intel_crtc->unpin_work_count) == 0))
+		atomic_dec(&intel_crtc->unpin_work_count);
 
 	kfree(work);
 }
@@ -11010,7 +11090,7 @@ static void do_intel_finish_page_flip(struct drm_device *dev,
 
 	if (work == NULL ||
 	    atomic_read(&work->pending) == INTEL_FLIP_INACTIVE ||
-	    !page_flip_finished(intel_crtc, work)) {
+	    !page_flip_finished(intel_crtc, work) || work_busy(&work->unpin_work)) {
 		spin_unlock_irqrestore(&dev->event_lock, flags);
 		return;
 	}
@@ -11036,15 +11116,6 @@ void intel_finish_page_flip_plane(struct drm_device *dev, int plane)
 	do_intel_finish_page_flip(dev, crtc);
 }
 
-static inline void intel_mark_page_flip_active(struct intel_flip_work *work)
-{
-	/* Ensure that the work item is consistent when activating it ... */
-	smp_wmb();
-	atomic_set(&work->pending, INTEL_FLIP_PENDING);
-	/* and that it is marked active as soon as the irq could fire. */
-	smp_wmb();
-}
-
 static int intel_gen2_queue_flip(struct drm_device *dev,
 				 struct drm_crtc *crtc,
 				 struct drm_framebuffer *fb,
@@ -11276,71 +11347,204 @@ static int intel_gen7_queue_flip(struct drm_device *dev,
 	return 0;
 }
 
-static bool use_mmio_flip(struct intel_engine_cs *engine,
-			  struct drm_i915_gem_object *obj)
+static struct intel_engine_cs *
+intel_get_flip_engine(struct drm_device *dev,
+		      struct drm_i915_private *dev_priv,
+		      struct drm_i915_gem_object *obj)
 {
-	/*
-	 * This is not being used for older platforms, because
-	 * non-availability of flip done interrupt forces us to use
-	 * CS flips. Older platforms derive flip done using some clever
-	 * tricks involving the flip_pending status bits and vblank irqs.
-	 * So using MMIO flips there would disrupt this mechanism.
-	 */
+	if (IS_VALLEYVIEW(dev) || IS_IVYBRIDGE(dev) || IS_HASWELL(dev))
+		return &dev_priv->engine[BCS];
 
-	if (engine == NULL)
-		return true;
+	if (dev_priv->info.gen >= 7) {
+		struct intel_engine_cs *engine;
 
-	if (i915.use_mmio_flip < 0)
+		engine = i915_gem_request_get_engine(obj->last_write_req);
+		if (engine && engine->id == RCS)
+			return engine;
+
+		return &dev_priv->engine[BCS];
+	} else
+		return &dev_priv->engine[RCS];
+}
+
+static bool
+flip_fb_compatible(struct drm_device *dev,
+		   struct drm_framebuffer *fb,
+		   struct drm_framebuffer *old_fb)
+{
+	struct drm_i915_gem_object *obj = intel_fb_obj(fb);
+	struct drm_i915_gem_object *old_obj = intel_fb_obj(old_fb);
+
+	if (old_fb->pixel_format != fb->pixel_format)
 		return false;
-	else if (i915.use_mmio_flip > 0)
-		return true;
-	else if (i915.enable_execlists)
-		return true;
-	else if (obj->base.dma_buf &&
-		 !reservation_object_test_signaled_rcu(obj->base.dma_buf->resv,
-						       false))
-		return true;
-	else
-		return engine != i915_gem_request_get_engine(obj->last_write_req);
+
+	if (INTEL_INFO(dev)->gen > 3 &&
+	    (fb->offsets[0] != old_fb->offsets[0] ||
+	     fb->pitches[0] != old_fb->pitches[0]))
+		return false;
+
+			/* vlv: DISPLAY_FLIP fails to change tiling */
+	if (IS_VALLEYVIEW(dev) && obj->tiling_mode != old_obj->tiling_mode)
+		return false;
+
+	return true;
+}
+
+static void
+intel_display_flip_prepare(struct drm_device *dev, struct drm_crtc *crtc,
+			   struct intel_flip_work *work)
+{
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+
+	if (work->flip_prepared)
+		return;
+
+	work->flip_prepared = true;
+
+	if (INTEL_INFO(dev)->gen >= 5 || IS_G4X(dev))
+		work->flip_count = I915_READ(PIPE_FLIPCOUNT_G4X(intel_crtc->pipe)) + 1;
+	work->flip_queued_vblank = drm_crtc_vblank_count(crtc);
+
+	intel_frontbuffer_flip_prepare(dev, work->new_crtc_state->fb_bits);
+}
+
+static void intel_flip_schedule_request(struct intel_flip_work *work, struct drm_crtc *crtc)
+{
+	struct drm_device *dev = crtc->dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct intel_plane_state *new_state = work->new_plane_state[0];
+	struct intel_plane_state *old_state = work->old_plane_state[0];
+	struct drm_framebuffer *fb, *old_fb;
+	struct drm_i915_gem_request *request = NULL;
+	struct intel_engine_cs *engine;
+	struct drm_i915_gem_object *obj;
+	struct fence *fence;
+	int ret;
+
+	if (i915_terminally_wedged(&dev_priv->gpu_error) ||
+	    i915_reset_in_progress(&dev_priv->gpu_error) ||
+	    i915.enable_execlists || i915.use_mmio_flip > 0 ||
+	    !dev_priv->display.queue_flip)
+		goto mmio;
+
+	/* Not right after modesetting, surface parameters need to be updated */
+	if (needs_modeset(crtc->state) ||
+	    to_intel_crtc_state(crtc->state)->update_pipe)
+		goto mmio;
+
+	/* Only allow a mmio flip for a primary plane without a dma-buf fence */
+	if (work->num_planes != 1 ||
+	    new_state->base.plane != crtc->primary ||
+	    new_state->base.fence)
+		goto mmio;
+
+	fence = work->old_plane_state[0]->base.fence;
+	if (fence && !fence_is_signaled(fence))
+		goto mmio;
+
+	old_fb = old_state->base.fb;
+	fb = new_state->base.fb;
+	obj = intel_fb_obj(fb);
+
+	trace_i915_flip_request(to_intel_crtc(crtc)->plane, obj);
+
+	/* Only when updating a already visible fb. */
+	if (!new_state->visible || !old_state->visible)
+		goto mmio;
+
+	if (!flip_fb_compatible(dev, fb, old_fb))
+		goto mmio;
+
+	engine = intel_get_flip_engine(dev, dev_priv, obj);
+	if (i915.use_mmio_flip == 0 && obj->last_write_req &&
+	    i915_gem_request_get_engine(obj->last_write_req) != engine)
+		goto mmio;
+
+	work->gtt_offset = intel_plane_obj_offset(to_intel_plane(crtc->primary), obj, 0);
+	work->gtt_offset += to_intel_crtc(crtc)->dspaddr_offset;
+
+	ret = i915_gem_object_sync(obj, engine, &request);
+	if (!ret && !request) {
+		request = i915_gem_request_alloc(engine, NULL);
+		ret = PTR_ERR_OR_ZERO(request);
+
+		if (ret)
+			request = NULL;
+	}
+
+	intel_display_flip_prepare(dev, crtc, work);
+
+	if (!ret)
+		ret = dev_priv->display.queue_flip(dev, crtc, fb, obj, request, 0);
+
+	if (!ret) {
+		i915_gem_request_assign(&work->flip_queued_req, request);
+		smp_mb__before_atomic();
+		atomic_set(&work->pending, INTEL_FLIP_PENDING);
+		i915_add_request_no_flush(request);
+		return;
+	}
+	if (request)
+		i915_gem_request_cancel(request);
+
+mmio:
+	to_intel_crtc(crtc)->reset_counter =
+		atomic_read(&dev_priv->gpu_error.reset_counter);
+	schedule_work(&work->mmio_work);
 }
 
 static void intel_mmio_flip_work_func(struct work_struct *w)
 {
 	struct intel_flip_work *work =
 		container_of(w, struct intel_flip_work, mmio_work);
-	struct intel_crtc *crtc = to_intel_crtc(work->crtc);
-	struct drm_device *dev = crtc->base.dev;
+	struct drm_crtc *crtc = work->old_crtc_state->base.crtc;
+	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+	struct intel_crtc_state *crtc_state = work->new_crtc_state;
+	struct drm_device *dev = crtc->dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
-	struct intel_plane *primary = to_intel_plane(crtc->base.primary);
-	struct drm_i915_gem_object *obj = intel_fb_obj(primary->base.state->fb);
+	struct drm_i915_gem_request *req;
+	int i;
 
-	if (work->flip_queued_req)
-		WARN_ON(__i915_wait_request(work->flip_queued_req,
-					    crtc->reset_counter,
+	for (i = 0; i < work->num_planes; i++) {
+		struct intel_plane_state *old_plane_state = work->old_plane_state[i];
+
+		/* For framebuffer backed by dmabuf, wait for fence */
+		if (old_plane_state->base.fence)
+			WARN_ON(fence_wait(old_plane_state->base.fence, false) < 0);
+
+		req = old_plane_state->wait_req;
+		if (!req)
+			continue;
+
+		WARN_ON(__i915_wait_request(req, intel_crtc->reset_counter,
 					    false, NULL,
 					    &dev_priv->rps.mmioflips));
+	}
 
-	/* For framebuffer backed by dmabuf, wait for fence */
-	if (obj->base.dma_buf)
-		WARN_ON(reservation_object_wait_timeout_rcu(obj->base.dma_buf->resv,
-							    false, false,
-							    MAX_SCHEDULE_TIMEOUT) < 0);
+	intel_display_flip_prepare(dev, crtc, work);
 
-	intel_pipe_update_start(crtc);
-	primary->update_plane(&primary->base,
-			      crtc->config,
-			      to_intel_plane_state(primary->base.state));
-	intel_pipe_update_end(crtc, work);
-}
+	intel_pipe_update_start(intel_crtc);
+	if (!needs_modeset(&crtc_state->base)) {
+		if (crtc_state->base.color_mgmt_changed || crtc_state->update_pipe) {
+			intel_color_set_csc(&crtc_state->base);
+			intel_color_load_luts(&crtc_state->base);
+		}
 
-static int intel_default_queue_flip(struct drm_device *dev,
-				    struct drm_crtc *crtc,
-				    struct drm_framebuffer *fb,
-				    struct drm_i915_gem_object *obj,
-				    struct drm_i915_gem_request *req,
-				    uint64_t gtt_offset)
-{
-	return -ENODEV;
+		if (crtc_state->update_pipe)
+			intel_update_pipe_config(intel_crtc, work->old_crtc_state);
+		else if (INTEL_INFO(dev)->gen >= 9)
+			skl_detach_scalers(intel_crtc);
+	}
+
+	for (i = 0; i < work->num_planes; i++) {
+		struct intel_plane_state *new_plane_state = work->new_plane_state[i];
+		struct intel_plane *plane = to_intel_plane(new_plane_state->base.plane);
+
+		plane->update_plane(&plane->base, crtc_state, new_plane_state);
+	}
+
+	intel_pipe_update_end(intel_crtc, work);
 }
 
 static bool __intel_pageflip_stall_check(struct drm_device *dev,
@@ -11353,7 +11557,8 @@ static bool __intel_pageflip_stall_check(struct drm_device *dev,
 	u32 pending;
 
 	pending = atomic_read(&work->pending);
-	if (pending == INTEL_FLIP_INACTIVE)
+	if (pending == INTEL_FLIP_INACTIVE ||
+	    work_busy(&work->unpin_work))
 		return false;
 
 	smp_mb__after_atomic();
@@ -11419,6 +11624,33 @@ void intel_check_page_flip(struct drm_device *dev, int pipe)
 	spin_unlock(&dev->event_lock);
 }
 
+static struct fence *intel_get_excl_fence(struct drm_i915_gem_object *obj)
+{
+	struct reservation_object *resv;
+
+
+	if (!obj->base.dma_buf)
+		return NULL;
+
+	resv = obj->base.dma_buf->resv;
+
+	/* For framebuffer backed by dmabuf, wait for fence */
+	while (1) {
+		struct fence *fence_excl, *ret = NULL;
+
+		rcu_read_lock();
+
+		fence_excl = rcu_dereference(resv->fence_excl);
+		if (fence_excl)
+			ret = fence_get_rcu(fence_excl);
+
+		rcu_read_unlock();
+
+		if (ret == fence_excl)
+			return ret;
+	}
+}
+
 static int intel_crtc_page_flip(struct drm_crtc *crtc,
 				struct drm_framebuffer *fb,
 				struct drm_pending_vblank_event *event,
@@ -11426,17 +11658,20 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
 {
 	struct drm_device *dev = crtc->dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
-	struct drm_framebuffer *old_fb = crtc->primary->fb;
+	struct drm_plane_state *old_state, *new_state = NULL;
+	struct drm_crtc_state *new_crtc_state = NULL;
+	struct drm_framebuffer *old_fb = crtc->primary->state->fb;
 	struct drm_i915_gem_object *obj = intel_fb_obj(fb);
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 	struct drm_plane *primary = crtc->primary;
-	enum pipe pipe = intel_crtc->pipe;
 	struct intel_flip_work *work;
-	struct intel_engine_cs *engine;
-	bool mmio_flip;
-	struct drm_i915_gem_request *request = NULL;
 	int ret;
 
+	old_state = crtc->primary->state;
+
+	if (!crtc->state->active)
+		return -EINVAL;
+
 	/*
 	 * drm_mode_page_flip_ioctl() should already catch this, but double
 	 * check to be safe.  In the future we may enable pageflipping from
@@ -11446,7 +11681,7 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
 		return -EBUSY;
 
 	/* Can't change pixel format via MI display flips. */
-	if (fb->pixel_format != crtc->primary->fb->pixel_format)
+	if (fb->pixel_format != old_fb->pixel_format)
 		return -EINVAL;
 
 	/*
@@ -11454,25 +11689,44 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
 	 * Note that pitch changes could also affect these register.
 	 */
 	if (INTEL_INFO(dev)->gen > 3 &&
-	    (fb->offsets[0] != crtc->primary->fb->offsets[0] ||
-	     fb->pitches[0] != crtc->primary->fb->pitches[0]))
+	    (fb->offsets[0] != old_fb->offsets[0] ||
+	     fb->pitches[0] != old_fb->pitches[0]))
 		return -EINVAL;
 
-	if (i915_terminally_wedged(&dev_priv->gpu_error))
-		goto out_hang;
-
 	work = kzalloc(sizeof(*work), GFP_KERNEL);
-	if (work == NULL)
-		return -ENOMEM;
+	new_crtc_state = intel_crtc_duplicate_state(crtc);
+	new_state = intel_plane_duplicate_state(primary);
+
+	if (!work || !new_crtc_state || !new_state) {
+		ret = -ENOMEM;
+		goto cleanup;
+	}
+
+	drm_framebuffer_unreference(new_state->fb);
+	drm_framebuffer_reference(fb);
+	new_state->fb = fb;
 
 	work->event = event;
-	work->crtc = crtc;
-	work->old_fb = old_fb;
 	INIT_WORK(&work->unpin_work, intel_unpin_work_fn);
+	INIT_WORK(&work->mmio_work, intel_mmio_flip_work_func);
 
+	work->new_crtc_state = to_intel_crtc_state(new_crtc_state);
+	work->old_crtc_state = intel_crtc->config;
+
+	work->fb_bits = to_intel_plane(primary)->frontbuffer_bit;
+	work->new_crtc_state->fb_bits = work->fb_bits;
+
+	work->can_async_unpin = true;
+	work->num_planes = 1;
+	work->old_plane_state[0] = to_intel_plane_state(old_state);
+	work->new_plane_state[0] = to_intel_plane_state(new_state);
+
+	/* Step 1: vblank waiting and workqueue throttling,
+	 * similar to intel_atomic_prepare_commit
+	 */
 	ret = drm_crtc_vblank_get(crtc);
 	if (ret)
-		goto free_work;
+		goto cleanup;
 
 	/* We borrow the event spin lock for protecting flip_work */
 	spin_lock_irq(&dev->event_lock);
@@ -11492,9 +11746,8 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
 			DRM_DEBUG_DRIVER("flip queue: crtc already busy\n");
 			spin_unlock_irq(&dev->event_lock);
 
-			drm_crtc_vblank_put(crtc);
-			kfree(work);
-			return -EBUSY;
+			ret = -EBUSY;
+			goto cleanup_vblank;
 		}
 	}
 	list_add_tail(&work->head, &intel_crtc->flip_work);
@@ -11503,157 +11756,62 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
 	if (atomic_read(&intel_crtc->unpin_work_count) >= 2)
 		flush_workqueue(dev_priv->wq);
 
-	/* Reference the objects for the scheduled work. */
-	drm_framebuffer_reference(work->old_fb);
-	drm_gem_object_reference(&obj->base);
-
-	crtc->primary->fb = fb;
-	update_state_fb(crtc->primary);
-	intel_fbc_pre_update(intel_crtc);
-
-	work->pending_flip_obj = obj;
-
-	ret = i915_mutex_lock_interruptible(dev);
+	/* step 2, similar to intel_prepare_plane_fb */
+	ret = mutex_lock_interruptible(&dev->struct_mutex);
 	if (ret)
-		goto cleanup;
-
-	atomic_inc(&intel_crtc->unpin_work_count);
-	intel_crtc->reset_counter = atomic_read(&dev_priv->gpu_error.reset_counter);
-
-	if (INTEL_INFO(dev)->gen >= 5 || IS_G4X(dev))
-		work->flip_count = I915_READ(PIPE_FLIPCOUNT_G4X(pipe)) + 1;
-
-	if (IS_VALLEYVIEW(dev) || IS_CHERRYVIEW(dev)) {
-		engine = &dev_priv->engine[BCS];
-		if (obj->tiling_mode != intel_fb_obj(work->old_fb)->tiling_mode)
-			/* vlv: DISPLAY_FLIP fails to change tiling */
-			engine = NULL;
-	} else if (IS_IVYBRIDGE(dev) || IS_HASWELL(dev)) {
-		engine = &dev_priv->engine[BCS];
-	} else if (INTEL_INFO(dev)->gen >= 7) {
-		engine = i915_gem_request_get_engine(obj->last_write_req);
-		if (engine == NULL || engine->id != RCS)
-			engine = &dev_priv->engine[BCS];
-	} else {
-		engine = &dev_priv->engine[RCS];
-	}
-
-	mmio_flip = use_mmio_flip(engine, obj);
+		goto cleanup_work;
 
-	/* When using CS flips, we want to emit semaphores between rings.
-	 * However, when using mmio flips we will create a task to do the
-	 * synchronisation, so all we want here is to pin the framebuffer
-	 * into the display plane and skip any waits.
-	 */
-	if (!mmio_flip) {
-		ret = i915_gem_object_sync(obj, engine, &request);
-		if (!ret && !request) {
-			request = i915_gem_request_alloc(engine, NULL);
-			ret = PTR_ERR_OR_ZERO(request);
-		}
-
-		if (ret)
-			goto cleanup_pending;
-	}
-
-	ret = intel_pin_and_fence_fb_obj(fb, primary->state->rotation);
+	ret = intel_pin_and_fence_fb_obj(fb, new_state->rotation);
 	if (ret)
-		goto cleanup_pending;
+		goto cleanup_unlock;
 
-	work->gtt_offset = intel_plane_obj_offset(to_intel_plane(primary),
-						  obj, 0);
-	work->gtt_offset += intel_crtc->dspaddr_offset;
-	work->flip_queued_vblank = drm_crtc_vblank_count(crtc);
+	i915_gem_track_fb(intel_fb_obj(old_fb), obj,
+			  to_intel_plane(primary)->frontbuffer_bit);
 
-	if (mmio_flip) {
-		INIT_WORK(&work->mmio_work, intel_mmio_flip_work_func);
+	/* point of no return, swap state */
+	primary->state = new_state;
+	crtc->state = new_crtc_state;
+	intel_crtc->config = to_intel_crtc_state(new_crtc_state);
+	primary->fb = fb;
 
-		i915_gem_request_assign(&work->flip_queued_req,
+	/* scheduling flip work */
+	atomic_inc(&intel_crtc->unpin_work_count);
+
+	if (obj->last_write_req &&
+	    !i915_gem_request_completed(obj->last_write_req, true))
+		i915_gem_request_assign(&work->old_plane_state[0]->wait_req,
 					obj->last_write_req);
 
-		schedule_work(&work->mmio_work);
-	} else {
-		i915_gem_request_assign(&work->flip_queued_req, request);
-		ret = dev_priv->display.queue_flip(dev, crtc, fb, obj, request,
-						   work->gtt_offset);
-		if (ret)
-			goto cleanup_unpin;
+	if (obj->base.dma_buf)
+		work->old_plane_state[0]->base.fence = intel_get_excl_fence(obj);
 
-		intel_mark_page_flip_active(work);
-		i915_add_request_no_flush(request);
-	}
+	intel_fbc_pre_update(intel_crtc);
 
-	work->flip_queued_vblank = drm_crtc_vblank_count(crtc);
+	intel_flip_schedule_request(work, crtc);
 
-	i915_gem_track_fb(intel_fb_obj(work->old_fb), obj,
-			  to_intel_plane(primary)->frontbuffer_bit);
 	mutex_unlock(&dev->struct_mutex);
 
-	intel_frontbuffer_flip_prepare(dev,
-				       to_intel_plane(primary)->frontbuffer_bit);
-
 	trace_i915_flip_request(intel_crtc->plane, obj);
 
 	return 0;
 
-cleanup_unpin:
-	intel_unpin_fb_obj(fb, crtc->primary->state->rotation);
-cleanup_pending:
-	if (!IS_ERR_OR_NULL(request))
-		i915_gem_request_cancel(request);
-	atomic_dec(&intel_crtc->unpin_work_count);
+cleanup_unlock:
 	mutex_unlock(&dev->struct_mutex);
-cleanup:
-	crtc->primary->fb = old_fb;
-	update_state_fb(crtc->primary);
-
-	drm_gem_object_unreference_unlocked(&obj->base);
-	drm_framebuffer_unreference(work->old_fb);
-
+cleanup_work:
 	spin_lock_irq(&dev->event_lock);
 	list_del(&work->head);
 	spin_unlock_irq(&dev->event_lock);
 
+cleanup_vblank:
 	drm_crtc_vblank_put(crtc);
-free_work:
-	kfree(work);
-
-	if (ret == -EIO) {
-		struct drm_atomic_state *state;
-		struct drm_plane_state *plane_state;
-
-out_hang:
-		state = drm_atomic_state_alloc(dev);
-		if (!state)
-			return -ENOMEM;
-		state->acquire_ctx = drm_modeset_legacy_acquire_ctx(crtc);
-
-retry:
-		plane_state = drm_atomic_get_plane_state(state, primary);
-		ret = PTR_ERR_OR_ZERO(plane_state);
-		if (!ret) {
-			drm_atomic_set_fb_for_plane(plane_state, fb);
-
-			ret = drm_atomic_set_crtc_for_plane(plane_state, crtc);
-			if (!ret)
-				ret = drm_atomic_commit(state);
-		}
-
-		if (ret == -EDEADLK) {
-			drm_modeset_backoff(state->acquire_ctx);
-			drm_atomic_state_clear(state);
-			goto retry;
-		}
+cleanup:
+	if (new_state)
+		intel_plane_destroy_state(primary, new_state);
 
-		if (ret)
-			drm_atomic_state_free(state);
+	if (new_crtc_state)
+		intel_crtc_destroy_state(crtc, new_crtc_state);
 
-		if (ret == 0 && event) {
-			spin_lock_irq(&dev->event_lock);
-			drm_send_vblank_event(dev, pipe, event);
-			spin_unlock_irq(&dev->event_lock);
-		}
-	}
+	kfree(work);
 	return ret;
 }
 
@@ -13710,33 +13868,6 @@ static const struct drm_crtc_funcs intel_crtc_funcs = {
 	.atomic_destroy_state = intel_crtc_destroy_state,
 };
 
-static struct fence *intel_get_excl_fence(struct drm_i915_gem_object *obj)
-{
-	struct reservation_object *resv;
-
-
-	if (!obj->base.dma_buf)
-		return NULL;
-
-	resv = obj->base.dma_buf->resv;
-
-	/* For framebuffer backed by dmabuf, wait for fence */
-	while (1) {
-		struct fence *fence_excl, *ret = NULL;
-
-		rcu_read_lock();
-
-		fence_excl = rcu_dereference(resv->fence_excl);
-		if (fence_excl)
-			ret = fence_get_rcu(fence_excl);
-
-		rcu_read_unlock();
-
-		if (ret == fence_excl)
-			return ret;
-	}
-}
-
 /**
  * intel_prepare_plane_fb - Prepare fb for usage on plane
  * @plane: drm plane to prepare for
@@ -15034,7 +15165,7 @@ void intel_init_display_hooks(struct drm_i915_private *dev_priv)
 		/* Drop through - unsupported since execlist only. */
 	default:
 		/* Default just returns -ENODEV to indicate unsupported */
-		dev_priv->display.queue_flip = intel_default_queue_flip;
+		break;
 	}
 }
 
@@ -15969,9 +16100,9 @@ void intel_modeset_gem_init(struct drm_device *dev)
 			DRM_ERROR("failed to pin boot fb on pipe %d\n",
 				  to_intel_crtc(c)->pipe);
 			drm_framebuffer_unreference(c->primary->fb);
-			c->primary->fb = NULL;
+			drm_framebuffer_unreference(c->primary->state->fb);
+			c->primary->fb = c->primary->state->fb = NULL;
 			c->primary->crtc = c->primary->state->crtc = NULL;
-			update_state_fb(c->primary);
 			c->state->plane_mask &= ~(1 << drm_plane_index(c->primary));
 		}
 	}
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index e584538ad776..7ecebf0c22e0 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -935,9 +935,6 @@ struct intel_flip_work {
 	struct work_struct unpin_work;
 	struct work_struct mmio_work;
 
-	struct drm_crtc *crtc;
-	struct drm_framebuffer *old_fb;
-	struct drm_i915_gem_object *pending_flip_obj;
 	struct drm_pending_vblank_event *event;
 	atomic_t pending;
 #define INTEL_FLIP_INACTIVE	0
@@ -947,6 +944,16 @@ struct intel_flip_work {
 	struct drm_i915_gem_request *flip_queued_req;
 	u32 flip_queued_vblank;
 	u32 flip_ready_vblank;
+
+	unsigned put_power_domains;
+	unsigned num_planes;
+
+	bool can_async_unpin, flip_prepared;
+	unsigned fb_bits;
+
+	struct intel_crtc_state *old_crtc_state, *new_crtc_state;
+	struct intel_plane_state *old_plane_state[I915_MAX_PLANES + 1];
+	struct intel_plane_state *new_plane_state[I915_MAX_PLANES + 1];
 };
 
 struct intel_load_detect_pipe {
-- 
2.1.0

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

[PATCH v2 09/11] drm/i915: Remove cs based page flip support.

[Maarten Lankhorst]

With mmio flips now available on all platforms it's time to remove
support for cs flips.

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c  |  21 +--
 drivers/gpu/drm/i915/intel_display.c | 273 +----------------------------------
 drivers/gpu/drm/i915/intel_drv.h     |   6 +-
 3 files changed, 12 insertions(+), 288 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 04ea4129c9f5..ccc390b10258 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -566,7 +566,6 @@ static void i915_dump_pageflip(struct seq_file *m,
 {
 	const char pipe = pipe_name(crtc->pipe);
 	u32 pending;
-	u32 addr;
 	int i;
 
 	pending = atomic_read(&work->pending);
@@ -578,7 +577,6 @@ static void i915_dump_pageflip(struct seq_file *m,
 			   pipe, plane_name(crtc->plane));
 	}
 
-
 	for (i = 0; i < work->num_planes; i++) {
 		struct intel_plane_state *old_plane_state = work->old_plane_state[i];
 		struct drm_plane *plane = old_plane_state->base.plane;
@@ -602,22 +600,9 @@ static void i915_dump_pageflip(struct seq_file *m,
 			   i915_gem_request_completed(req, true));
 	}
 
-	seq_printf(m, "Flip queued on frame %d, (was ready on frame %d), now %d\n",
-		   work->flip_queued_vblank,
-		   work->flip_ready_vblank,
-		   drm_crtc_vblank_count(&crtc->base));
-	seq_printf(m, "%d prepares\n", atomic_read(&work->pending));
-
-	if (INTEL_INFO(dev_priv)->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->flip_queued_req) {
-		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);
-	}
+	seq_printf(m, "Flip queued on frame %d, now %d\n",
+		   pending ? work->flip_queued_vblank : -1,
+		   intel_crtc_get_vblank_counter(crtc));
 }
 
 static int i915_gem_pageflip_info(struct seq_file *m, void *data)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 2a8392ada270..45fd84ebaccf 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -48,11 +48,6 @@
 #include <linux/reservation.h>
 #include <linux/dma-buf.h>
 
-static bool is_mmio_work(struct intel_flip_work *work)
-{
-	return !work->flip_queued_req;
-}
-
 /* Primary plane formats for gen <= 3 */
 static const uint32_t i8xx_primary_formats[] = {
 	DRM_FORMAT_C8,
@@ -3906,21 +3901,7 @@ static int intel_crtc_wait_for_pending_flips(struct drm_crtc *crtc)
 	if (ret < 0)
 		return ret;
 
-	if (ret == 0) {
-		struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
-		struct intel_flip_work *work;
-
-		spin_lock_irq(&dev->event_lock);
-		work = list_first_entry_or_null(&intel_crtc->flip_work,
-						struct intel_flip_work, head);
-
-		if (work && !is_mmio_work(work) &&
-		    !work_busy(&work->unpin_work)) {
-			WARN_ONCE(1, "Removing stuck page flip\n");
-			page_flip_completed(intel_crtc, work);
-		}
-		spin_unlock_irq(&dev->event_lock);
-	}
+	WARN(ret == 0, "Stuck page flip\n");
 
 	return 0;
 }
@@ -10981,9 +10962,6 @@ static void intel_unpin_work_fn(struct work_struct *__work)
 
 	intel_crtc_destroy_state(crtc, &work->old_crtc_state->base);
 
-	if (work->flip_queued_req)
-		i915_gem_request_unreference__unlocked(work->flip_queued_req);
-
 	for (i = 0; i < work->num_planes; i++) {
 		struct intel_plane_state *old_plane_state =
 			work->old_plane_state[i];
@@ -11015,59 +10993,6 @@ static void intel_unpin_work_fn(struct work_struct *__work)
 	kfree(work);
 }
 
-/* Is 'a' after or equal to 'b'? */
-static bool g4x_flip_count_after_eq(u32 a, u32 b)
-{
-	return !((a - b) & 0x80000000);
-}
-
-static bool page_flip_finished(struct intel_crtc *crtc,
-			       struct intel_flip_work *work)
-{
-	struct drm_device *dev = crtc->base.dev;
-	struct drm_i915_private *dev_priv = dev->dev_private;
-
-	if (i915_reset_in_progress(&dev_priv->gpu_error) ||
-	    crtc->reset_counter != atomic_read(&dev_priv->gpu_error.reset_counter))
-		return true;
-
-	/*
-	 * The relevant registers doen't exist on pre-ctg.
-	 * As the flip done interrupt doesn't trigger for mmio
-	 * flips on gmch platforms, a flip count check isn't
-	 * really needed there. But since ctg has the registers,
-	 * include it in the check anyway.
-	 */
-	if (INTEL_INFO(dev)->gen < 5 && !IS_G4X(dev))
-		return true;
-
-	/*
-	 * BDW signals flip done immediately if the plane
-	 * is disabled, even if the plane enable is already
-	 * armed to occur at the next vblank :(
-	 */
-
-	/*
-	 * A DSPSURFLIVE check isn't enough in case the mmio and CS flips
-	 * used the same base address. In that case the mmio flip might
-	 * have completed, but the CS hasn't even executed the flip yet.
-	 *
-	 * A flip count check isn't enough as the CS might have updated
-	 * the base address just after start of vblank, but before we
-	 * managed to process the interrupt. This means we'd complete the
-	 * CS flip too soon.
-	 *
-	 * Combining both checks should get us a good enough result. It may
-	 * still happen that the CS flip has been executed, but has not
-	 * yet actually completed. But in case the base address is the same
-	 * anyway, we don't really care.
-	 */
-	return (I915_READ(DSPSURFLIVE(crtc->plane)) & ~0xfff) ==
-		work->gtt_offset &&
-		g4x_flip_count_after_eq(I915_READ(PIPE_FLIPCOUNT_G4X(crtc->pipe)),
-					work->flip_count);
-}
-
 static void do_intel_finish_page_flip(struct drm_device *dev,
 				      struct drm_crtc *crtc)
 {
@@ -11090,7 +11015,7 @@ static void do_intel_finish_page_flip(struct drm_device *dev,
 
 	if (work == NULL ||
 	    atomic_read(&work->pending) == INTEL_FLIP_INACTIVE ||
-	    !page_flip_finished(intel_crtc, work) || work_busy(&work->unpin_work)) {
+	    work_busy(&work->unpin_work)) {
 		spin_unlock_irqrestore(&dev->event_lock, flags);
 		return;
 	}
@@ -11347,153 +11272,6 @@ static int intel_gen7_queue_flip(struct drm_device *dev,
 	return 0;
 }
 
-static struct intel_engine_cs *
-intel_get_flip_engine(struct drm_device *dev,
-		      struct drm_i915_private *dev_priv,
-		      struct drm_i915_gem_object *obj)
-{
-	if (IS_VALLEYVIEW(dev) || IS_IVYBRIDGE(dev) || IS_HASWELL(dev))
-		return &dev_priv->engine[BCS];
-
-	if (dev_priv->info.gen >= 7) {
-		struct intel_engine_cs *engine;
-
-		engine = i915_gem_request_get_engine(obj->last_write_req);
-		if (engine && engine->id == RCS)
-			return engine;
-
-		return &dev_priv->engine[BCS];
-	} else
-		return &dev_priv->engine[RCS];
-}
-
-static bool
-flip_fb_compatible(struct drm_device *dev,
-		   struct drm_framebuffer *fb,
-		   struct drm_framebuffer *old_fb)
-{
-	struct drm_i915_gem_object *obj = intel_fb_obj(fb);
-	struct drm_i915_gem_object *old_obj = intel_fb_obj(old_fb);
-
-	if (old_fb->pixel_format != fb->pixel_format)
-		return false;
-
-	if (INTEL_INFO(dev)->gen > 3 &&
-	    (fb->offsets[0] != old_fb->offsets[0] ||
-	     fb->pitches[0] != old_fb->pitches[0]))
-		return false;
-
-			/* vlv: DISPLAY_FLIP fails to change tiling */
-	if (IS_VALLEYVIEW(dev) && obj->tiling_mode != old_obj->tiling_mode)
-		return false;
-
-	return true;
-}
-
-static void
-intel_display_flip_prepare(struct drm_device *dev, struct drm_crtc *crtc,
-			   struct intel_flip_work *work)
-{
-	struct drm_i915_private *dev_priv = dev->dev_private;
-	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
-
-	if (work->flip_prepared)
-		return;
-
-	work->flip_prepared = true;
-
-	if (INTEL_INFO(dev)->gen >= 5 || IS_G4X(dev))
-		work->flip_count = I915_READ(PIPE_FLIPCOUNT_G4X(intel_crtc->pipe)) + 1;
-	work->flip_queued_vblank = drm_crtc_vblank_count(crtc);
-
-	intel_frontbuffer_flip_prepare(dev, work->new_crtc_state->fb_bits);
-}
-
-static void intel_flip_schedule_request(struct intel_flip_work *work, struct drm_crtc *crtc)
-{
-	struct drm_device *dev = crtc->dev;
-	struct drm_i915_private *dev_priv = dev->dev_private;
-	struct intel_plane_state *new_state = work->new_plane_state[0];
-	struct intel_plane_state *old_state = work->old_plane_state[0];
-	struct drm_framebuffer *fb, *old_fb;
-	struct drm_i915_gem_request *request = NULL;
-	struct intel_engine_cs *engine;
-	struct drm_i915_gem_object *obj;
-	struct fence *fence;
-	int ret;
-
-	if (i915_terminally_wedged(&dev_priv->gpu_error) ||
-	    i915_reset_in_progress(&dev_priv->gpu_error) ||
-	    i915.enable_execlists || i915.use_mmio_flip > 0 ||
-	    !dev_priv->display.queue_flip)
-		goto mmio;
-
-	/* Not right after modesetting, surface parameters need to be updated */
-	if (needs_modeset(crtc->state) ||
-	    to_intel_crtc_state(crtc->state)->update_pipe)
-		goto mmio;
-
-	/* Only allow a mmio flip for a primary plane without a dma-buf fence */
-	if (work->num_planes != 1 ||
-	    new_state->base.plane != crtc->primary ||
-	    new_state->base.fence)
-		goto mmio;
-
-	fence = work->old_plane_state[0]->base.fence;
-	if (fence && !fence_is_signaled(fence))
-		goto mmio;
-
-	old_fb = old_state->base.fb;
-	fb = new_state->base.fb;
-	obj = intel_fb_obj(fb);
-
-	trace_i915_flip_request(to_intel_crtc(crtc)->plane, obj);
-
-	/* Only when updating a already visible fb. */
-	if (!new_state->visible || !old_state->visible)
-		goto mmio;
-
-	if (!flip_fb_compatible(dev, fb, old_fb))
-		goto mmio;
-
-	engine = intel_get_flip_engine(dev, dev_priv, obj);
-	if (i915.use_mmio_flip == 0 && obj->last_write_req &&
-	    i915_gem_request_get_engine(obj->last_write_req) != engine)
-		goto mmio;
-
-	work->gtt_offset = intel_plane_obj_offset(to_intel_plane(crtc->primary), obj, 0);
-	work->gtt_offset += to_intel_crtc(crtc)->dspaddr_offset;
-
-	ret = i915_gem_object_sync(obj, engine, &request);
-	if (!ret && !request) {
-		request = i915_gem_request_alloc(engine, NULL);
-		ret = PTR_ERR_OR_ZERO(request);
-
-		if (ret)
-			request = NULL;
-	}
-
-	intel_display_flip_prepare(dev, crtc, work);
-
-	if (!ret)
-		ret = dev_priv->display.queue_flip(dev, crtc, fb, obj, request, 0);
-
-	if (!ret) {
-		i915_gem_request_assign(&work->flip_queued_req, request);
-		smp_mb__before_atomic();
-		atomic_set(&work->pending, INTEL_FLIP_PENDING);
-		i915_add_request_no_flush(request);
-		return;
-	}
-	if (request)
-		i915_gem_request_cancel(request);
-
-mmio:
-	to_intel_crtc(crtc)->reset_counter =
-		atomic_read(&dev_priv->gpu_error.reset_counter);
-	schedule_work(&work->mmio_work);
-}
-
 static void intel_mmio_flip_work_func(struct work_struct *w)
 {
 	struct intel_flip_work *work =
@@ -11522,7 +11300,7 @@ static void intel_mmio_flip_work_func(struct work_struct *w)
 					    &dev_priv->rps.mmioflips));
 	}
 
-	intel_display_flip_prepare(dev, crtc, work);
+	intel_frontbuffer_flip_prepare(dev, crtc_state->fb_bits);
 
 	intel_pipe_update_start(intel_crtc);
 	if (!needs_modeset(&crtc_state->base)) {
@@ -11551,9 +11329,7 @@ static bool __intel_pageflip_stall_check(struct drm_device *dev,
 					 struct drm_crtc *crtc,
 					 struct intel_flip_work *work)
 {
-	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
-	u32 addr;
 	u32 pending;
 
 	pending = atomic_read(&work->pending);
@@ -11563,36 +11339,8 @@ static bool __intel_pageflip_stall_check(struct drm_device *dev,
 
 	smp_mb__after_atomic();
 
-	if (is_mmio_work(work)) {
-		u32 cur = intel_crtc_get_vblank_counter(intel_crtc);
-
-		/* MMIO work completes when vblank is different from flip_queued_vblank. */
-		return cur != work->flip_queued_vblank;
-	}
-
-	if (work->flip_ready_vblank == 0) {
-		if (work->flip_queued_req &&
-		    !i915_gem_request_completed(work->flip_queued_req, true))
-			return false;
-
-		work->flip_ready_vblank = drm_crtc_vblank_count(crtc);
-	}
-
-	if (drm_crtc_vblank_count(crtc) - work->flip_ready_vblank < 3)
-		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. We could address this by checking for a
-	 * non-incrementing frame counter.
-	 */
-	return addr == work->gtt_offset;
+	/* MMIO work completes when vblank is different from flip_queued_vblank. */
+	return work->flip_queued_vblank != intel_crtc_get_vblank_counter(intel_crtc);
 }
 
 void intel_check_page_flip(struct drm_device *dev, int pipe)
@@ -11612,15 +11360,8 @@ void intel_check_page_flip(struct drm_device *dev, int pipe)
 					struct intel_flip_work, head);
 
 	if (work != NULL && __intel_pageflip_stall_check(dev, crtc, work)) {
-		WARN_ONCE(!is_mmio_work(work),
-			  "Kicking stuck page flip: queued at %d, now %d\n",
-			 work->flip_queued_vblank, drm_vblank_count(dev, pipe));
 		page_flip_completed(intel_crtc, work);
-		work = NULL;
 	}
-	if (work != NULL && !is_mmio_work(work) &&
-	    drm_vblank_count(dev, pipe) - work->flip_queued_vblank > 1)
-		intel_queue_rps_boost_for_request(dev, work->flip_queued_req);
 	spin_unlock(&dev->event_lock);
 }
 
@@ -11787,7 +11528,9 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
 
 	intel_fbc_pre_update(intel_crtc);
 
-	intel_flip_schedule_request(work, crtc);
+	intel_crtc->reset_counter =
+		atomic_read(&dev_priv->gpu_error.reset_counter);
+	schedule_work(&work->mmio_work);
 
 	mutex_unlock(&dev->struct_mutex);
 
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 7ecebf0c22e0..02ea5adfda72 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -939,16 +939,12 @@ struct intel_flip_work {
 	atomic_t pending;
 #define INTEL_FLIP_INACTIVE	0
 #define INTEL_FLIP_PENDING	1
-	u32 flip_count;
-	u32 gtt_offset;
-	struct drm_i915_gem_request *flip_queued_req;
 	u32 flip_queued_vblank;
-	u32 flip_ready_vblank;
 
 	unsigned put_power_domains;
 	unsigned num_planes;
 
-	bool can_async_unpin, flip_prepared;
+	bool can_async_unpin;
 	unsigned fb_bits;
 
 	struct intel_crtc_state *old_crtc_state, *new_crtc_state;
-- 
2.1.0

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

[PATCH v2 10/11] drm/i915: Remove use_mmio_flip kernel parameter.

[Maarten Lankhorst]

With the removal of cs flips this is always force enabled.

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_params.c | 5 -----
 drivers/gpu/drm/i915/i915_params.h | 1 -
 drivers/gpu/drm/i915/intel_lrc.c   | 3 +--
 3 files changed, 1 insertion(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c
index 80ce581793dc..765a37376860 100644
--- a/drivers/gpu/drm/i915/i915_params.c
+++ b/drivers/gpu/drm/i915/i915_params.c
@@ -50,7 +50,6 @@ struct i915_params i915 __read_mostly = {
 	.invert_brightness = 0,
 	.disable_display = 0,
 	.enable_cmd_parser = 1,
-	.use_mmio_flip = 0,
 	.mmio_debug = 0,
 	.verbose_state_checks = 1,
 	.nuclear_pageflip = 0,
@@ -179,10 +178,6 @@ module_param_named_unsafe(enable_cmd_parser, i915.enable_cmd_parser, int, 0600);
 MODULE_PARM_DESC(enable_cmd_parser,
 		 "Enable command parsing (1=enabled [default], 0=disabled)");
 
-module_param_named_unsafe(use_mmio_flip, i915.use_mmio_flip, int, 0600);
-MODULE_PARM_DESC(use_mmio_flip,
-		 "use MMIO flips (-1=never, 0=driver discretion [default], 1=always)");
-
 module_param_named(mmio_debug, i915.mmio_debug, int, 0600);
 MODULE_PARM_DESC(mmio_debug,
 	"Enable the MMIO debug code for the first N failures (default: off). "
diff --git a/drivers/gpu/drm/i915/i915_params.h b/drivers/gpu/drm/i915/i915_params.h
index 3934c4300427..dae2dbd87147 100644
--- a/drivers/gpu/drm/i915/i915_params.h
+++ b/drivers/gpu/drm/i915/i915_params.h
@@ -46,7 +46,6 @@ struct i915_params {
 	int invert_brightness;
 	int enable_cmd_parser;
 	int guc_log_level;
-	int use_mmio_flip;
 	int mmio_debug;
 	int edp_vswing;
 	unsigned int inject_load_failure;
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index f209ecfdcb5c..b05bb3879063 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -259,8 +259,7 @@ int intel_sanitize_enable_execlists(struct drm_device *dev, int enable_execlists
 	if (enable_execlists == 0)
 		return 0;
 
-	if (HAS_LOGICAL_RING_CONTEXTS(dev) && USES_PPGTT(dev) &&
-	    i915.use_mmio_flip >= 0)
+	if (HAS_LOGICAL_RING_CONTEXTS(dev) && USES_PPGTT(dev))
 		return 1;
 
 	return 0;
-- 
2.1.0

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

[PATCH v2 11/11] drm/i915: Remove queue_flip pointer.

[Maarten Lankhorst]

With the removal of cs support this is no longer reachable.
Can be revived if needed.

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h      |   5 -
 drivers/gpu/drm/i915/intel_display.c | 259 -----------------------------------
 2 files changed, 264 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 7927bd667a6d..3612201af119 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -601,11 +601,6 @@ struct drm_i915_display_funcs {
 	void (*audio_codec_disable)(struct intel_encoder *encoder);
 	void (*fdi_link_train)(struct drm_crtc *crtc);
 	void (*init_clock_gating)(struct drm_device *dev);
-	int (*queue_flip)(struct drm_device *dev, struct drm_crtc *crtc,
-			  struct drm_framebuffer *fb,
-			  struct drm_i915_gem_object *obj,
-			  struct drm_i915_gem_request *req,
-			  uint64_t gtt_offset);
 	void (*hpd_irq_setup)(struct drm_device *dev);
 	/* clock updates for mode set */
 	/* cursor updates */
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 45fd84ebaccf..581fe054b240 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -11041,237 +11041,6 @@ void intel_finish_page_flip_plane(struct drm_device *dev, int plane)
 	do_intel_finish_page_flip(dev, crtc);
 }
 
-static int intel_gen2_queue_flip(struct drm_device *dev,
-				 struct drm_crtc *crtc,
-				 struct drm_framebuffer *fb,
-				 struct drm_i915_gem_object *obj,
-				 struct drm_i915_gem_request *req,
-				 uint64_t gtt_offset)
-{
-	struct intel_engine_cs *engine = req->engine;
-	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
-	u32 flip_mask;
-	int ret;
-
-	ret = intel_ring_begin(req, 6);
-	if (ret)
-		return ret;
-
-	/* Can't queue multiple flips, so wait for the previous
-	 * one to finish before executing the next.
-	 */
-	if (intel_crtc->plane)
-		flip_mask = MI_WAIT_FOR_PLANE_B_FLIP;
-	else
-		flip_mask = MI_WAIT_FOR_PLANE_A_FLIP;
-	intel_ring_emit(engine, MI_WAIT_FOR_EVENT | flip_mask);
-	intel_ring_emit(engine, MI_NOOP);
-	intel_ring_emit(engine, MI_DISPLAY_FLIP |
-			MI_DISPLAY_FLIP_PLANE(intel_crtc->plane));
-	intel_ring_emit(engine, fb->pitches[0]);
-	intel_ring_emit(engine, gtt_offset);
-	intel_ring_emit(engine, 0); /* aux display base address, unused */
-
-	return 0;
-}
-
-static int intel_gen3_queue_flip(struct drm_device *dev,
-				 struct drm_crtc *crtc,
-				 struct drm_framebuffer *fb,
-				 struct drm_i915_gem_object *obj,
-				 struct drm_i915_gem_request *req,
-				 uint64_t gtt_offset)
-{
-	struct intel_engine_cs *engine = req->engine;
-	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
-	u32 flip_mask;
-	int ret;
-
-	ret = intel_ring_begin(req, 6);
-	if (ret)
-		return ret;
-
-	if (intel_crtc->plane)
-		flip_mask = MI_WAIT_FOR_PLANE_B_FLIP;
-	else
-		flip_mask = MI_WAIT_FOR_PLANE_A_FLIP;
-	intel_ring_emit(engine, MI_WAIT_FOR_EVENT | flip_mask);
-	intel_ring_emit(engine, MI_NOOP);
-	intel_ring_emit(engine, MI_DISPLAY_FLIP_I915 |
-			MI_DISPLAY_FLIP_PLANE(intel_crtc->plane));
-	intel_ring_emit(engine, fb->pitches[0]);
-	intel_ring_emit(engine, gtt_offset);
-	intel_ring_emit(engine, MI_NOOP);
-
-	return 0;
-}
-
-static int intel_gen4_queue_flip(struct drm_device *dev,
-				 struct drm_crtc *crtc,
-				 struct drm_framebuffer *fb,
-				 struct drm_i915_gem_object *obj,
-				 struct drm_i915_gem_request *req,
-				 uint64_t gtt_offset)
-{
-	struct intel_engine_cs *engine = req->engine;
-	struct drm_i915_private *dev_priv = dev->dev_private;
-	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
-	uint32_t pf, pipesrc;
-	int ret;
-
-	ret = intel_ring_begin(req, 4);
-	if (ret)
-		return ret;
-
-	/* i965+ uses the linear or tiled offsets from the
-	 * Display Registers (which do not change across a page-flip)
-	 * so we need only reprogram the base address.
-	 */
-	intel_ring_emit(engine, MI_DISPLAY_FLIP |
-			MI_DISPLAY_FLIP_PLANE(intel_crtc->plane));
-	intel_ring_emit(engine, fb->pitches[0]);
-	intel_ring_emit(engine, gtt_offset | obj->tiling_mode);
-
-	/* XXX Enabling the panel-fitter across page-flip is so far
-	 * untested on non-native modes, so ignore it for now.
-	 * pf = I915_READ(pipe == 0 ? PFA_CTL_1 : PFB_CTL_1) & PF_ENABLE;
-	 */
-	pf = 0;
-	pipesrc = I915_READ(PIPESRC(intel_crtc->pipe)) & 0x0fff0fff;
-	intel_ring_emit(engine, pf | pipesrc);
-
-	return 0;
-}
-
-static int intel_gen6_queue_flip(struct drm_device *dev,
-				 struct drm_crtc *crtc,
-				 struct drm_framebuffer *fb,
-				 struct drm_i915_gem_object *obj,
-				 struct drm_i915_gem_request *req,
-				 uint64_t gtt_offset)
-{
-	struct intel_engine_cs *engine = req->engine;
-	struct drm_i915_private *dev_priv = dev->dev_private;
-	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
-	uint32_t pf, pipesrc;
-	int ret;
-
-	ret = intel_ring_begin(req, 4);
-	if (ret)
-		return ret;
-
-	intel_ring_emit(engine, MI_DISPLAY_FLIP |
-			MI_DISPLAY_FLIP_PLANE(intel_crtc->plane));
-	intel_ring_emit(engine, fb->pitches[0] | obj->tiling_mode);
-	intel_ring_emit(engine, gtt_offset);
-
-	/* Contrary to the suggestions in the documentation,
-	 * "Enable Panel Fitter" does not seem to be required when page
-	 * flipping with a non-native mode, and worse causes a normal
-	 * modeset to fail.
-	 * pf = I915_READ(PF_CTL(intel_crtc->pipe)) & PF_ENABLE;
-	 */
-	pf = 0;
-	pipesrc = I915_READ(PIPESRC(intel_crtc->pipe)) & 0x0fff0fff;
-	intel_ring_emit(engine, pf | pipesrc);
-
-	return 0;
-}
-
-static int intel_gen7_queue_flip(struct drm_device *dev,
-				 struct drm_crtc *crtc,
-				 struct drm_framebuffer *fb,
-				 struct drm_i915_gem_object *obj,
-				 struct drm_i915_gem_request *req,
-				 uint64_t gtt_offset)
-{
-	struct intel_engine_cs *engine = req->engine;
-	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
-	uint32_t plane_bit = 0;
-	int len, ret;
-
-	switch (intel_crtc->plane) {
-	case PLANE_A:
-		plane_bit = MI_DISPLAY_FLIP_IVB_PLANE_A;
-		break;
-	case PLANE_B:
-		plane_bit = MI_DISPLAY_FLIP_IVB_PLANE_B;
-		break;
-	case PLANE_C:
-		plane_bit = MI_DISPLAY_FLIP_IVB_PLANE_C;
-		break;
-	default:
-		WARN_ONCE(1, "unknown plane in flip command\n");
-		return -ENODEV;
-	}
-
-	len = 4;
-	if (engine->id == RCS) {
-		len += 6;
-		/*
-		 * On Gen 8, SRM is now taking an extra dword to accommodate
-		 * 48bits addresses, and we need a NOOP for the batch size to
-		 * stay even.
-		 */
-		if (IS_GEN8(dev))
-			len += 2;
-	}
-
-	/*
-	 * BSpec MI_DISPLAY_FLIP for IVB:
-	 * "The full packet must be contained within the same cache line."
-	 *
-	 * Currently the LRI+SRM+MI_DISPLAY_FLIP all fit within the same
-	 * cacheline, if we ever start emitting more commands before
-	 * the MI_DISPLAY_FLIP we may need to first emit everything else,
-	 * then do the cacheline alignment, and finally emit the
-	 * MI_DISPLAY_FLIP.
-	 */
-	ret = intel_ring_cacheline_align(req);
-	if (ret)
-		return ret;
-
-	ret = intel_ring_begin(req, len);
-	if (ret)
-		return ret;
-
-	/* Unmask the flip-done completion message. Note that the bspec says that
-	 * we should do this for both the BCS and RCS, and that we must not unmask
-	 * more than one flip event at any time (or ensure that one flip message
-	 * can be sent by waiting for flip-done prior to queueing new flips).
-	 * Experimentation says that BCS works despite DERRMR masking all
-	 * flip-done completion events and that unmasking all planes at once
-	 * for the RCS also doesn't appear to drop events. Setting the DERRMR
-	 * to zero does lead to lockups within MI_DISPLAY_FLIP.
-	 */
-	if (engine->id == RCS) {
-		intel_ring_emit(engine, MI_LOAD_REGISTER_IMM(1));
-		intel_ring_emit_reg(engine, DERRMR);
-		intel_ring_emit(engine, ~(DERRMR_PIPEA_PRI_FLIP_DONE |
-					  DERRMR_PIPEB_PRI_FLIP_DONE |
-					  DERRMR_PIPEC_PRI_FLIP_DONE));
-		if (IS_GEN8(dev))
-			intel_ring_emit(engine, MI_STORE_REGISTER_MEM_GEN8 |
-					      MI_SRM_LRM_GLOBAL_GTT);
-		else
-			intel_ring_emit(engine, MI_STORE_REGISTER_MEM |
-					      MI_SRM_LRM_GLOBAL_GTT);
-		intel_ring_emit_reg(engine, DERRMR);
-		intel_ring_emit(engine, engine->scratch.gtt_offset + 256);
-		if (IS_GEN8(dev)) {
-			intel_ring_emit(engine, 0);
-			intel_ring_emit(engine, MI_NOOP);
-		}
-	}
-
-	intel_ring_emit(engine, MI_DISPLAY_FLIP_I915 | plane_bit);
-	intel_ring_emit(engine, (fb->pitches[0] | obj->tiling_mode));
-	intel_ring_emit(engine, gtt_offset);
-	intel_ring_emit(engine, (MI_NOOP));
-
-	return 0;
-}
-
 static void intel_mmio_flip_work_func(struct work_struct *w)
 {
 	struct intel_flip_work *work =
@@ -14882,34 +14651,6 @@ void intel_init_display_hooks(struct drm_i915_private *dev_priv)
 		dev_priv->display.modeset_calc_cdclk =
 			broxton_modeset_calc_cdclk;
 	}
-
-	switch (INTEL_INFO(dev_priv)->gen) {
-	case 2:
-		dev_priv->display.queue_flip = intel_gen2_queue_flip;
-		break;
-
-	case 3:
-		dev_priv->display.queue_flip = intel_gen3_queue_flip;
-		break;
-
-	case 4:
-	case 5:
-		dev_priv->display.queue_flip = intel_gen4_queue_flip;
-		break;
-
-	case 6:
-		dev_priv->display.queue_flip = intel_gen6_queue_flip;
-		break;
-	case 7:
-	case 8: /* FIXME(BDW): Check that the gen8 RCS flip works. */
-		dev_priv->display.queue_flip = intel_gen7_queue_flip;
-		break;
-	case 9:
-		/* Drop through - unsupported since execlist only. */
-	default:
-		/* Default just returns -ENODEV to indicate unsupported */
-		break;
-	}
 }
 
 /*
-- 
2.1.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

✗ Fi.CI.BAT: failure for drm/i915: Rework page flip to be more atomic like, and remove cs flips.

[Patchwork]

== Series Details ==

Series: drm/i915: Rework page flip to be more atomic like, and remove cs flips.
URL   : https://patchwork.freedesktop.org/series/5647/
State : failure

== Summary ==

Series 5647v1 drm/i915: Rework page flip to be more atomic like, and remove cs flips.
http://patchwork.freedesktop.org/api/1.0/series/5647/revisions/1/mbox/

Test gem_exec_suspend:
        Subgroup basic-s3:
                incomplete -> PASS       (hsw-gt2)
Test kms_flip:
        Subgroup basic-flip-vs-dpms:
                pass       -> DMESG-FAIL (skl-i7k-2)

bdw-nuci7        total:203  pass:191  dwarn:0   dfail:0   fail:0   skip:12 
bdw-ultra        total:203  pass:180  dwarn:0   dfail:0   fail:0   skip:23 
bsw-nuc-2        total:202  pass:163  dwarn:0   dfail:0   fail:0   skip:39 
byt-nuc          total:202  pass:164  dwarn:0   dfail:0   fail:0   skip:38 
hsw-brixbox      total:203  pass:179  dwarn:0   dfail:0   fail:0   skip:24 
hsw-gt2          total:203  pass:184  dwarn:0   dfail:0   fail:0   skip:19 
ilk-hp8440p      total:203  pass:135  dwarn:0   dfail:0   fail:0   skip:68 
ivb-t430s        total:203  pass:175  dwarn:0   dfail:0   fail:0   skip:28 
skl-i7k-2        total:203  pass:177  dwarn:0   dfail:1   fail:0   skip:25 
skl-nuci5        total:203  pass:192  dwarn:0   dfail:0   fail:0   skip:11 
snb-x220t        total:203  pass:165  dwarn:0   dfail:0   fail:1   skip:37 
BOOT FAILED for snb-dellxps

Results at /archive/results/CI_IGT_test/Patchwork_1885/

631ffd2f45bb43964f729e8661532fb115f5eeec drm-intel-nightly: 2016y-04m-13d-13h-00m-18s UTC integration manifest
c6140c2 drm/i915: Remove queue_flip pointer.
99fa013 drm/i915: Remove use_mmio_flip kernel parameter.
5f9f5e1 drm/i915: Remove cs based page flip support.
3914993 drm/i915: Rework intel_crtc_page_flip to be almost atomic, v3.
5b90d28 drm/i915: Add the exclusive fence to plane_state.
cb36a16 drm/i915: Convert flip_work to a list.
f34a39b drm/i915: Allow mmio updates on all platforms, v2.
0f894e7 drm/i915: Add support for detecting vblanks when hw frame counter is unavailable.
a794120 drm/i915: Remove intel_prepare_page_flip.
7df2297 drm/i915: Remove stallcheck special handling.
810b37f drm/core: Add drm_accurate_vblank_count, v4.

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

Re: [PATCH v2 02/11] drm/i915: Remove stallcheck special handling.

[Ander Conselvan De Oliveira]

On Wed, 2016-04-13 at 11:18 +0200, Maarten Lankhorst wrote:
> Re-use unpin_work->pending, but also set vblank count before
> intel_mark_page_flip_active to be sure.

Be sure of what?

> 
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_debugfs.c  | 11 ++++++-----
>  drivers/gpu/drm/i915/intel_display.c | 31 ++++++++++++-------------------
>  drivers/gpu/drm/i915/intel_drv.h     |  1 -
>  3 files changed, 18 insertions(+), 25 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
> b/drivers/gpu/drm/i915/i915_debugfs.c
> index 9640738aabf2..df8073a2ffbe 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -582,9 +582,14 @@ 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 pending;
>  			u32 addr;
>  
> -			if (atomic_read(&work->pending) <
> INTEL_FLIP_COMPLETE) {
> +			pending = atomic_read(&work->pending);
> +			if (pending == INTEL_FLIP_INACTIVE) {
> +				seq_printf(m, "Flip ioctl preparing on pipe
> %c (plane %c)\n",
> +					   pipe, plane);
> +			} else if (pending >= INTEL_FLIP_COMPLETE) {
>  				seq_printf(m, "Flip queued on pipe %c (plane
> %c)\n",
>  					   pipe, plane);
>  			} else {
> @@ -606,10 +611,6 @@ static int i915_gem_pageflip_info(struct seq_file *m,
> void *data)
>  				   work->flip_queued_vblank,
>  				   work->flip_ready_vblank,
>  				   drm_crtc_vblank_count(&crtc->base));
> -			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 (INTEL_INFO(dev)->gen >= 4)
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index f2be54a48727..618e034a7a5e 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -11415,8 +11415,6 @@ static void intel_do_mmio_flip(struct intel_mmio_flip
> *mmio_flip)
>  	if (work == NULL)
>  		return;
>  
> -	intel_mark_page_flip_active(work);
> -
>  	intel_pipe_update_start(crtc);
>  
>  	if (INTEL_INFO(mmio_flip->i915)->gen >= 9)
> @@ -11426,6 +11424,8 @@ static void intel_do_mmio_flip(struct intel_mmio_flip
> *mmio_flip)
>  		ilk_do_mmio_flip(crtc, work);
>  
>  	intel_pipe_update_end(crtc);
> +
> +	intel_mark_page_flip_active(work);

Is this to avoid triggering the stall check during the wait from a vblank
evasion?


>  }
>  
>  static void intel_mmio_flip_work_func(struct work_struct *work)
> @@ -11492,15 +11492,11 @@ static bool __intel_pageflip_stall_check(struct
> drm_device *dev,
>  	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
>  	struct intel_unpin_work *work = intel_crtc->unpin_work;
>  	u32 addr;
> +	u32 pending;
>  
> -	if (atomic_read(&work->pending) >= INTEL_FLIP_COMPLETE)
> -		return true;
> -
> -	if (atomic_read(&work->pending) < INTEL_FLIP_PENDING)
> -		return false;
> -
> -	if (!work->enable_stall_check)
> -		return false;
> +	pending = atomic_read(&work->pending);
> +	if (pending != INTEL_FLIP_PENDING)
> +		return pending == INTEL_FLIP_COMPLETE;
>  
>  	if (work->flip_ready_vblank == 0) {
>  		if (work->flip_queued_req &&
> @@ -11676,6 +11672,11 @@ static int intel_crtc_page_flip(struct drm_crtc
> *crtc,
>  	 */
>  	if (!mmio_flip) {
>  		ret = i915_gem_object_sync(obj, engine, &request);
> +		if (!ret && !request) {
> +			request = i915_gem_request_alloc(engine, NULL);
> +			ret = PTR_ERR_OR_ZERO(request);
> +		}
> +
>  		if (ret)
>  			goto cleanup_pending;
>  	}
> @@ -11687,6 +11688,7 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
>  	work->gtt_offset = intel_plane_obj_offset(to_intel_plane(primary),
>  						  obj, 0);
>  	work->gtt_offset += intel_crtc->dspaddr_offset;
> +	work->flip_queued_vblank = drm_crtc_vblank_count(crtc);
>  
>  	if (mmio_flip) {
>  		ret = intel_queue_mmio_flip(dev, crtc, obj);
> @@ -11696,14 +11698,6 @@ static int intel_crtc_page_flip(struct drm_crtc
> *crtc,
>  		i915_gem_request_assign(&work->flip_queued_req,
>  					obj->last_write_req);
>  	} else {
> -		if (!request) {
> -			request = i915_gem_request_alloc(engine, NULL);
> -			if (IS_ERR(request)) {
> -				ret = PTR_ERR(request);
> -				goto cleanup_unpin;
> -			}
> -		}
> -
>  		ret = dev_priv->display.queue_flip(dev, crtc, fb, obj,
> request,
>  						   page_flip_flags);
>  		if (ret)
> @@ -11716,7 +11710,6 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
>  		i915_add_request_no_flush(request);
>  
>  	work->flip_queued_vblank = drm_crtc_vblank_count(crtc);

Do we still need the assigment above?

Ander

> -	work->enable_stall_check = true;
>  
>  	i915_gem_track_fb(intel_fb_obj(work->old_fb), obj,
>  			  to_intel_plane(primary)->frontbuffer_bit);
> diff --git a/drivers/gpu/drm/i915/intel_drv.h
> b/drivers/gpu/drm/i915/intel_drv.h
> index e0fcfa1683cc..ce2a6d985bbe 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -952,7 +952,6 @@ struct intel_unpin_work {
>  	struct drm_i915_gem_request *flip_queued_req;
>  	u32 flip_queued_vblank;
>  	u32 flip_ready_vblank;
> -	bool enable_stall_check;
>  };
>  
>  struct intel_load_detect_pipe {
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 03/11] drm/i915: Remove intel_prepare_page_flip.

[Ander Conselvan De Oliveira]

On Wed, 2016-04-13 at 11:18 +0200, Maarten Lankhorst wrote:
> Do it in 1 step instead, use atomic_read since INTEL_FLIP_COMPLETE
> is no longer useful.

What's the deal with "use atomic_read"? I couldn't find where that is different
from the old code.


> 
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_debugfs.c  |  3 --
>  drivers/gpu/drm/i915/i915_irq.c      | 18 ++-----
>  drivers/gpu/drm/i915/intel_display.c | 96 ++++++++++++++---------------------
> -
>  drivers/gpu/drm/i915/intel_drv.h     |  2 -
>  4 files changed, 40 insertions(+), 79 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
> b/drivers/gpu/drm/i915/i915_debugfs.c
> index df8073a2ffbe..c3b029e7bffd 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -589,9 +589,6 @@ static int i915_gem_pageflip_info(struct seq_file *m, void
> *data)
>  			if (pending == INTEL_FLIP_INACTIVE) {
>  				seq_printf(m, "Flip ioctl preparing on pipe
> %c (plane %c)\n",
>  					   pipe, plane);
> -			} else if (pending >= INTEL_FLIP_COMPLETE) {
> -				seq_printf(m, "Flip queued on pipe %c (plane
> %c)\n",
> -					   pipe, plane);
>  			} else {
>  				seq_printf(m, "Flip pending (waiting for
> vsync) on pipe %c (plane %c)\n",
>  					   pipe, plane);
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index 679f08c944ef..5ed2f73a7ea9 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -1707,10 +1707,8 @@ static void valleyview_pipestat_irq_handler(struct
> drm_device *dev, u32 iir)
>  		    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);
> +		if (pipe_stats[pipe] & PLANE_FLIP_DONE_INT_STATUS_VLV)
>  			intel_finish_page_flip(dev, pipe);
> -		}
>  
>  		if (pipe_stats[pipe] & PIPE_CRC_DONE_INTERRUPT_STATUS)
>  			i9xx_pipe_crc_irq_handler(dev, pipe);
> @@ -2109,10 +2107,8 @@ static void ilk_display_irq_handler(struct drm_device
> *dev, u32 de_iir)
>  			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);
> +		if (de_iir & DE_PLANE_FLIP_DONE(pipe))
>  			intel_finish_page_flip_plane(dev, pipe);
> -		}
>  	}
>  
>  	/* check event from PCH */
> @@ -2156,10 +2152,8 @@ static void ivb_display_irq_handler(struct drm_device
> *dev, u32 de_iir)
>  			intel_check_page_flip(dev, pipe);
>  
>  		/* plane/pipes map 1:1 on ilk+ */
> -		if (de_iir & DE_PLANE_FLIP_DONE_IVB(pipe)) {
> -			intel_prepare_page_flip(dev, pipe);
> +		if (de_iir & DE_PLANE_FLIP_DONE_IVB(pipe))
>  			intel_finish_page_flip_plane(dev, pipe);
> -		}
>  	}
>  
>  	/* check event from PCH */
> @@ -2363,10 +2357,8 @@ gen8_de_irq_handler(struct drm_i915_private *dev_priv,
> u32 master_ctl)
>  		else
>  			flip_done &= GEN8_PIPE_PRIMARY_FLIP_DONE;
>  
> -		if (flip_done) {
> -			intel_prepare_page_flip(dev, pipe);
> +		if (flip_done)
>  			intel_finish_page_flip_plane(dev, pipe);
> -		}
>  
>  		if (iir & GEN8_PIPE_CDCLK_CRC_DONE)
>  			hsw_pipe_crc_irq_handler(dev, pipe);
> @@ -4024,7 +4016,6 @@ static bool i8xx_handle_vblank(struct drm_device *dev,
>  	if (I915_READ16(ISR) & flip_pending)
>  		goto check_page_flip;
>  
> -	intel_prepare_page_flip(dev, plane);
>  	intel_finish_page_flip(dev, pipe);
>  	return true;
>  
> @@ -4215,7 +4206,6 @@ static bool i915_handle_vblank(struct drm_device *dev,
>  	if (I915_READ(ISR) & flip_pending)
>  		goto check_page_flip;
>  
> -	intel_prepare_page_flip(dev, plane);
>  	intel_finish_page_flip(dev, pipe);
>  	return true;
>  
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index 618e034a7a5e..dc42335409b3 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -3101,7 +3101,6 @@ static void intel_complete_page_flips(struct drm_device
> *dev)
>  		struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
>  		enum plane plane = intel_crtc->plane;
>  
> -		intel_prepare_page_flip(dev, plane);
>  		intel_finish_page_flip_plane(dev, plane);
>  	}
>  }
> @@ -10925,53 +10924,6 @@ static void intel_unpin_work_fn(struct work_struct
> *__work)
>  	kfree(work);
>  }
>  
> -static void do_intel_finish_page_flip(struct drm_device *dev,
> -				      struct drm_crtc *crtc)
> -{
> -	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> -	struct intel_unpin_work *work;
> -	unsigned long flags;
> -
> -	/* Ignore early vblank irqs */
> -	if (intel_crtc == NULL)
> -		return;
> -
> -	/*
> -	 * This is called both by irq handlers and the reset code (to
> complete
> -	 * lost pageflips) so needs the full irqsave spinlocks.
> -	 */
> -	spin_lock_irqsave(&dev->event_lock, flags);
> -	work = intel_crtc->unpin_work;
> -
> -	/* Ensure we don't miss a work->pending update ... */
> -	smp_rmb();

If this is not needed anymore, should the matching smp_wmb() from 
intel_mark_page_flip_active() be removed?


> -
> -	if (work == NULL || atomic_read(&work->pending) <
> INTEL_FLIP_COMPLETE) {
> -		spin_unlock_irqrestore(&dev->event_lock, flags);
> -		return;
> -	}
> -
> -	page_flip_completed(intel_crtc);
> -
> -	spin_unlock_irqrestore(&dev->event_lock, flags);
> -}
> -
> -void intel_finish_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];
> -
> -	do_intel_finish_page_flip(dev, crtc);
> -}
> -
> -void intel_finish_page_flip_plane(struct drm_device *dev, int plane)
> -{
> -	struct drm_i915_private *dev_priv = dev->dev_private;
> -	struct drm_crtc *crtc = dev_priv->plane_to_crtc_mapping[plane];
> -
> -	do_intel_finish_page_flip(dev, crtc);
> -}
> -
>  /* Is 'a' after or equal to 'b'? */
>  static bool g4x_flip_count_after_eq(u32 a, u32 b)
>  {
> @@ -11024,28 +10976,52 @@ static bool page_flip_finished(struct intel_crtc
> *crtc)
>  				    crtc->unpin_work->flip_count);
>  }
>  
> -void intel_prepare_page_flip(struct drm_device *dev, int plane)
> +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(dev_priv->plane_to_crtc_mapping[plane]);
> +	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> +	struct intel_unpin_work *work;
>  	unsigned long flags;
>  
> +	/* Ignore early vblank irqs */
> +	if (intel_crtc == NULL)
> +		return;
>  
>  	/*
>  	 * This is called both by irq handlers and the reset code (to
> complete
>  	 * lost pageflips) so needs the full irqsave spinlocks.
> -	 *
> -	 * NB: An MMIO update of the plane base pointer will also
> -	 * generate a page-flip completion irq, i.e. every modeset
> -	 * is also accompanied by a spurious intel_prepare_page_flip().
>  	 */
>  	spin_lock_irqsave(&dev->event_lock, flags);
> -	if (intel_crtc->unpin_work && page_flip_finished(intel_crtc))
> -		atomic_inc_not_zero(&intel_crtc->unpin_work->pending);
> +	work = intel_crtc->unpin_work;
> +
> +	if (work == NULL ||
> +	    atomic_read(&work->pending) == INTEL_FLIP_INACTIVE ||
> +	   !page_flip_finished(intel_crtc)) {

This line is misaligned. Also, if the condition is was inverted you could save
the extra unlock and return.


Ander

> +		spin_unlock_irqrestore(&dev->event_lock, flags);
> +		return;
> +	}
> +
> +	page_flip_completed(intel_crtc);
> +
>  	spin_unlock_irqrestore(&dev->event_lock, flags);
>  }
>  
> +void intel_finish_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];
> +
> +	do_intel_finish_page_flip(dev, crtc);
> +}
> +
> +void intel_finish_page_flip_plane(struct drm_device *dev, int plane)
> +{
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	struct drm_crtc *crtc = dev_priv->plane_to_crtc_mapping[plane];
> +
> +	do_intel_finish_page_flip(dev, crtc);
> +}
> +
>  static inline void intel_mark_page_flip_active(struct intel_unpin_work *work)
>  {
>  	/* Ensure that the work item is consistent when activating it ... */
> @@ -11495,8 +11471,8 @@ static bool __intel_pageflip_stall_check(struct
> drm_device *dev,
>  	u32 pending;
>  
>  	pending = atomic_read(&work->pending);
> -	if (pending != INTEL_FLIP_PENDING)
> -		return pending == INTEL_FLIP_COMPLETE;
> +	if (pending == INTEL_FLIP_INACTIVE)
> +		return false;
>  
>  	if (work->flip_ready_vblank == 0) {
>  		if (work->flip_queued_req &&
> diff --git a/drivers/gpu/drm/i915/intel_drv.h
> b/drivers/gpu/drm/i915/intel_drv.h
> index ce2a6d985bbe..a74edecfd179 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -946,7 +946,6 @@ struct intel_unpin_work {
>  	atomic_t pending;
>  #define INTEL_FLIP_INACTIVE	0
>  #define INTEL_FLIP_PENDING	1
> -#define INTEL_FLIP_COMPLETE	2
>  	u32 flip_count;
>  	u32 gtt_offset;
>  	struct drm_i915_gem_request *flip_queued_req;
> @@ -1159,7 +1158,6 @@ struct drm_framebuffer *
>  __intel_framebuffer_create(struct drm_device *dev,
>  			   struct drm_mode_fb_cmd2 *mode_cmd,
>  			   struct drm_i915_gem_object *obj);
> -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);
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 05/11] drm/i915: Allow mmio updates on all platforms, v2.

[Ander Conselvan De Oliveira]

On Wed, 2016-04-13 at 11:18 +0200, Maarten Lankhorst wrote:
> Rename intel_unpin_work to intel_flip_work and use it for mmio flips
> and unpinning.

I think the rename should be a separate patch.

Ander

>  Use flip_queued_req to hold the wait request in the
> mmio case and allow the vblank interrupt to complete mmio work to
> have mmio flips run correctly on g4 and earlier.
> 
> Changes since v1:
> - Add smp_mb__after_atomic() to __intel_pageflip_stall_check,
>   to match the smp_mb__before_atomic() in pipe_update_end().
> - Check for cur != flip_queued_vblank in pageflip_stall_check.
> 
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_debugfs.c  |   4 +-
>  drivers/gpu/drm/i915/intel_display.c | 271 +++++++++++-----------------------
> -
>  drivers/gpu/drm/i915/intel_drv.h     |  18 +--
>  drivers/gpu/drm/i915/intel_sprite.c  |   8 +-
>  4 files changed, 95 insertions(+), 206 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
> b/drivers/gpu/drm/i915/i915_debugfs.c
> index c3b029e7bffd..5662cd5a1a9d 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -574,10 +574,10 @@ static int i915_gem_pageflip_info(struct seq_file *m,
> void *data)
>  	for_each_intel_crtc(dev, crtc) {
>  		const char pipe = pipe_name(crtc->pipe);
>  		const char plane = plane_name(crtc->plane);
> -		struct intel_unpin_work *work;
> +		struct intel_flip_work *work;
>  
>  		spin_lock_irq(&dev->event_lock);
> -		work = crtc->unpin_work;
> +		work = crtc->flip_work;
>  		if (work == NULL) {
>  			seq_printf(m, "No flip due on pipe %c (plane %c)\n",
>  				   pipe, plane);
> diff --git a/drivers/gpu/drm/i915/intel_display.c
> b/drivers/gpu/drm/i915/intel_display.c
> index f1a895153e64..b614f118b973 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -48,6 +48,11 @@
>  #include <linux/reservation.h>
>  #include <linux/dma-buf.h>
>  
> +static bool is_mmio_work(struct intel_flip_work *work)
> +{
> +	return work->mmio_work.func;
> +}
> +
>  /* Primary plane formats for gen <= 3 */
>  static const uint32_t i8xx_primary_formats[] = {
>  	DRM_FORMAT_C8,
> @@ -3285,7 +3290,7 @@ static bool intel_crtc_has_pending_flip(struct drm_crtc
> *crtc)
>  		return false;
>  
>  	spin_lock_irq(&dev->event_lock);
> -	pending = to_intel_crtc(crtc)->unpin_work != NULL;
> +	pending = to_intel_crtc(crtc)->flip_work != NULL;
>  	spin_unlock_irq(&dev->event_lock);
>  
>  	return pending;
> @@ -3864,7 +3869,7 @@ bool intel_has_pending_fb_unpin(struct drm_device *dev)
>  		if (atomic_read(&crtc->unpin_work_count) == 0)
>  			continue;
>  
> -		if (crtc->unpin_work)
> +		if (crtc->flip_work)
>  			intel_wait_for_vblank(dev, crtc->pipe);
>  
>  		return true;
> @@ -3876,11 +3881,11 @@ bool intel_has_pending_fb_unpin(struct drm_device
> *dev)
>  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;
> +	struct intel_flip_work *work = intel_crtc->flip_work;
>  
> -	/* ensure that the unpin work is consistent wrt ->pending. */
> +	/* ensure that the flip work is consistent wrt ->pending. */
>  	smp_rmb();
> -	intel_crtc->unpin_work = NULL;
> +	intel_crtc->flip_work = NULL;
>  
>  	if (work->event)
>  		drm_send_vblank_event(intel_crtc->base.dev,
> @@ -3890,7 +3895,7 @@ static void page_flip_completed(struct intel_crtc
> *intel_crtc)
>  	drm_crtc_vblank_put(&intel_crtc->base);
>  
>  	wake_up_all(&dev_priv->pending_flip_queue);
> -	queue_work(dev_priv->wq, &work->work);
> +	queue_work(dev_priv->wq, &work->unpin_work);
>  
>  	trace_i915_flip_complete(intel_crtc->plane,
>  				 work->pending_flip_obj);
> @@ -3914,9 +3919,11 @@ static int intel_crtc_wait_for_pending_flips(struct
> drm_crtc *crtc)
>  
>  	if (ret == 0) {
>  		struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> +		struct intel_flip_work *work;
>  
>  		spin_lock_irq(&dev->event_lock);
> -		if (intel_crtc->unpin_work) {
> +		work = intel_crtc->flip_work;
> +		if (work && !is_mmio_work(work)) {
>  			WARN_ONCE(1, "Removing stuck page flip\n");
>  			page_flip_completed(intel_crtc);
>  		}
> @@ -6308,7 +6315,7 @@ static void intel_crtc_disable_noatomic(struct drm_crtc
> *crtc)
>  		return;
>  
>  	if (to_intel_plane_state(crtc->primary->state)->visible) {
> -		WARN_ON(intel_crtc->unpin_work);
> +		WARN_ON(intel_crtc->flip_work);
>  
>  		intel_pre_disable_primary_noatomic(crtc);
>  
> @@ -10881,15 +10888,16 @@ static void intel_crtc_destroy(struct drm_crtc
> *crtc)
>  {
>  	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
>  	struct drm_device *dev = crtc->dev;
> -	struct intel_unpin_work *work;
> +	struct intel_flip_work *work;
>  
>  	spin_lock_irq(&dev->event_lock);
> -	work = intel_crtc->unpin_work;
> -	intel_crtc->unpin_work = NULL;
> +	work = intel_crtc->flip_work;
> +	intel_crtc->flip_work = NULL;
>  	spin_unlock_irq(&dev->event_lock);
>  
>  	if (work) {
> -		cancel_work_sync(&work->work);
> +		cancel_work_sync(&work->mmio_work);
> +		cancel_work_sync(&work->unpin_work);
>  		kfree(work);
>  	}
>  
> @@ -10900,12 +10908,15 @@ static void intel_crtc_destroy(struct drm_crtc
> *crtc)
>  
>  static void intel_unpin_work_fn(struct work_struct *__work)
>  {
> -	struct intel_unpin_work *work =
> -		container_of(__work, struct intel_unpin_work, work);
> +	struct intel_flip_work *work =
> +		container_of(__work, struct intel_flip_work, unpin_work);
>  	struct intel_crtc *crtc = to_intel_crtc(work->crtc);
>  	struct drm_device *dev = crtc->base.dev;
>  	struct drm_plane *primary = crtc->base.primary;
>  
> +	if (is_mmio_work(work))
> +		flush_work(&work->mmio_work);
> +
>  	mutex_lock(&dev->struct_mutex);
>  	intel_unpin_fb_obj(work->old_fb, primary->state->rotation);
>  	drm_gem_object_unreference(&work->pending_flip_obj->base);
> @@ -10971,16 +10982,16 @@ static bool page_flip_finished(struct intel_crtc
> *crtc)
>  	 * anyway, we don't really care.
>  	 */
>  	return (I915_READ(DSPSURFLIVE(crtc->plane)) & ~0xfff) ==
> -		crtc->unpin_work->gtt_offset &&
> +		crtc->flip_work->gtt_offset &&
>  		g4x_flip_count_after_eq(I915_READ(PIPE_FLIPCOUNT_G4X(crtc
> ->pipe)),
> -				    crtc->unpin_work->flip_count);
> +				    crtc->flip_work->flip_count);
>  }
>  
>  static void do_intel_finish_page_flip(struct drm_device *dev,
>  				      struct drm_crtc *crtc)
>  {
>  	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> -	struct intel_unpin_work *work;
> +	struct intel_flip_work *work;
>  	unsigned long flags;
>  
>  	/* Ignore early vblank irqs */
> @@ -10992,11 +11003,11 @@ static void do_intel_finish_page_flip(struct
> drm_device *dev,
>  	 * lost pageflips) so needs the full irqsave spinlocks.
>  	 */
>  	spin_lock_irqsave(&dev->event_lock, flags);
> -	work = intel_crtc->unpin_work;
> +	work = intel_crtc->flip_work;
>  
>  	if (work == NULL ||
>  	    atomic_read(&work->pending) == INTEL_FLIP_INACTIVE ||
> -	   !page_flip_finished(intel_crtc)) {
> +	    !page_flip_finished(intel_crtc)) {
>  		spin_unlock_irqrestore(&dev->event_lock, flags);
>  		return;
>  	}
> @@ -11022,7 +11033,7 @@ void intel_finish_page_flip_plane(struct drm_device
> *dev, int plane)
>  	do_intel_finish_page_flip(dev, crtc);
>  }
>  
> -static inline void intel_mark_page_flip_active(struct intel_unpin_work *work)
> +static inline void intel_mark_page_flip_active(struct intel_flip_work *work)
>  {
>  	/* Ensure that the work item is consistent when activating it ... */
>  	smp_wmb();
> @@ -11059,10 +11070,9 @@ static int intel_gen2_queue_flip(struct drm_device
> *dev,
>  	intel_ring_emit(engine, MI_DISPLAY_FLIP |
>  			MI_DISPLAY_FLIP_PLANE(intel_crtc->plane));
>  	intel_ring_emit(engine, fb->pitches[0]);
> -	intel_ring_emit(engine, intel_crtc->unpin_work->gtt_offset);
> +	intel_ring_emit(engine, intel_crtc->flip_work->gtt_offset);
>  	intel_ring_emit(engine, 0); /* aux display base address, unused */
>  
> -	intel_mark_page_flip_active(intel_crtc->unpin_work);
>  	return 0;
>  }
>  
> @@ -11091,10 +11101,9 @@ static int intel_gen3_queue_flip(struct drm_device
> *dev,
>  	intel_ring_emit(engine, MI_DISPLAY_FLIP_I915 |
>  			MI_DISPLAY_FLIP_PLANE(intel_crtc->plane));
>  	intel_ring_emit(engine, fb->pitches[0]);
> -	intel_ring_emit(engine, intel_crtc->unpin_work->gtt_offset);
> +	intel_ring_emit(engine, intel_crtc->flip_work->gtt_offset);
>  	intel_ring_emit(engine, MI_NOOP);
>  
> -	intel_mark_page_flip_active(intel_crtc->unpin_work);
>  	return 0;
>  }
>  
> @@ -11122,7 +11131,7 @@ static int intel_gen4_queue_flip(struct drm_device
> *dev,
>  	intel_ring_emit(engine, MI_DISPLAY_FLIP |
>  			MI_DISPLAY_FLIP_PLANE(intel_crtc->plane));
>  	intel_ring_emit(engine, fb->pitches[0]);
> -	intel_ring_emit(engine, intel_crtc->unpin_work->gtt_offset |
> +	intel_ring_emit(engine, intel_crtc->flip_work->gtt_offset |
>  			obj->tiling_mode);
>  
>  	/* XXX Enabling the panel-fitter across page-flip is so far
> @@ -11133,7 +11142,6 @@ static int intel_gen4_queue_flip(struct drm_device
> *dev,
>  	pipesrc = I915_READ(PIPESRC(intel_crtc->pipe)) & 0x0fff0fff;
>  	intel_ring_emit(engine, pf | pipesrc);
>  
> -	intel_mark_page_flip_active(intel_crtc->unpin_work);
>  	return 0;
>  }
>  
> @@ -11157,7 +11165,7 @@ static int intel_gen6_queue_flip(struct drm_device
> *dev,
>  	intel_ring_emit(engine, MI_DISPLAY_FLIP |
>  			MI_DISPLAY_FLIP_PLANE(intel_crtc->plane));
>  	intel_ring_emit(engine, fb->pitches[0] | obj->tiling_mode);
> -	intel_ring_emit(engine, intel_crtc->unpin_work->gtt_offset);
> +	intel_ring_emit(engine, intel_crtc->flip_work->gtt_offset);
>  
>  	/* Contrary to the suggestions in the documentation,
>  	 * "Enable Panel Fitter" does not seem to be required when page
> @@ -11169,7 +11177,6 @@ static int intel_gen6_queue_flip(struct drm_device
> *dev,
>  	pipesrc = I915_READ(PIPESRC(intel_crtc->pipe)) & 0x0fff0fff;
>  	intel_ring_emit(engine, pf | pipesrc);
>  
> -	intel_mark_page_flip_active(intel_crtc->unpin_work);
>  	return 0;
>  }
>  
> @@ -11261,10 +11268,9 @@ static int intel_gen7_queue_flip(struct drm_device
> *dev,
>  
>  	intel_ring_emit(engine, MI_DISPLAY_FLIP_I915 | plane_bit);
>  	intel_ring_emit(engine, (fb->pitches[0] | obj->tiling_mode));
> -	intel_ring_emit(engine, intel_crtc->unpin_work->gtt_offset);
> +	intel_ring_emit(engine, intel_crtc->flip_work->gtt_offset);
>  	intel_ring_emit(engine, (MI_NOOP));
>  
> -	intel_mark_page_flip_active(intel_crtc->unpin_work);
>  	return 0;
>  }
>  
> @@ -11282,9 +11288,6 @@ static bool use_mmio_flip(struct intel_engine_cs
> *engine,
>  	if (engine == NULL)
>  		return true;
>  
> -	if (INTEL_INFO(engine->dev)->gen < 5)
> -		return false;
> -
>  	if (i915.use_mmio_flip < 0)
>  		return false;
>  	else if (i915.use_mmio_flip > 0)
> @@ -11299,126 +11302,21 @@ static bool use_mmio_flip(struct intel_engine_cs
> *engine,
>  		return engine != i915_gem_request_get_engine(obj
> ->last_write_req);
>  }
>  
> -static void skl_do_mmio_flip(struct intel_crtc *intel_crtc,
> -			     unsigned int rotation,
> -			     struct intel_unpin_work *work)
> +static void intel_mmio_flip_work_func(struct work_struct *w)
>  {
> -	struct drm_device *dev = intel_crtc->base.dev;
> -	struct drm_i915_private *dev_priv = dev->dev_private;
> -	struct drm_framebuffer *fb = intel_crtc->base.primary->fb;
> -	const enum pipe pipe = intel_crtc->pipe;
> -	u32 ctl, stride, tile_height;
> -
> -	ctl = I915_READ(PLANE_CTL(pipe, 0));
> -	ctl &= ~PLANE_CTL_TILED_MASK;
> -	switch (fb->modifier[0]) {
> -	case DRM_FORMAT_MOD_NONE:
> -		break;
> -	case I915_FORMAT_MOD_X_TILED:
> -		ctl |= PLANE_CTL_TILED_X;
> -		break;
> -	case I915_FORMAT_MOD_Y_TILED:
> -		ctl |= PLANE_CTL_TILED_Y;
> -		break;
> -	case I915_FORMAT_MOD_Yf_TILED:
> -		ctl |= PLANE_CTL_TILED_YF;
> -		break;
> -	default:
> -		MISSING_CASE(fb->modifier[0]);
> -	}
> -
> -	/*
> -	 * The stride is either expressed as a multiple of 64 bytes chunks
> for
> -	 * linear buffers or in number of tiles for tiled buffers.
> -	 */
> -	if (intel_rotation_90_or_270(rotation)) {
> -		/* stride = Surface height in tiles */
> -		tile_height = intel_tile_height(dev_priv, fb->modifier[0],
> 0);
> -		stride = DIV_ROUND_UP(fb->height, tile_height);
> -	} else {
> -		stride = fb->pitches[0] /
> -			intel_fb_stride_alignment(dev_priv, fb->modifier[0],
> -						  fb->pixel_format);
> -	}
> -
> -	/*
> -	 * Both PLANE_CTL and PLANE_STRIDE are not updated on vblank but on
> -	 * PLANE_SURF updates, the update is then guaranteed to be atomic.
> -	 */
> -	I915_WRITE(PLANE_CTL(pipe, 0), ctl);
> -	I915_WRITE(PLANE_STRIDE(pipe, 0), stride);
> -
> -	I915_WRITE(PLANE_SURF(pipe, 0), work->gtt_offset);
> -	POSTING_READ(PLANE_SURF(pipe, 0));
> -}
> -
> -static void ilk_do_mmio_flip(struct intel_crtc *intel_crtc,
> -			     struct intel_unpin_work *work)
> -{
> -	struct drm_device *dev = intel_crtc->base.dev;
> +	struct intel_flip_work *work =
> +		container_of(w, struct intel_flip_work, mmio_work);
> +	struct intel_crtc *crtc = to_intel_crtc(work->crtc);
> +	struct drm_device *dev = crtc->base.dev;
>  	struct drm_i915_private *dev_priv = dev->dev_private;
> -	struct intel_framebuffer *intel_fb =
> -		to_intel_framebuffer(intel_crtc->base.primary->fb);
> -	struct drm_i915_gem_object *obj = intel_fb->obj;
> -	i915_reg_t reg = DSPCNTR(intel_crtc->plane);
> -	u32 dspcntr;
> -
> -	dspcntr = I915_READ(reg);
> -
> -	if (obj->tiling_mode != I915_TILING_NONE)
> -		dspcntr |= DISPPLANE_TILED;
> -	else
> -		dspcntr &= ~DISPPLANE_TILED;
> +	struct intel_plane *primary = to_intel_plane(crtc->base.primary);
> +	struct drm_i915_gem_object *obj = intel_fb_obj(primary->base.state
> ->fb);
>  
> -	I915_WRITE(reg, dspcntr);
> -
> -	I915_WRITE(DSPSURF(intel_crtc->plane), work->gtt_offset);
> -	POSTING_READ(DSPSURF(intel_crtc->plane));
> -}
> -
> -/*
> - * XXX: This is the temporary way to update the plane registers until we get
> - * around to using the usual plane update functions for MMIO flips
> - */
> -static void intel_do_mmio_flip(struct intel_mmio_flip *mmio_flip)
> -{
> -	struct intel_crtc *crtc = mmio_flip->crtc;
> -	struct intel_unpin_work *work;
> -
> -	spin_lock_irq(&crtc->base.dev->event_lock);
> -	work = crtc->unpin_work;
> -	spin_unlock_irq(&crtc->base.dev->event_lock);
> -	if (work == NULL)
> -		return;
> -
> -	intel_pipe_update_start(crtc);
> -
> -	if (INTEL_INFO(mmio_flip->i915)->gen >= 9)
> -		skl_do_mmio_flip(crtc, mmio_flip->rotation, work);
> -	else
> -		/* use_mmio_flip() retricts MMIO flips to ilk+ */
> -		ilk_do_mmio_flip(crtc, work);
> -
> -	intel_pipe_update_end(crtc);
> -
> -	intel_mark_page_flip_active(work);
> -}
> -
> -static void intel_mmio_flip_work_func(struct work_struct *work)
> -{
> -	struct intel_mmio_flip *mmio_flip =
> -		container_of(work, struct intel_mmio_flip, work);
> -	struct intel_framebuffer *intel_fb =
> -		to_intel_framebuffer(mmio_flip->crtc->base.primary->fb);
> -	struct drm_i915_gem_object *obj = intel_fb->obj;
> -
> -	if (mmio_flip->req) {
> -		WARN_ON(__i915_wait_request(mmio_flip->req,
> -					    mmio_flip->crtc->reset_counter,
> +	if (work->flip_queued_req)
> +		WARN_ON(__i915_wait_request(work->flip_queued_req,
> +					    crtc->reset_counter,
>  					    false, NULL,
> -					    &mmio_flip->i915
> ->rps.mmioflips));
> -		i915_gem_request_unreference__unlocked(mmio_flip->req);
> -	}
> +					    &dev_priv->rps.mmioflips));
>  
>  	/* For framebuffer backed by dmabuf, wait for fence */
>  	if (obj->base.dma_buf)
> @@ -11426,29 +11324,11 @@ static void intel_mmio_flip_work_func(struct
> work_struct *work)
>  							    false, false,
>  							   
>  MAX_SCHEDULE_TIMEOUT) < 0);
>  
> -	intel_do_mmio_flip(mmio_flip);
> -	kfree(mmio_flip);
> -}
> -
> -static int intel_queue_mmio_flip(struct drm_device *dev,
> -				 struct drm_crtc *crtc,
> -				 struct drm_i915_gem_object *obj)
> -{
> -	struct intel_mmio_flip *mmio_flip;
> -
> -	mmio_flip = kmalloc(sizeof(*mmio_flip), GFP_KERNEL);
> -	if (mmio_flip == NULL)
> -		return -ENOMEM;
> -
> -	mmio_flip->i915 = to_i915(dev);
> -	mmio_flip->req = i915_gem_request_reference(obj->last_write_req);
> -	mmio_flip->crtc = to_intel_crtc(crtc);
> -	mmio_flip->rotation = crtc->primary->state->rotation;
> -
> -	INIT_WORK(&mmio_flip->work, intel_mmio_flip_work_func);
> -	schedule_work(&mmio_flip->work);
> -
> -	return 0;
> +	intel_pipe_update_start(crtc);
> +	primary->update_plane(&primary->base,
> +			      crtc->config,
> +			      to_intel_plane_state(primary->base.state));
> +	intel_pipe_update_end(crtc, work);
>  }
>  
>  static int intel_default_queue_flip(struct drm_device *dev,
> @@ -11466,7 +11346,7 @@ static bool __intel_pageflip_stall_check(struct
> drm_device *dev,
>  {
>  	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;
> +	struct intel_flip_work *work = intel_crtc->flip_work;
>  	u32 addr;
>  	u32 pending;
>  
> @@ -11474,6 +11354,15 @@ static bool __intel_pageflip_stall_check(struct
> drm_device *dev,
>  	if (pending == INTEL_FLIP_INACTIVE)
>  		return false;
>  
> +	smp_mb__after_atomic();
> +
> +	if (is_mmio_work(work)) {
> +		u32 cur = intel_crtc_get_vblank_counter(intel_crtc);
> +
> +		/* MMIO work completes when vblank is different from
> flip_queued_vblank. */
> +		return cur != work->flip_queued_vblank;
> +	}
> +
>  	if (work->flip_ready_vblank == 0) {
>  		if (work->flip_queued_req &&
>  		    !i915_gem_request_completed(work->flip_queued_req, true))
> @@ -11504,7 +11393,7 @@ 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);
> -	struct intel_unpin_work *work;
> +	struct intel_flip_work *work;
>  
>  	WARN_ON(!in_interrupt());
>  
> @@ -11512,14 +11401,15 @@ void intel_check_page_flip(struct drm_device *dev,
> int pipe)
>  		return;
>  
>  	spin_lock(&dev->event_lock);
> -	work = intel_crtc->unpin_work;
> +	work = intel_crtc->flip_work;
>  	if (work != NULL && __intel_pageflip_stall_check(dev, crtc)) {
> -		WARN_ONCE(1, "Kicking stuck page flip: queued at %d, now
> %d\n",
> +		WARN_ONCE(!is_mmio_work(work),
> +			  "Kicking stuck page flip: queued at %d, now %d\n",
>  			 work->flip_queued_vblank, drm_vblank_count(dev,
> pipe));
>  		page_flip_completed(intel_crtc);
>  		work = NULL;
>  	}
> -	if (work != NULL &&
> +	if (work != NULL && !is_mmio_work(work) &&
>  	    drm_vblank_count(dev, pipe) - work->flip_queued_vblank > 1)
>  		intel_queue_rps_boost_for_request(dev, work
> ->flip_queued_req);
>  	spin_unlock(&dev->event_lock);
> @@ -11537,7 +11427,7 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
>  	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
>  	struct drm_plane *primary = crtc->primary;
>  	enum pipe pipe = intel_crtc->pipe;
> -	struct intel_unpin_work *work;
> +	struct intel_flip_work *work;
>  	struct intel_engine_cs *engine;
>  	bool mmio_flip;
>  	struct drm_i915_gem_request *request = NULL;
> @@ -11574,15 +11464,15 @@ static int intel_crtc_page_flip(struct drm_crtc
> *crtc,
>  	work->event = event;
>  	work->crtc = crtc;
>  	work->old_fb = old_fb;
> -	INIT_WORK(&work->work, intel_unpin_work_fn);
> +	INIT_WORK(&work->unpin_work, intel_unpin_work_fn);
>  
>  	ret = drm_crtc_vblank_get(crtc);
>  	if (ret)
>  		goto free_work;
>  
> -	/* We borrow the event spin lock for protecting unpin_work */
> +	/* We borrow the event spin lock for protecting flip_work */
>  	spin_lock_irq(&dev->event_lock);
> -	if (intel_crtc->unpin_work) {
> +	if (intel_crtc->flip_work) {
>  		/* Before declaring the flip queue wedged, check if
>  		 * the hardware completed the operation behind our backs.
>  		 */
> @@ -11598,7 +11488,7 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
>  			return -EBUSY;
>  		}
>  	}
> -	intel_crtc->unpin_work = work;
> +	intel_crtc->flip_work = work;
>  	spin_unlock_irq(&dev->event_lock);
>  
>  	if (atomic_read(&intel_crtc->unpin_work_count) >= 2)
> @@ -11667,23 +11557,22 @@ static int intel_crtc_page_flip(struct drm_crtc
> *crtc,
>  	work->flip_queued_vblank = drm_crtc_vblank_count(crtc);
>  
>  	if (mmio_flip) {
> -		ret = intel_queue_mmio_flip(dev, crtc, obj);
> -		if (ret)
> -			goto cleanup_unpin;
> +		INIT_WORK(&work->mmio_work, intel_mmio_flip_work_func);
>  
>  		i915_gem_request_assign(&work->flip_queued_req,
>  					obj->last_write_req);
> +
> +		schedule_work(&work->mmio_work);
>  	} else {
> +		i915_gem_request_assign(&work->flip_queued_req, request);
>  		ret = dev_priv->display.queue_flip(dev, crtc, fb, obj,
> request,
>  						   page_flip_flags);
>  		if (ret)
>  			goto cleanup_unpin;
>  
> -		i915_gem_request_assign(&work->flip_queued_req, request);
> -	}
> -
> -	if (request)
> +		intel_mark_page_flip_active(work);
>  		i915_add_request_no_flush(request);
> +	}
>  
>  	work->flip_queued_vblank = drm_crtc_vblank_count(crtc);
>  
> @@ -11713,7 +11602,7 @@ cleanup:
>  	drm_framebuffer_unreference(work->old_fb);
>  
>  	spin_lock_irq(&dev->event_lock);
> -	intel_crtc->unpin_work = NULL;
> +	intel_crtc->flip_work = NULL;
>  	spin_unlock_irq(&dev->event_lock);
>  
>  	drm_crtc_vblank_put(crtc);
> @@ -14018,7 +13907,7 @@ static void intel_finish_crtc_commit(struct drm_crtc
> *crtc,
>  {
>  	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
>  
> -	intel_pipe_update_end(intel_crtc);
> +	intel_pipe_update_end(intel_crtc, NULL);
>  }
>  
>  /**
> diff --git a/drivers/gpu/drm/i915/intel_drv.h
> b/drivers/gpu/drm/i915/intel_drv.h
> index a2712f6f1eb3..d46aff0350a0 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -592,14 +592,6 @@ struct vlv_wm_state {
>  	bool cxsr;
>  };
>  
> -struct intel_mmio_flip {
> -	struct work_struct work;
> -	struct drm_i915_private *i915;
> -	struct drm_i915_gem_request *req;
> -	struct intel_crtc *crtc;
> -	unsigned int rotation;
> -};
> -
>  struct intel_crtc {
>  	struct drm_crtc base;
>  	enum pipe pipe;
> @@ -614,7 +606,7 @@ struct intel_crtc {
>  	unsigned long enabled_power_domains;
>  	bool lowfreq_avail;
>  	struct intel_overlay *overlay;
> -	struct intel_unpin_work *unpin_work;
> +	struct intel_flip_work *flip_work;
>  
>  	atomic_t unpin_work_count;
>  
> @@ -937,8 +929,10 @@ intel_get_crtc_for_plane(struct drm_device *dev, int
> plane)
>  	return dev_priv->plane_to_crtc_mapping[plane];
>  }
>  
> -struct intel_unpin_work {
> -	struct work_struct work;
> +struct intel_flip_work {
> +	struct work_struct unpin_work;
> +	struct work_struct mmio_work;
> +
>  	struct drm_crtc *crtc;
>  	struct drm_framebuffer *old_fb;
>  	struct drm_i915_gem_object *pending_flip_obj;
> @@ -1618,7 +1612,7 @@ int intel_plane_init(struct drm_device *dev, enum pipe
> pipe, int plane);
>  int intel_sprite_set_colorkey(struct drm_device *dev, void *data,
>  			      struct drm_file *file_priv);
>  void intel_pipe_update_start(struct intel_crtc *crtc);
> -void intel_pipe_update_end(struct intel_crtc *crtc);
> +void intel_pipe_update_end(struct intel_crtc *crtc, struct intel_flip_work
> *work);
>  
>  /* intel_tv.c */
>  void intel_tv_init(struct drm_device *dev);
> diff --git a/drivers/gpu/drm/i915/intel_sprite.c
> b/drivers/gpu/drm/i915/intel_sprite.c
> index e2de6b0df5a8..8ec7ce549835 100644
> --- a/drivers/gpu/drm/i915/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> @@ -151,13 +151,19 @@ void intel_pipe_update_start(struct intel_crtc *crtc)
>   * re-enables interrupts and verifies the update was actually completed
>   * before a vblank using the value of @start_vbl_count.
>   */
> -void intel_pipe_update_end(struct intel_crtc *crtc)
> +void intel_pipe_update_end(struct intel_crtc *crtc, struct intel_flip_work
> *work)
>  {
>  	enum pipe pipe = crtc->pipe;
>  	int scanline_end = intel_get_crtc_scanline(crtc);
>  	u32 end_vbl_count = intel_crtc_get_vblank_counter(crtc);
>  	ktime_t end_vbl_time = ktime_get();
>  
> +	if (work) {
> +		work->flip_queued_vblank = end_vbl_count;
> +		smp_mb__before_atomic();
> +		atomic_set(&work->pending, INTEL_FLIP_PENDING);
> +	}
> +
>  	trace_i915_pipe_update_end(crtc, end_vbl_count, scanline_end);
>  
>  	local_irq_enable();
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 03/11] drm/i915: Remove intel_prepare_page_flip.

[Maarten Lankhorst]

Op 15-04-16 om 14:21 schreef Ander Conselvan De Oliveira:
> On Wed, 2016-04-13 at 11:18 +0200, Maarten Lankhorst wrote:
>> Do it in 1 step instead, use atomic_read since INTEL_FLIP_COMPLETE
>> is no longer useful.
> What's the deal with "use atomic_read"? I couldn't find where that is different
> from the old code.
This could probably use some rewording, what I believe was that with the removal of
prepare_page_flip, do_intel_finish_page_flip only needs a single atomic_read to complete.
>> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>> ---
>>  drivers/gpu/drm/i915/i915_debugfs.c  |  3 --
>>  drivers/gpu/drm/i915/i915_irq.c      | 18 ++-----
>>  drivers/gpu/drm/i915/intel_display.c | 96 ++++++++++++++---------------------
>> -
>>  drivers/gpu/drm/i915/intel_drv.h     |  2 -
>>  4 files changed, 40 insertions(+), 79 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
>> b/drivers/gpu/drm/i915/i915_debugfs.c
>> index df8073a2ffbe..c3b029e7bffd 100644
>> --- a/drivers/gpu/drm/i915/i915_debugfs.c
>> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
>> @@ -589,9 +589,6 @@ static int i915_gem_pageflip_info(struct seq_file *m, void
>> *data)
>>  			if (pending == INTEL_FLIP_INACTIVE) {
>>  				seq_printf(m, "Flip ioctl preparing on pipe
>> %c (plane %c)\n",
>>  					   pipe, plane);
>> -			} else if (pending >= INTEL_FLIP_COMPLETE) {
>> -				seq_printf(m, "Flip queued on pipe %c (plane
>> %c)\n",
>> -					   pipe, plane);
>>  			} else {
>>  				seq_printf(m, "Flip pending (waiting for
>> vsync) on pipe %c (plane %c)\n",
>>  					   pipe, plane);
>> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
>> index 679f08c944ef..5ed2f73a7ea9 100644
>> --- a/drivers/gpu/drm/i915/i915_irq.c
>> +++ b/drivers/gpu/drm/i915/i915_irq.c
>> @@ -1707,10 +1707,8 @@ static void valleyview_pipestat_irq_handler(struct
>> drm_device *dev, u32 iir)
>>  		    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);
>> +		if (pipe_stats[pipe] & PLANE_FLIP_DONE_INT_STATUS_VLV)
>>  			intel_finish_page_flip(dev, pipe);
>> -		}
>>  
>>  		if (pipe_stats[pipe] & PIPE_CRC_DONE_INTERRUPT_STATUS)
>>  			i9xx_pipe_crc_irq_handler(dev, pipe);
>> @@ -2109,10 +2107,8 @@ static void ilk_display_irq_handler(struct drm_device
>> *dev, u32 de_iir)
>>  			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);
>> +		if (de_iir & DE_PLANE_FLIP_DONE(pipe))
>>  			intel_finish_page_flip_plane(dev, pipe);
>> -		}
>>  	}
>>  
>>  	/* check event from PCH */
>> @@ -2156,10 +2152,8 @@ static void ivb_display_irq_handler(struct drm_device
>> *dev, u32 de_iir)
>>  			intel_check_page_flip(dev, pipe);
>>  
>>  		/* plane/pipes map 1:1 on ilk+ */
>> -		if (de_iir & DE_PLANE_FLIP_DONE_IVB(pipe)) {
>> -			intel_prepare_page_flip(dev, pipe);
>> +		if (de_iir & DE_PLANE_FLIP_DONE_IVB(pipe))
>>  			intel_finish_page_flip_plane(dev, pipe);
>> -		}
>>  	}
>>  
>>  	/* check event from PCH */
>> @@ -2363,10 +2357,8 @@ gen8_de_irq_handler(struct drm_i915_private *dev_priv,
>> u32 master_ctl)
>>  		else
>>  			flip_done &= GEN8_PIPE_PRIMARY_FLIP_DONE;
>>  
>> -		if (flip_done) {
>> -			intel_prepare_page_flip(dev, pipe);
>> +		if (flip_done)
>>  			intel_finish_page_flip_plane(dev, pipe);
>> -		}
>>  
>>  		if (iir & GEN8_PIPE_CDCLK_CRC_DONE)
>>  			hsw_pipe_crc_irq_handler(dev, pipe);
>> @@ -4024,7 +4016,6 @@ static bool i8xx_handle_vblank(struct drm_device *dev,
>>  	if (I915_READ16(ISR) & flip_pending)
>>  		goto check_page_flip;
>>  
>> -	intel_prepare_page_flip(dev, plane);
>>  	intel_finish_page_flip(dev, pipe);
>>  	return true;
>>  
>> @@ -4215,7 +4206,6 @@ static bool i915_handle_vblank(struct drm_device *dev,
>>  	if (I915_READ(ISR) & flip_pending)
>>  		goto check_page_flip;
>>  
>> -	intel_prepare_page_flip(dev, plane);
>>  	intel_finish_page_flip(dev, pipe);
>>  	return true;
>>  
>> diff --git a/drivers/gpu/drm/i915/intel_display.c
>> b/drivers/gpu/drm/i915/intel_display.c
>> index 618e034a7a5e..dc42335409b3 100644
>> --- a/drivers/gpu/drm/i915/intel_display.c
>> +++ b/drivers/gpu/drm/i915/intel_display.c
>> @@ -3101,7 +3101,6 @@ static void intel_complete_page_flips(struct drm_device
>> *dev)
>>  		struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
>>  		enum plane plane = intel_crtc->plane;
>>  
>> -		intel_prepare_page_flip(dev, plane);
>>  		intel_finish_page_flip_plane(dev, plane);
>>  	}
>>  }
>> @@ -10925,53 +10924,6 @@ static void intel_unpin_work_fn(struct work_struct
>> *__work)
>>  	kfree(work);
>>  }
>>  
>> -static void do_intel_finish_page_flip(struct drm_device *dev,
>> -				      struct drm_crtc *crtc)
>> -{
>> -	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
>> -	struct intel_unpin_work *work;
>> -	unsigned long flags;
>> -
>> -	/* Ignore early vblank irqs */
>> -	if (intel_crtc == NULL)
>> -		return;
>> -
>> -	/*
>> -	 * This is called both by irq handlers and the reset code (to
>> complete
>> -	 * lost pageflips) so needs the full irqsave spinlocks.
>> -	 */
>> -	spin_lock_irqsave(&dev->event_lock, flags);
>> -	work = intel_crtc->unpin_work;
>> -
>> -	/* Ensure we don't miss a work->pending update ... */
>> -	smp_rmb();
> If this is not needed anymore, should the matching smp_wmb() from 
> intel_mark_page_flip_active() be removed?
>
>
>> -
>> -	if (work == NULL || atomic_read(&work->pending) <
>> INTEL_FLIP_COMPLETE) {
>> -		spin_unlock_irqrestore(&dev->event_lock, flags);
>> -		return;
>> -	}
>> -
>> -	page_flip_completed(intel_crtc);
>> -
>> -	spin_unlock_irqrestore(&dev->event_lock, flags);
>> -}
>> -
>> -void intel_finish_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];
>> -
>> -	do_intel_finish_page_flip(dev, crtc);
>> -}
>> -
>> -void intel_finish_page_flip_plane(struct drm_device *dev, int plane)
>> -{
>> -	struct drm_i915_private *dev_priv = dev->dev_private;
>> -	struct drm_crtc *crtc = dev_priv->plane_to_crtc_mapping[plane];
>> -
>> -	do_intel_finish_page_flip(dev, crtc);
>> -}
>> -
>>  /* Is 'a' after or equal to 'b'? */
>>  static bool g4x_flip_count_after_eq(u32 a, u32 b)
>>  {
>> @@ -11024,28 +10976,52 @@ static bool page_flip_finished(struct intel_crtc
>> *crtc)
>>  				    crtc->unpin_work->flip_count);
>>  }
>>  
>> -void intel_prepare_page_flip(struct drm_device *dev, int plane)
>> +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(dev_priv->plane_to_crtc_mapping[plane]);
>> +	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
>> +	struct intel_unpin_work *work;
>>  	unsigned long flags;
>>  
>> +	/* Ignore early vblank irqs */
>> +	if (intel_crtc == NULL)
>> +		return;
>>  
>>  	/*
>>  	 * This is called both by irq handlers and the reset code (to
>> complete
>>  	 * lost pageflips) so needs the full irqsave spinlocks.
>> -	 *
>> -	 * NB: An MMIO update of the plane base pointer will also
>> -	 * generate a page-flip completion irq, i.e. every modeset
>> -	 * is also accompanied by a spurious intel_prepare_page_flip().
>>  	 */
>>  	spin_lock_irqsave(&dev->event_lock, flags);
>> -	if (intel_crtc->unpin_work && page_flip_finished(intel_crtc))
>> -		atomic_inc_not_zero(&intel_crtc->unpin_work->pending);
>> +	work = intel_crtc->unpin_work;
>> +
>> +	if (work == NULL ||
>> +	    atomic_read(&work->pending) == INTEL_FLIP_INACTIVE ||
>> +	   !page_flip_finished(intel_crtc)) {
> This line is misaligned. Also, if the condition is was inverted you could save
> the extra unlock and return.
>
>
> Ander
>
>> +		spin_unlock_irqrestore(&dev->event_lock, flags);
>> +		return;
>> +	}
>> +
>> +	page_flip_completed(intel_crtc);
>> +
>>  	spin_unlock_irqrestore(&dev->event_lock, flags);
>>  }
>>  
>> +void intel_finish_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];
>> +
>> +	do_intel_finish_page_flip(dev, crtc);
>> +}
>> +
>> +void intel_finish_page_flip_plane(struct drm_device *dev, int plane)
>> +{
>> +	struct drm_i915_private *dev_priv = dev->dev_private;
>> +	struct drm_crtc *crtc = dev_priv->plane_to_crtc_mapping[plane];
>> +
>> +	do_intel_finish_page_flip(dev, crtc);
>> +}
>> +
>>  static inline void intel_mark_page_flip_active(struct intel_unpin_work *work)
>>  {
>>  	/* Ensure that the work item is consistent when activating it ... */
>> @@ -11495,8 +11471,8 @@ static bool __intel_pageflip_stall_check(struct
>> drm_device *dev,
>>  	u32 pending;
>>  
>>  	pending = atomic_read(&work->pending);
>> -	if (pending != INTEL_FLIP_PENDING)
>> -		return pending == INTEL_FLIP_COMPLETE;
>> +	if (pending == INTEL_FLIP_INACTIVE)
>> +		return false;
>>  
>>  	if (work->flip_ready_vblank == 0) {
>>  		if (work->flip_queued_req &&
>> diff --git a/drivers/gpu/drm/i915/intel_drv.h
>> b/drivers/gpu/drm/i915/intel_drv.h
>> index ce2a6d985bbe..a74edecfd179 100644
>> --- a/drivers/gpu/drm/i915/intel_drv.h
>> +++ b/drivers/gpu/drm/i915/intel_drv.h
>> @@ -946,7 +946,6 @@ struct intel_unpin_work {
>>  	atomic_t pending;
>>  #define INTEL_FLIP_INACTIVE	0
>>  #define INTEL_FLIP_PENDING	1
>> -#define INTEL_FLIP_COMPLETE	2
>>  	u32 flip_count;
>>  	u32 gtt_offset;
>>  	struct drm_i915_gem_request *flip_queued_req;
>> @@ -1159,7 +1158,6 @@ struct drm_framebuffer *
>>  __intel_framebuffer_create(struct drm_device *dev,
>>  			   struct drm_mode_fb_cmd2 *mode_cmd,
>>  			   struct drm_i915_gem_object *obj);
>> -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);

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 02/11] drm/i915: Remove stallcheck special handling.

[Maarten Lankhorst]

Op 15-04-16 om 09:07 schreef Ander Conselvan De Oliveira:
> On Wed, 2016-04-13 at 11:18 +0200, Maarten Lankhorst wrote:
>> Re-use unpin_work->pending, but also set vblank count before
>> intel_mark_page_flip_active to be sure.
> Be sure of what?
>
>> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>> ---
>>  drivers/gpu/drm/i915/i915_debugfs.c  | 11 ++++++-----
>>  drivers/gpu/drm/i915/intel_display.c | 31 ++++++++++++-------------------
>>  drivers/gpu/drm/i915/intel_drv.h     |  1 -
>>  3 files changed, 18 insertions(+), 25 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
>> b/drivers/gpu/drm/i915/i915_debugfs.c
>> index 9640738aabf2..df8073a2ffbe 100644
>> --- a/drivers/gpu/drm/i915/i915_debugfs.c
>> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
>> @@ -582,9 +582,14 @@ 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 pending;
>>  			u32 addr;
>>  
>> -			if (atomic_read(&work->pending) <
>> INTEL_FLIP_COMPLETE) {
>> +			pending = atomic_read(&work->pending);
>> +			if (pending == INTEL_FLIP_INACTIVE) {
>> +				seq_printf(m, "Flip ioctl preparing on pipe
>> %c (plane %c)\n",
>> +					   pipe, plane);
>> +			} else if (pending >= INTEL_FLIP_COMPLETE) {
>>  				seq_printf(m, "Flip queued on pipe %c (plane
>> %c)\n",
>>  					   pipe, plane);
>>  			} else {
>> @@ -606,10 +611,6 @@ static int i915_gem_pageflip_info(struct seq_file *m,
>> void *data)
>>  				   work->flip_queued_vblank,
>>  				   work->flip_ready_vblank,
>>  				   drm_crtc_vblank_count(&crtc->base));
>> -			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 (INTEL_INFO(dev)->gen >= 4)
>> diff --git a/drivers/gpu/drm/i915/intel_display.c
>> b/drivers/gpu/drm/i915/intel_display.c
>> index f2be54a48727..618e034a7a5e 100644
>> --- a/drivers/gpu/drm/i915/intel_display.c
>> +++ b/drivers/gpu/drm/i915/intel_display.c
>> @@ -11415,8 +11415,6 @@ static void intel_do_mmio_flip(struct intel_mmio_flip
>> *mmio_flip)
>>  	if (work == NULL)
>>  		return;
>>  
>> -	intel_mark_page_flip_active(work);
>> -
>>  	intel_pipe_update_start(crtc);
>>  
>>  	if (INTEL_INFO(mmio_flip->i915)->gen >= 9)
>> @@ -11426,6 +11424,8 @@ static void intel_do_mmio_flip(struct intel_mmio_flip
>> *mmio_flip)
>>  		ilk_do_mmio_flip(crtc, work);
>>  
>>  	intel_pipe_update_end(crtc);
>> +
>> +	intel_mark_page_flip_active(work);
> Is this to avoid triggering the stall check during the wait from a vblank
> evasion?
It's to ensure that if a vblank happens before pipe_update_end, we don't mark the flip as completed until we actually updated the mmio registers.
>
>>  }
>>  
>>  static void intel_mmio_flip_work_func(struct work_struct *work)
>> @@ -11492,15 +11492,11 @@ static bool __intel_pageflip_stall_check(struct
>> drm_device *dev,
>>  	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
>>  	struct intel_unpin_work *work = intel_crtc->unpin_work;
>>  	u32 addr;
>> +	u32 pending;
>>  
>> -	if (atomic_read(&work->pending) >= INTEL_FLIP_COMPLETE)
>> -		return true;
>> -
>> -	if (atomic_read(&work->pending) < INTEL_FLIP_PENDING)
>> -		return false;
>> -
>> -	if (!work->enable_stall_check)
>> -		return false;
>> +	pending = atomic_read(&work->pending);
>> +	if (pending != INTEL_FLIP_PENDING)
>> +		return pending == INTEL_FLIP_COMPLETE;
>>  
>>  	if (work->flip_ready_vblank == 0) {
>>  		if (work->flip_queued_req &&
>> @@ -11676,6 +11672,11 @@ static int intel_crtc_page_flip(struct drm_crtc
>> *crtc,
>>  	 */
>>  	if (!mmio_flip) {
>>  		ret = i915_gem_object_sync(obj, engine, &request);
>> +		if (!ret && !request) {
>> +			request = i915_gem_request_alloc(engine, NULL);
>> +			ret = PTR_ERR_OR_ZERO(request);
>> +		}
>> +
>>  		if (ret)
>>  			goto cleanup_pending;
>>  	}
>> @@ -11687,6 +11688,7 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
>>  	work->gtt_offset = intel_plane_obj_offset(to_intel_plane(primary),
>>  						  obj, 0);
>>  	work->gtt_offset += intel_crtc->dspaddr_offset;
>> +	work->flip_queued_vblank = drm_crtc_vblank_count(crtc);
>>  
>>  	if (mmio_flip) {
>>  		ret = intel_queue_mmio_flip(dev, crtc, obj);
>> @@ -11696,14 +11698,6 @@ static int intel_crtc_page_flip(struct drm_crtc
>> *crtc,
>>  		i915_gem_request_assign(&work->flip_queued_req,
>>  					obj->last_write_req);
>>  	} else {
>> -		if (!request) {
>> -			request = i915_gem_request_alloc(engine, NULL);
>> -			if (IS_ERR(request)) {
>> -				ret = PTR_ERR(request);
>> -				goto cleanup_unpin;
>> -			}
>> -		}
>> -
>>  		ret = dev_priv->display.queue_flip(dev, crtc, fb, obj,
>> request,
>>  						   page_flip_flags);
>>  		if (ret)
>> @@ -11716,7 +11710,6 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
>>  		i915_add_request_no_flush(request);
>>  
>>  	work->flip_queued_vblank = drm_crtc_vblank_count(crtc);
> Do we still need the assigment above?
>
It's used for rps boosting, so likely...
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 05/11] drm/i915: Allow mmio updates on all platforms, v2.

[Maarten Lankhorst]

Op 15-04-16 om 14:31 schreef Ander Conselvan De Oliveira:
> On Wed, 2016-04-13 at 11:18 +0200, Maarten Lankhorst wrote:
>> Rename intel_unpin_work to intel_flip_work and use it for mmio flips
>> and unpinning.
> I think the rename should be a separate patch.
>
intel_unpin_work becomes used for mmio updates and unpinning, hence it's renamed to intel_flip_work. I don't think it makes much sense as a separate patch.

I could try though, see what happens. :)

~Maarten
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 02/11] drm/i915: Remove stallcheck special handling.

[Ander Conselvan De Oliveira]

On Mon, 2016-04-18 at 07:31 +0200, Maarten Lankhorst wrote:
> Op 15-04-16 om 09:07 schreef Ander Conselvan De Oliveira:
> > On Wed, 2016-04-13 at 11:18 +0200, Maarten Lankhorst wrote:
> > > Re-use unpin_work->pending, but also set vblank count before
> > > intel_mark_page_flip_active to be sure.
> > Be sure of what?
> > 
> > > Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/i915_debugfs.c  | 11 ++++++-----
> > >  drivers/gpu/drm/i915/intel_display.c | 31 ++++++++++++-------------------
> > >  drivers/gpu/drm/i915/intel_drv.h     |  1 -
> > >  3 files changed, 18 insertions(+), 25 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
> > > b/drivers/gpu/drm/i915/i915_debugfs.c
> > > index 9640738aabf2..df8073a2ffbe 100644
> > > --- a/drivers/gpu/drm/i915/i915_debugfs.c
> > > +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> > > @@ -582,9 +582,14 @@ 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 pending;
> > >  			u32 addr;
> > >  
> > > -			if (atomic_read(&work->pending) <
> > > INTEL_FLIP_COMPLETE) {
> > > +			pending = atomic_read(&work->pending);
> > > +			if (pending == INTEL_FLIP_INACTIVE) {
> > > +				seq_printf(m, "Flip ioctl preparing on
> > > pipe
> > > %c (plane %c)\n",
> > > +					   pipe, plane);
> > > +			} else if (pending >= INTEL_FLIP_COMPLETE) {
> > >  				seq_printf(m, "Flip queued on pipe %c
> > > (plane
> > > %c)\n",
> > >  					   pipe, plane);
> > >  			} else {
> > > @@ -606,10 +611,6 @@ static int i915_gem_pageflip_info(struct seq_file *m,
> > > void *data)
> > >  				   work->flip_queued_vblank,
> > >  				   work->flip_ready_vblank,
> > >  				   drm_crtc_vblank_count(&crtc->base));
> > > -			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 (INTEL_INFO(dev)->gen >= 4)
> > > diff --git a/drivers/gpu/drm/i915/intel_display.c
> > > b/drivers/gpu/drm/i915/intel_display.c
> > > index f2be54a48727..618e034a7a5e 100644
> > > --- a/drivers/gpu/drm/i915/intel_display.c
> > > +++ b/drivers/gpu/drm/i915/intel_display.c
> > > @@ -11415,8 +11415,6 @@ static void intel_do_mmio_flip(struct
> > > intel_mmio_flip
> > > *mmio_flip)
> > >  	if (work == NULL)
> > >  		return;
> > >  
> > > -	intel_mark_page_flip_active(work);
> > > -
> > >  	intel_pipe_update_start(crtc);
> > >  
> > >  	if (INTEL_INFO(mmio_flip->i915)->gen >= 9)
> > > @@ -11426,6 +11424,8 @@ static void intel_do_mmio_flip(struct
> > > intel_mmio_flip
> > > *mmio_flip)
> > >  		ilk_do_mmio_flip(crtc, work);
> > >  
> > >  	intel_pipe_update_end(crtc);
> > > +
> > > +	intel_mark_page_flip_active(work);
> > Is this to avoid triggering the stall check during the wait from a vblank
> > evasion?
> It's to ensure that if a vblank happens before pipe_update_end, we don't mark 
> the flip as completed until we actually updated the mmio registers.

But interrupts are disabled between pipe_update_start() and pipe_update_end(),
so if that happens it either happens before or during pipe_update_start(), no?

Is it possible the vblank happens just after pipe_update_end() and before
marking it active? Seems to me that in that case, first prepare_page_flip() will
increase unpin_work->pending (so it will go from INACTIVE to PENDING) and then
marking it active will set it again to PENDING, so it never gets to COMPLETE.

But even if the above can happen, that is fixed by the removal of the COMPLETE
state in patch 3.

> > 
> > >  }
> > >  
> > >  static void intel_mmio_flip_work_func(struct work_struct *work)
> > > @@ -11492,15 +11492,11 @@ static bool __intel_pageflip_stall_check(struct
> > > drm_device *dev,
> > >  	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> > >  	struct intel_unpin_work *work = intel_crtc->unpin_work;
> > >  	u32 addr;
> > > +	u32 pending;
> > >  
> > > -	if (atomic_read(&work->pending) >= INTEL_FLIP_COMPLETE)
> > > -		return true;
> > > -
> > > -	if (atomic_read(&work->pending) < INTEL_FLIP_PENDING)
> > > -		return false;
> > > -
> > > -	if (!work->enable_stall_check)
> > > -		return false;
> > > +	pending = atomic_read(&work->pending);
> > > +	if (pending != INTEL_FLIP_PENDING)
> > > +		return pending == INTEL_FLIP_COMPLETE;
> > >  
> > >  	if (work->flip_ready_vblank == 0) {
> > >  		if (work->flip_queued_req &&
> > > @@ -11676,6 +11672,11 @@ static int intel_crtc_page_flip(struct drm_crtc
> > > *crtc,
> > >  	 */
> > >  	if (!mmio_flip) {
> > >  		ret = i915_gem_object_sync(obj, engine, &request);
> > > +		if (!ret && !request) {
> > > +			request = i915_gem_request_alloc(engine, NULL);
> > > +			ret = PTR_ERR_OR_ZERO(request);
> > > +		}
> > > +
> > >  		if (ret)
> > >  			goto cleanup_pending;
> > >  	}
> > > @@ -11687,6 +11688,7 @@ static int intel_crtc_page_flip(struct drm_crtc
> > > *crtc,
> > >  	work->gtt_offset =
> > > intel_plane_obj_offset(to_intel_plane(primary),
> > >  						  obj, 0);
> > >  	work->gtt_offset += intel_crtc->dspaddr_offset;
> > > +	work->flip_queued_vblank = drm_crtc_vblank_count(crtc);
> > >  
> > >  	if (mmio_flip) {
> > >  		ret = intel_queue_mmio_flip(dev, crtc, obj);
> > > @@ -11696,14 +11698,6 @@ static int intel_crtc_page_flip(struct drm_crtc
> > > *crtc,
> > >  		i915_gem_request_assign(&work->flip_queued_req,
> > >  					obj->last_write_req);
> > >  	} else {
> > > -		if (!request) {
> > > -			request = i915_gem_request_alloc(engine, NULL);
> > > -			if (IS_ERR(request)) {
> > > -				ret = PTR_ERR(request);
> > > -				goto cleanup_unpin;
> > > -			}
> > > -		}
> > > -
> > >  		ret = dev_priv->display.queue_flip(dev, crtc, fb, obj,
> > > request,
> > >  						   page_flip_flags);
> > >  		if (ret)
> > > @@ -11716,7 +11710,6 @@ static int intel_crtc_page_flip(struct drm_crtc
> > > *crtc,
> > >  		i915_add_request_no_flush(request);
> > >  
> > >  	work->flip_queued_vblank = drm_crtc_vblank_count(crtc);
> > Do we still need the assigment above?
> > 
> It's used for rps boosting, so likely...

But you added the same assignment above, so removing it would only change the
timing of the boost if the count were to flip while queuing but not prevent the
boost from happening. It's no big deal, I just find it odd that we need to set
that value twice.

Ander


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

Re: [PATCH v2 02/11] drm/i915: Remove stallcheck special handling.

[Maarten Lankhorst]

Op 18-04-16 om 09:57 schreef Ander Conselvan De Oliveira:
> On Mon, 2016-04-18 at 07:31 +0200, Maarten Lankhorst wrote:
>> Op 15-04-16 om 09:07 schreef Ander Conselvan De Oliveira:
>>> On Wed, 2016-04-13 at 11:18 +0200, Maarten Lankhorst wrote:
>>>> Re-use unpin_work->pending, but also set vblank count before
>>>> intel_mark_page_flip_active to be sure.
>>> Be sure of what?
>>>
>>>> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>>>> ---
>>>>  drivers/gpu/drm/i915/i915_debugfs.c  | 11 ++++++-----
>>>>  drivers/gpu/drm/i915/intel_display.c | 31 ++++++++++++-------------------
>>>>  drivers/gpu/drm/i915/intel_drv.h     |  1 -
>>>>  3 files changed, 18 insertions(+), 25 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
>>>> b/drivers/gpu/drm/i915/i915_debugfs.c
>>>> index 9640738aabf2..df8073a2ffbe 100644
>>>> --- a/drivers/gpu/drm/i915/i915_debugfs.c
>>>> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
>>>> @@ -582,9 +582,14 @@ 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 pending;
>>>>  			u32 addr;
>>>>  
>>>> -			if (atomic_read(&work->pending) <
>>>> INTEL_FLIP_COMPLETE) {
>>>> +			pending = atomic_read(&work->pending);
>>>> +			if (pending == INTEL_FLIP_INACTIVE) {
>>>> +				seq_printf(m, "Flip ioctl preparing on
>>>> pipe
>>>> %c (plane %c)\n",
>>>> +					   pipe, plane);
>>>> +			} else if (pending >= INTEL_FLIP_COMPLETE) {
>>>>  				seq_printf(m, "Flip queued on pipe %c
>>>> (plane
>>>> %c)\n",
>>>>  					   pipe, plane);
>>>>  			} else {
>>>> @@ -606,10 +611,6 @@ static int i915_gem_pageflip_info(struct seq_file *m,
>>>> void *data)
>>>>  				   work->flip_queued_vblank,
>>>>  				   work->flip_ready_vblank,
>>>>  				   drm_crtc_vblank_count(&crtc->base));
>>>> -			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 (INTEL_INFO(dev)->gen >= 4)
>>>> diff --git a/drivers/gpu/drm/i915/intel_display.c
>>>> b/drivers/gpu/drm/i915/intel_display.c
>>>> index f2be54a48727..618e034a7a5e 100644
>>>> --- a/drivers/gpu/drm/i915/intel_display.c
>>>> +++ b/drivers/gpu/drm/i915/intel_display.c
>>>> @@ -11415,8 +11415,6 @@ static void intel_do_mmio_flip(struct
>>>> intel_mmio_flip
>>>> *mmio_flip)
>>>>  	if (work == NULL)
>>>>  		return;
>>>>  
>>>> -	intel_mark_page_flip_active(work);
>>>> -
>>>>  	intel_pipe_update_start(crtc);
>>>>  
>>>>  	if (INTEL_INFO(mmio_flip->i915)->gen >= 9)
>>>> @@ -11426,6 +11424,8 @@ static void intel_do_mmio_flip(struct
>>>> intel_mmio_flip
>>>> *mmio_flip)
>>>>  		ilk_do_mmio_flip(crtc, work);
>>>>  
>>>>  	intel_pipe_update_end(crtc);
>>>> +
>>>> +	intel_mark_page_flip_active(work);
>>> Is this to avoid triggering the stall check during the wait from a vblank
>>> evasion?
>> It's to ensure that if a vblank happens before pipe_update_end, we don't mark 
>> the flip as completed until we actually updated the mmio registers.
> But interrupts are disabled between pipe_update_start() and pipe_update_end(),
> so if that happens it either happens before or during pipe_update_start(), no?
>
> Is it possible the vblank happens just after pipe_update_end() and before
> marking it active? Seems to me that in that case, first prepare_page_flip() will
> increase unpin_work->pending (so it will go from INACTIVE to PENDING) and then
> marking it active will set it again to PENDING, so it never gets to COMPLETE.
>
> But even if the above can happen, that is fixed by the removal of the COMPLETE
> state in patch 3.
We disable local interrupts, but interrupts can still happen on another cpu.
>>>>  }
>>>>  
>>>>  static void intel_mmio_flip_work_func(struct work_struct *work)
>>>> @@ -11492,15 +11492,11 @@ static bool __intel_pageflip_stall_check(struct
>>>> drm_device *dev,
>>>>  	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
>>>>  	struct intel_unpin_work *work = intel_crtc->unpin_work;
>>>>  	u32 addr;
>>>> +	u32 pending;
>>>>  
>>>> -	if (atomic_read(&work->pending) >= INTEL_FLIP_COMPLETE)
>>>> -		return true;
>>>> -
>>>> -	if (atomic_read(&work->pending) < INTEL_FLIP_PENDING)
>>>> -		return false;
>>>> -
>>>> -	if (!work->enable_stall_check)
>>>> -		return false;
>>>> +	pending = atomic_read(&work->pending);
>>>> +	if (pending != INTEL_FLIP_PENDING)
>>>> +		return pending == INTEL_FLIP_COMPLETE;
>>>>  
>>>>  	if (work->flip_ready_vblank == 0) {
>>>>  		if (work->flip_queued_req &&
>>>> @@ -11676,6 +11672,11 @@ static int intel_crtc_page_flip(struct drm_crtc
>>>> *crtc,
>>>>  	 */
>>>>  	if (!mmio_flip) {
>>>>  		ret = i915_gem_object_sync(obj, engine, &request);
>>>> +		if (!ret && !request) {
>>>> +			request = i915_gem_request_alloc(engine, NULL);
>>>> +			ret = PTR_ERR_OR_ZERO(request);
>>>> +		}
>>>> +
>>>>  		if (ret)
>>>>  			goto cleanup_pending;
>>>>  	}
>>>> @@ -11687,6 +11688,7 @@ static int intel_crtc_page_flip(struct drm_crtc
>>>> *crtc,
>>>>  	work->gtt_offset =
>>>> intel_plane_obj_offset(to_intel_plane(primary),
>>>>  						  obj, 0);
>>>>  	work->gtt_offset += intel_crtc->dspaddr_offset;
>>>> +	work->flip_queued_vblank = drm_crtc_vblank_count(crtc);
>>>>  
>>>>  	if (mmio_flip) {
>>>>  		ret = intel_queue_mmio_flip(dev, crtc, obj);
>>>> @@ -11696,14 +11698,6 @@ static int intel_crtc_page_flip(struct drm_crtc
>>>> *crtc,
>>>>  		i915_gem_request_assign(&work->flip_queued_req,
>>>>  					obj->last_write_req);
>>>>  	} else {
>>>> -		if (!request) {
>>>> -			request = i915_gem_request_alloc(engine, NULL);
>>>> -			if (IS_ERR(request)) {
>>>> -				ret = PTR_ERR(request);
>>>> -				goto cleanup_unpin;
>>>> -			}
>>>> -		}
>>>> -
>>>>  		ret = dev_priv->display.queue_flip(dev, crtc, fb, obj,
>>>> request,
>>>>  						   page_flip_flags);
>>>>  		if (ret)
>>>> @@ -11716,7 +11710,6 @@ static int intel_crtc_page_flip(struct drm_crtc
>>>> *crtc,
>>>>  		i915_add_request_no_flush(request);
>>>>  
>>>>  	work->flip_queued_vblank = drm_crtc_vblank_count(crtc);
>>> Do we still need the assigment above?
>>>
>> It's used for rps boosting, so likely...
> But you added the same assignment above, so removing it would only change the
> timing of the boost if the count were to flip while queuing but not prevent the
> boost from happening. It's no big deal, I just find it odd that we need to set
> that value twice.
>
I think I'm not paranoid enough here. I should probably stop touching work as soon as I call intel_mark_page_flip_active,
so moving it from .queue_flip to after i915_gem_request_assign would be best.

This would allow us to ensure the work struct is completely filled in when mark_page_flip_active is called, with no intermediate states to worry about.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v2.1 02/11] drm/i915: Remove stallcheck special handling, v2.

[Maarten Lankhorst]

Both intel_unpin_work.pending and intel_unpin_work.enable_stall_check
were used to see if work should be enabled. By only using pending
some special cases are gone, and access to unpin_work can be simplified.

Use this to only access work members untilintel_mark_page_flip_active
is called, or intel_queue_mmio_flip is used. This will prevent
use-after-free, and makes it easier to verify accesses.

Changes since v1:
- Reword commit message.
- Do not access unpin_work after intel_mark_page_flip_active.
- Add the right memory barriers.

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 2d11b4948a74..99a461e34f60 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -582,9 +582,14 @@ 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 pending;
 			u32 addr;
 
-			if (atomic_read(&work->pending) < INTEL_FLIP_COMPLETE) {
+			pending = atomic_read(&work->pending);
+			if (pending == INTEL_FLIP_INACTIVE) {
+				seq_printf(m, "Flip ioctl preparing on pipe %c (plane %c)\n",
+					   pipe, plane);
+			} else if (pending >= INTEL_FLIP_COMPLETE) {
 				seq_printf(m, "Flip queued on pipe %c (plane %c)\n",
 					   pipe, plane);
 			} else {
@@ -606,10 +611,6 @@ static int i915_gem_pageflip_info(struct seq_file *m, void *data)
 				   work->flip_queued_vblank,
 				   work->flip_ready_vblank,
 				   drm_crtc_vblank_count(&crtc->base));
-			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 (INTEL_INFO(dev)->gen >= 4)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index a3908dda0ece..8061b329f974 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3879,8 +3879,6 @@ 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)
@@ -10948,16 +10946,13 @@ static void do_intel_finish_page_flip(struct drm_device *dev,
 	spin_lock_irqsave(&dev->event_lock, flags);
 	work = intel_crtc->unpin_work;
 
-	/* Ensure we don't miss a work->pending update ... */
-	smp_rmb();
+	if (work && atomic_read(&work->pending) >= INTEL_FLIP_COMPLETE) {
+		/* ensure that the unpin work is consistent wrt ->pending. */
+		smp_mb__after_atomic();
 
-	if (work == NULL || atomic_read(&work->pending) < INTEL_FLIP_COMPLETE) {
-		spin_unlock_irqrestore(&dev->event_lock, flags);
-		return;
+		page_flip_completed(intel_crtc);
 	}
 
-	page_flip_completed(intel_crtc);
-
 	spin_unlock_irqrestore(&dev->event_lock, flags);
 }
 
@@ -11054,10 +11049,8 @@ void intel_prepare_page_flip(struct drm_device *dev, int plane)
 static inline void intel_mark_page_flip_active(struct intel_unpin_work *work)
 {
 	/* Ensure that the work item is consistent when activating it ... */
-	smp_wmb();
+	smp_mb__before_atomic();
 	atomic_set(&work->pending, INTEL_FLIP_PENDING);
-	/* and that it is marked active as soon as the irq could fire. */
-	smp_wmb();
 }
 
 static int intel_gen2_queue_flip(struct drm_device *dev,
@@ -11091,7 +11084,6 @@ static int intel_gen2_queue_flip(struct drm_device *dev,
 	intel_ring_emit(engine, intel_crtc->unpin_work->gtt_offset);
 	intel_ring_emit(engine, 0); /* aux display base address, unused */
 
-	intel_mark_page_flip_active(intel_crtc->unpin_work);
 	return 0;
 }
 
@@ -11123,7 +11115,6 @@ static int intel_gen3_queue_flip(struct drm_device *dev,
 	intel_ring_emit(engine, intel_crtc->unpin_work->gtt_offset);
 	intel_ring_emit(engine, MI_NOOP);
 
-	intel_mark_page_flip_active(intel_crtc->unpin_work);
 	return 0;
 }
 
@@ -11162,7 +11153,6 @@ static int intel_gen4_queue_flip(struct drm_device *dev,
 	pipesrc = I915_READ(PIPESRC(intel_crtc->pipe)) & 0x0fff0fff;
 	intel_ring_emit(engine, pf | pipesrc);
 
-	intel_mark_page_flip_active(intel_crtc->unpin_work);
 	return 0;
 }
 
@@ -11198,7 +11188,6 @@ static int intel_gen6_queue_flip(struct drm_device *dev,
 	pipesrc = I915_READ(PIPESRC(intel_crtc->pipe)) & 0x0fff0fff;
 	intel_ring_emit(engine, pf | pipesrc);
 
-	intel_mark_page_flip_active(intel_crtc->unpin_work);
 	return 0;
 }
 
@@ -11293,7 +11282,6 @@ static int intel_gen7_queue_flip(struct drm_device *dev,
 	intel_ring_emit(engine, intel_crtc->unpin_work->gtt_offset);
 	intel_ring_emit(engine, (MI_NOOP));
 
-	intel_mark_page_flip_active(intel_crtc->unpin_work);
 	return 0;
 }
 
@@ -11420,8 +11408,6 @@ static void intel_do_mmio_flip(struct intel_mmio_flip *mmio_flip)
 	if (work == NULL)
 		return;
 
-	intel_mark_page_flip_active(work);
-
 	intel_pipe_update_start(crtc);
 
 	if (INTEL_INFO(mmio_flip->i915)->gen >= 9)
@@ -11431,6 +11417,8 @@ static void intel_do_mmio_flip(struct intel_mmio_flip *mmio_flip)
 		ilk_do_mmio_flip(crtc, work);
 
 	intel_pipe_update_end(crtc);
+
+	intel_mark_page_flip_active(work);
 }
 
 static void intel_mmio_flip_work_func(struct work_struct *work)
@@ -11497,15 +11485,14 @@ static bool __intel_pageflip_stall_check(struct drm_device *dev,
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 	struct intel_unpin_work *work = intel_crtc->unpin_work;
 	u32 addr;
+	u32 pending;
 
-	if (atomic_read(&work->pending) >= INTEL_FLIP_COMPLETE)
-		return true;
-
-	if (atomic_read(&work->pending) < INTEL_FLIP_PENDING)
-		return false;
+	pending = atomic_read(&work->pending);
+	/* ensure that the unpin work is consistent wrt ->pending. */
+	smp_mb__after_atomic();
 
-	if (!work->enable_stall_check)
-		return false;
+	if (pending != INTEL_FLIP_PENDING)
+		return pending == INTEL_FLIP_COMPLETE;
 
 	if (work->flip_ready_vblank == 0) {
 		if (work->flip_queued_req &&
@@ -11681,6 +11668,11 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
 	 */
 	if (!mmio_flip) {
 		ret = i915_gem_object_sync(obj, engine, &request);
+		if (!ret && !request) {
+			request = i915_gem_request_alloc(engine, NULL);
+			ret = PTR_ERR_OR_ZERO(request);
+		}
+
 		if (ret)
 			goto cleanup_pending;
 	}
@@ -11694,36 +11686,29 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
 	work->gtt_offset += intel_crtc->dspaddr_offset;
 
 	if (mmio_flip) {
-		ret = intel_queue_mmio_flip(dev, crtc, obj);
-		if (ret)
-			goto cleanup_unpin;
+		work->flip_queued_vblank = drm_crtc_vblank_count(crtc);
 
 		i915_gem_request_assign(&work->flip_queued_req,
 					obj->last_write_req);
-	} else {
-		if (!request) {
-			request = i915_gem_request_alloc(engine, NULL);
-			if (IS_ERR(request)) {
-				ret = PTR_ERR(request);
-				goto cleanup_unpin;
-			}
-		}
 
+		ret = intel_queue_mmio_flip(dev, crtc, obj);
+		if (ret)
+			goto cleanup_unpin;
+	} else {
 		ret = dev_priv->display.queue_flip(dev, crtc, fb, obj, request,
 						   page_flip_flags);
 		if (ret)
 			goto cleanup_unpin;
 
 		i915_gem_request_assign(&work->flip_queued_req, request);
-	}
 
-	if (request)
-		i915_add_request_no_flush(request);
+		work->flip_queued_vblank = drm_crtc_vblank_count(crtc);
+		intel_mark_page_flip_active(work);
 
-	work->flip_queued_vblank = drm_crtc_vblank_count(crtc);
-	work->enable_stall_check = true;
+		i915_add_request_no_flush(request);
+	}
 
-	i915_gem_track_fb(intel_fb_obj(work->old_fb), obj,
+	i915_gem_track_fb(intel_fb_obj(old_fb), obj,
 			  to_intel_plane(primary)->frontbuffer_bit);
 	mutex_unlock(&dev->struct_mutex);
 
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index e0fcfa1683cc..ce2a6d985bbe 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -952,7 +952,6 @@ struct intel_unpin_work {
 	struct drm_i915_gem_request *flip_queued_req;
 	u32 flip_queued_vblank;
 	u32 flip_ready_vblank;
-	bool enable_stall_check;
 };
 
 struct intel_load_detect_pipe {

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

[PATCH v2.1 03/11] drm/i915: Remove intel_prepare_page_flip, v2.

[Maarten Lankhorst]

Instead of calling prepare_flip right before calling finish_page_flip
do everything from prepare_page_flip in finish_page_flip.

Putting prepare and finish page_flip in a single step removes the need
for INTEL_FLIP_COMPLETE, so it can be removed. This simplifies the code
slightly.

Changes since v1:
- Invert if case to simplify code.
- Add missing atomic barrier.
- Reword commit message.

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 99a461e34f60..566c5afdd300 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -589,9 +589,6 @@ static int i915_gem_pageflip_info(struct seq_file *m, void *data)
 			if (pending == INTEL_FLIP_INACTIVE) {
 				seq_printf(m, "Flip ioctl preparing on pipe %c (plane %c)\n",
 					   pipe, plane);
-			} else if (pending >= INTEL_FLIP_COMPLETE) {
-				seq_printf(m, "Flip queued on pipe %c (plane %c)\n",
-					   pipe, plane);
 			} else {
 				seq_printf(m, "Flip pending (waiting for vsync) on pipe %c (plane %c)\n",
 					   pipe, plane);
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 247d962afabb..6e3b13c7bf31 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -1707,10 +1707,8 @@ static void valleyview_pipestat_irq_handler(struct drm_device *dev, u32 iir)
 		    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);
+		if (pipe_stats[pipe] & PLANE_FLIP_DONE_INT_STATUS_VLV)
 			intel_finish_page_flip(dev, pipe);
-		}
 
 		if (pipe_stats[pipe] & PIPE_CRC_DONE_INTERRUPT_STATUS)
 			i9xx_pipe_crc_irq_handler(dev, pipe);
@@ -2109,10 +2107,8 @@ static void ilk_display_irq_handler(struct drm_device *dev, u32 de_iir)
 			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);
+		if (de_iir & DE_PLANE_FLIP_DONE(pipe))
 			intel_finish_page_flip_plane(dev, pipe);
-		}
 	}
 
 	/* check event from PCH */
@@ -2156,10 +2152,8 @@ static void ivb_display_irq_handler(struct drm_device *dev, u32 de_iir)
 			intel_check_page_flip(dev, pipe);
 
 		/* plane/pipes map 1:1 on ilk+ */
-		if (de_iir & DE_PLANE_FLIP_DONE_IVB(pipe)) {
-			intel_prepare_page_flip(dev, pipe);
+		if (de_iir & DE_PLANE_FLIP_DONE_IVB(pipe))
 			intel_finish_page_flip_plane(dev, pipe);
-		}
 	}
 
 	/* check event from PCH */
@@ -2363,10 +2357,8 @@ gen8_de_irq_handler(struct drm_i915_private *dev_priv, u32 master_ctl)
 		else
 			flip_done &= GEN8_PIPE_PRIMARY_FLIP_DONE;
 
-		if (flip_done) {
-			intel_prepare_page_flip(dev, pipe);
+		if (flip_done)
 			intel_finish_page_flip_plane(dev, pipe);
-		}
 
 		if (iir & GEN8_PIPE_CDCLK_CRC_DONE)
 			hsw_pipe_crc_irq_handler(dev, pipe);
@@ -3962,7 +3954,6 @@ static bool i8xx_handle_vblank(struct drm_device *dev,
 	if (I915_READ16(ISR) & flip_pending)
 		goto check_page_flip;
 
-	intel_prepare_page_flip(dev, plane);
 	intel_finish_page_flip(dev, pipe);
 	return true;
 
@@ -4153,7 +4144,6 @@ static bool i915_handle_vblank(struct drm_device *dev,
 	if (I915_READ(ISR) & flip_pending)
 		goto check_page_flip;
 
-	intel_prepare_page_flip(dev, plane);
 	intel_finish_page_flip(dev, pipe);
 	return true;
 
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 8061b329f974..fd683146d453 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3101,7 +3101,6 @@ static void intel_complete_page_flips(struct drm_device *dev)
 		struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 		enum plane plane = intel_crtc->plane;
 
-		intel_prepare_page_flip(dev, plane);
 		intel_finish_page_flip_plane(dev, plane);
 	}
 }
@@ -10928,50 +10927,6 @@ static void intel_unpin_work_fn(struct work_struct *__work)
 	kfree(work);
 }
 
-static void do_intel_finish_page_flip(struct drm_device *dev,
-				      struct drm_crtc *crtc)
-{
-	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
-	struct intel_unpin_work *work;
-	unsigned long flags;
-
-	/* Ignore early vblank irqs */
-	if (intel_crtc == NULL)
-		return;
-
-	/*
-	 * This is called both by irq handlers and the reset code (to complete
-	 * lost pageflips) so needs the full irqsave spinlocks.
-	 */
-	spin_lock_irqsave(&dev->event_lock, flags);
-	work = intel_crtc->unpin_work;
-
-	if (work && atomic_read(&work->pending) >= INTEL_FLIP_COMPLETE) {
-		/* ensure that the unpin work is consistent wrt ->pending. */
-		smp_mb__after_atomic();
-
-		page_flip_completed(intel_crtc);
-	}
-
-	spin_unlock_irqrestore(&dev->event_lock, flags);
-}
-
-void intel_finish_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];
-
-	do_intel_finish_page_flip(dev, crtc);
-}
-
-void intel_finish_page_flip_plane(struct drm_device *dev, int plane)
-{
-	struct drm_i915_private *dev_priv = dev->dev_private;
-	struct drm_crtc *crtc = dev_priv->plane_to_crtc_mapping[plane];
-
-	do_intel_finish_page_flip(dev, crtc);
-}
-
 /* Is 'a' after or equal to 'b'? */
 static bool g4x_flip_count_after_eq(u32 a, u32 b)
 {
@@ -11024,28 +10979,48 @@ static bool page_flip_finished(struct intel_crtc *crtc)
 				    crtc->unpin_work->flip_count);
 }
 
-void intel_prepare_page_flip(struct drm_device *dev, int plane)
+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(dev_priv->plane_to_crtc_mapping[plane]);
+	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+	struct intel_unpin_work *work;
 	unsigned long flags;
 
+	/* Ignore early vblank irqs */
+	if (intel_crtc == NULL)
+		return;
 
 	/*
 	 * This is called both by irq handlers and the reset code (to complete
 	 * lost pageflips) so needs the full irqsave spinlocks.
-	 *
-	 * NB: An MMIO update of the plane base pointer will also
-	 * generate a page-flip completion irq, i.e. every modeset
-	 * is also accompanied by a spurious intel_prepare_page_flip().
 	 */
 	spin_lock_irqsave(&dev->event_lock, flags);
-	if (intel_crtc->unpin_work && page_flip_finished(intel_crtc))
-		atomic_inc_not_zero(&intel_crtc->unpin_work->pending);
+	work = intel_crtc->unpin_work;
+
+	if (work != NULL &&
+	    atomic_read(&work->pending) == INTEL_FLIP_PENDING &&
+	    page_flip_finished(intel_crtc))
+		page_flip_completed(intel_crtc);
+
 	spin_unlock_irqrestore(&dev->event_lock, flags);
 }
 
+void intel_finish_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];
+
+	do_intel_finish_page_flip(dev, crtc);
+}
+
+void intel_finish_page_flip_plane(struct drm_device *dev, int plane)
+{
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct drm_crtc *crtc = dev_priv->plane_to_crtc_mapping[plane];
+
+	do_intel_finish_page_flip(dev, crtc);
+}
+
 static inline void intel_mark_page_flip_active(struct intel_unpin_work *work)
 {
 	/* Ensure that the work item is consistent when activating it ... */
@@ -11491,8 +11466,8 @@ static bool __intel_pageflip_stall_check(struct drm_device *dev,
 	/* ensure that the unpin work is consistent wrt ->pending. */
 	smp_mb__after_atomic();
 
-	if (pending != INTEL_FLIP_PENDING)
-		return pending == INTEL_FLIP_COMPLETE;
+	if (pending == INTEL_FLIP_INACTIVE)
+		return false;
 
 	if (work->flip_ready_vblank == 0) {
 		if (work->flip_queued_req &&
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index ce2a6d985bbe..a74edecfd179 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -946,7 +946,6 @@ struct intel_unpin_work {
 	atomic_t pending;
 #define INTEL_FLIP_INACTIVE	0
 #define INTEL_FLIP_PENDING	1
-#define INTEL_FLIP_COMPLETE	2
 	u32 flip_count;
 	u32 gtt_offset;
 	struct drm_i915_gem_request *flip_queued_req;
@@ -1159,7 +1158,6 @@ struct drm_framebuffer *
 __intel_framebuffer_create(struct drm_device *dev,
 			   struct drm_mode_fb_cmd2 *mode_cmd,
 			   struct drm_i915_gem_object *obj);
-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);

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

[PATCH v2.2 03/11] drm/i915: Remove intel_prepare_page_flip, v2.

[Maarten Lankhorst]

Instead of calling prepare_flip right before calling finish_page_flip
do everything from prepare_page_flip in finish_page_flip.

Putting prepare and finish page_flip in a single step removes the need
for INTEL_FLIP_COMPLETE, so it can be removed. This simplifies the code
slightly.

Changes since v1:
- Invert if case to simplify code.
- Add missing barrier.
- Reword commit message.

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
Uh oh, forgot to refresh, this version has the missing memory barrier the commit says it has.

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 99a461e34f60..566c5afdd300 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -589,9 +589,6 @@ static int i915_gem_pageflip_info(struct seq_file *m, void *data)
 			if (pending == INTEL_FLIP_INACTIVE) {
 				seq_printf(m, "Flip ioctl preparing on pipe %c (plane %c)\n",
 					   pipe, plane);
-			} else if (pending >= INTEL_FLIP_COMPLETE) {
-				seq_printf(m, "Flip queued on pipe %c (plane %c)\n",
-					   pipe, plane);
 			} else {
 				seq_printf(m, "Flip pending (waiting for vsync) on pipe %c (plane %c)\n",
 					   pipe, plane);
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 247d962afabb..6e3b13c7bf31 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -1707,10 +1707,8 @@ static void valleyview_pipestat_irq_handler(struct drm_device *dev, u32 iir)
 		    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);
+		if (pipe_stats[pipe] & PLANE_FLIP_DONE_INT_STATUS_VLV)
 			intel_finish_page_flip(dev, pipe);
-		}
 
 		if (pipe_stats[pipe] & PIPE_CRC_DONE_INTERRUPT_STATUS)
 			i9xx_pipe_crc_irq_handler(dev, pipe);
@@ -2109,10 +2107,8 @@ static void ilk_display_irq_handler(struct drm_device *dev, u32 de_iir)
 			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);
+		if (de_iir & DE_PLANE_FLIP_DONE(pipe))
 			intel_finish_page_flip_plane(dev, pipe);
-		}
 	}
 
 	/* check event from PCH */
@@ -2156,10 +2152,8 @@ static void ivb_display_irq_handler(struct drm_device *dev, u32 de_iir)
 			intel_check_page_flip(dev, pipe);
 
 		/* plane/pipes map 1:1 on ilk+ */
-		if (de_iir & DE_PLANE_FLIP_DONE_IVB(pipe)) {
-			intel_prepare_page_flip(dev, pipe);
+		if (de_iir & DE_PLANE_FLIP_DONE_IVB(pipe))
 			intel_finish_page_flip_plane(dev, pipe);
-		}
 	}
 
 	/* check event from PCH */
@@ -2363,10 +2357,8 @@ gen8_de_irq_handler(struct drm_i915_private *dev_priv, u32 master_ctl)
 		else
 			flip_done &= GEN8_PIPE_PRIMARY_FLIP_DONE;
 
-		if (flip_done) {
-			intel_prepare_page_flip(dev, pipe);
+		if (flip_done)
 			intel_finish_page_flip_plane(dev, pipe);
-		}
 
 		if (iir & GEN8_PIPE_CDCLK_CRC_DONE)
 			hsw_pipe_crc_irq_handler(dev, pipe);
@@ -3962,7 +3954,6 @@ static bool i8xx_handle_vblank(struct drm_device *dev,
 	if (I915_READ16(ISR) & flip_pending)
 		goto check_page_flip;
 
-	intel_prepare_page_flip(dev, plane);
 	intel_finish_page_flip(dev, pipe);
 	return true;
 
@@ -4153,7 +4144,6 @@ static bool i915_handle_vblank(struct drm_device *dev,
 	if (I915_READ(ISR) & flip_pending)
 		goto check_page_flip;
 
-	intel_prepare_page_flip(dev, plane);
 	intel_finish_page_flip(dev, pipe);
 	return true;
 
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 8061b329f974..1e929b081163 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3101,7 +3101,6 @@ static void intel_complete_page_flips(struct drm_device *dev)
 		struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 		enum plane plane = intel_crtc->plane;
 
-		intel_prepare_page_flip(dev, plane);
 		intel_finish_page_flip_plane(dev, plane);
 	}
 }
@@ -10928,50 +10927,6 @@ static void intel_unpin_work_fn(struct work_struct *__work)
 	kfree(work);
 }
 
-static void do_intel_finish_page_flip(struct drm_device *dev,
-				      struct drm_crtc *crtc)
-{
-	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
-	struct intel_unpin_work *work;
-	unsigned long flags;
-
-	/* Ignore early vblank irqs */
-	if (intel_crtc == NULL)
-		return;
-
-	/*
-	 * This is called both by irq handlers and the reset code (to complete
-	 * lost pageflips) so needs the full irqsave spinlocks.
-	 */
-	spin_lock_irqsave(&dev->event_lock, flags);
-	work = intel_crtc->unpin_work;
-
-	if (work && atomic_read(&work->pending) >= INTEL_FLIP_COMPLETE) {
-		/* ensure that the unpin work is consistent wrt ->pending. */
-		smp_mb__after_atomic();
-
-		page_flip_completed(intel_crtc);
-	}
-
-	spin_unlock_irqrestore(&dev->event_lock, flags);
-}
-
-void intel_finish_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];
-
-	do_intel_finish_page_flip(dev, crtc);
-}
-
-void intel_finish_page_flip_plane(struct drm_device *dev, int plane)
-{
-	struct drm_i915_private *dev_priv = dev->dev_private;
-	struct drm_crtc *crtc = dev_priv->plane_to_crtc_mapping[plane];
-
-	do_intel_finish_page_flip(dev, crtc);
-}
-
 /* Is 'a' after or equal to 'b'? */
 static bool g4x_flip_count_after_eq(u32 a, u32 b)
 {
@@ -10983,6 +10938,9 @@ static bool page_flip_finished(struct intel_crtc *crtc)
 	struct drm_device *dev = crtc->base.dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
 
+	/* ensure that the unpin work is consistent wrt ->pending. */
+	smp_mb__after_atomic();
+
 	if (i915_reset_in_progress(&dev_priv->gpu_error) ||
 	    crtc->reset_counter != atomic_read(&dev_priv->gpu_error.reset_counter))
 		return true;
@@ -11024,28 +10982,48 @@ static bool page_flip_finished(struct intel_crtc *crtc)
 				    crtc->unpin_work->flip_count);
 }
 
-void intel_prepare_page_flip(struct drm_device *dev, int plane)
+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(dev_priv->plane_to_crtc_mapping[plane]);
+	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+	struct intel_unpin_work *work;
 	unsigned long flags;
 
+	/* Ignore early vblank irqs */
+	if (intel_crtc == NULL)
+		return;
 
 	/*
 	 * This is called both by irq handlers and the reset code (to complete
 	 * lost pageflips) so needs the full irqsave spinlocks.
-	 *
-	 * NB: An MMIO update of the plane base pointer will also
-	 * generate a page-flip completion irq, i.e. every modeset
-	 * is also accompanied by a spurious intel_prepare_page_flip().
 	 */
 	spin_lock_irqsave(&dev->event_lock, flags);
-	if (intel_crtc->unpin_work && page_flip_finished(intel_crtc))
-		atomic_inc_not_zero(&intel_crtc->unpin_work->pending);
+	work = intel_crtc->unpin_work;
+
+	if (work != NULL &&
+	    atomic_read(&work->pending) == INTEL_FLIP_PENDING &&
+	    page_flip_finished(intel_crtc))
+		page_flip_completed(intel_crtc);
+
 	spin_unlock_irqrestore(&dev->event_lock, flags);
 }
 
+void intel_finish_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];
+
+	do_intel_finish_page_flip(dev, crtc);
+}
+
+void intel_finish_page_flip_plane(struct drm_device *dev, int plane)
+{
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct drm_crtc *crtc = dev_priv->plane_to_crtc_mapping[plane];
+
+	do_intel_finish_page_flip(dev, crtc);
+}
+
 static inline void intel_mark_page_flip_active(struct intel_unpin_work *work)
 {
 	/* Ensure that the work item is consistent when activating it ... */
@@ -11491,8 +11469,8 @@ static bool __intel_pageflip_stall_check(struct drm_device *dev,
 	/* ensure that the unpin work is consistent wrt ->pending. */
 	smp_mb__after_atomic();
 
-	if (pending != INTEL_FLIP_PENDING)
-		return pending == INTEL_FLIP_COMPLETE;
+	if (pending == INTEL_FLIP_INACTIVE)
+		return false;
 
 	if (work->flip_ready_vblank == 0) {
 		if (work->flip_queued_req &&
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index ce2a6d985bbe..a74edecfd179 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -946,7 +946,6 @@ struct intel_unpin_work {
 	atomic_t pending;
 #define INTEL_FLIP_INACTIVE	0
 #define INTEL_FLIP_PENDING	1
-#define INTEL_FLIP_COMPLETE	2
 	u32 flip_count;
 	u32 gtt_offset;
 	struct drm_i915_gem_request *flip_queued_req;
@@ -1159,7 +1158,6 @@ struct drm_framebuffer *
 __intel_framebuffer_create(struct drm_device *dev,
 			   struct drm_mode_fb_cmd2 *mode_cmd,
 			   struct drm_i915_gem_object *obj);
-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);

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel