[PATCH 0/2] drm/i915: Fix warnings from atomic nonblocking unpin.

[PATCH 0/2] drm/i915: Fix warnings from atomic nonblocking unpin.

[Maarten Lankhorst]

Some small fixes to make CI more happy.

First is a fix to make all blocking calls wait for update to complete, so any call
done right doesn't have to block on it or return -EBUSY.

pin_count was limited to 15, so many cursor updates in a single vblank would cause it to reach the limit
and a warning. Bump this upwards for the time being. The proper fix is > 1 vblank depth, or passing
vma in the plane_state.

Maarten Lankhorst (2):
  drm/i915: Wait for flips to complete before returning.
  drm/i915: Bump pin_count to UINT_MAX.

 drivers/gpu/drm/i915/i915_gem_gtt.h  |  4 ++--
 drivers/gpu/drm/i915/intel_display.c | 26 ++++++++++++++++++++------
 2 files changed, 22 insertions(+), 8 deletions(-)

-- 
2.5.5

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

[PATCH] drm/i915: Wait for flips to complete before returning.

[Maarten Lankhorst]

Doing a page flip right after setcrtc would fail because part of the update is run
asynchronously. This is a regression and is caused kms_flip to fails without
the atomic page flip patch, and kms_frontbuffer_tracking to fail on ro-bdw-i7-5600u.

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 26 ++++++++++++++++++++------
 1 file changed, 20 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index a500f08ec0ac..21c0a2f3102b 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3788,7 +3788,7 @@ static void page_flip_completed(struct intel_crtc *intel_crtc, struct intel_flip
 	queue_work(dev_priv->wq, &work->unpin_work);
 }
 
