All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/7] drm/i915: Missed interrupt simulation is no more, tell the world
@ 2018-09-06  7:51 Chris Wilson
  2018-09-06  7:51 ` [PATCH 2/7] drm/i915/execlists: Reset CSB pointers on canceling requests (wedging) Chris Wilson
                   ` (8 more replies)
  0 siblings, 9 replies; 10+ messages in thread
From: Chris Wilson @ 2018-09-06  7:51 UTC (permalink / raw)
  To: intel-gfx

Using the guc, we cannot disable the user interrupt generation as we use
it for driving submission. And from Icelake, we no longer have the
ability to individually mask interrupt generation from each engine,
disabling our ability to fake missed interrupts.

In both cases, report back to userspace that the missed interrupt
generator is no longer available.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 1f7051e97afb..b4744a68cd88 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -4117,6 +4117,17 @@ i915_ring_test_irq_set(void *data, u64 val)
 {
 	struct drm_i915_private *i915 = data;
 
+	/* GuC keeps the user interrupt permanently enabled for submission */
+	if (USES_GUC_SUBMISSION(i915))
+		return -ENODEV;
+
+	/*
+	 * From icl, we can no longer individually mask interrupt generation
+	 * from each engine.
+	 */
+	if (INTEL_GEN(i915) >= 11)
+		return -ENODEV;
+
 	val &= INTEL_INFO(i915)->ring_mask;
 	DRM_DEBUG_DRIVER("Masking interrupts on rings 0x%08llx\n", val);
 
-- 
2.19.0.rc2

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

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 2/7] drm/i915/execlists: Reset CSB pointers on canceling requests (wedging)
  2018-09-06  7:51 [PATCH 1/7] drm/i915: Missed interrupt simulation is no more, tell the world Chris Wilson
@ 2018-09-06  7:51 ` Chris Wilson
  2018-09-06  7:51 ` [PATCH 3/7] drm/i915/execlists: Avoid kicking priority on the current context Chris Wilson
                   ` (7 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Chris Wilson @ 2018-09-06  7:51 UTC (permalink / raw)
  To: intel-gfx

The prior assumption was that we did not need to reset the CSB on
wedging when cancelling the outstanding requests as it would be cleaned
up in the subsequent reset prior to restarting the GPU. However, what
was not accounted for was that in performing the reset, we would try to
process the outstanding CSB entries. If the GPU happened to complete a
CS event just as we were performing the cancellation of requests, that
event would be kept in the CSB until the reset -- but our bookkeeping
was cleared, causing confusion when trying to complete the CS event.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_lrc.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 9b1f0e5211a0..066ab178a8b2 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -850,6 +850,7 @@ static void execlists_cancel_requests(struct intel_engine_cs *engine)
 	/* Cancel the requests on the HW and clear the ELSP tracker. */
 	execlists_cancel_port_requests(execlists);
 	execlists_user_end(execlists);
+	reset_csb_pointers(execlists);
 
 	/* Mark all executing requests as skipped. */
 	list_for_each_entry(rq, &engine->timeline.requests, link) {
-- 
2.19.0.rc2

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

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 3/7] drm/i915/execlists: Avoid kicking priority on the current context
  2018-09-06  7:51 [PATCH 1/7] drm/i915: Missed interrupt simulation is no more, tell the world Chris Wilson
  2018-09-06  7:51 ` [PATCH 2/7] drm/i915/execlists: Reset CSB pointers on canceling requests (wedging) Chris Wilson
@ 2018-09-06  7:51 ` Chris Wilson
  2018-09-06  7:51 ` [PATCH 4/7] drm/i915/selftests: Basic stress test for rapid context switching Chris Wilson
                   ` (6 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Chris Wilson @ 2018-09-06  7:51 UTC (permalink / raw)
  To: intel-gfx

If the request is currently on the HW (in port 0), then we do not need
to kick the submission tasklet to evaluate whether we should be
preempting itself in order to execute it again.

In the case that was annoying me:

   execlists_schedule: rq(18:211173).prio=0 -> 2
   need_preempt: last(18:211174).prio=0, queue.prio=2

We are bumping the priority of the first of a pair of requests running
in the current context. Then when evaluating preempt, we would see that
that our priority request is higher than the last executing request in
ELSP0 and so trigger preemption, not realising that our intended request
was already executing.

v2: As we assume state of the execlists->port[] that is only valid while
we hold the timeline lock we have to repeat some earlier tests that on
the validity of the node.
v3: Wrap guc submission under the timeline.lock as is now the way of all
things.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
---
 drivers/gpu/drm/i915/intel_guc_submission.c | 18 +++------
 drivers/gpu/drm/i915/intel_lrc.c            | 41 +++++++++++++++------
 2 files changed, 36 insertions(+), 23 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_guc_submission.c b/drivers/gpu/drm/i915/intel_guc_submission.c
index 07b9d313b019..7b878790228a 100644
--- a/drivers/gpu/drm/i915/intel_guc_submission.c
+++ b/drivers/gpu/drm/i915/intel_guc_submission.c
@@ -771,19 +771,8 @@ static bool __guc_dequeue(struct intel_engine_cs *engine)
 
 static void guc_dequeue(struct intel_engine_cs *engine)
 {
-	unsigned long flags;
-	bool submit;
-
-	local_irq_save(flags);
-
-	spin_lock(&engine->timeline.lock);
-	submit = __guc_dequeue(engine);
-	spin_unlock(&engine->timeline.lock);
-
-	if (submit)
+	if ( __guc_dequeue(engine))
 		guc_submit(engine);
-
-	local_irq_restore(flags);
 }
 
 static void guc_submission_tasklet(unsigned long data)
@@ -792,6 +781,9 @@ static void guc_submission_tasklet(unsigned long data)
 	struct intel_engine_execlists * const execlists = &engine->execlists;
 	struct execlist_port *port = execlists->port;
 	struct i915_request *rq;
+	unsigned long flags;
+
+	spin_lock_irqsave(&engine->timeline.lock, flags);
 
 	rq = port_request(port);
 	while (rq && i915_request_completed(rq)) {
@@ -815,6 +807,8 @@ static void guc_submission_tasklet(unsigned long data)
 
 	if (!execlists_is_active(execlists, EXECLISTS_ACTIVE_PREEMPT))
 		guc_dequeue(engine);
+
+	spin_unlock_irqrestore(&engine->timeline.lock, flags);
 }
 
 static struct i915_request *
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 066ab178a8b2..881ab979f02f 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -355,13 +355,8 @@ execlists_unwind_incomplete_requests(struct intel_engine_execlists *execlists)
 {
 	struct intel_engine_cs *engine =
 		container_of(execlists, typeof(*engine), execlists);
-	unsigned long flags;
-
-	spin_lock_irqsave(&engine->timeline.lock, flags);
 
 	__unwind_incomplete_requests(engine);
-
-	spin_unlock_irqrestore(&engine->timeline.lock, flags);
 }
 
 static inline void
@@ -1234,9 +1229,13 @@ static void execlists_schedule(struct i915_request *request,
 
 		engine = sched_lock_engine(node, engine);
 
+		/* Recheck after acquiring the engine->timeline.lock */
 		if (prio <= node->attr.priority)
 			continue;
 
+		if (i915_sched_node_signaled(node))
+			continue;
+
 		node->attr.priority = prio;
 		if (!list_empty(&node->link)) {
 			if (last != engine) {
@@ -1245,14 +1244,34 @@ static void execlists_schedule(struct i915_request *request,
 			}
 			GEM_BUG_ON(pl->priority != prio);
 			list_move_tail(&node->link, &pl->requests);
+		} else {
+			/*
+			 * If the request is not in the priolist queue because
+			 * it is not yet runnable, then it doesn't contribute
+			 * to our preemption decisions. On the other hand,
+			 * if the request is on the HW, it too is not in the
+			 * queue; but in that case we may still need to reorder
+			 * the inflight requests.
+			 */
+			if (!i915_sw_fence_done(&sched_to_request(node)->submit))
+				continue;
 		}
 
-		if (prio > engine->execlists.queue_priority &&
-		    i915_sw_fence_done(&sched_to_request(node)->submit)) {
-			/* defer submission until after all of our updates */
-			__update_queue(engine, prio);
-			tasklet_hi_schedule(&engine->execlists.tasklet);
-		}
+		if (prio <= engine->execlists.queue_priority)
+			continue;
+
+		/*
+		 * If we are already the currently executing context, don't
+		 * bother evaluating if we should preempt ourselves.
+		 */
+		if (sched_to_request(node)->global_seqno &&
+		    i915_seqno_passed(port_request(engine->execlists.port)->global_seqno,
+				      sched_to_request(node)->global_seqno))
+			continue;
+
+		/* Defer (tasklet) submission until after all of our updates. */
+		__update_queue(engine, prio);
+		tasklet_hi_schedule(&engine->execlists.tasklet);
 	}
 
 	spin_unlock_irq(&engine->timeline.lock);
-- 
2.19.0.rc2

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

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 4/7] drm/i915/selftests: Basic stress test for rapid context switching
  2018-09-06  7:51 [PATCH 1/7] drm/i915: Missed interrupt simulation is no more, tell the world Chris Wilson
  2018-09-06  7:51 ` [PATCH 2/7] drm/i915/execlists: Reset CSB pointers on canceling requests (wedging) Chris Wilson
  2018-09-06  7:51 ` [PATCH 3/7] drm/i915/execlists: Avoid kicking priority on the current context Chris Wilson
@ 2018-09-06  7:51 ` Chris Wilson
  2018-09-06  7:51 ` [PATCH 5/7] drm/i915/execlists: Delay updating ring register state after resume Chris Wilson
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Chris Wilson @ 2018-09-06  7:51 UTC (permalink / raw)
  To: intel-gfx

We need to exercise the HW and submission paths for switching contexts
rapidly to check that features such as execlists' wa_tail are adequate.
Plus it's an interesting baseline latency metric.

v2: Check the initial request for allocation errors

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 .../gpu/drm/i915/selftests/i915_gem_context.c | 188 ++++++++++++++++++
 1 file changed, 188 insertions(+)

diff --git a/drivers/gpu/drm/i915/selftests/i915_gem_context.c b/drivers/gpu/drm/i915/selftests/i915_gem_context.c
index 1c92560d35da..24734dd91309 100644
--- a/drivers/gpu/drm/i915/selftests/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/selftests/i915_gem_context.c
@@ -22,6 +22,8 @@
  *
  */
 
+#include <linux/prime_numbers.h>
+
 #include "../i915_selftest.h"
 #include "i915_random.h"
 #include "igt_flush_test.h"
@@ -32,6 +34,191 @@
 
 #define DW_PER_PAGE (PAGE_SIZE / sizeof(u32))
 
+struct live_test {
+	struct drm_i915_private *i915;
+	const char *func;
+	const char *name;
+
+	unsigned int reset_count;
+};
+
+static int begin_live_test(struct live_test *t,
+			   struct drm_i915_private *i915,
+			   const char *func,
+			   const char *name)
+{
+	int err;
+
+	t->i915 = i915;
+	t->func = func;
+	t->name = name;
+
+	err = i915_gem_wait_for_idle(i915,
+				     I915_WAIT_LOCKED,
+				     MAX_SCHEDULE_TIMEOUT);
+	if (err) {
+		pr_err("%s(%s): failed to idle before, with err=%d!",
+		       func, name, err);
+		return err;
+	}
+
+	i915->gpu_error.missed_irq_rings = 0;
+	t->reset_count = i915_reset_count(&i915->gpu_error);
+
+	return 0;
+}
+
+static int end_live_test(struct live_test *t)
+{
+	struct drm_i915_private *i915 = t->i915;
+
+	i915_retire_requests(i915);
+
+	if (wait_for(intel_engines_are_idle(i915), 10)) {
+		pr_err("%s(%s): GPU not idle\n", t->func, t->name);
+		return -EIO;
+	}
+
+	if (t->reset_count != i915_reset_count(&i915->gpu_error)) {
+		pr_err("%s(%s): GPU was reset %d times!\n",
+		       t->func, t->name,
+		       i915_reset_count(&i915->gpu_error) - t->reset_count);
+		return -EIO;
+	}
+
+	if (i915->gpu_error.missed_irq_rings) {
+		pr_err("%s(%s): Missed interrupts on engines %lx\n",
+		       t->func, t->name, i915->gpu_error.missed_irq_rings);
+		return -EIO;
+	}
+
+	return 0;
+}
+
+static int live_nop_switch(void *arg)
+{
+	const unsigned int nctx = 1024;
+	struct drm_i915_private *i915 = arg;
+	struct intel_engine_cs *engine;
+	struct i915_gem_context **ctx;
+	enum intel_engine_id id;
+	struct drm_file *file;
+	struct live_test t;
+	unsigned long n;
+	int err = -ENODEV;
+
+	/*
+	 * Create as many contexts as we can feasibly get away with
+	 * and check we can switch between them rapidly.
+	 *
+	 * Serves as very simple stress test for submission and HW switching
+	 * between contexts.
+	 */
+
+	if (!DRIVER_CAPS(i915)->has_logical_contexts)
+		return 0;
+
+	file = mock_file(i915);
+	if (IS_ERR(file))
+		return PTR_ERR(file);
+
+	mutex_lock(&i915->drm.struct_mutex);
+
+	ctx = kcalloc(nctx, sizeof(*ctx), GFP_KERNEL);
+	if (!ctx) {
+		err = -ENOMEM;
+		goto out_unlock;
+	}
+
+	for (n = 0; n < nctx; n++) {
+		ctx[n] = i915_gem_create_context(i915, file->driver_priv);
+		if (IS_ERR(ctx[n])) {
+			err = PTR_ERR(ctx[n]);
+			goto out_unlock;
+		}
+	}
+
+	for_each_engine(engine, i915, id) {
+		struct i915_request *rq;
+		unsigned long end_time, prime;
+		ktime_t times[2] = {};
+
+		times[0] = ktime_get_raw();
+		for (n = 0; n < nctx; n++) {
+			rq = i915_request_alloc(engine, ctx[n]);
+			if (IS_ERR(rq)) {
+				err = PTR_ERR(rq);
+				goto out_unlock;
+			}
+			i915_request_add(rq);
+		}
+		i915_request_wait(rq,
+				  I915_WAIT_LOCKED,
+				  MAX_SCHEDULE_TIMEOUT);
+		times[1] = ktime_get_raw();
+
+		pr_info("Populated %d contexts on %s in %lluns\n",
+			nctx, engine->name, ktime_to_ns(times[1] - times[0]));
+
+		err = begin_live_test(&t, i915, __func__, engine->name);
+		if (err)
+			goto out_unlock;
+
+		end_time = jiffies + i915_selftest.timeout_jiffies;
+		for_each_prime_number_from(prime, 2, 8192) {
+			times[1] = ktime_get_raw();
+
+			for (n = 0; n < prime; n++) {
+				rq = i915_request_alloc(engine, ctx[n % nctx]);
+				if (IS_ERR(rq)) {
+					err = PTR_ERR(rq);
+					goto out_unlock;
+				}
+
+				/*
+				 * This space is left intentionally blank.
+				 *
+				 * We do not actually want to perform any
+				 * action with this request, we just want
+				 * to measure the latency in allocation
+				 * and submission of our breadcrumbs -
+				 * ensuring that the bare request is sufficient
+				 * for the system to work (i.e. proper HEAD
+				 * tracking of the rings, interrupt handling,
+				 * etc). It also gives us the lowest bounds
+				 * for latency.
+				 */
+
+				i915_request_add(rq);
+			}
+			i915_request_wait(rq,
+					  I915_WAIT_LOCKED,
+					  MAX_SCHEDULE_TIMEOUT);
+
+			times[1] = ktime_sub(ktime_get_raw(), times[1]);
+			if (prime == 2)
+				times[0] = times[1];
+
+			if (__igt_timeout(end_time, NULL))
+				break;
+		}
+
+		err = end_live_test(&t);
+		if (err)
+			goto out_unlock;
+
+		pr_info("Switch latencies on %s: 1 = %lluns, %lu = %lluns\n",
+			engine->name,
+			ktime_to_ns(times[0]),
+			prime - 1, div64_u64(ktime_to_ns(times[1]), prime - 1));
+	}
+
+out_unlock:
+	mutex_unlock(&i915->drm.struct_mutex);
+	mock_file_free(i915, file);
+	return err;
+}
+
 static struct i915_vma *
 gpu_fill_dw(struct i915_vma *vma, u64 offset, unsigned long count, u32 value)
 {
@@ -713,6 +900,7 @@ int i915_gem_context_live_selftests(struct drm_i915_private *dev_priv)
 {
 	static const struct i915_subtest tests[] = {
 		SUBTEST(igt_switch_to_kernel_context),
+		SUBTEST(live_nop_switch),
 		SUBTEST(igt_ctx_exec),
 		SUBTEST(igt_ctx_readonly),
 	};
-- 
2.19.0.rc2

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

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 5/7] drm/i915/execlists: Delay updating ring register state after resume
  2018-09-06  7:51 [PATCH 1/7] drm/i915: Missed interrupt simulation is no more, tell the world Chris Wilson
                   ` (2 preceding siblings ...)
  2018-09-06  7:51 ` [PATCH 4/7] drm/i915/selftests: Basic stress test for rapid context switching Chris Wilson
@ 2018-09-06  7:51 ` Chris Wilson
  2018-09-06  7:52 ` [PATCH 6/7] drm/i915/execlists: Use coherent writes into the context image Chris Wilson
                   ` (4 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Chris Wilson @ 2018-09-06  7:51 UTC (permalink / raw)
  To: intel-gfx

Now that we reload both RING_HEAD and RING_TAIL when rebinding the
context, we do not need to scrub those registers immediately on resume.

v2: Handle the perma-pinned contexts.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_lrc.c | 29 +++++++++++------------------
 1 file changed, 11 insertions(+), 18 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 881ab979f02f..595ff42e7662 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -2861,13 +2861,14 @@ static int execlists_context_deferred_alloc(struct i915_gem_context *ctx,
 	return ret;
 }
 
-void intel_lr_context_resume(struct drm_i915_private *dev_priv)
+void intel_lr_context_resume(struct drm_i915_private *i915)
 {
 	struct intel_engine_cs *engine;
 	struct i915_gem_context *ctx;
 	enum intel_engine_id id;
 
-	/* Because we emit WA_TAIL_DWORDS there may be a disparity
+	/*
+	 * Because we emit WA_TAIL_DWORDS there may be a disparity
 	 * between our bookkeeping in ce->ring->head and ce->ring->tail and
 	 * that stored in context. As we only write new commands from
 	 * ce->ring->tail onwards, everything before that is junk. If the GPU
@@ -2877,28 +2878,20 @@ void intel_lr_context_resume(struct drm_i915_private *dev_priv)
 	 * So to avoid that we reset the context images upon resume. For
 	 * simplicity, we just zero everything out.
 	 */
-	list_for_each_entry(ctx, &dev_priv->contexts.list, link) {
-		for_each_engine(engine, dev_priv, id) {
+	list_for_each_entry(ctx, &i915->contexts.list, link) {
+		for_each_engine(engine, i915, id) {
 			struct intel_context *ce =
 				to_intel_context(ctx, engine);
-			u32 *reg;
-
-			if (!ce->state)
-				continue;
 
-			reg = i915_gem_object_pin_map(ce->state->obj,
-						      I915_MAP_WB);
-			if (WARN_ON(IS_ERR(reg)))
+			if (!ce->ring)
 				continue;
 
-			reg += LRC_STATE_PN * PAGE_SIZE / sizeof(*reg);
-			reg[CTX_RING_HEAD+1] = 0;
-			reg[CTX_RING_TAIL+1] = 0;
-
-			ce->state->obj->mm.dirty = true;
-			i915_gem_object_unpin_map(ce->state->obj);
-
 			intel_ring_reset(ce->ring, 0);
+
+			if (ce->pin_count) { /* otherwise done in context_pin */
+				ce->lrc_reg_state[CTX_RING_HEAD+1] = 0;
+				ce->lrc_reg_state[CTX_RING_TAIL+1] = 0;
+			}
 		}
 	}
 }
-- 
2.19.0.rc2

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

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 6/7] drm/i915/execlists: Use coherent writes into the context image
  2018-09-06  7:51 [PATCH 1/7] drm/i915: Missed interrupt simulation is no more, tell the world Chris Wilson
                   ` (3 preceding siblings ...)
  2018-09-06  7:51 ` [PATCH 5/7] drm/i915/execlists: Delay updating ring register state after resume Chris Wilson
@ 2018-09-06  7:52 ` Chris Wilson
  2018-09-06  7:52 ` [PATCH 7/7] drm/i915/execlists: Onion unwind for logical_ring_init() failure Chris Wilson
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Chris Wilson @ 2018-09-06  7:52 UTC (permalink / raw)
  To: intel-gfx

That we use a WB mapping for updating the RING_TAIL register inside the
context image even on !llc machines has been a source of consternation
for every reader. It appears to work on bsw+, but it may just have been
that we have been incredibly bad at detecting the errors.

v2: With extra enthusiasm.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_drv.h         | 6 ++++++
 drivers/gpu/drm/i915/i915_gem.c         | 2 ++
 drivers/gpu/drm/i915/i915_perf.c        | 3 ++-
 drivers/gpu/drm/i915/intel_engine_cs.c  | 2 +-
 drivers/gpu/drm/i915/intel_lrc.c        | 8 +++++---
 drivers/gpu/drm/i915/intel_ringbuffer.c | 2 +-
 6 files changed, 17 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 767615ecdea5..db006ff73827 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -3073,6 +3073,12 @@ enum i915_map_type {
 	I915_MAP_FORCE_WC = I915_MAP_WC | I915_MAP_OVERRIDE,
 };
 
+static inline enum i915_map_type
+i915_coherent_map_type(struct drm_i915_private *i915)
+{
+	return HAS_LLC(i915) ? I915_MAP_WB : I915_MAP_WC;
+}
+
 /**
  * i915_gem_object_pin_map - return a contiguous mapping of the entire object
  * @obj: the object to map into kernel address space
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 89834ce19acd..d6f2bbd6a0dc 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -5417,6 +5417,8 @@ static int __intel_engines_record_defaults(struct drm_i915_private *i915)
 	for_each_engine(engine, i915, id) {
 		struct i915_vma *state;
 
+		GEM_BUG_ON(to_intel_context(ctx, engine)->pin_count);
+
 		state = to_intel_context(ctx, engine)->state;
 		if (!state)
 			continue;
diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c
index ccb20230df2c..0dabfeb2297a 100644
--- a/drivers/gpu/drm/i915/i915_perf.c
+++ b/drivers/gpu/drm/i915/i915_perf.c
@@ -1847,7 +1847,8 @@ static int gen8_configure_all_contexts(struct drm_i915_private *dev_priv,
 		if (!ce->state)
 			continue;
 
-		regs = i915_gem_object_pin_map(ce->state->obj, I915_MAP_WB);
+		regs = i915_gem_object_pin_map(ce->state->obj,
+					      i915_coherent_map_type(dev_priv));
 		if (IS_ERR(regs))
 			return PTR_ERR(regs);
 
diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c
index 10cd051ba29e..c99f2cb9b0e1 100644
--- a/drivers/gpu/drm/i915/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/intel_engine_cs.c
@@ -1150,7 +1150,7 @@ void intel_engines_unpark(struct drm_i915_private *i915)
 		map = NULL;
 		if (engine->default_state)
 			map = i915_gem_object_pin_map(engine->default_state,
-						      I915_MAP_WB);
+						      I915_MAP_FORCE_WB);
 		if (!IS_ERR_OR_NULL(map))
 			engine->pinned_default_state = map;
 
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 595ff42e7662..b8648449817c 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -1314,7 +1314,7 @@ static int __context_pin(struct i915_gem_context *ctx, struct i915_vma *vma)
 	 * on an active context (which by nature is already on the GPU).
 	 */
 	if (!(vma->flags & I915_VMA_GLOBAL_BIND)) {
-		err = i915_gem_object_set_to_gtt_domain(vma->obj, true);
+		err = i915_gem_object_set_to_wc_domain(vma->obj, true);
 		if (err)
 			return err;
 	}
@@ -1342,7 +1342,9 @@ __execlists_context_pin(struct intel_engine_cs *engine,
 	if (ret)
 		goto err;
 
-	vaddr = i915_gem_object_pin_map(ce->state->obj, I915_MAP_WB);
+	vaddr = i915_gem_object_pin_map(ce->state->obj,
+					i915_coherent_map_type(ctx->i915) |
+					I915_MAP_OVERRIDE);
 	if (IS_ERR(vaddr)) {
 		ret = PTR_ERR(vaddr);
 		goto unpin_vma;
@@ -2771,7 +2773,7 @@ populate_lr_context(struct i915_gem_context *ctx,
 		void *defaults;
 
 		defaults = i915_gem_object_pin_map(engine->default_state,
-						   I915_MAP_WB);
+						   I915_MAP_FORCE_WB);
 		if (IS_ERR(defaults)) {
 			ret = PTR_ERR(defaults);
 			goto err_unpin_ctx;
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 472939f5c18f..266c6d047d10 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -1288,7 +1288,7 @@ alloc_context_vma(struct intel_engine_cs *engine)
 		}
 
 		defaults = i915_gem_object_pin_map(engine->default_state,
-						   I915_MAP_WB);
+						   I915_MAP_FORCE_WB);
 		if (IS_ERR(defaults)) {
 			err = PTR_ERR(defaults);
 			goto err_map;
-- 
2.19.0.rc2

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

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 7/7] drm/i915/execlists: Onion unwind for logical_ring_init() failure
  2018-09-06  7:51 [PATCH 1/7] drm/i915: Missed interrupt simulation is no more, tell the world Chris Wilson
                   ` (4 preceding siblings ...)
  2018-09-06  7:52 ` [PATCH 6/7] drm/i915/execlists: Use coherent writes into the context image Chris Wilson
@ 2018-09-06  7:52 ` Chris Wilson
  2018-09-06  8:32 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/7] drm/i915: Missed interrupt simulation is no more, tell the world Patchwork
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Chris Wilson @ 2018-09-06  7:52 UTC (permalink / raw)
  To: intel-gfx

Fix up the error unwind for logical_ring_init() failing by moving the
cleanup into the callers who own the various bits of state during
initialisation.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/intel_lrc.c | 18 +++++++++++-------
 1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index b8648449817c..ebf05be35f4d 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -2414,7 +2414,7 @@ static int logical_ring_init(struct intel_engine_cs *engine)
 
 	ret = intel_engine_init_common(engine);
 	if (ret)
-		goto error;
+		return ret;
 
 	if (HAS_LOGICAL_RING_ELSQ(i915)) {
 		execlists->submit_reg = i915->regs +
@@ -2456,10 +2456,6 @@ static int logical_ring_init(struct intel_engine_cs *engine)
 	reset_csb_pointers(execlists);
 
 	return 0;
-
-error:
-	intel_logical_ring_cleanup(engine);
-	return ret;
 }
 
 int logical_render_ring_init(struct intel_engine_cs *engine)
@@ -2482,10 +2478,14 @@ int logical_render_ring_init(struct intel_engine_cs *engine)
 	engine->emit_breadcrumb = gen8_emit_breadcrumb_rcs;
 	engine->emit_breadcrumb_sz = gen8_emit_breadcrumb_rcs_sz;
 
-	ret = intel_engine_create_scratch(engine, PAGE_SIZE);
+	ret = logical_ring_init(engine);
 	if (ret)
 		return ret;
 
+	ret = intel_engine_create_scratch(engine, PAGE_SIZE);
+	if (ret)
+		goto err_cleanup_common;
+
 	ret = intel_init_workaround_bb(engine);
 	if (ret) {
 		/*
@@ -2497,7 +2497,11 @@ int logical_render_ring_init(struct intel_engine_cs *engine)
 			  ret);
 	}
 
-	return logical_ring_init(engine);
+	return 0;
+
+err_cleanup_common:
+	intel_engine_cleanup_common(engine);
+	return ret;
 }
 
 int logical_xcs_ring_init(struct intel_engine_cs *engine)
-- 
2.19.0.rc2

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

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/7] drm/i915: Missed interrupt simulation is no more, tell the world
  2018-09-06  7:51 [PATCH 1/7] drm/i915: Missed interrupt simulation is no more, tell the world Chris Wilson
                   ` (5 preceding siblings ...)
  2018-09-06  7:52 ` [PATCH 7/7] drm/i915/execlists: Onion unwind for logical_ring_init() failure Chris Wilson
@ 2018-09-06  8:32 ` Patchwork
  2018-09-06  8:34 ` ✗ Fi.CI.SPARSE: " Patchwork
  2018-09-06  8:50 ` ✗ Fi.CI.BAT: failure " Patchwork
  8 siblings, 0 replies; 10+ messages in thread
From: Patchwork @ 2018-09-06  8:32 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx

== Series Details ==

Series: series starting with [1/7] drm/i915: Missed interrupt simulation is no more, tell the world
URL   : https://patchwork.freedesktop.org/series/49255/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
ac19c8f1d890 drm/i915: Missed interrupt simulation is no more, tell the world
7e6243ec098e drm/i915/execlists: Reset CSB pointers on canceling requests (wedging)
08bf3205457a drm/i915/execlists: Avoid kicking priority on the current context
-:49: ERROR:SPACING: space prohibited after that open parenthesis '('
#49: FILE: drivers/gpu/drm/i915/intel_guc_submission.c:774:
+	if ( __guc_dequeue(engine))

total: 1 errors, 0 warnings, 0 checks, 103 lines checked
55badd0ca312 drm/i915/selftests: Basic stress test for rapid context switching
53a11c83e442 drm/i915/execlists: Delay updating ring register state after resume
-:68: CHECK:SPACING: spaces preferred around that '+' (ctx:VxV)
#68: FILE: drivers/gpu/drm/i915/intel_lrc.c:2892:
+				ce->lrc_reg_state[CTX_RING_HEAD+1] = 0;
 				                               ^

-:69: CHECK:SPACING: spaces preferred around that '+' (ctx:VxV)
#69: FILE: drivers/gpu/drm/i915/intel_lrc.c:2893:
+				ce->lrc_reg_state[CTX_RING_TAIL+1] = 0;
 				                               ^

total: 0 errors, 0 warnings, 2 checks, 52 lines checked
b6970b210332 drm/i915/execlists: Use coherent writes into the context image
-:56: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#56: FILE: drivers/gpu/drm/i915/i915_perf.c:1851:
+		regs = i915_gem_object_pin_map(ce->state->obj,
+					      i915_coherent_map_type(dev_priv));

total: 0 errors, 0 warnings, 1 checks, 71 lines checked
b818668614c0 drm/i915/execlists: Onion unwind for logical_ring_init() failure

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* ✗ Fi.CI.SPARSE: warning for series starting with [1/7] drm/i915: Missed interrupt simulation is no more, tell the world
  2018-09-06  7:51 [PATCH 1/7] drm/i915: Missed interrupt simulation is no more, tell the world Chris Wilson
                   ` (6 preceding siblings ...)
  2018-09-06  8:32 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/7] drm/i915: Missed interrupt simulation is no more, tell the world Patchwork
@ 2018-09-06  8:34 ` Patchwork
  2018-09-06  8:50 ` ✗ Fi.CI.BAT: failure " Patchwork
  8 siblings, 0 replies; 10+ messages in thread
From: Patchwork @ 2018-09-06  8:34 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx

== Series Details ==

Series: series starting with [1/7] drm/i915: Missed interrupt simulation is no more, tell the world
URL   : https://patchwork.freedesktop.org/series/49255/
State : warning

== Summary ==

$ dim sparse origin/drm-tip
Commit: drm/i915: Missed interrupt simulation is no more, tell the world
Okay!

Commit: drm/i915/execlists: Reset CSB pointers on canceling requests (wedging)
Okay!

Commit: drm/i915/execlists: Avoid kicking priority on the current context
Okay!

Commit: drm/i915/selftests: Basic stress test for rapid context switching
+./include/linux/slab.h:631:13: error: undefined identifier '__builtin_mul_overflow'
+./include/linux/slab.h:631:13: warning: call with no type!

Commit: drm/i915/execlists: Delay updating ring register state after resume
Okay!

Commit: drm/i915/execlists: Use coherent writes into the context image
-drivers/gpu/drm/i915/selftests/../i915_drv.h:3688:16: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/selftests/../i915_drv.h:3694:16: warning: expression using sizeof(void)

Commit: drm/i915/execlists: Onion unwind for logical_ring_init() failure
Okay!

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* ✗ Fi.CI.BAT: failure for series starting with [1/7] drm/i915: Missed interrupt simulation is no more, tell the world
  2018-09-06  7:51 [PATCH 1/7] drm/i915: Missed interrupt simulation is no more, tell the world Chris Wilson
                   ` (7 preceding siblings ...)
  2018-09-06  8:34 ` ✗ Fi.CI.SPARSE: " Patchwork
@ 2018-09-06  8:50 ` Patchwork
  8 siblings, 0 replies; 10+ messages in thread
From: Patchwork @ 2018-09-06  8:50 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx

== Series Details ==

Series: series starting with [1/7] drm/i915: Missed interrupt simulation is no more, tell the world
URL   : https://patchwork.freedesktop.org/series/49255/
State : failure

== Summary ==

= CI Bug Log - changes from CI_DRM_4778 -> Patchwork_10106 =

== Summary - FAILURE ==

  Serious unknown changes coming with Patchwork_10106 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_10106, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/49255/revisions/1/mbox/

== Possible new issues ==

  Here are the unknown changes that may have been introduced in Patchwork_10106:

  === IGT changes ===

    ==== Possible regressions ====

    igt@drv_selftest@live_hangcheck:
      fi-cfl-s3:          PASS -> DMESG-FAIL

    
== Known issues ==

  Here are the changes found in Patchwork_10106 that come from known issues:

  === IGT changes ===

    ==== Issues hit ====

    igt@gem_exec_suspend@basic-s4-devices:
      fi-kbl-7500u:       PASS -> DMESG-WARN (fdo#107139, fdo#105128)

    igt@kms_chamelium@dp-edid-read:
      fi-kbl-7500u:       PASS -> FAIL (fdo#103841)

    igt@kms_pipe_crc_basic@nonblocking-crc-pipe-a:
      fi-byt-clapper:     PASS -> FAIL (fdo#107362)

    igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b:
      fi-byt-clapper:     PASS -> FAIL (fdo#107362, fdo#103191)

    igt@kms_psr@primary_page_flip:
      fi-icl-u:           NOTRUN -> FAIL (fdo#107383) +3

    igt@pm_backlight@basic-brightness:
      fi-glk-dsi:         PASS -> INCOMPLETE (k.org#198133, fdo#103359)

    igt@prime_vgem@basic-fence-flip:
      fi-ilk-650:         PASS -> FAIL (fdo#104008)

    
    ==== Possible fixes ====

    igt@kms_frontbuffer_tracking@basic:
      fi-byt-clapper:     FAIL (fdo#103167) -> PASS

    igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a:
      fi-byt-clapper:     FAIL (fdo#107362, fdo#103191) -> PASS +1

    
  fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167
  fdo#103191 https://bugs.freedesktop.org/show_bug.cgi?id=103191
  fdo#103359 https://bugs.freedesktop.org/show_bug.cgi?id=103359
  fdo#103841 https://bugs.freedesktop.org/show_bug.cgi?id=103841
  fdo#104008 https://bugs.freedesktop.org/show_bug.cgi?id=104008
  fdo#105128 https://bugs.freedesktop.org/show_bug.cgi?id=105128
  fdo#107139 https://bugs.freedesktop.org/show_bug.cgi?id=107139
  fdo#107362 https://bugs.freedesktop.org/show_bug.cgi?id=107362
  fdo#107383 https://bugs.freedesktop.org/show_bug.cgi?id=107383
  k.org#198133 https://bugzilla.kernel.org/show_bug.cgi?id=198133


== Participating hosts (53 -> 49) ==

  Additional (1): fi-icl-u 
  Missing    (5): fi-ctg-p8600 fi-ilk-m540 fi-byt-squawks fi-bsw-cyan fi-hsw-4200u 


== Build changes ==

    * Linux: CI_DRM_4778 -> Patchwork_10106

  CI_DRM_4778: 456cfc52e9f12423c6e597f677d8acb05851c3e3 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4631: 8884101aa01aedee01b2c3d0ac075473384551b7 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_10106: b818668614c06148e9f7d8bbf2d894679200589b @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

b818668614c0 drm/i915/execlists: Onion unwind for logical_ring_init() failure
b6970b210332 drm/i915/execlists: Use coherent writes into the context image
53a11c83e442 drm/i915/execlists: Delay updating ring register state after resume
55badd0ca312 drm/i915/selftests: Basic stress test for rapid context switching
08bf3205457a drm/i915/execlists: Avoid kicking priority on the current context
7e6243ec098e drm/i915/execlists: Reset CSB pointers on canceling requests (wedging)
ac19c8f1d890 drm/i915: Missed interrupt simulation is no more, tell the world

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_10106/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2018-09-06  8:50 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-09-06  7:51 [PATCH 1/7] drm/i915: Missed interrupt simulation is no more, tell the world Chris Wilson
2018-09-06  7:51 ` [PATCH 2/7] drm/i915/execlists: Reset CSB pointers on canceling requests (wedging) Chris Wilson
2018-09-06  7:51 ` [PATCH 3/7] drm/i915/execlists: Avoid kicking priority on the current context Chris Wilson
2018-09-06  7:51 ` [PATCH 4/7] drm/i915/selftests: Basic stress test for rapid context switching Chris Wilson
2018-09-06  7:51 ` [PATCH 5/7] drm/i915/execlists: Delay updating ring register state after resume Chris Wilson
2018-09-06  7:52 ` [PATCH 6/7] drm/i915/execlists: Use coherent writes into the context image Chris Wilson
2018-09-06  7:52 ` [PATCH 7/7] drm/i915/execlists: Onion unwind for logical_ring_init() failure Chris Wilson
2018-09-06  8:32 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/7] drm/i915: Missed interrupt simulation is no more, tell the world Patchwork
2018-09-06  8:34 ` ✗ Fi.CI.SPARSE: " Patchwork
2018-09-06  8:50 ` ✗ Fi.CI.BAT: failure " Patchwork

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.