All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/5] drm/i915: Signal drm events for atomic
@ 2016-06-13 14:13 Daniel Vetter
  2016-06-13 14:13 ` [PATCH 2/5] drm/i915: Roll out the helper nonblock tracking Daniel Vetter
                   ` (5 more replies)
  0 siblings, 6 replies; 18+ messages in thread
From: Daniel Vetter @ 2016-06-13 14:13 UTC (permalink / raw)
  To: Intel Graphics Development; +Cc: Daniel Vetter

This is part of what atomic must implement. And it's also required
to be able to use the helper nonblocking support.

v2: Always send out the drm event, remove the planes_changed check.

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/intel_display.c | 13 ++++++++++---
 drivers/gpu/drm/i915/intel_sprite.c  | 14 ++++++++++++++
 2 files changed, 24 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 801e4c17dd8d..a4ebf9df7dc3 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -13774,13 +13774,21 @@ static int intel_atomic_commit(struct drm_device *dev,
 		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;
 
 		if (modeset && crtc->state->active) {
 			update_scanline_offset(to_intel_crtc(crtc));
 			dev_priv->display.crtc_enable(crtc);
 		}
 
+		/* Complete events for now disable pipes here. */
+		if (modeset && !crtc->state->active && crtc->state->event) {
+			spin_lock_irq(&dev->event_lock);
+			drm_crtc_send_vblank_event(crtc, crtc->state->event);
+			spin_unlock_irq(&dev->event_lock);
+
+			crtc->state->event = NULL;
+		}
+
 		if (!modeset)
 			intel_pre_plane_update(to_intel_crtc_state(old_crtc_state));
 
@@ -13788,8 +13796,7 @@ static int intel_atomic_commit(struct drm_device *dev,
 		    drm_atomic_get_existing_plane_state(state, crtc->primary))
 			intel_fbc_enable(intel_crtc);
 
-		if (crtc->state->active &&
-		    (crtc->state->planes_changed || update_pipe))
+		if (crtc->state->active)
 			drm_atomic_helper_commit_planes_on_crtc(old_crtc_state);
 
 		if (pipe_config->base.active && needs_vblank_wait(pipe_config))
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index 324ccb06397d..fc654173c491 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -166,6 +166,20 @@ void intel_pipe_update_end(struct intel_crtc *crtc, struct intel_flip_work *work
 
 	trace_i915_pipe_update_end(crtc, end_vbl_count, scanline_end);
 
+	/* We're still in the vblank-evade critical section, this can't race.
+	 * Would be slightly nice to just grab the vblank count and arm the
+	 * event outside of the critical section - the spinlock might spin for a
+	 * while ... */
+	if (crtc->base.state->event) {
+		WARN_ON(drm_crtc_vblank_get(&crtc->base) != 0);
+
+		spin_lock(&crtc->base.dev->event_lock);
+		drm_crtc_arm_vblank_event(&crtc->base, crtc->base.state->event);
+		spin_unlock(&crtc->base.dev->event_lock);
+
+		crtc->base.state->event = NULL;
+	}
+
 	local_irq_enable();
 
 	if (crtc->debug.start_vbl_count &&
-- 
2.8.1

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

^ permalink raw reply related	[flat|nested] 18+ messages in thread
* [PATCH 2/5] drm/i915: Roll out the helper nonblock tracking
  2016-06-13 14:13 [PATCH 1/5] drm/i915: Signal drm events for atomic Daniel Vetter
@ 2016-06-13 14:13 ` Daniel Vetter
  2016-06-14 12:24   ` [PATCH 2.5/5] Reapply "drm/i915: Pass atomic states to fbc update, functions." Maarten Lankhorst
  2016-06-13 14:13 ` [PATCH 3/5] drm/i915: nonblocking commit Daniel Vetter
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 18+ messages in thread
From: Daniel Vetter @ 2016-06-13 14:13 UTC (permalink / raw)
  To: Intel Graphics Development; +Cc: Daniel Vetter, Daniel Vetter

Right now still all blocking, no worker anywhere to be seen.

Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index a4ebf9df7dc3..d68ca2825cd7 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -13697,6 +13697,10 @@ static int intel_atomic_commit(struct drm_device *dev,
 	unsigned long put_domains[I915_MAX_PIPES] = {};
 	unsigned crtc_vblank_mask = 0;
 
+	ret = drm_atomic_helper_setup_commit(state, nonblock);
+	if (ret)
+		return ret;
+
 	ret = intel_atomic_prepare_commit(dev, state, nonblock);
 	if (ret) {
 		DRM_DEBUG_ATOMIC("Preparing state failed with %i\n", ret);
@@ -13708,6 +13712,8 @@ static int intel_atomic_commit(struct drm_device *dev,
 	dev_priv->wm.skl_results = intel_state->wm_results;
 	intel_shared_dpll_commit(state);
 
+	drm_atomic_helper_wait_for_dependencies(state);
+
 	if (intel_state->modeset) {
 		memcpy(dev_priv->min_pixclk, intel_state->min_pixclk,
 		       sizeof(intel_state->min_pixclk));
@@ -13803,7 +13809,7 @@ static int intel_atomic_commit(struct drm_device *dev,
 			crtc_vblank_mask |= 1 << i;
 	}
 
-	/* FIXME: add subpixel order */
+	drm_atomic_helper_commit_hw_done(state);
 
 	if (!state->legacy_cursor_update)
 		intel_atomic_wait_for_vblanks(dev, dev_priv, crtc_vblank_mask);
@@ -13838,6 +13844,8 @@ static int intel_atomic_commit(struct drm_device *dev,
 	drm_atomic_helper_cleanup_planes(dev, state);
 	mutex_unlock(&dev->struct_mutex);
 
+	drm_atomic_helper_commit_cleanup_done(state);
+
 	drm_atomic_state_free(state);
 
 	/* As one of the primary mmio accessors, KMS has a high likelihood
-- 
2.8.1

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

^ permalink raw reply related	[flat|nested] 18+ messages in thread
* [PATCH 3/5] drm/i915: nonblocking commit
  2016-06-13 14:13 [PATCH 1/5] drm/i915: Signal drm events for atomic Daniel Vetter
  2016-06-13 14:13 ` [PATCH 2/5] drm/i915: Roll out the helper nonblock tracking Daniel Vetter
@ 2016-06-13 14:13 ` Daniel Vetter
  2016-06-14  7:24   ` Maarten Lankhorst
                     ` (2 more replies)
  2016-06-13 14:13 ` [PATCH 4/5] drm/i915: Move fb_bits updating later in atomic_commit Daniel Vetter
                   ` (3 subsequent siblings)
  5 siblings, 3 replies; 18+ messages in thread
From: Daniel Vetter @ 2016-06-13 14:13 UTC (permalink / raw)
  To: Intel Graphics Development; +Cc: Daniel Vetter, Daniel Vetter

Simply split intel_atomic_commit in half and place the new
nonblocking commit helpers at the right spots.

NOTE: There's still trouble with obj->frontbuffer bits getting mangled
when pipelining atomic commits.

v2:
- Remove the check for nonblocking which returned -EINVAL.
- Do wait for requests in the worker thread before committing
  hw state.

v3: Move hw_done after the optimize_wm/post_plane_update step, plus
add FIXME comment how to fix that up again properly.

v4: Update FIXME for intel_atomic_commit - more stuff works now.

v5: Still reject nonblocking modeset commits (Maarten).

Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 126 ++++++++++++++++++++++++-----------
 1 file changed, 87 insertions(+), 39 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index d68ca2825cd7..a16a307dbb76 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -13544,12 +13544,12 @@ 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) {
+		if (needs_modeset(crtc_state) && nonblock) {
+			DRM_DEBUG_KMS("nonblocking commit for modeset not yet implemented.\n");
+			return -EINVAL;
+		}
+
 		if (state->legacy_cursor_update)
 			continue;
 
@@ -13667,50 +13667,34 @@ static bool needs_vblank_wait(struct intel_crtc_state *crtc_state)
 	return false;
 }
 
-/**
- * intel_atomic_commit - commit validated state object
- * @dev: DRM device
- * @state: the top-level driver state object
- * @nonblock: nonblocking commit
- *
- * This function commits a top-level state object that has been validated
- * with drm_atomic_helper_check().
- *
- * FIXME:  Atomic modeset support for i915 is not yet complete.  At the moment
- * we can only handle plane-related operations and do not yet support
- * nonblocking commit.
- *
- * RETURNS
- * Zero for success or -errno.
- */
-static int intel_atomic_commit(struct drm_device *dev,
-			       struct drm_atomic_state *state,
-			       bool nonblock)
+static void intel_atomic_commit_tail(struct drm_atomic_state *state)
 {
+	struct drm_device *dev = state->dev;
 	struct intel_atomic_state *intel_state = to_intel_atomic_state(state);
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct drm_crtc_state *old_crtc_state;
 	struct drm_crtc *crtc;
 	struct intel_crtc_state *intel_cstate;
-	int ret = 0, i;
+	struct drm_plane *plane;
+	struct drm_plane_state *plane_state;
 	bool hw_check = intel_state->modeset;
 	unsigned long put_domains[I915_MAX_PIPES] = {};
 	unsigned crtc_vblank_mask = 0;
+	int i, ret;
 
-	ret = drm_atomic_helper_setup_commit(state, nonblock);
-	if (ret)
-		return ret;
+	for_each_plane_in_state(state, plane, plane_state, i) {
+		struct intel_plane_state *intel_plane_state =
+			to_intel_plane_state(plane_state);
 
-	ret = intel_atomic_prepare_commit(dev, state, nonblock);
-	if (ret) {
-		DRM_DEBUG_ATOMIC("Preparing state failed with %i\n", ret);
-		return ret;
-	}
+		if (!intel_plane_state->wait_req)
+			continue;
 
-	drm_atomic_helper_swap_state(state, true);
-	dev_priv->wm.distrust_bios_wm = false;
-	dev_priv->wm.skl_results = intel_state->wm_results;
-	intel_shared_dpll_commit(state);
+		ret = __i915_wait_request(intel_plane_state->wait_req,
+					  true, NULL, NULL);
+		/* EIO should be eaten, and we can't get interrupted in the
+		 * worker, and blocking commits have waited already. */
+		WARN_ON(ret);
+	}
 
 	drm_atomic_helper_wait_for_dependencies(state);
 
@@ -13809,8 +13793,15 @@ static int intel_atomic_commit(struct drm_device *dev,
 			crtc_vblank_mask |= 1 << i;
 	}
 
-	drm_atomic_helper_commit_hw_done(state);
-
+	/* FIXME: We should call drm_atomic_helper_commit_hw_done() here
+	 * already, but still need the state for the delayed optimization. To
+	 * fix this:
+	 * - wrap the optimization/post_plane_update stuff into a per-crtc work.
+	 * - schedule that vblank worker _before_ calling hw_done
+	 * - at the start of commit_tail, cancel it _synchrously
+	 * - switch over to the vblank wait helper in the core after that since
+	 *   we don't need out special handling any more.
+	 */
 	if (!state->legacy_cursor_update)
 		intel_atomic_wait_for_vblanks(dev, dev_priv, crtc_vblank_mask);
 
@@ -13837,6 +13828,8 @@ static int intel_atomic_commit(struct drm_device *dev,
 		intel_modeset_verify_crtc(crtc, old_crtc_state, crtc->state);
 	}
 
+	drm_atomic_helper_commit_hw_done(state);
+
 	if (intel_state->modeset)
 		intel_display_power_put(dev_priv, POWER_DOMAIN_MODESET);
 
@@ -13860,6 +13853,61 @@ static int intel_atomic_commit(struct drm_device *dev,
 	 * can happen also when the device is completely off.
 	 */
 	intel_uncore_arm_unclaimed_mmio_detection(dev_priv);
+}
+
+static void intel_atomic_commit_work(struct work_struct *work)
+{
+	struct drm_atomic_state *state = container_of(work,
+						      struct drm_atomic_state,
+						      commit_work);
+	intel_atomic_commit_tail(state);
+}
+
+/**
+ * intel_atomic_commit - commit validated state object
+ * @dev: DRM device
+ * @state: the top-level driver state object
+ * @nonblock: nonblocking commit
+ *
+ * This function commits a top-level state object that has been validated
+ * with drm_atomic_helper_check().
+ *
+ * FIXME:  Atomic modeset support for i915 is not yet complete.  At the moment
+ * nonblocking commits are only safe for pure plane updates. Everything else
+ * should work though.
+ *
+ * RETURNS
+ * Zero for success or -errno.
+ */
+static int intel_atomic_commit(struct drm_device *dev,
+			       struct drm_atomic_state *state,
+			       bool nonblock)
+{
+	struct intel_atomic_state *intel_state = to_intel_atomic_state(state);
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	int ret = 0;
+
+	ret = drm_atomic_helper_setup_commit(state, nonblock);
+	if (ret)
+		return ret;
+
+	INIT_WORK(&state->commit_work, intel_atomic_commit_work);
+
+	ret = intel_atomic_prepare_commit(dev, state, nonblock);
+	if (ret) {
+		DRM_DEBUG_ATOMIC("Preparing state failed with %i\n", ret);
+		return ret;
+	}
+
+	drm_atomic_helper_swap_state(state, true);
+	dev_priv->wm.distrust_bios_wm = false;
+	dev_priv->wm.skl_results = intel_state->wm_results;
+	intel_shared_dpll_commit(state);
+
+	if (nonblock)
+		queue_work(system_unbound_wq, &state->commit_work);
+	else
+		intel_atomic_commit_tail(state);
 
 	return 0;
 }
-- 
2.8.1

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

^ permalink raw reply related	[flat|nested] 18+ messages in thread
* [PATCH 4/5] drm/i915: Move fb_bits updating later in atomic_commit
  2016-06-13 14:13 [PATCH 1/5] drm/i915: Signal drm events for atomic Daniel Vetter
  2016-06-13 14:13 ` [PATCH 2/5] drm/i915: Roll out the helper nonblock tracking Daniel Vetter
  2016-06-13 14:13 ` [PATCH 3/5] drm/i915: nonblocking commit Daniel Vetter
@ 2016-06-13 14:13 ` Daniel Vetter
  2016-06-13 14:57   ` Chris Wilson
  2016-06-13 14:13 ` [PATCH 5/5] drm/i915: Use atomic commits for legacy page_flips Daniel Vetter
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 18+ messages in thread
From: Daniel Vetter @ 2016-06-13 14:13 UTC (permalink / raw)
  To: Intel Graphics Development; +Cc: Daniel Vetter, Daniel Vetter

Currently it's part of prepare_fb, still in the first phase of
atomic_commit which might fail. Which means that we need to have some
heuristics in cleanup_fb to figure out whether things failed, or
whether we just clean up the old fbs.

That's fragile, and worse, once we start pipelining commits gets
confused: While the last commit is still getting cleanup up we already
hammer in the new one, and fb_bits aren't refcounted, resulting in
lost bits and WARN_ON galore. We could instead try to make cleanup_fb
more clever, but a simpler fix is to postpone the fb_bits tracking
past the point of no return, where we commit all the software state.

That also makes conceptually more sense, since fb_bits must be updated
synchronously from the ioctl (they track usage from userspace pov, not
from the hw pov), right before we're fully committed to the updated.

This fixes WARNING splats from track_fb with page_flip implemented
through atomic_commit.

Testcase: igt/kms_flip/flip-vs-rmfb
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 41 ++++++++++++++++++++++--------------
 1 file changed, 25 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index a16a307dbb76..fd46db4882e5 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -13863,6 +13863,25 @@ static void intel_atomic_commit_work(struct work_struct *work)
 	intel_atomic_commit_tail(state);
 }
 
+static void intel_atomic_track_fbs(struct drm_atomic_state *state)
+{
+	struct drm_plane_state *old_plane_state;
+	struct drm_plane *plane;
+	struct drm_i915_gem_object *obj, *old_obj;
+	struct intel_plane *intel_plane;
+	int i;
+
+	mutex_lock(&state->dev->struct_mutex);
+	for_each_plane_in_state(state, plane, old_plane_state, i) {
+		obj = intel_fb_obj(plane->state->fb);
+		old_obj = intel_fb_obj(old_plane_state->fb);
+		intel_plane = to_intel_plane(plane);
+
+		i915_gem_track_fb(old_obj, obj, intel_plane->frontbuffer_bit);
+	}
+	mutex_unlock(&state->dev->struct_mutex);
+}
+
 /**
  * intel_atomic_commit - commit validated state object
  * @dev: DRM device
@@ -13903,6 +13922,7 @@ static int intel_atomic_commit(struct drm_device *dev,
 	dev_priv->wm.distrust_bios_wm = false;
 	dev_priv->wm.skl_results = intel_state->wm_results;
 	intel_shared_dpll_commit(state);
+	intel_atomic_track_fbs(state);
 
 	if (nonblock)
 		queue_work(system_unbound_wq, &state->commit_work);
@@ -13982,7 +14002,6 @@ intel_prepare_plane_fb(struct drm_plane *plane,
 {
 	struct drm_device *dev = plane->dev;
 	struct drm_framebuffer *fb = new_state->fb;
-	struct intel_plane *intel_plane = to_intel_plane(plane);
 	struct drm_i915_gem_object *obj = intel_fb_obj(fb);
 	struct drm_i915_gem_object *old_obj = intel_fb_obj(plane->state->fb);
 	int ret = 0;
@@ -14039,16 +14058,12 @@ intel_prepare_plane_fb(struct drm_plane *plane,
 		ret = intel_pin_and_fence_fb_obj(fb, new_state->rotation);
 	}
 
-	if (ret == 0) {
-		if (obj) {
-			struct intel_plane_state *plane_state =
-				to_intel_plane_state(new_state);
-
-			i915_gem_request_assign(&plane_state->wait_req,
-						obj->last_write_req);
-		}
+	if (ret == 0 && obj) {
+		struct intel_plane_state *plane_state =
+			to_intel_plane_state(new_state);
 
-		i915_gem_track_fb(old_obj, obj, intel_plane->frontbuffer_bit);
+		i915_gem_request_assign(&plane_state->wait_req,
+					obj->last_write_req);
 	}
 
 	return ret;
@@ -14068,7 +14083,6 @@ intel_cleanup_plane_fb(struct drm_plane *plane,
 		       const struct drm_plane_state *old_state)
 {
 	struct drm_device *dev = plane->dev;
-	struct intel_plane *intel_plane = to_intel_plane(plane);
 	struct intel_plane_state *old_intel_state;
 	struct drm_i915_gem_object *old_obj = intel_fb_obj(old_state->fb);
 	struct drm_i915_gem_object *obj = intel_fb_obj(plane->state->fb);
@@ -14082,11 +14096,6 @@ intel_cleanup_plane_fb(struct drm_plane *plane,
 	    !INTEL_INFO(dev)->cursor_needs_physical))
 		intel_unpin_fb_obj(old_state->fb, old_state->rotation);
 
-	/* prepare_fb aborted? */
-	if ((old_obj && (old_obj->frontbuffer_bits & intel_plane->frontbuffer_bit)) ||
-	    (obj && !(obj->frontbuffer_bits & intel_plane->frontbuffer_bit)))
-		i915_gem_track_fb(old_obj, obj, intel_plane->frontbuffer_bit);
-
 	i915_gem_request_assign(&old_intel_state->wait_req, NULL);
 }
 
-- 
2.8.1

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

^ permalink raw reply related	[flat|nested] 18+ messages in thread
* [PATCH 5/5] drm/i915: Use atomic commits for legacy page_flips
  2016-06-13 14:13 [PATCH 1/5] drm/i915: Signal drm events for atomic Daniel Vetter
                   ` (2 preceding siblings ...)
  2016-06-13 14:13 ` [PATCH 4/5] drm/i915: Move fb_bits updating later in atomic_commit Daniel Vetter
@ 2016-06-13 14:13 ` Daniel Vetter
  2016-06-15  8:01   ` Maarten Lankhorst
  2016-06-13 14:44 ` ✗ Ro.CI.BAT: failure for series starting with [1/5] drm/i915: Signal drm events for atomic Patchwork
  2016-06-14  8:21 ` [PATCH 1/5] " Maarten Lankhorst
  5 siblings, 1 reply; 18+ messages in thread
From: Daniel Vetter @ 2016-06-13 14:13 UTC (permalink / raw)
  To: Intel Graphics Development; +Cc: Daniel Vetter

Note that I didn't start garbage collecting all the legacy flip code
yet, to make it easier to revert this. But there will be _lots_ of
code that can be removed once this is tested on all platforms.

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/intel_display.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index fd46db4882e5..de1ff4a85b5e 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -11619,6 +11619,7 @@ void intel_check_page_flip(struct drm_i915_private *dev_priv, int pipe)
 	spin_unlock(&dev->event_lock);
 }
 
+__attribute__((unused))
 static int intel_crtc_page_flip(struct drm_crtc *crtc,
 				struct drm_framebuffer *fb,
 				struct drm_pending_vblank_event *event,
@@ -13977,7 +13978,7 @@ 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 = intel_crtc_page_flip,
+	.page_flip = drm_atomic_helper_page_flip,
 	.atomic_duplicate_state = intel_crtc_duplicate_state,
 	.atomic_destroy_state = intel_crtc_destroy_state,
 };
-- 
2.8.1

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

^ permalink raw reply related	[flat|nested] 18+ messages in thread
* ✗ Ro.CI.BAT: failure for series starting with [1/5] drm/i915: Signal drm events for atomic
  2016-06-13 14:13 [PATCH 1/5] drm/i915: Signal drm events for atomic Daniel Vetter
                   ` (3 preceding siblings ...)
  2016-06-13 14:13 ` [PATCH 5/5] drm/i915: Use atomic commits for legacy page_flips Daniel Vetter
@ 2016-06-13 14:44 ` Patchwork
  2016-06-14  8:21 ` [PATCH 1/5] " Maarten Lankhorst
  5 siblings, 0 replies; 18+ messages in thread
From: Patchwork @ 2016-06-13 14:44 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: intel-gfx

== Series Details ==

Series: series starting with [1/5] drm/i915: Signal drm events for atomic
URL   : https://patchwork.freedesktop.org/series/8632/
State : failure

== Summary ==

Series 8632v1 Series without cover letter
http://patchwork.freedesktop.org/api/1.0/series/8632/revisions/1/mbox

Test drv_module_reload_basic:
                pass       -> SKIP       (ro-hsw-i3-4010u)
                dmesg-warn -> PASS       (fi-skl-i5-6260u)
Test gem_exec_flush:
        Subgroup basic-batch-kernel-default-cmd:
                pass       -> FAIL       (ro-byt-n2820)
Test kms_flip:
        Subgroup basic-flip-vs-dpms:
                pass       -> DMESG-WARN (ro-hsw-i7-4770r)
                pass       -> DMESG-WARN (ro-bdw-i7-5600u)
                pass       -> DMESG-WARN (ro-hsw-i3-4010u)
                pass       -> DMESG-WARN (ro-bdw-i5-5250u)
        Subgroup basic-flip-vs-modeset:
                pass       -> DMESG-WARN (ro-hsw-i7-4770r)
                pass       -> DMESG-WARN (ro-bdw-i7-5600u)
                pass       -> DMESG-WARN (ro-hsw-i3-4010u)
                pass       -> DMESG-WARN (ro-bdw-i5-5250u)
        Subgroup basic-flip-vs-wf_vblank:
                pass       -> DMESG-WARN (ro-hsw-i7-4770r)
                pass       -> DMESG-WARN (ro-bdw-i5-5250u)
                pass       -> DMESG-WARN (ro-hsw-i3-4010u)
                pass       -> DMESG-WARN (ro-bdw-i7-5600u)
                pass       -> DMESG-WARN (fi-bdw-i7-5557u)
        Subgroup basic-plain-flip:
                pass       -> DMESG-WARN (ro-hsw-i7-4770r)
                pass       -> DMESG-WARN (ro-bdw-i5-5250u)
                pass       -> DMESG-WARN (ro-hsw-i3-4010u)
                pass       -> DMESG-WARN (ro-bdw-i7-5600u)
                pass       -> DMESG-WARN (fi-bdw-i7-5557u)
Test kms_frontbuffer_tracking:
        Subgroup basic:
                pass       -> DMESG-WARN (ro-hsw-i7-4770r)
                pass       -> DMESG-WARN (ro-bdw-i5-5250u)
                pass       -> DMESG-WARN (ro-hsw-i3-4010u)
                pass       -> DMESG-WARN (ro-bdw-i7-5600u)
                pass       -> DMESG-WARN (fi-bdw-i7-5557u)
Test kms_pipe_crc_basic:
        Subgroup hang-read-crc-pipe-c:
                pass       -> DMESG-WARN (ro-ivb2-i7-3770)
        Subgroup nonblocking-crc-pipe-c-frame-sequence:
                pass       -> SKIP       (fi-skl-i5-6260u)
        Subgroup suspend-read-crc-pipe-b:
                dmesg-warn -> SKIP       (ro-bdw-i5-5250u)

fi-bdw-i7-5557u  total:213  pass:198  dwarn:3   dfail:0   fail:0   skip:12 
fi-skl-i5-6260u  total:213  pass:201  dwarn:0   dfail:0   fail:0   skip:12 
fi-skl-i7-6700k  total:213  pass:188  dwarn:0   dfail:0   fail:0   skip:25 
fi-snb-i7-2600   total:213  pass:174  dwarn:0   dfail:0   fail:0   skip:39 
ro-bdw-i5-5250u  total:213  pass:192  dwarn:6   dfail:0   fail:0   skip:15 
ro-bdw-i7-5600u  total:213  pass:180  dwarn:5   dfail:0   fail:0   skip:28 
ro-bsw-n3050     total:213  pass:171  dwarn:0   dfail:0   fail:2   skip:40 
ro-byt-n2820     total:213  pass:173  dwarn:0   dfail:0   fail:3   skip:37 
ro-hsw-i3-4010u  total:213  pass:184  dwarn:5   dfail:0   fail:0   skip:24 
ro-hsw-i7-4770r  total:213  pass:185  dwarn:5   dfail:0   fail:0   skip:23 
ro-ilk-i7-620lm  total:213  pass:150  dwarn:0   dfail:0   fail:1   skip:62 
ro-ilk1-i5-650   total:208  pass:150  dwarn:0   dfail:0   fail:1   skip:57 
ro-ivb-i7-3770   total:213  pass:181  dwarn:0   dfail:0   fail:0   skip:32 
ro-ivb2-i7-3770  total:213  pass:184  dwarn:1   dfail:0   fail:0   skip:28 
ro-skl3-i5-6260u total:213  pass:201  dwarn:1   dfail:0   fail:0   skip:11 
ro-snb-i7-2620M  total:213  pass:174  dwarn:0   dfail:0   fail:1   skip:38 
fi-hsw-i7-4770k failed to connect after reboot
ro-bdw-i7-5557U failed to connect after reboot

Results at /archive/results/CI_IGT_test/RO_Patchwork_1177/

9dfa9b5 drm-intel-nightly: 2016y-06m-13d-13h-48m-21s UTC integration manifest
9d5272a drm/i915: Use atomic commits for legacy page_flips
f20c9ba drm/i915: Move fb_bits updating later in atomic_commit
188aae0 drm/i915: nonblocking commit
073e2d3 drm/i915: Roll out the helper nonblock tracking
d4ae869 drm/i915: Signal drm events for atomic

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

^ permalink raw reply	[flat|nested] 18+ messages in thread
* Re: [PATCH 4/5] drm/i915: Move fb_bits updating later in atomic_commit
  2016-06-13 14:13 ` [PATCH 4/5] drm/i915: Move fb_bits updating later in atomic_commit Daniel Vetter
@ 2016-06-13 14:57   ` Chris Wilson
  0 siblings, 0 replies; 18+ messages in thread
From: Chris Wilson @ 2016-06-13 14:57 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: Daniel Vetter, Intel Graphics Development

On Mon, Jun 13, 2016 at 04:13:48PM +0200, Daniel Vetter wrote:
> Currently it's part of prepare_fb, still in the first phase of
> atomic_commit which might fail. Which means that we need to have some
> heuristics in cleanup_fb to figure out whether things failed, or
> whether we just clean up the old fbs.
> 
> That's fragile, and worse, once we start pipelining commits gets
> confused: While the last commit is still getting cleanup up we already
> hammer in the new one, and fb_bits aren't refcounted, resulting in
> lost bits and WARN_ON galore. We could instead try to make cleanup_fb
> more clever, but a simpler fix is to postpone the fb_bits tracking
> past the point of no return, where we commit all the software state.
> 
> That also makes conceptually more sense, since fb_bits must be updated
> synchronously from the ioctl (they track usage from userspace pov, not
> from the hw pov), right before we're fully committed to the updated.
> 
> This fixes WARNING splats from track_fb with page_flip implemented
> through atomic_commit.
> 
> Testcase: igt/kms_flip/flip-vs-rmfb
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 41 ++++++++++++++++++++++--------------
>  1 file changed, 25 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index a16a307dbb76..fd46db4882e5 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -13863,6 +13863,25 @@ static void intel_atomic_commit_work(struct work_struct *work)
>  	intel_atomic_commit_tail(state);
>  }
>  
> +static void intel_atomic_track_fbs(struct drm_atomic_state *state)
> +{
> +	struct drm_plane_state *old_plane_state;
> +	struct drm_plane *plane;
> +	struct drm_i915_gem_object *obj, *old_obj;
> +	struct intel_plane *intel_plane;
> +	int i;
> +
> +	mutex_lock(&state->dev->struct_mutex);
> +	for_each_plane_in_state(state, plane, old_plane_state, i) {
> +		obj = intel_fb_obj(plane->state->fb);
> +		old_obj = intel_fb_obj(old_plane_state->fb);
> +		intel_plane = to_intel_plane(plane);
> +
> +		i915_gem_track_fb(old_obj, obj, intel_plane->frontbuffer_bit);
> +	}
> +	mutex_unlock(&state->dev->struct_mutex);
> +}

Seems like an opportune time to pull in

https://cgit.freedesktop.org/~ickle/linux-2.6/commit/?h=tasklet&id=d16c3d8f3dd395b8d13a3691efb356a23e0b702b
https://cgit.freedesktop.org/~ickle/linux-2.6/commit/?h=tasklet&id=4a5c85282ba3c15a64dc0f600969234f30ebe820

and fwiw i915_gem_track_fb would be better off inside intel_display.c
-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

^ permalink raw reply	[flat|nested] 18+ messages in thread
* Re: [PATCH 3/5] drm/i915: nonblocking commit
  2016-06-13 14:13 ` [PATCH 3/5] drm/i915: nonblocking commit Daniel Vetter
@ 2016-06-14  7:24   ` Maarten Lankhorst
  2016-06-14 16:01   ` [PATCH] " Daniel Vetter
  2016-06-28  4:27   ` [PATCH 3/5] " Daniel Stone
  2 siblings, 0 replies; 18+ messages in thread
From: Maarten Lankhorst @ 2016-06-14  7:24 UTC (permalink / raw)
  To: Daniel Vetter, Intel Graphics Development; +Cc: Daniel Vetter

Op 13-06-16 om 16:13 schreef Daniel Vetter:
> Simply split intel_atomic_commit in half and place the new
> nonblocking commit helpers at the right spots.
>
> NOTE: There's still trouble with obj->frontbuffer bits getting mangled
> when pipelining atomic commits.
>
> v2:
> - Remove the check for nonblocking which returned -EINVAL.
> - Do wait for requests in the worker thread before committing
>   hw state.
>
> v3: Move hw_done after the optimize_wm/post_plane_update step, plus
> add FIXME comment how to fix that up again properly.
>
> v4: Update FIXME for intel_atomic_commit - more stuff works now.
>
> v5: Still reject nonblocking modeset commits (Maarten).
>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
if (intel_state->modeset && nonblock) is good enough, no need to iterate over crtc's. :-)
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 18+ messages in thread
* Re: [PATCH 1/5] drm/i915: Signal drm events for atomic
  2016-06-13 14:13 [PATCH 1/5] drm/i915: Signal drm events for atomic Daniel Vetter
                   ` (4 preceding siblings ...)
  2016-06-13 14:44 ` ✗ Ro.CI.BAT: failure for series starting with [1/5] drm/i915: Signal drm events for atomic Patchwork
@ 2016-06-14  8:21 ` Maarten Lankhorst
  2016-06-14 12:38   ` Daniel Vetter
  5 siblings, 1 reply; 18+ messages in thread
From: Maarten Lankhorst @ 2016-06-14  8:21 UTC (permalink / raw)
  To: Daniel Vetter, Intel Graphics Development

Op 13-06-16 om 16:13 schreef Daniel Vetter:
> This is part of what atomic must implement. And it's also required
> to be able to use the helper nonblocking support.
>
> v2: Always send out the drm event, remove the planes_changed check.
Hey,

There used to be a patch to add a test in kms_atomic for testing vblank events, has this ever been committed?

~Maarten
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 13 ++++++++++---
>  drivers/gpu/drm/i915/intel_sprite.c  | 14 ++++++++++++++
>  2 files changed, 24 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 801e4c17dd8d..a4ebf9df7dc3 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -13774,13 +13774,21 @@ static int intel_atomic_commit(struct drm_device *dev,
>  		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;
>  
>  		if (modeset && crtc->state->active) {
>  			update_scanline_offset(to_intel_crtc(crtc));
>  			dev_priv->display.crtc_enable(crtc);
>  		}
>  
> +		/* Complete events for now disable pipes here. */
> +		if (modeset && !crtc->state->active && crtc->state->event) {
> +			spin_lock_irq(&dev->event_lock);
> +			drm_crtc_send_vblank_event(crtc, crtc->state->event);
> +			spin_unlock_irq(&dev->event_lock);
> +
> +			crtc->state->event = NULL;
> +		}
> +
>  		if (!modeset)
>  			intel_pre_plane_update(to_intel_crtc_state(old_crtc_state));
>  
> @@ -13788,8 +13796,7 @@ static int intel_atomic_commit(struct drm_device *dev,
>  		    drm_atomic_get_existing_plane_state(state, crtc->primary))
>  			intel_fbc_enable(intel_crtc);
>  
> -		if (crtc->state->active &&
> -		    (crtc->state->planes_changed || update_pipe))
> +		if (crtc->state->active)
>  			drm_atomic_helper_commit_planes_on_crtc(old_crtc_state);
>  
>  		if (pipe_config->base.active && needs_vblank_wait(pipe_config))
> diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> index 324ccb06397d..fc654173c491 100644
> --- a/drivers/gpu/drm/i915/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> @@ -166,6 +166,20 @@ void intel_pipe_update_end(struct intel_crtc *crtc, struct intel_flip_work *work
>  
>  	trace_i915_pipe_update_end(crtc, end_vbl_count, scanline_end);
>  
> +	/* We're still in the vblank-evade critical section, this can't race.
> +	 * Would be slightly nice to just grab the vblank count and arm the
> +	 * event outside of the critical section - the spinlock might spin for a
> +	 * while ... */
> +	if (crtc->base.state->event) {
> +		WARN_ON(drm_crtc_vblank_get(&crtc->base) != 0);
> +
> +		spin_lock(&crtc->base.dev->event_lock);
> +		drm_crtc_arm_vblank_event(&crtc->base, crtc->base.state->event);
> +		spin_unlock(&crtc->base.dev->event_lock);
> +
> +		crtc->base.state->event = NULL;
> +	}
> +
>  	local_irq_enable();
>  
>  	if (crtc->debug.start_vbl_count &&


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

^ permalink raw reply	[flat|nested] 18+ messages in thread
* [PATCH 2.5/5] Reapply "drm/i915: Pass atomic states to fbc update, functions."
  2016-06-13 14:13 ` [PATCH 2/5] drm/i915: Roll out the helper nonblock tracking Daniel Vetter
@ 2016-06-14 12:24   ` Maarten Lankhorst
  2016-06-14 12:37     ` Daniel Vetter
  0 siblings, 1 reply; 18+ messages in thread
From: Maarten Lankhorst @ 2016-06-14 12:24 UTC (permalink / raw)
  To: Daniel Vetter, Intel Graphics Development; +Cc: Daniel Vetter

The patch was reverted as part of the original nonblocking commit
support, but is required for any kind of nonblocking commit.

This is required to let fbc updates run async. It has a lot of
checks whether certain locks are taken, which can be removed when
the relevant states are passed in as pointers.

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/1463490484-19540-17-git-send-email-maarten.lankhorst@linux.intel.com
Reviewed-by: Patrik Jakobsson <patrik.jakobsson@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c |  8 +++++---
 drivers/gpu/drm/i915/intel_drv.h     |  8 ++++++--
 drivers/gpu/drm/i915/intel_fbc.c     | 39 +++++++++++++++++-------------------
 3 files changed, 29 insertions(+), 26 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 5d0081067bac..ddcf966c515a 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4641,7 +4641,7 @@ static void intel_pre_plane_update(struct intel_crtc_state *old_crtc_state)
 		struct intel_plane_state *old_primary_state =
 			to_intel_plane_state(old_pri_state);
 
-		intel_fbc_pre_update(crtc);
+		intel_fbc_pre_update(crtc, pipe_config, primary_state);
 
 		if (old_primary_state->visible &&
 		    (modeset || !primary_state->visible))
@@ -11708,7 +11708,9 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
 
 	crtc->primary->fb = fb;
 	update_state_fb(crtc->primary);
-	intel_fbc_pre_update(intel_crtc);
+
+	intel_fbc_pre_update(intel_crtc, intel_crtc->config,
+			     to_intel_plane_state(primary->state));
 
 	work->pending_flip_obj = obj;
 
@@ -13799,7 +13801,7 @@ static int intel_atomic_commit(struct drm_device *dev,
 
 		if (crtc->state->active &&
 		    drm_atomic_get_existing_plane_state(state, crtc->primary))
-			intel_fbc_enable(intel_crtc);
+			intel_fbc_enable(intel_crtc, pipe_config, to_intel_plane_state(crtc->primary->state));
 
 		if (crtc->state->active)
 			drm_atomic_helper_commit_planes_on_crtc(old_crtc_state);
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 8dc67adace6b..0c1dc9bae170 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1431,11 +1431,15 @@ static inline void intel_fbdev_restore_mode(struct drm_device *dev)
 void intel_fbc_choose_crtc(struct drm_i915_private *dev_priv,
 			   struct drm_atomic_state *state);
 bool intel_fbc_is_active(struct drm_i915_private *dev_priv);
-void intel_fbc_pre_update(struct intel_crtc *crtc);
+void intel_fbc_pre_update(struct intel_crtc *crtc,
+			  struct intel_crtc_state *crtc_state,
+			  struct intel_plane_state *plane_state);
 void intel_fbc_post_update(struct intel_crtc *crtc);
 void intel_fbc_init(struct drm_i915_private *dev_priv);
 void intel_fbc_init_pipe_state(struct drm_i915_private *dev_priv);
-void intel_fbc_enable(struct intel_crtc *crtc);
+void intel_fbc_enable(struct intel_crtc *crtc,
+		      struct intel_crtc_state *crtc_state,
+		      struct intel_plane_state *plane_state);
 void intel_fbc_disable(struct intel_crtc *crtc);
 void intel_fbc_global_disable(struct drm_i915_private *dev_priv);
 void intel_fbc_invalidate(struct drm_i915_private *dev_priv,
diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c
index 45ee07b888a0..ecabd59ffbaf 100644
--- a/drivers/gpu/drm/i915/intel_fbc.c
+++ b/drivers/gpu/drm/i915/intel_fbc.c
@@ -481,10 +481,10 @@ static void intel_fbc_deactivate(struct drm_i915_private *dev_priv)
 		intel_fbc_hw_deactivate(dev_priv);
 }
 
-static bool multiple_pipes_ok(struct intel_crtc *crtc)
+static bool multiple_pipes_ok(struct intel_crtc *crtc,
+			      struct intel_plane_state *plane_state)
 {
-	struct drm_i915_private *dev_priv = crtc->base.dev->dev_private;
-	struct drm_plane *primary = crtc->base.primary;
+	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
 	struct intel_fbc *fbc = &dev_priv->fbc;
 	enum pipe pipe = crtc->pipe;
 
@@ -492,9 +492,7 @@ static bool multiple_pipes_ok(struct intel_crtc *crtc)
 	if (!no_fbc_on_multiple_pipes(dev_priv))
 		return true;
 
-	WARN_ON(!drm_modeset_is_locked(&primary->mutex));
-
-	if (to_intel_plane_state(primary->state)->visible)
+	if (plane_state->visible)
 		fbc->visible_pipes_mask |= (1 << pipe);
 	else
 		fbc->visible_pipes_mask &= ~(1 << pipe);
@@ -709,21 +707,16 @@ static bool intel_fbc_hw_tracking_covers_screen(struct intel_crtc *crtc)
 	return effective_w <= max_w && effective_h <= max_h;
 }
 
-static void intel_fbc_update_state_cache(struct intel_crtc *crtc)
+static void intel_fbc_update_state_cache(struct intel_crtc *crtc,
+					 struct intel_crtc_state *crtc_state,
+					 struct intel_plane_state *plane_state)
 {
 	struct drm_i915_private *dev_priv = crtc->base.dev->dev_private;
 	struct intel_fbc *fbc = &dev_priv->fbc;
 	struct intel_fbc_state_cache *cache = &fbc->state_cache;
-	struct intel_crtc_state *crtc_state =
-		to_intel_crtc_state(crtc->base.state);
-	struct intel_plane_state *plane_state =
-		to_intel_plane_state(crtc->base.primary->state);
 	struct drm_framebuffer *fb = plane_state->base.fb;
 	struct drm_i915_gem_object *obj;
 
-	WARN_ON(!drm_modeset_is_locked(&crtc->base.mutex));
-	WARN_ON(!drm_modeset_is_locked(&crtc->base.primary->mutex));
-
 	cache->crtc.mode_flags = crtc_state->base.adjusted_mode.flags;
 	if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv))
 		cache->crtc.hsw_bdw_pixel_rate =
@@ -888,7 +881,9 @@ static bool intel_fbc_reg_params_equal(struct intel_fbc_reg_params *params1,
 	return memcmp(params1, params2, sizeof(*params1)) == 0;
 }
 
-void intel_fbc_pre_update(struct intel_crtc *crtc)
+void intel_fbc_pre_update(struct intel_crtc *crtc,
+			  struct intel_crtc_state *crtc_state,
+			  struct intel_plane_state *plane_state)
 {
 	struct drm_i915_private *dev_priv = crtc->base.dev->dev_private;
 	struct intel_fbc *fbc = &dev_priv->fbc;
@@ -898,7 +893,7 @@ void intel_fbc_pre_update(struct intel_crtc *crtc)
 
 	mutex_lock(&fbc->lock);
 
-	if (!multiple_pipes_ok(crtc)) {
+	if (!multiple_pipes_ok(crtc, plane_state)) {
 		fbc->no_fbc_reason = "more than one pipe active";
 		goto deactivate;
 	}
@@ -906,7 +901,7 @@ void intel_fbc_pre_update(struct intel_crtc *crtc)
 	if (!fbc->enabled || fbc->crtc != crtc)
 		goto unlock;
 
-	intel_fbc_update_state_cache(crtc);
+	intel_fbc_update_state_cache(crtc, crtc_state, plane_state);
 
 deactivate:
 	intel_fbc_deactivate(dev_priv);
@@ -1090,7 +1085,9 @@ out:
  * intel_fbc_enable multiple times for the same pipe without an
  * intel_fbc_disable in the middle, as long as it is deactivated.
  */
-void intel_fbc_enable(struct intel_crtc *crtc)
+void intel_fbc_enable(struct intel_crtc *crtc,
+		      struct intel_crtc_state *crtc_state,
+		      struct intel_plane_state *plane_state)
 {
 	struct drm_i915_private *dev_priv = crtc->base.dev->dev_private;
 	struct intel_fbc *fbc = &dev_priv->fbc;
@@ -1103,19 +1100,19 @@ void intel_fbc_enable(struct intel_crtc *crtc)
 	if (fbc->enabled) {
 		WARN_ON(fbc->crtc == NULL);
 		if (fbc->crtc == crtc) {
-			WARN_ON(!crtc->config->enable_fbc);
+			WARN_ON(!crtc_state->enable_fbc);
 			WARN_ON(fbc->active);
 		}
 		goto out;
 	}
 
-	if (!crtc->config->enable_fbc)
+	if (!crtc_state->enable_fbc)
 		goto out;
 
 	WARN_ON(fbc->active);
 	WARN_ON(fbc->crtc != NULL);
 
-	intel_fbc_update_state_cache(crtc);
+	intel_fbc_update_state_cache(crtc, crtc_state, plane_state);
 	if (intel_fbc_alloc_cfb(crtc)) {
 		fbc->no_fbc_reason = "not enough stolen memory";
 		goto out;
-- 
2.5.5


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

^ permalink raw reply related	[flat|nested] 18+ messages in thread
* Re: [PATCH 2.5/5] Reapply "drm/i915: Pass atomic states to fbc update, functions."
  2016-06-14 12:24   ` [PATCH 2.5/5] Reapply "drm/i915: Pass atomic states to fbc update, functions." Maarten Lankhorst
@ 2016-06-14 12:37     ` Daniel Vetter
  0 siblings, 0 replies; 18+ messages in thread
From: Daniel Vetter @ 2016-06-14 12:37 UTC (permalink / raw)
  To: Maarten Lankhorst
  Cc: Daniel Vetter, Intel Graphics Development, Daniel Vetter

On Tue, Jun 14, 2016 at 02:24:20PM +0200, Maarten Lankhorst wrote:
> The patch was reverted as part of the original nonblocking commit
> support, but is required for any kind of nonblocking commit.
> 
> This is required to let fbc updates run async. It has a lot of
> checks whether certain locks are taken, which can be removed when
> the relevant states are passed in as pointers.
> 
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Link: http://patchwork.freedesktop.org/patch/msgid/1463490484-19540-17-git-send-email-maarten.lankhorst@linux.intel.com
> Reviewed-by: Patrik Jakobsson <patrik.jakobsson@linux.intel.com>

lgtm too, Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

> ---
>  drivers/gpu/drm/i915/intel_display.c |  8 +++++---
>  drivers/gpu/drm/i915/intel_drv.h     |  8 ++++++--
>  drivers/gpu/drm/i915/intel_fbc.c     | 39 +++++++++++++++++-------------------
>  3 files changed, 29 insertions(+), 26 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 5d0081067bac..ddcf966c515a 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -4641,7 +4641,7 @@ static void intel_pre_plane_update(struct intel_crtc_state *old_crtc_state)
>  		struct intel_plane_state *old_primary_state =
>  			to_intel_plane_state(old_pri_state);
>  
> -		intel_fbc_pre_update(crtc);
> +		intel_fbc_pre_update(crtc, pipe_config, primary_state);
>  
>  		if (old_primary_state->visible &&
>  		    (modeset || !primary_state->visible))
> @@ -11708,7 +11708,9 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
>  
>  	crtc->primary->fb = fb;
>  	update_state_fb(crtc->primary);
> -	intel_fbc_pre_update(intel_crtc);
> +
> +	intel_fbc_pre_update(intel_crtc, intel_crtc->config,
> +			     to_intel_plane_state(primary->state));
>  
>  	work->pending_flip_obj = obj;
>  
> @@ -13799,7 +13801,7 @@ static int intel_atomic_commit(struct drm_device *dev,
>  
>  		if (crtc->state->active &&
>  		    drm_atomic_get_existing_plane_state(state, crtc->primary))
> -			intel_fbc_enable(intel_crtc);
> +			intel_fbc_enable(intel_crtc, pipe_config, to_intel_plane_state(crtc->primary->state));
>  
>  		if (crtc->state->active)
>  			drm_atomic_helper_commit_planes_on_crtc(old_crtc_state);
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 8dc67adace6b..0c1dc9bae170 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1431,11 +1431,15 @@ static inline void intel_fbdev_restore_mode(struct drm_device *dev)
>  void intel_fbc_choose_crtc(struct drm_i915_private *dev_priv,
>  			   struct drm_atomic_state *state);
>  bool intel_fbc_is_active(struct drm_i915_private *dev_priv);
> -void intel_fbc_pre_update(struct intel_crtc *crtc);
> +void intel_fbc_pre_update(struct intel_crtc *crtc,
> +			  struct intel_crtc_state *crtc_state,
> +			  struct intel_plane_state *plane_state);
>  void intel_fbc_post_update(struct intel_crtc *crtc);
>  void intel_fbc_init(struct drm_i915_private *dev_priv);
>  void intel_fbc_init_pipe_state(struct drm_i915_private *dev_priv);
> -void intel_fbc_enable(struct intel_crtc *crtc);
> +void intel_fbc_enable(struct intel_crtc *crtc,
> +		      struct intel_crtc_state *crtc_state,
> +		      struct intel_plane_state *plane_state);
>  void intel_fbc_disable(struct intel_crtc *crtc);
>  void intel_fbc_global_disable(struct drm_i915_private *dev_priv);
>  void intel_fbc_invalidate(struct drm_i915_private *dev_priv,
> diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c
> index 45ee07b888a0..ecabd59ffbaf 100644
> --- a/drivers/gpu/drm/i915/intel_fbc.c
> +++ b/drivers/gpu/drm/i915/intel_fbc.c
> @@ -481,10 +481,10 @@ static void intel_fbc_deactivate(struct drm_i915_private *dev_priv)
>  		intel_fbc_hw_deactivate(dev_priv);
>  }
>  
> -static bool multiple_pipes_ok(struct intel_crtc *crtc)
> +static bool multiple_pipes_ok(struct intel_crtc *crtc,
> +			      struct intel_plane_state *plane_state)
>  {
> -	struct drm_i915_private *dev_priv = crtc->base.dev->dev_private;
> -	struct drm_plane *primary = crtc->base.primary;
> +	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
>  	struct intel_fbc *fbc = &dev_priv->fbc;
>  	enum pipe pipe = crtc->pipe;
>  
> @@ -492,9 +492,7 @@ static bool multiple_pipes_ok(struct intel_crtc *crtc)
>  	if (!no_fbc_on_multiple_pipes(dev_priv))
>  		return true;
>  
> -	WARN_ON(!drm_modeset_is_locked(&primary->mutex));
> -
> -	if (to_intel_plane_state(primary->state)->visible)
> +	if (plane_state->visible)
>  		fbc->visible_pipes_mask |= (1 << pipe);
>  	else
>  		fbc->visible_pipes_mask &= ~(1 << pipe);
> @@ -709,21 +707,16 @@ static bool intel_fbc_hw_tracking_covers_screen(struct intel_crtc *crtc)
>  	return effective_w <= max_w && effective_h <= max_h;
>  }
>  
> -static void intel_fbc_update_state_cache(struct intel_crtc *crtc)
> +static void intel_fbc_update_state_cache(struct intel_crtc *crtc,
> +					 struct intel_crtc_state *crtc_state,
> +					 struct intel_plane_state *plane_state)
>  {
>  	struct drm_i915_private *dev_priv = crtc->base.dev->dev_private;
>  	struct intel_fbc *fbc = &dev_priv->fbc;
>  	struct intel_fbc_state_cache *cache = &fbc->state_cache;
> -	struct intel_crtc_state *crtc_state =
> -		to_intel_crtc_state(crtc->base.state);
> -	struct intel_plane_state *plane_state =
> -		to_intel_plane_state(crtc->base.primary->state);
>  	struct drm_framebuffer *fb = plane_state->base.fb;
>  	struct drm_i915_gem_object *obj;
>  
> -	WARN_ON(!drm_modeset_is_locked(&crtc->base.mutex));
> -	WARN_ON(!drm_modeset_is_locked(&crtc->base.primary->mutex));
> -
>  	cache->crtc.mode_flags = crtc_state->base.adjusted_mode.flags;
>  	if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv))
>  		cache->crtc.hsw_bdw_pixel_rate =
> @@ -888,7 +881,9 @@ static bool intel_fbc_reg_params_equal(struct intel_fbc_reg_params *params1,
>  	return memcmp(params1, params2, sizeof(*params1)) == 0;
>  }
>  
> -void intel_fbc_pre_update(struct intel_crtc *crtc)
> +void intel_fbc_pre_update(struct intel_crtc *crtc,
> +			  struct intel_crtc_state *crtc_state,
> +			  struct intel_plane_state *plane_state)
>  {
>  	struct drm_i915_private *dev_priv = crtc->base.dev->dev_private;
>  	struct intel_fbc *fbc = &dev_priv->fbc;
> @@ -898,7 +893,7 @@ void intel_fbc_pre_update(struct intel_crtc *crtc)
>  
>  	mutex_lock(&fbc->lock);
>  
> -	if (!multiple_pipes_ok(crtc)) {
> +	if (!multiple_pipes_ok(crtc, plane_state)) {
>  		fbc->no_fbc_reason = "more than one pipe active";
>  		goto deactivate;
>  	}
> @@ -906,7 +901,7 @@ void intel_fbc_pre_update(struct intel_crtc *crtc)
>  	if (!fbc->enabled || fbc->crtc != crtc)
>  		goto unlock;
>  
> -	intel_fbc_update_state_cache(crtc);
> +	intel_fbc_update_state_cache(crtc, crtc_state, plane_state);
>  
>  deactivate:
>  	intel_fbc_deactivate(dev_priv);
> @@ -1090,7 +1085,9 @@ out:
>   * intel_fbc_enable multiple times for the same pipe without an
>   * intel_fbc_disable in the middle, as long as it is deactivated.
>   */
> -void intel_fbc_enable(struct intel_crtc *crtc)
> +void intel_fbc_enable(struct intel_crtc *crtc,
> +		      struct intel_crtc_state *crtc_state,
> +		      struct intel_plane_state *plane_state)
>  {
>  	struct drm_i915_private *dev_priv = crtc->base.dev->dev_private;
>  	struct intel_fbc *fbc = &dev_priv->fbc;
> @@ -1103,19 +1100,19 @@ void intel_fbc_enable(struct intel_crtc *crtc)
>  	if (fbc->enabled) {
>  		WARN_ON(fbc->crtc == NULL);
>  		if (fbc->crtc == crtc) {
> -			WARN_ON(!crtc->config->enable_fbc);
> +			WARN_ON(!crtc_state->enable_fbc);
>  			WARN_ON(fbc->active);
>  		}
>  		goto out;
>  	}
>  
> -	if (!crtc->config->enable_fbc)
> +	if (!crtc_state->enable_fbc)
>  		goto out;
>  
>  	WARN_ON(fbc->active);
>  	WARN_ON(fbc->crtc != NULL);
>  
> -	intel_fbc_update_state_cache(crtc);
> +	intel_fbc_update_state_cache(crtc, crtc_state, plane_state);
>  	if (intel_fbc_alloc_cfb(crtc)) {
>  		fbc->no_fbc_reason = "not enough stolen memory";
>  		goto out;
> -- 
> 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

^ permalink raw reply	[flat|nested] 18+ messages in thread
* Re: [PATCH 1/5] drm/i915: Signal drm events for atomic
  2016-06-14  8:21 ` [PATCH 1/5] " Maarten Lankhorst
@ 2016-06-14 12:38   ` Daniel Vetter
  0 siblings, 0 replies; 18+ messages in thread
From: Daniel Vetter @ 2016-06-14 12:38 UTC (permalink / raw)
  To: Maarten Lankhorst; +Cc: Daniel Vetter, Intel Graphics Development

On Tue, Jun 14, 2016 at 10:21:02AM +0200, Maarten Lankhorst wrote:
> Op 13-06-16 om 16:13 schreef Daniel Vetter:
> > This is part of what atomic must implement. And it's also required
> > to be able to use the helper nonblocking support.
> >
> > v2: Always send out the drm event, remove the planes_changed check.
> Hey,
> 
> There used to be a patch to add a test in kms_atomic for testing vblank events, has this ever been committed?

Not sure, I'll ping Daniel Stone/Gustavo on irc a bit about this one.
-Daniel

> 
> ~Maarten
> > Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> > ---
> >  drivers/gpu/drm/i915/intel_display.c | 13 ++++++++++---
> >  drivers/gpu/drm/i915/intel_sprite.c  | 14 ++++++++++++++
> >  2 files changed, 24 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index 801e4c17dd8d..a4ebf9df7dc3 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -13774,13 +13774,21 @@ static int intel_atomic_commit(struct drm_device *dev,
> >  		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;
> >  
> >  		if (modeset && crtc->state->active) {
> >  			update_scanline_offset(to_intel_crtc(crtc));
> >  			dev_priv->display.crtc_enable(crtc);
> >  		}
> >  
> > +		/* Complete events for now disable pipes here. */
> > +		if (modeset && !crtc->state->active && crtc->state->event) {
> > +			spin_lock_irq(&dev->event_lock);
> > +			drm_crtc_send_vblank_event(crtc, crtc->state->event);
> > +			spin_unlock_irq(&dev->event_lock);
> > +
> > +			crtc->state->event = NULL;
> > +		}
> > +
> >  		if (!modeset)
> >  			intel_pre_plane_update(to_intel_crtc_state(old_crtc_state));
> >  
> > @@ -13788,8 +13796,7 @@ static int intel_atomic_commit(struct drm_device *dev,
> >  		    drm_atomic_get_existing_plane_state(state, crtc->primary))
> >  			intel_fbc_enable(intel_crtc);
> >  
> > -		if (crtc->state->active &&
> > -		    (crtc->state->planes_changed || update_pipe))
> > +		if (crtc->state->active)
> >  			drm_atomic_helper_commit_planes_on_crtc(old_crtc_state);
> >  
> >  		if (pipe_config->base.active && needs_vblank_wait(pipe_config))
> > diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> > index 324ccb06397d..fc654173c491 100644
> > --- a/drivers/gpu/drm/i915/intel_sprite.c
> > +++ b/drivers/gpu/drm/i915/intel_sprite.c
> > @@ -166,6 +166,20 @@ void intel_pipe_update_end(struct intel_crtc *crtc, struct intel_flip_work *work
> >  
> >  	trace_i915_pipe_update_end(crtc, end_vbl_count, scanline_end);
> >  
> > +	/* We're still in the vblank-evade critical section, this can't race.
> > +	 * Would be slightly nice to just grab the vblank count and arm the
> > +	 * event outside of the critical section - the spinlock might spin for a
> > +	 * while ... */
> > +	if (crtc->base.state->event) {
> > +		WARN_ON(drm_crtc_vblank_get(&crtc->base) != 0);
> > +
> > +		spin_lock(&crtc->base.dev->event_lock);
> > +		drm_crtc_arm_vblank_event(&crtc->base, crtc->base.state->event);
> > +		spin_unlock(&crtc->base.dev->event_lock);
> > +
> > +		crtc->base.state->event = NULL;
> > +	}
> > +
> >  	local_irq_enable();
> >  
> >  	if (crtc->debug.start_vbl_count &&
> 
> 

-- 
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

^ permalink raw reply	[flat|nested] 18+ messages in thread
* [PATCH] drm/i915: nonblocking commit
  2016-06-13 14:13 ` [PATCH 3/5] drm/i915: nonblocking commit Daniel Vetter
  2016-06-14  7:24   ` Maarten Lankhorst
@ 2016-06-14 16:01   ` Daniel Vetter
  2016-06-28  4:27   ` [PATCH 3/5] " Daniel Stone
  2 siblings, 0 replies; 18+ messages in thread
From: Daniel Vetter @ 2016-06-14 16:01 UTC (permalink / raw)
  To: Intel Graphics Development; +Cc: Daniel Vetter, Daniel Vetter

Simply split intel_atomic_commit in half and place the new
nonblocking commit helpers at the right spots.

NOTE: There's still trouble with obj->frontbuffer bits getting mangled
when pipelining atomic commits.

v2:
- Remove the check for nonblocking which returned -EINVAL.
- Do wait for requests in the worker thread before committing
  hw state.

v3: Move hw_done after the optimize_wm/post_plane_update step, plus
add FIXME comment how to fix that up again properly.

v4: Update FIXME for intel_atomic_commit - more stuff works now.

v5: Still reject nonblocking modeset commits (Maarten).

v6: Use intel_state->modeset (Maarten).

Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 126 ++++++++++++++++++++++++-----------
 1 file changed, 87 insertions(+), 39 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 78fa0c1d7847..f20fa569b446 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -13549,11 +13549,6 @@ 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) {
 		if (state->legacy_cursor_update)
 			continue;
@@ -13672,50 +13667,34 @@ static bool needs_vblank_wait(struct intel_crtc_state *crtc_state)
 	return false;
 }
 
-/**
- * intel_atomic_commit - commit validated state object
- * @dev: DRM device
- * @state: the top-level driver state object
- * @nonblock: nonblocking commit
- *
- * This function commits a top-level state object that has been validated
- * with drm_atomic_helper_check().
- *
- * FIXME:  Atomic modeset support for i915 is not yet complete.  At the moment
- * we can only handle plane-related operations and do not yet support
- * nonblocking commit.
- *
- * RETURNS
- * Zero for success or -errno.
- */
-static int intel_atomic_commit(struct drm_device *dev,
-			       struct drm_atomic_state *state,
-			       bool nonblock)
+static void intel_atomic_commit_tail(struct drm_atomic_state *state)
 {
+	struct drm_device *dev = state->dev;
 	struct intel_atomic_state *intel_state = to_intel_atomic_state(state);
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct drm_crtc_state *old_crtc_state;
 	struct drm_crtc *crtc;
 	struct intel_crtc_state *intel_cstate;
-	int ret = 0, i;
+	struct drm_plane *plane;
+	struct drm_plane_state *plane_state;
 	bool hw_check = intel_state->modeset;
 	unsigned long put_domains[I915_MAX_PIPES] = {};
 	unsigned crtc_vblank_mask = 0;
+	int i, ret;
 
-	ret = drm_atomic_helper_setup_commit(state, nonblock);
-	if (ret)
-		return ret;
+	for_each_plane_in_state(state, plane, plane_state, i) {
+		struct intel_plane_state *intel_plane_state =
+			to_intel_plane_state(plane_state);
 
-	ret = intel_atomic_prepare_commit(dev, state, nonblock);
-	if (ret) {
-		DRM_DEBUG_ATOMIC("Preparing state failed with %i\n", ret);
-		return ret;
-	}
+		if (!intel_plane_state->wait_req)
+			continue;
 
-	drm_atomic_helper_swap_state(state, true);
-	dev_priv->wm.distrust_bios_wm = false;
-	dev_priv->wm.skl_results = intel_state->wm_results;
-	intel_shared_dpll_commit(state);
+		ret = __i915_wait_request(intel_plane_state->wait_req,
+					  true, NULL, NULL);
+		/* EIO should be eaten, and we can't get interrupted in the
+		 * worker, and blocking commits have waited already. */
+		WARN_ON(ret);
+	}
 
 	drm_atomic_helper_wait_for_dependencies(state);
 
@@ -13814,8 +13793,15 @@ static int intel_atomic_commit(struct drm_device *dev,
 			crtc_vblank_mask |= 1 << i;
 	}
 
-	drm_atomic_helper_commit_hw_done(state);
-
+	/* FIXME: We should call drm_atomic_helper_commit_hw_done() here
+	 * already, but still need the state for the delayed optimization. To
+	 * fix this:
+	 * - wrap the optimization/post_plane_update stuff into a per-crtc work.
+	 * - schedule that vblank worker _before_ calling hw_done
+	 * - at the start of commit_tail, cancel it _synchrously
+	 * - switch over to the vblank wait helper in the core after that since
+	 *   we don't need out special handling any more.
+	 */
 	if (!state->legacy_cursor_update)
 		intel_atomic_wait_for_vblanks(dev, dev_priv, crtc_vblank_mask);
 
@@ -13842,6 +13828,8 @@ static int intel_atomic_commit(struct drm_device *dev,
 		intel_modeset_verify_crtc(crtc, old_crtc_state, crtc->state);
 	}
 
+	drm_atomic_helper_commit_hw_done(state);
+
 	if (intel_state->modeset)
 		intel_display_power_put(dev_priv, POWER_DOMAIN_MODESET);
 
@@ -13865,6 +13853,66 @@ static int intel_atomic_commit(struct drm_device *dev,
 	 * can happen also when the device is completely off.
 	 */
 	intel_uncore_arm_unclaimed_mmio_detection(dev_priv);
+}
+
+static void intel_atomic_commit_work(struct work_struct *work)
+{
+	struct drm_atomic_state *state = container_of(work,
+						      struct drm_atomic_state,
+						      commit_work);
+	intel_atomic_commit_tail(state);
+}
+
+/**
+ * intel_atomic_commit - commit validated state object
+ * @dev: DRM device
+ * @state: the top-level driver state object
+ * @nonblock: nonblocking commit
+ *
+ * This function commits a top-level state object that has been validated
+ * with drm_atomic_helper_check().
+ *
+ * FIXME:  Atomic modeset support for i915 is not yet complete.  At the moment
+ * nonblocking commits are only safe for pure plane updates. Everything else
+ * should work though.
+ *
+ * RETURNS
+ * Zero for success or -errno.
+ */
+static int intel_atomic_commit(struct drm_device *dev,
+			       struct drm_atomic_state *state,
+			       bool nonblock)
+{
+	struct intel_atomic_state *intel_state = to_intel_atomic_state(state);
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	int ret = 0;
+
+	if (intel_state->modeset && nonblock) {
+		DRM_DEBUG_KMS("nonblocking commit for modeset not yet implemented.\n");
+		return -EINVAL;
+	}
+
+	ret = drm_atomic_helper_setup_commit(state, nonblock);
+	if (ret)
+		return ret;
+
+	INIT_WORK(&state->commit_work, intel_atomic_commit_work);
+
+	ret = intel_atomic_prepare_commit(dev, state, nonblock);
+	if (ret) {
+		DRM_DEBUG_ATOMIC("Preparing state failed with %i\n", ret);
+		return ret;
+	}
+
+	drm_atomic_helper_swap_state(state, true);
+	dev_priv->wm.distrust_bios_wm = false;
+	dev_priv->wm.skl_results = intel_state->wm_results;
+	intel_shared_dpll_commit(state);
+
+	if (nonblock)
+		queue_work(system_unbound_wq, &state->commit_work);
+	else
+		intel_atomic_commit_tail(state);
 
 	return 0;
 }
-- 
2.8.1

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

^ permalink raw reply related	[flat|nested] 18+ messages in thread
* Re: [PATCH 5/5] drm/i915: Use atomic commits for legacy page_flips
  2016-06-13 14:13 ` [PATCH 5/5] drm/i915: Use atomic commits for legacy page_flips Daniel Vetter
@ 2016-06-15  8:01   ` Maarten Lankhorst
  2016-06-20 18:30     ` Matthew Auld
  0 siblings, 1 reply; 18+ messages in thread
From: Maarten Lankhorst @ 2016-06-15  8:01 UTC (permalink / raw)
  To: Daniel Vetter, Intel Graphics Development

Op 13-06-16 om 16:13 schreef Daniel Vetter:
> Note that I didn't start garbage collecting all the legacy flip code
> yet, to make it easier to revert this. But there will be _lots_ of
> code that can be removed once this is tested on all platforms.
>
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index fd46db4882e5..de1ff4a85b5e 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -11619,6 +11619,7 @@ void intel_check_page_flip(struct drm_i915_private *dev_priv, int pipe)
>  	spin_unlock(&dev->event_lock);
>  }
>  
> +__attribute__((unused))
>  static int intel_crtc_page_flip(struct drm_crtc *crtc,
>  				struct drm_framebuffer *fb,
>  				struct drm_pending_vblank_event *event,
> @@ -13977,7 +13978,7 @@ 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 = intel_crtc_page_flip,
> +	.page_flip = drm_atomic_helper_page_flip,
>  	.atomic_duplicate_state = intel_crtc_duplicate_state,
>  	.atomic_destroy_state = intel_crtc_destroy_state,
>  };

Change this to __maybe_unused and you have my

Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> for the whole series.

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

^ permalink raw reply	[flat|nested] 18+ messages in thread
* Re: [PATCH 5/5] drm/i915: Use atomic commits for legacy page_flips
  2016-06-15  8:01   ` Maarten Lankhorst
@ 2016-06-20 18:30     ` Matthew Auld
  2016-06-21 14:23       ` Daniel Vetter
  0 siblings, 1 reply; 18+ messages in thread
From: Matthew Auld @ 2016-06-20 18:30 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: Intel Graphics Development

I seem to be hitting some issues with this, at least in gnome-shell
which I bisected to this commit. Somewhat hard to describe but the
background window or desktop is partiality and intermittently drawn,
almost like a flicker onto the foreground window, title-bar etc.
typically during any kind of transition animation, like switching
windows, though sometimes just at random. Any ideas, I take it you
haven't seen anything like this while testing?
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 18+ messages in thread
* Re: [PATCH 5/5] drm/i915: Use atomic commits for legacy page_flips
  2016-06-20 18:30     ` Matthew Auld
@ 2016-06-21 14:23       ` Daniel Vetter
  2016-06-21 21:00         ` Chris Wilson
  0 siblings, 1 reply; 18+ messages in thread
From: Daniel Vetter @ 2016-06-21 14:23 UTC (permalink / raw)
  To: Matthew Auld; +Cc: Intel Graphics Development

Adding Maarten/Daniela since I'm going on vacation.
-Daniel

On Mon, Jun 20, 2016 at 8:30 PM, Matthew Auld
<matthew.william.auld@gmail.com> wrote:
> I seem to be hitting some issues with this, at least in gnome-shell
> which I bisected to this commit. Somewhat hard to describe but the
> background window or desktop is partiality and intermittently drawn,
> almost like a flicker onto the foreground window, title-bar etc.
> typically during any kind of transition animation, like switching
> windows, though sometimes just at random. Any ideas, I take it you
> haven't seen anything like this while testing?



-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 18+ messages in thread
* Re: [PATCH 5/5] drm/i915: Use atomic commits for legacy page_flips
  2016-06-21 14:23       ` Daniel Vetter
@ 2016-06-21 21:00         ` Chris Wilson
  0 siblings, 0 replies; 18+ messages in thread
From: Chris Wilson @ 2016-06-21 21:00 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: Intel Graphics Development

On Tue, Jun 21, 2016 at 04:23:42PM +0200, Daniel Vetter wrote:
> Adding Maarten/Daniela since I'm going on vacation.
> -Daniel
> 
> On Mon, Jun 20, 2016 at 8:30 PM, Matthew Auld
> <matthew.william.auld@gmail.com> wrote:
> > I seem to be hitting some issues with this, at least in gnome-shell
> > which I bisected to this commit. Somewhat hard to describe but the
> > background window or desktop is partiality and intermittently drawn,
> > almost like a flicker onto the foreground window, title-bar etc.
> > typically during any kind of transition animation, like switching
> > windows, though sometimes just at random. Any ideas, I take it you
> > haven't seen anything like this while testing?

The effect is that we are either rendering onto the current scanout or
the next scanout is being presented with rendering unflushed.
Probably the former. Underruns have been known to have similar flicker
(at least on bdw+) but this feels more like rendering to the wrong
buffer.
-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

^ permalink raw reply	[flat|nested] 18+ messages in thread
* Re: [PATCH 3/5] drm/i915: nonblocking commit
  2016-06-13 14:13 ` [PATCH 3/5] drm/i915: nonblocking commit Daniel Vetter
  2016-06-14  7:24   ` Maarten Lankhorst
  2016-06-14 16:01   ` [PATCH] " Daniel Vetter
@ 2016-06-28  4:27   ` Daniel Stone
  2 siblings, 0 replies; 18+ messages in thread
From: Daniel Stone @ 2016-06-28  4:27 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: Daniel Vetter, Intel Graphics Development

Hi,

On 14 June 2016 at 00:13, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> +static void intel_atomic_commit_tail(struct drm_atomic_state *state)
>  {
> +       struct drm_device *dev = state->dev;
>         struct intel_atomic_state *intel_state = to_intel_atomic_state(state);
>         struct drm_i915_private *dev_priv = dev->dev_private;
>         struct drm_crtc_state *old_crtc_state;
>         struct drm_crtc *crtc;
>         struct intel_crtc_state *intel_cstate;
> -       int ret = 0, i;
> +       struct drm_plane *plane;
> +       struct drm_plane_state *plane_state;
>         bool hw_check = intel_state->modeset;
>         unsigned long put_domains[I915_MAX_PIPES] = {};
>         unsigned crtc_vblank_mask = 0;
> +       int i, ret;
>
> -       ret = drm_atomic_helper_setup_commit(state, nonblock);
> -       if (ret)
> -               return ret;
> +       for_each_plane_in_state(state, plane, plane_state, i) {
> +               struct intel_plane_state *intel_plane_state =
> +                       to_intel_plane_state(plane_state);

Nope nope nope - as we're now after swap_state, this needs to be
plane->state rather than plane_state.

Renaming the arg to this function to old_state probably would've made
this clearer. It showed up pretty violently when running
Mutter/Wayland though. Changing this lets me run it without seeing
tearing on every draw, and no ill effects seen thus far.

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

^ permalink raw reply	[flat|nested] 18+ messages in thread
end of thread, other threads:[~2016-06-28  4:27 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-06-13 14:13 [PATCH 1/5] drm/i915: Signal drm events for atomic Daniel Vetter
2016-06-13 14:13 ` [PATCH 2/5] drm/i915: Roll out the helper nonblock tracking Daniel Vetter
2016-06-14 12:24   ` [PATCH 2.5/5] Reapply "drm/i915: Pass atomic states to fbc update, functions." Maarten Lankhorst
2016-06-14 12:37     ` Daniel Vetter
2016-06-13 14:13 ` [PATCH 3/5] drm/i915: nonblocking commit Daniel Vetter
2016-06-14  7:24   ` Maarten Lankhorst
2016-06-14 16:01   ` [PATCH] " Daniel Vetter
2016-06-28  4:27   ` [PATCH 3/5] " Daniel Stone
2016-06-13 14:13 ` [PATCH 4/5] drm/i915: Move fb_bits updating later in atomic_commit Daniel Vetter
2016-06-13 14:57   ` Chris Wilson
2016-06-13 14:13 ` [PATCH 5/5] drm/i915: Use atomic commits for legacy page_flips Daniel Vetter
2016-06-15  8:01   ` Maarten Lankhorst
2016-06-20 18:30     ` Matthew Auld
2016-06-21 14:23       ` Daniel Vetter
2016-06-21 21:00         ` Chris Wilson
2016-06-13 14:44 ` ✗ Ro.CI.BAT: failure for series starting with [1/5] drm/i915: Signal drm events for atomic Patchwork
2016-06-14  8:21 ` [PATCH 1/5] " Maarten Lankhorst
2016-06-14 12:38   ` Daniel Vetter

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.