-static int intel_crtc_wait_for_pending_flips(struct drm_crtc *crtc)
+static int intel_crtc_wait_for_pending_flips(struct drm_crtc *crtc, bool intr)
 {
 	struct drm_device *dev = crtc->dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
@@ -3796,10 +3796,15 @@ static int intel_crtc_wait_for_pending_flips(struct drm_crtc *crtc)
 
 	WARN_ON(waitqueue_active(&dev_priv->pending_flip_queue));
 
-	ret = wait_event_interruptible_timeout(
-					dev_priv->pending_flip_queue,
-					!intel_crtc_has_pending_flip(crtc),
-					60*HZ);
+	if (intr)
+		ret = wait_event_interruptible_timeout(
+						dev_priv->pending_flip_queue,
+						!intel_crtc_has_pending_flip(crtc),
+						60*HZ);
+	else
+		ret = wait_event_timeout(dev_priv->pending_flip_queue,
+					 !intel_crtc_has_pending_flip(crtc),
+					 60*HZ);
 
 	if (ret < 0)
 		return ret;
@@ -12736,7 +12741,7 @@ static int intel_atomic_prepare_commit(struct drm_device *dev,
 		struct intel_flip_work *work;
 
 		if (!state->legacy_cursor_update) {
-			ret = intel_crtc_wait_for_pending_flips(crtc);
+			ret = intel_crtc_wait_for_pending_flips(crtc, true);
 			if (ret)
 				return ret;
 
@@ -13005,6 +13010,7 @@ static int intel_atomic_commit(struct drm_device *dev,
 	struct drm_crtc_state *old_crtc_state;
 	struct drm_crtc *crtc;
 	int ret = 0, i;
+	unsigned crtc_mask = 0;
 
 	ret = intel_atomic_prepare_commit(dev, state, nonblock);
 	if (ret) {
@@ -13075,6 +13081,8 @@ static int intel_atomic_commit(struct drm_device *dev,
 		struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 		bool modeset = needs_modeset(crtc->state);
 
+		crtc_mask |= drm_crtc_mask(crtc);
+
 		if (modeset && crtc->state->active) {
 			update_scanline_offset(to_intel_crtc(crtc));
 			dev_priv->display.crtc_enable(crtc);
@@ -13105,6 +13113,12 @@ static int intel_atomic_commit(struct drm_device *dev,
 		intel_schedule_update(crtc, intel_state, work, nonblock);
 	}
 
+	if (!nonblock && !state->legacy_cursor_update) {
+		drm_for_each_crtc(crtc, dev)
+			if (crtc_mask & drm_crtc_mask(crtc))
+				intel_crtc_wait_for_pending_flips(crtc, false);
+	}
+
 	/* FIXME: add subpixel order */
 
 	drm_atomic_state_free(state);
-- 
2.5.5

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

[PATCH 2/2] drm/i915: Bump pin_count to UINT_MAX.

[Maarten Lankhorst]

With nonblocking unpin there can be many cursor pins before they're
cleared by the next page flip.

Fix this by extending pin_count to the full 32-bit to prevent a
WARN_ON(vma->pin_count == DRM_I915_GEM_OBJECT_MAX_PIN_COUNT)

Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reported-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Fixes: a6747b7304a9 ("drm/i915: Make unpin async.")
---
 drivers/gpu/drm/i915/i915_gem_gtt.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h b/drivers/gpu/drm/i915/i915_gem_gtt.h
index 62be77cac5cd..1d43cc290f71 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.h
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
@@ -218,8 +218,8 @@ struct i915_vma {
 	 *
 	 * In the worst case this is 1 + 1 + 1 + 2*2 = 7. That would fit into 3
 	 * bits with absolutely no headroom. So use 4 bits. */
-	unsigned int pin_count:4;
-#define DRM_I915_GEM_OBJECT_MAX_PIN_COUNT 0xf
+	unsigned int pin_count;
+#define DRM_I915_GEM_OBJECT_MAX_PIN_COUNT UINT_MAX
 };
 
 struct i915_page_dma {
-- 
2.5.5

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

[PATCH 1/2] drm/i915: Wait for flips to complete before returning.

[Maarten Lankhorst]

Doing a page flip right after setcrtc would fail because part of the update is run
asynchronously. This is a regression and is caused kms_flip to fails without
the atomic page flip patch, and kms_frontbuffer_tracking to fail on ro-bdw-i7-5600u.

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
Oops, stripped 1/2 from subject, fixed!

 drivers/gpu/drm/i915/intel_display.c | 26 ++++++++++++++++++++------
 1 file changed, 20 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index a500f08ec0ac..21c0a2f3102b 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3788,7 +3788,7 @@ static void page_flip_completed(struct intel_crtc *intel_crtc, struct intel_flip
 	queue_work(dev_priv->wq, &work->unpin_work);
 }
 
-static int intel_crtc_wait_for_pending_flips(struct drm_crtc *crtc)
+static int intel_crtc_wait_for_pending_flips(struct drm_crtc *crtc, bool intr)
 {
 	struct drm_device *dev = crtc->dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
@@ -3796,10 +3796,15 @@ static int intel_crtc_wait_for_pending_flips(struct drm_crtc *crtc)
 
 	WARN_ON(waitqueue_active(&dev_priv->pending_flip_queue));
 
-	ret = wait_event_interruptible_timeout(
-					dev_priv->pending_flip_queue,
-					!intel_crtc_has_pending_flip(crtc),
-					60*HZ);
+	if (intr)
+		ret = wait_event_interruptible_timeout(
+						dev_priv->pending_flip_queue,
+						!intel_crtc_has_pending_flip(crtc),
+						60*HZ);
+	else
+		ret = wait_event_timeout(dev_priv->pending_flip_queue,
+					 !intel_crtc_has_pending_flip(crtc),
+					 60*HZ);
 
 	if (ret < 0)
 		return ret;
@@ -12736,7 +12741,7 @@ static int intel_atomic_prepare_commit(struct drm_device *dev,
 		struct intel_flip_work *work;
 
 		if (!state->legacy_cursor_update) {
-			ret = intel_crtc_wait_for_pending_flips(crtc);
+			ret = intel_crtc_wait_for_pending_flips(crtc, true);
 			if (ret)
 				return ret;
 
@@ -13005,6 +13010,7 @@ static int intel_atomic_commit(struct drm_device *dev,
 	struct drm_crtc_state *old_crtc_state;
 	struct drm_crtc *crtc;
 	int ret = 0, i;
+	unsigned crtc_mask = 0;
 
 	ret = intel_atomic_prepare_commit(dev, state, nonblock);
 	if (ret) {
@@ -13075,6 +13081,8 @@ static int intel_atomic_commit(struct drm_device *dev,
 		struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 		bool modeset = needs_modeset(crtc->state);
 
+		crtc_mask |= drm_crtc_mask(crtc);
+
 		if (modeset && crtc->state->active) {
 			update_scanline_offset(to_intel_crtc(crtc));
 			dev_priv->display.crtc_enable(crtc);
@@ -13105,6 +13113,12 @@ static int intel_atomic_commit(struct drm_device *dev,
 		intel_schedule_update(crtc, intel_state, work, nonblock);
 	}
 
+	if (!nonblock && !state->legacy_cursor_update) {
+		drm_for_each_crtc(crtc, dev)
+			if (crtc_mask & drm_crtc_mask(crtc))
+				intel_crtc_wait_for_pending_flips(crtc, false);
+	}
+
 	/* FIXME: add subpixel order */
 
 	drm_atomic_state_free(state);
-- 
2.5.5

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

[PATCH 3/2] Revert "drm/i915: Allow nonblocking update of pageflips."

[Maarten Lankhorst]

This reverts commit d55dbd06bb5e1399aba9ab5227465339d1bbefff.

It lacks a description, removes the flip trace point,
doesn't handle -EBUSY if a flip is already queued
and needs to be redone.

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
Patch 3/2 which CI probably doesn't handle correctly,
so trybot results should be on https://patchwork.freedesktop.org/series/7570/

 drivers/gpu/drm/i915/intel_display.c | 350 ++++++++++++++++++++++++++---------
 1 file changed, 266 insertions(+), 84 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 21c0a2f3102b..f49ef629c4a8 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -108,6 +108,8 @@ static void vlv_prepare_pll(struct intel_crtc *crtc,
 			    const struct intel_crtc_state *pipe_config);
 static void chv_prepare_pll(struct intel_crtc *crtc,
 			    const struct intel_crtc_state *pipe_config);
+static void intel_begin_crtc_commit(struct drm_crtc *, struct drm_crtc_state *);
+static void intel_finish_crtc_commit(struct drm_crtc *, struct drm_crtc_state *);
 static void skl_init_scalers(struct drm_device *dev, struct intel_crtc *intel_crtc,
 	struct intel_crtc_state *crtc_state);
 static void skylake_pfit_enable(struct intel_crtc *crtc);
@@ -10984,7 +10986,7 @@ static void intel_mmio_flip_work_func(struct work_struct *w)
 	struct drm_device *dev = crtc->dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct drm_i915_gem_request *req;
-	int i, ret;
+	int i;
 
 	if (!needs_modeset(&crtc_state->base) && crtc_state->update_pipe) {
 		work->put_power_domains =
@@ -11006,14 +11008,7 @@ static void intel_mmio_flip_work_func(struct work_struct *w)
 					    &dev_priv->rps.mmioflips));
 	}
 
-	ret = drm_crtc_vblank_get(crtc);
-	I915_STATE_WARN(ret < 0, "enabling vblank failed with %i\n", ret);
-
-	if (work->num_planes &&
-	    work->old_plane_state[0]->base.plane == crtc->primary)
-		intel_fbc_enable(intel_crtc, work->new_crtc_state, work->new_plane_state[0]);
-
-	intel_frontbuffer_flip_prepare(dev, work->fb_bits);
+	intel_frontbuffer_flip_prepare(dev, crtc_state->fb_bits);
 
 	intel_pipe_update_start(intel_crtc);
 	if (!needs_modeset(&crtc_state->base)) {
@@ -11032,15 +11027,206 @@ static void intel_mmio_flip_work_func(struct work_struct *w)
 		struct intel_plane_state *new_plane_state = work->new_plane_state[i];
 		struct intel_plane *plane = to_intel_plane(new_plane_state->base.plane);
 
-		if (new_plane_state->visible)
-			plane->update_plane(&plane->base, crtc_state, new_plane_state);
-		else
-			plane->disable_plane(&plane->base, crtc);
+		plane->update_plane(&plane->base, crtc_state, new_plane_state);
 	}
 
 	intel_pipe_update_end(intel_crtc, work);
 }
 
+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,
+				uint32_t page_flip_flags)
+{
+	struct drm_device *dev = crtc->dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	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;
+	struct intel_flip_work *work;
+	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
+	 * a disabled primary plane.
+	 */
+	if (WARN_ON(intel_fb_obj(old_fb) == NULL))
+		return -EBUSY;
+
+	/* Can't change pixel format via MI display flips. */
+	if (fb->pixel_format != old_fb->pixel_format)
+		return -EINVAL;
+
+	/*
+	 * TILEOFF/LINOFF registers can't be changed via MI display flips.
+	 * Note that pitch changes could also affect these register.
+	 */
+	if (INTEL_INFO(dev)->gen > 3 &&
+	    (fb->offsets[0] != old_fb->offsets[0] ||
+	     fb->pitches[0] != old_fb->pitches[0]))
+		return -EINVAL;
+
+	work = kzalloc(sizeof(*work), GFP_KERNEL);
+	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;
+	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 cleanup;
+
+	/* We borrow the event spin lock for protecting flip_work */
+	spin_lock_irq(&dev->event_lock);
+	if (!list_empty(&intel_crtc->flip_work)) {
+		struct intel_flip_work *old_work;
+
+		old_work = list_last_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 (pageflip_finished(intel_crtc, old_work)) {
+			DRM_DEBUG_DRIVER("flip queue: previous flip completed, continuing\n");
+			page_flip_completed(intel_crtc, old_work);
+		} else {
+			DRM_DEBUG_DRIVER("flip queue: crtc already busy\n");
+			spin_unlock_irq(&dev->event_lock);
+
+			ret = -EBUSY;
+			goto cleanup_vblank;
+		}
+	}
+	list_add_tail(&work->head, &intel_crtc->flip_work);
+	spin_unlock_irq(&dev->event_lock);
+
+	if (atomic_read(&intel_crtc->unpin_work_count) >= 2)
+		flush_workqueue(dev_priv->wq);
+
+	/* step 2, similar to intel_prepare_plane_fb */
+	ret = mutex_lock_interruptible(&dev->struct_mutex);
+	if (ret)
+		goto cleanup_work;
+
+	ret = intel_pin_and_fence_fb_obj(fb, new_state->rotation);
+	if (ret)
+		goto cleanup_unlock;
+
+	i915_gem_track_fb(intel_fb_obj(old_fb), obj,
+			  to_intel_plane(primary)->frontbuffer_bit);
+
+	/* 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;
+
+	/* 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);
+
+	if (obj->base.dma_buf)
+		work->old_plane_state[0]->base.fence = intel_get_excl_fence(obj);
+
+	intel_fbc_pre_update(intel_crtc,
+			     to_intel_crtc_state(new_crtc_state),
+			     to_intel_plane_state(new_state));
+
+	schedule_work(&work->mmio_work);
+
+	mutex_unlock(&dev->struct_mutex);
+
+	trace_i915_flip_request(intel_crtc->plane, obj);
+
+	return 0;
+
+cleanup_unlock:
+	mutex_unlock(&dev->struct_mutex);
+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);
+cleanup:
+	if (new_state)
+		intel_plane_destroy_state(primary, new_state);
+
+	if (new_crtc_state)
+		intel_crtc_destroy_state(crtc, new_crtc_state);
+
+	intel_free_flip_work(work);
+	return ret;
+}
+
+
 /**
  * intel_wm_need_update - Check whether watermarks need updating
  * @plane: drm plane
@@ -11317,6 +11503,8 @@ static int intel_crtc_atomic_check(struct drm_crtc *crtc,
 
 static const struct drm_crtc_helper_funcs intel_helper_funcs = {
 	.mode_set_base_atomic = intel_pipe_set_base_atomic,
+	.atomic_begin = intel_begin_crtc_commit,
+	.atomic_flush = intel_finish_crtc_commit,
 	.atomic_check = intel_crtc_atomic_check,
 };
 
@@ -12736,6 +12924,11 @@ static int intel_atomic_prepare_commit(struct drm_device *dev,
 	struct drm_crtc *crtc;
 	int i, ret;
 
+	if (nonblock) {
+		DRM_DEBUG_KMS("i915 does not yet support nonblocking commit\n");
+		return -EINVAL;
+	}
+
 	for_each_crtc_in_state(state, crtc, crtc_state, i) {
 		struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 		struct intel_flip_work *work;
@@ -12777,11 +12970,6 @@ static int intel_atomic_prepare_commit(struct drm_device *dev,
 		}
 	}
 
-	if (intel_state->modeset && nonblock) {
-		DRM_DEBUG_ATOMIC("Nonblock modesets are not yet supported!\n");
-		return -EINVAL;
-	}
-
 	ret = mutex_lock_interruptible(&dev->struct_mutex);
 	if (ret)
 		return ret;
@@ -12935,41 +13123,13 @@ static void intel_schedule_unpin(struct drm_crtc *crtc,
 	queue_work(dev_priv->wq, &work->unpin_work);
 }
 
-static void intel_schedule_flip(struct drm_crtc *crtc,
-				struct intel_atomic_state *state,
-				struct intel_flip_work *work,
-				bool nonblock)
-{
-	struct intel_crtc_state *crtc_state = work->new_crtc_state;
-
-	if (crtc_state->base.planes_changed ||
-	    needs_modeset(&crtc_state->base) ||
-	    crtc_state->update_pipe) {
-		if (nonblock)
-			schedule_work(&work->mmio_work);
-		else
-			intel_mmio_flip_work_func(&work->mmio_work);
-	} else {
-		int ret;
-
-		ret = drm_crtc_vblank_get(crtc);
-		I915_STATE_WARN(ret < 0, "enabling vblank failed with %i\n", ret);
-
-		work->flip_queued_vblank = intel_crtc_get_vblank_counter(to_intel_crtc(crtc));
-		smp_mb__before_atomic();
-		atomic_set(&work->pending, 1);
-	}
-}
-
 static void intel_schedule_update(struct drm_crtc *crtc,
 				  struct intel_atomic_state *state,
-				  struct intel_flip_work *work,
-				  bool nonblock)
+				  struct intel_flip_work *work)
 {
 	struct drm_device *dev = crtc->dev;
-	struct intel_crtc_state *pipe_config = work->new_crtc_state;
 
-	if (!pipe_config->base.active && work->can_async_unpin) {
+	if (work->can_async_unpin) {
 		INIT_LIST_HEAD(&work->head);
 		intel_schedule_unpin(crtc, state, work);
 		return;
@@ -12979,10 +13139,7 @@ static void intel_schedule_update(struct drm_crtc *crtc,
 	list_add_tail(&work->head, &to_intel_crtc(crtc)->flip_work);
 	spin_unlock_irq(&dev->event_lock);
 
-	if (!pipe_config->base.active)
-		intel_schedule_unpin(crtc, state, work);
-	else
-		intel_schedule_flip(crtc, state, work, nonblock);
+	intel_schedule_unpin(crtc, state, work);
 }
 
 /**
@@ -13077,9 +13234,11 @@ static int intel_atomic_commit(struct drm_device *dev,
 
 	/* Now enable the clocks, plane, pipe, and connectors that we set up. */
 	for_each_crtc_in_state(state, crtc, old_crtc_state, i) {
-		struct intel_flip_work *work = intel_state->work[i];
 		struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 		bool modeset = needs_modeset(crtc->state);
+		struct intel_crtc_state *pipe_config =
+			to_intel_crtc_state(crtc->state);
+		bool update_pipe = !modeset && pipe_config->update_pipe;
 
 		crtc_mask |= drm_crtc_mask(crtc);
 
@@ -13091,6 +13250,22 @@ static int intel_atomic_commit(struct drm_device *dev,
 		if (!modeset)
 			intel_pre_plane_update(to_intel_crtc_state(old_crtc_state));
 
+		if (crtc->state->active &&
+		    drm_atomic_get_existing_plane_state(state, crtc->primary))
+			intel_fbc_enable(intel_crtc, pipe_config, to_intel_plane_state(crtc->primary->state));
+
+		if (crtc->state->active &&
+		    (crtc->state->planes_changed || update_pipe))
+			drm_atomic_helper_commit_planes_on_crtc(old_crtc_state);
+	}
+
+	/* FIXME: add subpixel order */
+
+	for_each_crtc_in_state(state, crtc, old_crtc_state, i) {
+		struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+		struct intel_flip_work *work =
+			intel_state->work[i];
+
 		if (!work) {
 			if (!list_empty_careful(&intel_crtc->flip_work)) {
 				spin_lock_irq(&dev->event_lock);
@@ -13110,7 +13285,7 @@ static int intel_atomic_commit(struct drm_device *dev,
 
 		intel_state->work[i] = NULL;
 		intel_prepare_work(crtc, work, state, old_crtc_state);
-		intel_schedule_update(crtc, intel_state, work, nonblock);
+		intel_schedule_update(crtc, intel_state, work);
 	}
 
 	if (!nonblock && !state->legacy_cursor_update) {
@@ -13119,8 +13294,6 @@ static int intel_atomic_commit(struct drm_device *dev,
 				intel_crtc_wait_for_pending_flips(crtc, false);
 	}
 
-	/* FIXME: add subpixel order */
-
 	drm_atomic_state_free(state);
 
 	/* As one of the primary mmio accessors, KMS has a high likelihood
@@ -13184,38 +13357,11 @@ static const struct drm_crtc_funcs intel_crtc_funcs = {
 	.set_config = drm_atomic_helper_set_config,
 	.set_property = drm_atomic_helper_crtc_set_property,
 	.destroy = intel_crtc_destroy,
-	.page_flip = drm_atomic_helper_page_flip,
+	.page_flip = intel_crtc_page_flip,
 	.atomic_duplicate_state = intel_crtc_duplicate_state,
 	.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
@@ -13402,6 +13548,42 @@ intel_check_primary_plane(struct drm_plane *plane,
 					     &state->visible);
 }
 
+static void intel_begin_crtc_commit(struct drm_crtc *crtc,
+				    struct drm_crtc_state *old_crtc_state)
+{
+	struct drm_device *dev = crtc->dev;
+	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+	struct intel_crtc_state *old_intel_state =
+		to_intel_crtc_state(old_crtc_state);
+	bool modeset = needs_modeset(crtc->state);
+
+	intel_frontbuffer_flip_prepare(dev, to_intel_crtc_state(crtc->state)->fb_bits);
+
+	/* Perform vblank evasion around commit operation */
+	intel_pipe_update_start(intel_crtc);
+
+	if (modeset)
+		return;
+
+	if (crtc->state->color_mgmt_changed || to_intel_crtc_state(crtc->state)->update_pipe) {
+		intel_color_set_csc(crtc->state);
+		intel_color_load_luts(crtc->state);
+	}
+
+	if (to_intel_crtc_state(crtc->state)->update_pipe)
+		intel_update_pipe_config(intel_crtc, old_intel_state);
+	else if (INTEL_INFO(dev)->gen >= 9)
+		skl_detach_scalers(intel_crtc);
+}
+
+static void intel_finish_crtc_commit(struct drm_crtc *crtc,
+				     struct drm_crtc_state *old_crtc_state)
+{
+	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+
+	intel_pipe_update_end(intel_crtc, NULL);
+}
+
 /**
  * intel_plane_destroy - destroy a plane
  * @plane: plane to destroy
-- 
2.5.5


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

Re: [PATCH 2/2] drm/i915: Bump pin_count to UINT_MAX.

[Chris Wilson]

On Mon, May 23, 2016 at 03:37:41PM +0200, Maarten Lankhorst wrote:
> With nonblocking unpin there can be many cursor pins before they're
> cleared by the next page flip.
> 
> Fix this by extending pin_count to the full 32-bit to prevent a
> WARN_ON(vma->pin_count == DRM_I915_GEM_OBJECT_MAX_PIN_COUNT)

This is a hack that affects non-KMS paths. Being able to process binding
in a single operation on all architectures is something we want to
preserve.

Why is every cursor movement generating an unpin work? Should I just
start poking registers from userspace to avoid a silly kerenl?
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Ro.CI.BAT: warning for drm/i915: Wait for flips to complete before returning.

[Patchwork]

== Series Details ==

Series: drm/i915: Wait for flips to complete before returning.
URL   : https://patchwork.freedesktop.org/series/7569/
State : warning

== Summary ==

Series 7569v1 drm/i915: Wait for flips to complete before returning.
http://patchwork.freedesktop.org/api/1.0/series/7569/revisions/1/mbox

Test gem_exec_flush:
        Subgroup basic-batch-kernel-default-cmd:
                fail       -> PASS       (fi-byt-n2820)
        Subgroup basic-uc-pro-default:
                pass       -> DMESG-WARN (ro-ivb-i7-3770)
Test kms_flip:
        Subgroup basic-flip-vs-wf_vblank:
                fail       -> PASS       (ro-bdw-i7-5600u)
Test kms_frontbuffer_tracking:
        Subgroup basic:
                fail       -> PASS       (ro-bdw-i7-5600u)

fi-bdw-i7-5557u  total:209  pass:197  dwarn:0   dfail:0   fail:0   skip:12 
fi-byt-n2820     total:209  pass:169  dwarn:0   dfail:0   fail:2   skip:38 
fi-hsw-i7-4770k  total:209  pass:190  dwarn:0   dfail:0   fail:0   skip:19 
fi-hsw-i7-4770r  total:209  pass:186  dwarn:0   dfail:0   fail:0   skip:23 
fi-skl-i5-6260u  total:209  pass:196  dwarn:2   dfail:0   fail:0   skip:11 
fi-skl-i7-6700k  total:209  pass:182  dwarn:2   dfail:0   fail:0   skip:25 
fi-snb-i7-2600   total:209  pass:170  dwarn:0   dfail:0   fail:0   skip:39 
ro-bdw-i5-5250u  total:209  pass:172  dwarn:0   dfail:0   fail:0   skip:37 
ro-bdw-i7-5557U  total:209  pass:197  dwarn:0   dfail:0   fail:0   skip:12 
ro-bdw-i7-5600u  total:209  pass:181  dwarn:0   dfail:0   fail:0   skip:28 
ro-bsw-n3050     total:209  pass:168  dwarn:0   dfail:0   fail:2   skip:39 
ro-byt-n2820     total:209  pass:169  dwarn:0   dfail:0   fail:3   skip:37 
ro-hsw-i3-4010u  total:209  pass:186  dwarn:0   dfail:0   fail:0   skip:23 
ro-hsw-i7-4770r  total:209  pass:186  dwarn:0   dfail:0   fail:0   skip:23 
ro-ilk-i7-620lm  total:209  pass:146  dwarn:0   dfail:0   fail:1   skip:62 
ro-ilk1-i5-650   total:204  pass:146  dwarn:0   dfail:0   fail:1   skip:57 
ro-ivb-i7-3770   total:209  pass:176  dwarn:1   dfail:0   fail:0   skip:32 
ro-ivb2-i7-3770  total:209  pass:181  dwarn:0   dfail:0   fail:0   skip:28 
ro-skl-i7-6700hq total:204  pass:180  dwarn:2   dfail:0   fail:0   skip:22 
ro-snb-i7-2620M  total:209  pass:170  dwarn:0   dfail:0   fail:1   skip:38 
fi-bsw-n3050 failed to connect after reboot

Results at /archive/results/CI_IGT_test/RO_Patchwork_975/

54afd5a drm-intel-nightly: 2016y-05m-23d-13h-38m-05s UTC integration manifest
cd73854 drm/i915: Bump pin_count to UINT_MAX.
efefadf drm/i915: Wait for flips to complete before returning.

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

Re: [PATCH 2/2] drm/i915: Bump pin_count to UINT_MAX.

[Maarten Lankhorst]

Op 23-05-16 om 16:25 schreef Chris Wilson:
> On Mon, May 23, 2016 at 03:37:41PM +0200, Maarten Lankhorst wrote:
>> With nonblocking unpin there can be many cursor pins before they're
>> cleared by the next page flip.
>>
>> Fix this by extending pin_count to the full 32-bit to prevent a
>> WARN_ON(vma->pin_count == DRM_I915_GEM_OBJECT_MAX_PIN_COUNT)
> This is a hack that affects non-KMS paths. Being able to process binding
> in a single operation on all architectures is something we want to
> preserve.
>
> Why is every cursor movement generating an unpin work? Should I just
> start poking registers from userspace to avoid a silly kerenl?
> -Chris
>
All the unpin work gets batched till after the next vblank, it's not very efficient
but if you want to fix it you should just add the vma to plane state already.

According to Ville unpin count would still be too low on BXT/SKL, so it wouldn't
remove the need for this patch anyway..

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

✗ Ro.CI.BAT: failure for drm/i915: Wait for flips to complete before returning.

[Patchwork]

== Series Details ==

Series: drm/i915: Wait for flips to complete before returning.
URL   : https://patchwork.freedesktop.org/series/7569/
State : failure

== Summary ==

Series 7569v1 drm/i915: Wait for flips to complete before returning.
http://patchwork.freedesktop.org/api/1.0/series/7569/revisions/1/mbox

Test kms_flip:
        Subgroup basic-flip-vs-wf_vblank:
                fail       -> PASS       (ro-bdw-i7-5600u)
Test kms_frontbuffer_tracking:
        Subgroup basic:
                fail       -> PASS       (ro-bdw-i7-5600u)
Test kms_pipe_crc_basic:
        Subgroup nonblocking-crc-pipe-b-frame-sequence:
                pass       -> FAIL       (ro-byt-n2820)
Test kms_sink_crc_basic:
                skip       -> PASS       (ro-skl-i7-6700hq)

fi-bdw-i7-5557u  total:209  pass:197  dwarn:0   dfail:0   fail:0   skip:12 
fi-bsw-n3050     total:209  pass:167  dwarn:0   dfail:0   fail:2   skip:40 
fi-hsw-i7-4770k  total:209  pass:190  dwarn:0   dfail:0   fail:0   skip:19 
fi-hsw-i7-4770r  total:209  pass:186  dwarn:0   dfail:0   fail:0   skip:23 
fi-skl-i5-6260u  total:209  pass:196  dwarn:2   dfail:0   fail:0   skip:11 
fi-skl-i7-6700k  total:209  pass:182  dwarn:2   dfail:0   fail:0   skip:25 
fi-snb-i7-2600   total:209  pass:170  dwarn:0   dfail:0   fail:0   skip:39 
ro-bdw-i5-5250u  total:209  pass:172  dwarn:0   dfail:0   fail:0   skip:37 
ro-bdw-i7-5557U  total:209  pass:197  dwarn:0   dfail:0   fail:0   skip:12 
ro-bdw-i7-5600u  total:209  pass:181  dwarn:0   dfail:0   fail:0   skip:28 
ro-bsw-n3050     total:209  pass:168  dwarn:0   dfail:0   fail:2   skip:39 
ro-byt-n2820     total:209  pass:168  dwarn:0   dfail:0   fail:4   skip:37 
ro-hsw-i3-4010u  total:209  pass:186  dwarn:0   dfail:0   fail:0   skip:23 
ro-hsw-i7-4770r  total:209  pass:186  dwarn:0   dfail:0   fail:0   skip:23 
ro-ilk-i7-620lm  total:209  pass:146  dwarn:0   dfail:0   fail:1   skip:62 
ro-ilk1-i5-650   total:204  pass:146  dwarn:0   dfail:0   fail:1   skip:57 
ro-ivb-i7-3770   total:209  pass:177  dwarn:0   dfail:0   fail:0   skip:32 
ro-ivb2-i7-3770  total:209  pass:181  dwarn:0   dfail:0   fail:0   skip:28 
ro-skl-i7-6700hq total:204  pass:181  dwarn:2   dfail:0   fail:0   skip:21 
fi-byt-n2820 failed to connect after reboot
ro-snb-i7-2620M failed to connect after reboot

Results at /archive/results/CI_IGT_test/RO_Patchwork_976/

54afd5a drm-intel-nightly: 2016y-05m-23d-13h-38m-05s UTC integration manifest
0c72178 drm/i915: Bump pin_count to UINT_MAX.
4ce00ee drm/i915: Wait for flips to complete before returning.

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

Re: [PATCH 2/2] drm/i915: Bump pin_count to UINT_MAX.

[Chris Wilson]

On Mon, May 23, 2016 at 05:09:05PM +0200, Maarten Lankhorst wrote:
> Op 23-05-16 om 16:25 schreef Chris Wilson:
> > On Mon, May 23, 2016 at 03:37:41PM +0200, Maarten Lankhorst wrote:
> >> With nonblocking unpin there can be many cursor pins before they're
> >> cleared by the next page flip.
> >>
> >> Fix this by extending pin_count to the full 32-bit to prevent a
> >> WARN_ON(vma->pin_count == DRM_I915_GEM_OBJECT_MAX_PIN_COUNT)
> > This is a hack that affects non-KMS paths. Being able to process binding
> > in a single operation on all architectures is something we want to
> > preserve.
> >
> > Why is every cursor movement generating an unpin work? Should I just
> > start poking registers from userspace to avoid a silly kerenl?
> > -Chris
> >
> All the unpin work gets batched till after the next vblank, it's not very efficient
> but if you want to fix it you should just add the vma to plane state already.

I still don't see where the flush will occur, or why vblanks would be
running at all for cursor updates.
 
> According to Ville unpin count would still be too low on BXT/SKL, so it wouldn't
> remove the need for this patch anyway..

Increasing the count is one thing, taking all 32bits as a workaround for
poor behaviour in the cursor update is another.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/2] drm/i915: Bump pin_count to UINT_MAX.

[Maarten Lankhorst]

Op 23-05-16 om 17:43 schreef Chris Wilson:
> On Mon, May 23, 2016 at 05:09:05PM +0200, Maarten Lankhorst wrote:
>> Op 23-05-16 om 16:25 schreef Chris Wilson:
>>> On Mon, May 23, 2016 at 03:37:41PM +0200, Maarten Lankhorst wrote:
>>>> With nonblocking unpin there can be many cursor pins before they're
>>>> cleared by the next page flip.
>>>>
>>>> Fix this by extending pin_count to the full 32-bit to prevent a
>>>> WARN_ON(vma->pin_count == DRM_I915_GEM_OBJECT_MAX_PIN_COUNT)
>>> This is a hack that affects non-KMS paths. Being able to process binding
>>> in a single operation on all architectures is something we want to
>>> preserve.
>>>
>>> Why is every cursor movement generating an unpin work? Should I just
>>> start poking registers from userspace to avoid a silly kerenl?
>>> -Chris
>>>
>> All the unpin work gets batched till after the next vblank, it's not very efficient
>> but if you want to fix it you should just add the vma to plane state already.
> I still don't see where the flush will occur, or why vblanks would be
> running at all for cursor updates.
Next page flip. All cursor updates are added to flip_work list and are cleared on vblank.
>> According to Ville unpin count would still be too low on BXT/SKL, so it wouldn't
>> remove the need for this patch anyway..
> Increasing the count is one thing, taking all 32bits as a workaround for
> poor behaviour in the cursor update is another.
> -Chris
>

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

Re: [PATCH 2/2] drm/i915: Bump pin_count to UINT_MAX.

[Chris Wilson]

On Mon, May 23, 2016 at 06:09:31PM +0200, Maarten Lankhorst wrote:
> Op 23-05-16 om 17:43 schreef Chris Wilson:
> > On Mon, May 23, 2016 at 05:09:05PM +0200, Maarten Lankhorst wrote:
> >> Op 23-05-16 om 16:25 schreef Chris Wilson:
> >>> On Mon, May 23, 2016 at 03:37:41PM +0200, Maarten Lankhorst wrote:
> >>>> With nonblocking unpin there can be many cursor pins before they're
> >>>> cleared by the next page flip.
> >>>>
> >>>> Fix this by extending pin_count to the full 32-bit to prevent a
> >>>> WARN_ON(vma->pin_count == DRM_I915_GEM_OBJECT_MAX_PIN_COUNT)
> >>> This is a hack that affects non-KMS paths. Being able to process binding
> >>> in a single operation on all architectures is something we want to
> >>> preserve.
> >>>
> >>> Why is every cursor movement generating an unpin work? Should I just
> >>> start poking registers from userspace to avoid a silly kerenl?
> >>> -Chris
> >>>
> >> All the unpin work gets batched till after the next vblank, it's not very efficient
> >> but if you want to fix it you should just add the vma to plane state already.
> > I still don't see where the flush will occur, or why vblanks would be
> > running at all for cursor updates.
> Next page flip. All cursor updates are added to flip_work list and are cleared on vblank.

Now knowing the trigger, I've written a reproducer: igt/kms_cursor_legacy
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/2] drm/i915: Bump pin_count to UINT_MAX.

[Daniel Vetter]

On Mon, May 23, 2016 at 03:37:41PM +0200, Maarten Lankhorst wrote:
> With nonblocking unpin there can be many cursor pins before they're
> cleared by the next page flip.
> 
> Fix this by extending pin_count to the full 32-bit to prevent a
> WARN_ON(vma->pin_count == DRM_I915_GEM_OBJECT_MAX_PIN_COUNT)
> 
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Reported-by: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Fixes: a6747b7304a9 ("drm/i915: Make unpin async.")
> ---
>  drivers/gpu/drm/i915/i915_gem_gtt.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h b/drivers/gpu/drm/i915/i915_gem_gtt.h
> index 62be77cac5cd..1d43cc290f71 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.h
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
> @@ -218,8 +218,8 @@ struct i915_vma {
>  	 *
>  	 * In the worst case this is 1 + 1 + 1 + 2*2 = 7. That would fit into 3
>  	 * bits with absolutely no headroom. So use 4 bits. */
> -	unsigned int pin_count:4;
> -#define DRM_I915_GEM_OBJECT_MAX_PIN_COUNT 0xf
> +	unsigned int pin_count;
> +#define DRM_I915_GEM_OBJECT_MAX_PIN_COUNT UINT_MAX

You need to read up on some of the history of this. The problem is that
some peeps have too many rt threads and managed to sufficiently stall our
unpin worker until we ran out of memory. Well, we would have if not for
this check here. The real fix should be to eventually stall for the unpin
workers to complete (like we do/did in the old pageflip code), or to have
an explicit (per-crtc probably) list of to-be-unpinned stuff, so that we
can process old unpins synchronously once they're completed. Just bumping
the limit so that no one notices that we leak pin counts like bad ain't a
fix ;-)
-Daniel

>  };
>  
>  struct i915_page_dma {
> -- 
> 2.5.5
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx