All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/i915/gt: Incrementally check for rewinding
@ 2020-06-09 12:28 ` Chris Wilson
  0 siblings, 0 replies; 18+ messages in thread
From: Chris Wilson @ 2020-06-09 12:28 UTC (permalink / raw)
  To: intel-gfx; +Cc: Chris Wilson, Mika Kuoppala, stable

In commit 5ba32c7be81e ("drm/i915/execlists: Always force a context
reload when rewinding RING_TAIL"), we placed the check for rewinding a
context on actually submitting the next request in that context. This
was so that we only had to check once, and could do so with precision
avoiding as many forced restores as possible. For example, to ensure
that we can resubmit the same request a couple of times, we include a
small wa_tail such that on the next submission, the ring->tail will
appear to move forwards when resubmitting the same request. This is very
common as it will happen for every lite-restore to fill the second port
after a context switch.

However, intel_ring_direction() is limited in precision to movements of
upto half the ring size. The consequence being that if we tried to
unwind many requests, we could exceed half the ring and flip the sense
of the direction, so missing a force restore. As no request can be
greater than half the ring (i.e. 2048 bytes in the smallest case), we
can check for rollback incrementally. As we check against the tail that
would be submitted, we do not lose any sensitivity and allow lite
restores for the simple case. We still need to double check upon
submitting the context, to allow for multiple preemptions and
resubmissions.

Fixes: 5ba32c7be81e ("drm/i915/execlists: Always force a context reload when rewinding RING_TAIL")
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Cc: <stable@vger.kernel.org> # v5.4+
---
 drivers/gpu/drm/i915/gt/intel_engine_cs.c     |   4 +-
 drivers/gpu/drm/i915/gt/intel_lrc.c           |  21 +++-
 drivers/gpu/drm/i915/gt/intel_ring.c          |   4 +
 drivers/gpu/drm/i915/gt/selftest_mocs.c       |  18 ++-
 drivers/gpu/drm/i915/gt/selftest_ring.c       | 110 ++++++++++++++++++
 .../drm/i915/selftests/i915_mock_selftests.h  |   1 +
 6 files changed, 154 insertions(+), 4 deletions(-)
 create mode 100644 drivers/gpu/drm/i915/gt/selftest_ring.c

diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
index e5141a897786..0a05301e00fb 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
@@ -646,7 +646,7 @@ static int engine_setup_common(struct intel_engine_cs *engine)
 struct measure_breadcrumb {
 	struct i915_request rq;
 	struct intel_ring ring;
-	u32 cs[1024];
+	u32 cs[2048];
 };
 
 static int measure_breadcrumb_dw(struct intel_context *ce)
@@ -667,6 +667,8 @@ static int measure_breadcrumb_dw(struct intel_context *ce)
 
 	frame->ring.vaddr = frame->cs;
 	frame->ring.size = sizeof(frame->cs);
+	frame->ring.wrap =
+		BITS_PER_TYPE(frame->ring.size) - ilog2(frame->ring.size);
 	frame->ring.effective_size = frame->ring.size;
 	intel_ring_update_space(&frame->ring);
 	frame->rq.ring = &frame->ring;
diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c
index a057f7a2a521..f66274e60bb6 100644
--- a/drivers/gpu/drm/i915/gt/intel_lrc.c
+++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
@@ -1137,6 +1137,13 @@ __unwind_incomplete_requests(struct intel_engine_cs *engine)
 			list_move(&rq->sched.link, pl);
 			set_bit(I915_FENCE_FLAG_PQUEUE, &rq->fence.flags);
 
+			/* Check for rollback incrementally */
+			if (intel_ring_direction(rq->ring,
+						 intel_ring_wrap(rq->ring,
+								 rq->tail),
+						 rq->ring->tail) <= 0)
+				rq->context->lrc.desc |= CTX_DESC_FORCE_RESTORE;
+
 			active = rq;
 		} else {
 			struct intel_engine_cs *owner = rq->context->engine;
@@ -1505,8 +1512,9 @@ static u64 execlists_update_context(struct i915_request *rq)
 	 * HW has a tendency to ignore us rewinding the TAIL to the end of
 	 * an earlier request.
 	 */
+	GEM_BUG_ON(ce->lrc_reg_state[CTX_RING_TAIL] != rq->ring->tail);
+	prev = rq->ring->tail;
 	tail = intel_ring_set_tail(rq->ring, rq->tail);
-	prev = ce->lrc_reg_state[CTX_RING_TAIL];
 	if (unlikely(intel_ring_direction(rq->ring, tail, prev) <= 0))
 		desc |= CTX_DESC_FORCE_RESTORE;
 	ce->lrc_reg_state[CTX_RING_TAIL] = tail;
@@ -4758,6 +4766,14 @@ static int gen12_emit_flush(struct i915_request *request, u32 mode)
 	return 0;
 }
 
+static void assert_request_valid(struct i915_request *rq)
+{
+	struct intel_ring *ring __maybe_unused = rq->ring;
+
+	/* Can we unwind this request without appearing to go forwards? */
+	GEM_BUG_ON(intel_ring_direction(ring, rq->wa_tail, rq->head) <= 0);
+}
+
 /*
  * Reserve space for 2 NOOPs at the end of each request to be
  * used as a workaround for not being allowed to do lite
@@ -4770,6 +4786,9 @@ static u32 *gen8_emit_wa_tail(struct i915_request *request, u32 *cs)
 	*cs++ = MI_NOOP;
 	request->wa_tail = intel_ring_offset(request, cs);
 
+	/* Check that entire request is less than half the ring */
+	assert_request_valid(request);
+
 	return cs;
 }
 
diff --git a/drivers/gpu/drm/i915/gt/intel_ring.c b/drivers/gpu/drm/i915/gt/intel_ring.c
index 8cda1b7e17ba..bdb324167ef3 100644
--- a/drivers/gpu/drm/i915/gt/intel_ring.c
+++ b/drivers/gpu/drm/i915/gt/intel_ring.c
@@ -315,3 +315,7 @@ int intel_ring_cacheline_align(struct i915_request *rq)
 	GEM_BUG_ON(rq->ring->emit & (CACHELINE_BYTES - 1));
 	return 0;
 }
+
+#if IS_ENABLED(CONFIG_DRM_I915_SELFTEST)
+#include "selftest_ring.c"
+#endif
diff --git a/drivers/gpu/drm/i915/gt/selftest_mocs.c b/drivers/gpu/drm/i915/gt/selftest_mocs.c
index 7bae64018ad9..b25eba50c88e 100644
--- a/drivers/gpu/drm/i915/gt/selftest_mocs.c
+++ b/drivers/gpu/drm/i915/gt/selftest_mocs.c
@@ -18,6 +18,20 @@ struct live_mocs {
 	void *vaddr;
 };
 
+static struct intel_context *mocs_context_create(struct intel_engine_cs *engine)
+{
+	struct intel_context *ce;
+
+	ce = intel_context_create(engine);
+	if (IS_ERR(ce))
+		return ce;
+
+	/* We build large requests to read the registers from the ring */
+	ce->ring = __intel_context_ring_size(SZ_16K);
+
+	return ce;
+}
+
 static int request_add_sync(struct i915_request *rq, int err)
 {
 	i915_request_get(rq);
@@ -301,7 +315,7 @@ static int live_mocs_clean(void *arg)
 	for_each_engine(engine, gt, id) {
 		struct intel_context *ce;
 
-		ce = intel_context_create(engine);
+		ce = mocs_context_create(engine);
 		if (IS_ERR(ce)) {
 			err = PTR_ERR(ce);
 			break;
@@ -395,7 +409,7 @@ static int live_mocs_reset(void *arg)
 	for_each_engine(engine, gt, id) {
 		struct intel_context *ce;
 
-		ce = intel_context_create(engine);
+		ce = mocs_context_create(engine);
 		if (IS_ERR(ce)) {
 			err = PTR_ERR(ce);
 			break;
diff --git a/drivers/gpu/drm/i915/gt/selftest_ring.c b/drivers/gpu/drm/i915/gt/selftest_ring.c
new file mode 100644
index 000000000000..2a8c534dc125
--- /dev/null
+++ b/drivers/gpu/drm/i915/gt/selftest_ring.c
@@ -0,0 +1,110 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright © 2020 Intel Corporation
+ */
+
+static struct intel_ring *mock_ring(unsigned long sz)
+{
+	struct intel_ring *ring;
+
+	ring = kzalloc(sizeof(*ring) + sz, GFP_KERNEL);
+	if (!ring)
+		return NULL;
+
+	kref_init(&ring->ref);
+	ring->size = sz;
+	ring->wrap = BITS_PER_TYPE(ring->size) - ilog2(sz);
+	ring->effective_size = sz;
+	ring->vaddr = (void *)(ring + 1);
+	atomic_set(&ring->pin_count, 1);
+
+	intel_ring_update_space(ring);
+
+	return ring;
+}
+
+static void mock_ring_free(struct intel_ring *ring)
+{
+	kfree(ring);
+}
+
+static int check_ring_direction(struct intel_ring *ring,
+				u32 next, u32 prev,
+				int expected)
+{
+	int result;
+
+	result = intel_ring_direction(ring, next, prev);
+	if (result < 0)
+		result = -1;
+	else if (result > 0)
+		result = 1;
+
+	if (result != expected) {
+		pr_err("intel_ring_direction(%u, %u):%d != %d\n",
+		       next, prev, result, expected);
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static int check_ring_step(struct intel_ring *ring, u32 x, u32 step)
+{
+	u32 prev = x, next = intel_ring_wrap(ring, x + step);
+	int err = 0;
+
+	err |= check_ring_direction(ring, next, next,  0);
+	err |= check_ring_direction(ring, prev, prev,  0);
+	err |= check_ring_direction(ring, next, prev,  1);
+	err |= check_ring_direction(ring, prev, next, -1);
+
+	return err;
+}
+
+static int check_ring_offset(struct intel_ring *ring, u32 x, u32 step)
+{
+	int err = 0;
+
+	err |= check_ring_step(ring, x, step);
+	err |= check_ring_step(ring, intel_ring_wrap(ring, x + 1), step);
+	err |= check_ring_step(ring, intel_ring_wrap(ring, x - 1), step);
+
+	return err;
+}
+
+static int igt_ring_direction(void *dummy)
+{
+	struct intel_ring *ring;
+	unsigned int half = 2048;
+	int step, err = 0;
+
+	ring = mock_ring(2 * half);
+	if (!ring)
+		return -ENOMEM;
+
+	GEM_BUG_ON(ring->size != 2 * half);
+
+	/* Precision of wrap detection is limited to ring->size / 2 */
+	for (step = 1; step < half; step <<= 1) {
+		err |= check_ring_offset(ring, 0, step);
+		err |= check_ring_offset(ring, half, step);
+	}
+	err |= check_ring_step(ring, 0, half - 64);
+
+	/* And check unwrapped handling for good measure */
+	err |= check_ring_offset(ring, 0, 2 * half + 64);
+	err |= check_ring_offset(ring, 3 * half, 1);
+
+	mock_ring_free(ring);
+	return err;
+}
+
+int intel_ring_mock_selftests(void)
+{
+	static const struct i915_subtest tests[] = {
+		SUBTEST(igt_ring_direction),
+	};
+
+	return i915_subtests(tests, NULL);
+}
diff --git a/drivers/gpu/drm/i915/selftests/i915_mock_selftests.h b/drivers/gpu/drm/i915/selftests/i915_mock_selftests.h
index 1929feba4e8e..3db34d3eea58 100644
--- a/drivers/gpu/drm/i915/selftests/i915_mock_selftests.h
+++ b/drivers/gpu/drm/i915/selftests/i915_mock_selftests.h
@@ -21,6 +21,7 @@ selftest(fence, i915_sw_fence_mock_selftests)
 selftest(scatterlist, scatterlist_mock_selftests)
 selftest(syncmap, i915_syncmap_mock_selftests)
 selftest(uncore, intel_uncore_mock_selftests)
+selftest(ring, intel_ring_mock_selftests)
 selftest(engine, intel_engine_cs_mock_selftests)
 selftest(timelines, intel_timeline_mock_selftests)
 selftest(requests, i915_request_mock_selftests)
-- 
2.20.1


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

* [Intel-gfx] [PATCH] drm/i915/gt: Incrementally check for rewinding
@ 2020-06-09 12:28 ` Chris Wilson
  0 siblings, 0 replies; 18+ messages in thread
From: Chris Wilson @ 2020-06-09 12:28 UTC (permalink / raw)
  To: intel-gfx; +Cc: stable, Chris Wilson

In commit 5ba32c7be81e ("drm/i915/execlists: Always force a context
reload when rewinding RING_TAIL"), we placed the check for rewinding a
context on actually submitting the next request in that context. This
was so that we only had to check once, and could do so with precision
avoiding as many forced restores as possible. For example, to ensure
that we can resubmit the same request a couple of times, we include a
small wa_tail such that on the next submission, the ring->tail will
appear to move forwards when resubmitting the same request. This is very
common as it will happen for every lite-restore to fill the second port
after a context switch.

However, intel_ring_direction() is limited in precision to movements of
upto half the ring size. The consequence being that if we tried to
unwind many requests, we could exceed half the ring and flip the sense
of the direction, so missing a force restore. As no request can be
greater than half the ring (i.e. 2048 bytes in the smallest case), we
can check for rollback incrementally. As we check against the tail that
would be submitted, we do not lose any sensitivity and allow lite
restores for the simple case. We still need to double check upon
submitting the context, to allow for multiple preemptions and
resubmissions.

Fixes: 5ba32c7be81e ("drm/i915/execlists: Always force a context reload when rewinding RING_TAIL")
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Cc: <stable@vger.kernel.org> # v5.4+
---
 drivers/gpu/drm/i915/gt/intel_engine_cs.c     |   4 +-
 drivers/gpu/drm/i915/gt/intel_lrc.c           |  21 +++-
 drivers/gpu/drm/i915/gt/intel_ring.c          |   4 +
 drivers/gpu/drm/i915/gt/selftest_mocs.c       |  18 ++-
 drivers/gpu/drm/i915/gt/selftest_ring.c       | 110 ++++++++++++++++++
 .../drm/i915/selftests/i915_mock_selftests.h  |   1 +
 6 files changed, 154 insertions(+), 4 deletions(-)
 create mode 100644 drivers/gpu/drm/i915/gt/selftest_ring.c

diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
index e5141a897786..0a05301e00fb 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
@@ -646,7 +646,7 @@ static int engine_setup_common(struct intel_engine_cs *engine)
 struct measure_breadcrumb {
 	struct i915_request rq;
 	struct intel_ring ring;
-	u32 cs[1024];
+	u32 cs[2048];
 };
 
 static int measure_breadcrumb_dw(struct intel_context *ce)
@@ -667,6 +667,8 @@ static int measure_breadcrumb_dw(struct intel_context *ce)
 
 	frame->ring.vaddr = frame->cs;
 	frame->ring.size = sizeof(frame->cs);
+	frame->ring.wrap =
+		BITS_PER_TYPE(frame->ring.size) - ilog2(frame->ring.size);
 	frame->ring.effective_size = frame->ring.size;
 	intel_ring_update_space(&frame->ring);
 	frame->rq.ring = &frame->ring;
diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c
index a057f7a2a521..f66274e60bb6 100644
--- a/drivers/gpu/drm/i915/gt/intel_lrc.c
+++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
@@ -1137,6 +1137,13 @@ __unwind_incomplete_requests(struct intel_engine_cs *engine)
 			list_move(&rq->sched.link, pl);
 			set_bit(I915_FENCE_FLAG_PQUEUE, &rq->fence.flags);
 
+			/* Check for rollback incrementally */
+			if (intel_ring_direction(rq->ring,
+						 intel_ring_wrap(rq->ring,
+								 rq->tail),
+						 rq->ring->tail) <= 0)
+				rq->context->lrc.desc |= CTX_DESC_FORCE_RESTORE;
+
 			active = rq;
 		} else {
 			struct intel_engine_cs *owner = rq->context->engine;
@@ -1505,8 +1512,9 @@ static u64 execlists_update_context(struct i915_request *rq)
 	 * HW has a tendency to ignore us rewinding the TAIL to the end of
 	 * an earlier request.
 	 */
+	GEM_BUG_ON(ce->lrc_reg_state[CTX_RING_TAIL] != rq->ring->tail);
+	prev = rq->ring->tail;
 	tail = intel_ring_set_tail(rq->ring, rq->tail);
-	prev = ce->lrc_reg_state[CTX_RING_TAIL];
 	if (unlikely(intel_ring_direction(rq->ring, tail, prev) <= 0))
 		desc |= CTX_DESC_FORCE_RESTORE;
 	ce->lrc_reg_state[CTX_RING_TAIL] = tail;
@@ -4758,6 +4766,14 @@ static int gen12_emit_flush(struct i915_request *request, u32 mode)
 	return 0;
 }
 
+static void assert_request_valid(struct i915_request *rq)
+{
+	struct intel_ring *ring __maybe_unused = rq->ring;
+
+	/* Can we unwind this request without appearing to go forwards? */
+	GEM_BUG_ON(intel_ring_direction(ring, rq->wa_tail, rq->head) <= 0);
+}
+
 /*
  * Reserve space for 2 NOOPs at the end of each request to be
  * used as a workaround for not being allowed to do lite
@@ -4770,6 +4786,9 @@ static u32 *gen8_emit_wa_tail(struct i915_request *request, u32 *cs)
 	*cs++ = MI_NOOP;
 	request->wa_tail = intel_ring_offset(request, cs);
 
+	/* Check that entire request is less than half the ring */
+	assert_request_valid(request);
+
 	return cs;
 }
 
diff --git a/drivers/gpu/drm/i915/gt/intel_ring.c b/drivers/gpu/drm/i915/gt/intel_ring.c
index 8cda1b7e17ba..bdb324167ef3 100644
--- a/drivers/gpu/drm/i915/gt/intel_ring.c
+++ b/drivers/gpu/drm/i915/gt/intel_ring.c
@@ -315,3 +315,7 @@ int intel_ring_cacheline_align(struct i915_request *rq)
 	GEM_BUG_ON(rq->ring->emit & (CACHELINE_BYTES - 1));
 	return 0;
 }
+
+#if IS_ENABLED(CONFIG_DRM_I915_SELFTEST)
+#include "selftest_ring.c"
+#endif
diff --git a/drivers/gpu/drm/i915/gt/selftest_mocs.c b/drivers/gpu/drm/i915/gt/selftest_mocs.c
index 7bae64018ad9..b25eba50c88e 100644
--- a/drivers/gpu/drm/i915/gt/selftest_mocs.c
+++ b/drivers/gpu/drm/i915/gt/selftest_mocs.c
@@ -18,6 +18,20 @@ struct live_mocs {
 	void *vaddr;
 };
 
+static struct intel_context *mocs_context_create(struct intel_engine_cs *engine)
+{
+	struct intel_context *ce;
+
+	ce = intel_context_create(engine);
+	if (IS_ERR(ce))
+		return ce;
+
+	/* We build large requests to read the registers from the ring */
+	ce->ring = __intel_context_ring_size(SZ_16K);
+
+	return ce;
+}
+
 static int request_add_sync(struct i915_request *rq, int err)
 {
 	i915_request_get(rq);
@@ -301,7 +315,7 @@ static int live_mocs_clean(void *arg)
 	for_each_engine(engine, gt, id) {
 		struct intel_context *ce;
 
-		ce = intel_context_create(engine);
+		ce = mocs_context_create(engine);
 		if (IS_ERR(ce)) {
 			err = PTR_ERR(ce);
 			break;
@@ -395,7 +409,7 @@ static int live_mocs_reset(void *arg)
 	for_each_engine(engine, gt, id) {
 		struct intel_context *ce;
 
-		ce = intel_context_create(engine);
+		ce = mocs_context_create(engine);
 		if (IS_ERR(ce)) {
 			err = PTR_ERR(ce);
 			break;
diff --git a/drivers/gpu/drm/i915/gt/selftest_ring.c b/drivers/gpu/drm/i915/gt/selftest_ring.c
new file mode 100644
index 000000000000..2a8c534dc125
--- /dev/null
+++ b/drivers/gpu/drm/i915/gt/selftest_ring.c
@@ -0,0 +1,110 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright © 2020 Intel Corporation
+ */
+
+static struct intel_ring *mock_ring(unsigned long sz)
+{
+	struct intel_ring *ring;
+
+	ring = kzalloc(sizeof(*ring) + sz, GFP_KERNEL);
+	if (!ring)
+		return NULL;
+
+	kref_init(&ring->ref);
+	ring->size = sz;
+	ring->wrap = BITS_PER_TYPE(ring->size) - ilog2(sz);
+	ring->effective_size = sz;
+	ring->vaddr = (void *)(ring + 1);
+	atomic_set(&ring->pin_count, 1);
+
+	intel_ring_update_space(ring);
+
+	return ring;
+}
+
+static void mock_ring_free(struct intel_ring *ring)
+{
+	kfree(ring);
+}
+
+static int check_ring_direction(struct intel_ring *ring,
+				u32 next, u32 prev,
+				int expected)
+{
+	int result;
+
+	result = intel_ring_direction(ring, next, prev);
+	if (result < 0)
+		result = -1;
+	else if (result > 0)
+		result = 1;
+
+	if (result != expected) {
+		pr_err("intel_ring_direction(%u, %u):%d != %d\n",
+		       next, prev, result, expected);
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static int check_ring_step(struct intel_ring *ring, u32 x, u32 step)
+{
+	u32 prev = x, next = intel_ring_wrap(ring, x + step);
+	int err = 0;
+
+	err |= check_ring_direction(ring, next, next,  0);
+	err |= check_ring_direction(ring, prev, prev,  0);
+	err |= check_ring_direction(ring, next, prev,  1);
+	err |= check_ring_direction(ring, prev, next, -1);
+
+	return err;
+}
+
+static int check_ring_offset(struct intel_ring *ring, u32 x, u32 step)
+{
+	int err = 0;
+
+	err |= check_ring_step(ring, x, step);
+	err |= check_ring_step(ring, intel_ring_wrap(ring, x + 1), step);
+	err |= check_ring_step(ring, intel_ring_wrap(ring, x - 1), step);
+
+	return err;
+}
+
+static int igt_ring_direction(void *dummy)
+{
+	struct intel_ring *ring;
+	unsigned int half = 2048;
+	int step, err = 0;
+
+	ring = mock_ring(2 * half);
+	if (!ring)
+		return -ENOMEM;
+
+	GEM_BUG_ON(ring->size != 2 * half);
+
+	/* Precision of wrap detection is limited to ring->size / 2 */
+	for (step = 1; step < half; step <<= 1) {
+		err |= check_ring_offset(ring, 0, step);
+		err |= check_ring_offset(ring, half, step);
+	}
+	err |= check_ring_step(ring, 0, half - 64);
+
+	/* And check unwrapped handling for good measure */
+	err |= check_ring_offset(ring, 0, 2 * half + 64);
+	err |= check_ring_offset(ring, 3 * half, 1);
+
+	mock_ring_free(ring);
+	return err;
+}
+
+int intel_ring_mock_selftests(void)
+{
+	static const struct i915_subtest tests[] = {
+		SUBTEST(igt_ring_direction),
+	};
+
+	return i915_subtests(tests, NULL);
+}
diff --git a/drivers/gpu/drm/i915/selftests/i915_mock_selftests.h b/drivers/gpu/drm/i915/selftests/i915_mock_selftests.h
index 1929feba4e8e..3db34d3eea58 100644
--- a/drivers/gpu/drm/i915/selftests/i915_mock_selftests.h
+++ b/drivers/gpu/drm/i915/selftests/i915_mock_selftests.h
@@ -21,6 +21,7 @@ selftest(fence, i915_sw_fence_mock_selftests)
 selftest(scatterlist, scatterlist_mock_selftests)
 selftest(syncmap, i915_syncmap_mock_selftests)
 selftest(uncore, intel_uncore_mock_selftests)
+selftest(ring, intel_ring_mock_selftests)
 selftest(engine, intel_engine_cs_mock_selftests)
 selftest(timelines, intel_timeline_mock_selftests)
 selftest(requests, i915_request_mock_selftests)
-- 
2.20.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

* [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915/gt: Incrementally check for rewinding
  2020-06-09 12:28 ` [Intel-gfx] " Chris Wilson
  (?)
@ 2020-06-09 12:47 ` Patchwork
  -1 siblings, 0 replies; 18+ messages in thread
From: Patchwork @ 2020-06-09 12:47 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx

== Series Details ==

Series: drm/i915/gt: Incrementally check for rewinding
URL   : https://patchwork.freedesktop.org/series/78163/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
e9fc78f4bfec drm/i915/gt: Incrementally check for rewinding
-:165: WARNING:FILE_PATH_CHANGES: added, moved or deleted file(s), does MAINTAINERS need updating?
#165: 
new file mode 100644

total: 0 errors, 1 warnings, 0 checks, 222 lines checked

_______________________________________________
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

* [Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915/gt: Incrementally check for rewinding
  2020-06-09 12:28 ` [Intel-gfx] " Chris Wilson
  (?)
  (?)
@ 2020-06-09 13:08 ` Patchwork
  -1 siblings, 0 replies; 18+ messages in thread
From: Patchwork @ 2020-06-09 13:08 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx

== Series Details ==

Series: drm/i915/gt: Incrementally check for rewinding
URL   : https://patchwork.freedesktop.org/series/78163/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_8602 -> Patchwork_17914
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17914/index.html

Known issues
------------

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

### IGT changes ###

#### Issues hit ####

  * igt@i915_pm_rpm@module-reload:
    - fi-glk-dsi:         [PASS][1] -> [DMESG-WARN][2] ([i915#1982])
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8602/fi-glk-dsi/igt@i915_pm_rpm@module-reload.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17914/fi-glk-dsi/igt@i915_pm_rpm@module-reload.html

  * igt@kms_busy@basic@flip:
    - fi-kbl-x1275:       [PASS][3] -> [DMESG-WARN][4] ([i915#62] / [i915#92] / [i915#95])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8602/fi-kbl-x1275/igt@kms_busy@basic@flip.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17914/fi-kbl-x1275/igt@kms_busy@basic@flip.html

  * igt@kms_cursor_legacy@basic-busy-flip-before-cursor-atomic:
    - fi-bsw-n3050:       [PASS][5] -> [DMESG-WARN][6] ([i915#1982])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8602/fi-bsw-n3050/igt@kms_cursor_legacy@basic-busy-flip-before-cursor-atomic.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17914/fi-bsw-n3050/igt@kms_cursor_legacy@basic-busy-flip-before-cursor-atomic.html

  
#### Possible fixes ####

  * igt@i915_pm_rpm@module-reload:
    - fi-byt-j1900:       [DMESG-WARN][7] ([i915#1982]) -> [PASS][8]
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8602/fi-byt-j1900/igt@i915_pm_rpm@module-reload.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17914/fi-byt-j1900/igt@i915_pm_rpm@module-reload.html

  * igt@kms_cursor_legacy@basic-flip-after-cursor-atomic:
    - fi-icl-u2:          [DMESG-WARN][9] ([i915#1982]) -> [PASS][10] +1 similar issue
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8602/fi-icl-u2/igt@kms_cursor_legacy@basic-flip-after-cursor-atomic.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17914/fi-icl-u2/igt@kms_cursor_legacy@basic-flip-after-cursor-atomic.html

  * {igt@kms_flip@basic-flip-vs-modeset@b-dsi1}:
    - {fi-tgl-dsi}:       [DMESG-WARN][11] ([i915#1982]) -> [PASS][12] +1 similar issue
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8602/fi-tgl-dsi/igt@kms_flip@basic-flip-vs-modeset@b-dsi1.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17914/fi-tgl-dsi/igt@kms_flip@basic-flip-vs-modeset@b-dsi1.html

  
#### Warnings ####

  * igt@i915_pm_rpm@module-reload:
    - fi-kbl-x1275:       [DMESG-FAIL][13] ([i915#62]) -> [SKIP][14] ([fdo#109271])
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8602/fi-kbl-x1275/igt@i915_pm_rpm@module-reload.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17914/fi-kbl-x1275/igt@i915_pm_rpm@module-reload.html

  * igt@kms_cursor_legacy@basic-flip-after-cursor-legacy:
    - fi-kbl-x1275:       [DMESG-WARN][15] ([i915#62] / [i915#92]) -> [DMESG-WARN][16] ([i915#62] / [i915#92] / [i915#95]) +8 similar issues
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8602/fi-kbl-x1275/igt@kms_cursor_legacy@basic-flip-after-cursor-legacy.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17914/fi-kbl-x1275/igt@kms_cursor_legacy@basic-flip-after-cursor-legacy.html

  * igt@kms_cursor_legacy@basic-flip-before-cursor-atomic:
    - fi-kbl-x1275:       [DMESG-WARN][17] ([i915#62] / [i915#92] / [i915#95]) -> [DMESG-WARN][18] ([i915#62] / [i915#92]) +2 similar issues
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8602/fi-kbl-x1275/igt@kms_cursor_legacy@basic-flip-before-cursor-atomic.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17914/fi-kbl-x1275/igt@kms_cursor_legacy@basic-flip-before-cursor-atomic.html

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [i915#1982]: https://gitlab.freedesktop.org/drm/intel/issues/1982
  [i915#62]: https://gitlab.freedesktop.org/drm/intel/issues/62
  [i915#92]: https://gitlab.freedesktop.org/drm/intel/issues/92
  [i915#95]: https://gitlab.freedesktop.org/drm/intel/issues/95


Participating hosts (48 -> 43)
------------------------------

  Additional (1): fi-kbl-7560u 
  Missing    (6): fi-ilk-m540 fi-byt-squawks fi-bsw-cyan fi-ctg-p8600 fi-byt-clapper fi-bdw-samus 


Build changes
-------------

  * Linux: CI_DRM_8602 -> Patchwork_17914

  CI-20190529: 20190529
  CI_DRM_8602: 8b5dcdfb116246ecc8e676961bb1cf82ec75e33f @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5699: 201da47cb57b8fadd9bc45be16b82617b32a2c01 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_17914: e9fc78f4bfec134476a0eed02ee2b2241c4f206b @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

e9fc78f4bfec drm/i915/gt: Incrementally check for rewinding

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17914/index.html
_______________________________________________
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

* [Intel-gfx] ✗ Fi.CI.IGT: failure for drm/i915/gt: Incrementally check for rewinding
  2020-06-09 12:28 ` [Intel-gfx] " Chris Wilson
                   ` (2 preceding siblings ...)
  (?)
@ 2020-06-09 14:10 ` Patchwork
  -1 siblings, 0 replies; 18+ messages in thread
From: Patchwork @ 2020-06-09 14:10 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx

== Series Details ==

Series: drm/i915/gt: Incrementally check for rewinding
URL   : https://patchwork.freedesktop.org/series/78163/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_8602_full -> Patchwork_17914_full
====================================================

Summary
-------

  **FAILURE**

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

  

Possible new issues
-------------------

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

### IGT changes ###

#### Possible regressions ####

  * igt@kms_flip@flip-vs-absolute-wf_vblank@a-dp1:
    - shard-kbl:          [PASS][1] -> [INCOMPLETE][2]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8602/shard-kbl6/igt@kms_flip@flip-vs-absolute-wf_vblank@a-dp1.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17914/shard-kbl1/igt@kms_flip@flip-vs-absolute-wf_vblank@a-dp1.html

  
New tests
---------

  New tests have been introduced between CI_DRM_8602_full and Patchwork_17914_full:

### New IGT tests (1) ###

  * igt@i915_selftest@mock@ring:
    - Statuses : 8 pass(s)
    - Exec time: [0.11, 1.13] s

  

Known issues
------------

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

### IGT changes ###

#### Issues hit ####

  * igt@gem_exec_whisper@basic-queues-priority-all:
    - shard-glk:          [PASS][3] -> [DMESG-WARN][4] ([i915#118] / [i915#95])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8602/shard-glk8/igt@gem_exec_whisper@basic-queues-priority-all.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17914/shard-glk8/igt@gem_exec_whisper@basic-queues-priority-all.html

  * igt@kms_big_fb@linear-16bpp-rotate-0:
    - shard-apl:          [PASS][5] -> [DMESG-WARN][6] ([i915#1982])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8602/shard-apl8/igt@kms_big_fb@linear-16bpp-rotate-0.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17914/shard-apl6/igt@kms_big_fb@linear-16bpp-rotate-0.html

  * igt@kms_color@pipe-c-ctm-0-25:
    - shard-skl:          [PASS][7] -> [DMESG-WARN][8] ([i915#1982]) +5 similar issues
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8602/shard-skl8/igt@kms_color@pipe-c-ctm-0-25.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17914/shard-skl5/igt@kms_color@pipe-c-ctm-0-25.html

  * igt@kms_color@pipe-c-ctm-blue-to-red:
    - shard-kbl:          [PASS][9] -> [DMESG-WARN][10] ([i915#93] / [i915#95]) +1 similar issue
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8602/shard-kbl3/igt@kms_color@pipe-c-ctm-blue-to-red.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17914/shard-kbl3/igt@kms_color@pipe-c-ctm-blue-to-red.html

  * igt@kms_cursor_crc@pipe-c-cursor-256x256-sliding:
    - shard-skl:          [PASS][11] -> [FAIL][12] ([i915#54])
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8602/shard-skl6/igt@kms_cursor_crc@pipe-c-cursor-256x256-sliding.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17914/shard-skl4/igt@kms_cursor_crc@pipe-c-cursor-256x256-sliding.html

  * igt@kms_flip@flip-vs-suspend-interruptible@a-dp1:
    - shard-kbl:          [PASS][13] -> [DMESG-WARN][14] ([i915#180]) +6 similar issues
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8602/shard-kbl6/igt@kms_flip@flip-vs-suspend-interruptible@a-dp1.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17914/shard-kbl1/igt@kms_flip@flip-vs-suspend-interruptible@a-dp1.html
    - shard-apl:          [PASS][15] -> [DMESG-WARN][16] ([i915#180])
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8602/shard-apl6/igt@kms_flip@flip-vs-suspend-interruptible@a-dp1.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17914/shard-apl4/igt@kms_flip@flip-vs-suspend-interruptible@a-dp1.html

  * igt@kms_hdr@bpc-switch-suspend:
    - shard-skl:          [PASS][17] -> [FAIL][18] ([i915#1188])
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8602/shard-skl10/igt@kms_hdr@bpc-switch-suspend.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17914/shard-skl8/igt@kms_hdr@bpc-switch-suspend.html

  * igt@kms_lease@page_flip_implicit_plane:
    - shard-snb:          [PASS][19] -> [TIMEOUT][20] ([i915#1958]) +1 similar issue
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8602/shard-snb2/igt@kms_lease@page_flip_implicit_plane.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17914/shard-snb4/igt@kms_lease@page_flip_implicit_plane.html

  * igt@kms_plane_alpha_blend@pipe-c-coverage-7efc:
    - shard-skl:          [PASS][21] -> [FAIL][22] ([fdo#108145] / [i915#265])
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8602/shard-skl6/igt@kms_plane_alpha_blend@pipe-c-coverage-7efc.html
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17914/shard-skl4/igt@kms_plane_alpha_blend@pipe-c-coverage-7efc.html

  * igt@kms_psr@psr2_cursor_plane_move:
    - shard-iclb:         [PASS][23] -> [SKIP][24] ([fdo#109441]) +2 similar issues
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8602/shard-iclb2/igt@kms_psr@psr2_cursor_plane_move.html
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17914/shard-iclb8/igt@kms_psr@psr2_cursor_plane_move.html

  * igt@kms_vblank@pipe-a-ts-continuation-suspend:
    - shard-skl:          [PASS][25] -> [INCOMPLETE][26] ([i915#69])
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8602/shard-skl7/igt@kms_vblank@pipe-a-ts-continuation-suspend.html
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17914/shard-skl4/igt@kms_vblank@pipe-a-ts-continuation-suspend.html

  * igt@syncobj_wait@multi-wait-for-submit-submitted-signaled:
    - shard-apl:          [PASS][27] -> [DMESG-WARN][28] ([i915#95]) +20 similar issues
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8602/shard-apl8/igt@syncobj_wait@multi-wait-for-submit-submitted-signaled.html
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17914/shard-apl7/igt@syncobj_wait@multi-wait-for-submit-submitted-signaled.html

  
#### Possible fixes ####

  * igt@gem_ctx_persistence@engines-mixed-process@vecs0:
    - shard-skl:          [FAIL][29] ([i915#1528]) -> [PASS][30]
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8602/shard-skl8/igt@gem_ctx_persistence@engines-mixed-process@vecs0.html
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17914/shard-skl6/igt@gem_ctx_persistence@engines-mixed-process@vecs0.html

  * igt@i915_pm_dc@dc5-dpms:
    - shard-skl:          [INCOMPLETE][31] ([i915#198]) -> [PASS][32]
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8602/shard-skl2/igt@i915_pm_dc@dc5-dpms.html
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17914/shard-skl10/igt@i915_pm_dc@dc5-dpms.html

  * igt@i915_pm_dc@dc6-psr:
    - shard-skl:          [FAIL][33] ([i915#454]) -> [PASS][34]
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8602/shard-skl6/igt@i915_pm_dc@dc6-psr.html
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17914/shard-skl2/igt@i915_pm_dc@dc6-psr.html

  * igt@kms_addfb_basic@bad-pitch-999:
    - shard-apl:          [DMESG-WARN][35] ([i915#95]) -> [PASS][36] +18 similar issues
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8602/shard-apl1/igt@kms_addfb_basic@bad-pitch-999.html
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17914/shard-apl8/igt@kms_addfb_basic@bad-pitch-999.html

  * igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions:
    - shard-skl:          [FAIL][37] ([IGT#5]) -> [PASS][38]
   [37]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8602/shard-skl8/igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions.html
   [38]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17914/shard-skl6/igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions.html

  * igt@kms_flip@2x-flip-vs-blocking-wf-vblank@ab-vga1-hdmi-a1:
    - shard-hsw:          [DMESG-WARN][39] ([i915#1982]) -> [PASS][40]
   [39]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8602/shard-hsw6/igt@kms_flip@2x-flip-vs-blocking-wf-vblank@ab-vga1-hdmi-a1.html
   [40]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17914/shard-hsw2/igt@kms_flip@2x-flip-vs-blocking-wf-vblank@ab-vga1-hdmi-a1.html

  * igt@kms_flip@basic-flip-vs-dpms@a-edp1:
    - shard-skl:          [DMESG-WARN][41] ([i915#1982]) -> [PASS][42] +4 similar issues
   [41]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8602/shard-skl10/igt@kms_flip@basic-flip-vs-dpms@a-edp1.html
   [42]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17914/shard-skl10/igt@kms_flip@basic-flip-vs-dpms@a-edp1.html

  * igt@kms_flip@flip-vs-expired-vblank@c-hdmi-a1:
    - shard-glk:          [FAIL][43] ([i915#79]) -> [PASS][44]
   [43]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8602/shard-glk4/igt@kms_flip@flip-vs-expired-vblank@c-hdmi-a1.html
   [44]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17914/shard-glk9/igt@kms_flip@flip-vs-expired-vblank@c-hdmi-a1.html

  * igt@kms_flip@flip-vs-suspend@b-dp1:
    - shard-apl:          [DMESG-WARN][45] ([i915#180]) -> [PASS][46] +1 similar issue
   [45]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8602/shard-apl8/igt@kms_flip@flip-vs-suspend@b-dp1.html
   [46]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17914/shard-apl7/igt@kms_flip@flip-vs-suspend@b-dp1.html

  * igt@kms_plane_alpha_blend@pipe-c-constant-alpha-min:
    - shard-skl:          [FAIL][47] ([fdo#108145] / [i915#265]) -> [PASS][48]
   [47]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8602/shard-skl3/igt@kms_plane_alpha_blend@pipe-c-constant-alpha-min.html
   [48]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17914/shard-skl1/igt@kms_plane_alpha_blend@pipe-c-constant-alpha-min.html

  * igt@kms_psr@psr2_sprite_plane_move:
    - shard-iclb:         [SKIP][49] ([fdo#109441]) -> [PASS][50] +3 similar issues
   [49]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8602/shard-iclb6/igt@kms_psr@psr2_sprite_plane_move.html
   [50]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17914/shard-iclb2/igt@kms_psr@psr2_sprite_plane_move.html

  * igt@kms_vblank@pipe-a-ts-continuation-suspend:
    - shard-kbl:          [DMESG-WARN][51] ([i915#180]) -> [PASS][52] +3 similar issues
   [51]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8602/shard-kbl4/igt@kms_vblank@pipe-a-ts-continuation-suspend.html
   [52]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17914/shard-kbl7/igt@kms_vblank@pipe-a-ts-continuation-suspend.html

  * igt@kms_vblank@pipe-b-ts-continuation-suspend:
    - shard-kbl:          [INCOMPLETE][53] ([i915#155]) -> [PASS][54]
   [53]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8602/shard-kbl1/igt@kms_vblank@pipe-b-ts-continuation-suspend.html
   [54]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17914/shard-kbl2/igt@kms_vblank@pipe-b-ts-continuation-suspend.html

  * igt@perf@polling-parameterized:
    - shard-hsw:          [FAIL][55] ([i915#1542]) -> [PASS][56]
   [55]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8602/shard-hsw6/igt@perf@polling-parameterized.html
   [56]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17914/shard-hsw7/igt@perf@polling-parameterized.html

  
#### Warnings ####

  * igt@gem_exec_reloc@basic-concurrent16:
    - shard-snb:          [FAIL][57] ([i915#1930]) -> [TIMEOUT][58] ([i915#1958])
   [57]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8602/shard-snb2/igt@gem_exec_reloc@basic-concurrent16.html
   [58]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17914/shard-snb4/igt@gem_exec_reloc@basic-concurrent16.html

  * igt@i915_pm_dc@dc3co-vpb-simulation:
    - shard-iclb:         [SKIP][59] ([i915#588]) -> [SKIP][60] ([i915#658])
   [59]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8602/shard-iclb2/igt@i915_pm_dc@dc3co-vpb-simulation.html
   [60]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17914/shard-iclb8/igt@i915_pm_dc@dc3co-vpb-simulation.html

  * igt@i915_pm_dc@dc6-psr:
    - shard-snb:          [INCOMPLETE][61] ([i915#82]) -> [SKIP][62] ([fdo#109271])
   [61]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8602/shard-snb5/igt@i915_pm_dc@dc6-psr.html
   [62]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17914/shard-snb6/igt@i915_pm_dc@dc6-psr.html

  * igt@kms_content_protection@atomic:
    - shard-apl:          [TIMEOUT][63] ([i915#1319] / [i915#1635]) -> [DMESG-FAIL][64] ([fdo#110321] / [i915#95])
   [63]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8602/shard-apl6/igt@kms_content_protection@atomic.html
   [64]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17914/shard-apl4/igt@kms_content_protection@atomic.html

  * igt@kms_content_protection@legacy:
    - shard-kbl:          [TIMEOUT][65] ([i915#1319] / [i915#1958]) -> [TIMEOUT][66] ([i915#1319])
   [65]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8602/shard-kbl4/igt@kms_content_protection@legacy.html
   [66]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17914/shard-kbl7/igt@kms_content_protection@legacy.html

  * igt@kms_content_protection@srm:
    - shard-apl:          [TIMEOUT][67] ([i915#1319] / [i915#1635]) -> [TIMEOUT][68] ([i915#1319])
   [67]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8602/shard-apl3/igt@kms_content_protection@srm.html
   [68]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17914/shard-apl3/igt@kms_content_protection@srm.html

  * igt@kms_frontbuffer_tracking@fbc-suspend:
    - shard-kbl:          [DMESG-WARN][69] ([i915#93] / [i915#95]) -> [DMESG-WARN][70] ([i915#180] / [i915#93] / [i915#95])
   [69]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8602/shard-kbl3/igt@kms_frontbuffer_tracking@fbc-suspend.html
   [70]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17914/shard-kbl3/igt@kms_frontbuffer_tracking@fbc-suspend.html

  * igt@kms_plane_alpha_blend@pipe-c-alpha-basic:
    - shard-skl:          [FAIL][71] ([fdo#108145] / [i915#265]) -> [DMESG-FAIL][72] ([fdo#108145] / [i915#1982])
   [71]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8602/shard-skl1/igt@kms_plane_alpha_blend@pipe-c-alpha-basic.html
   [72]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17914/shard-skl10/igt@kms_plane_alpha_blend@pipe-c-alpha-basic.html

  * igt@kms_psr@suspend:
    - shard-snb:          [SKIP][73] ([fdo#109271]) -> [TIMEOUT][74] ([i915#1958]) +2 similar issues
   [73]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8602/shard-snb2/igt@kms_psr@suspend.html
   [74]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17914/shard-snb4/igt@kms_psr@suspend.html

  
  [IGT#5]: https://gitlab.freedesktop.org/drm/igt-gpu-tools/issues/5
  [fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109441]: https://bugs.freedesktop.org/show_bug.cgi?id=109441
  [fdo#110321]: https://bugs.freedesktop.org/show_bug.cgi?id=110321
  [i915#118]: https://gitlab.freedesktop.org/drm/intel/issues/118
  [i915#1188]: https://gitlab.freedesktop.org/drm/intel/issues/1188
  [i915#1319]: https://gitlab.freedesktop.org/drm/intel/issues/1319
  [i915#1528]: https://gitlab.freedesktop.org/drm/intel/issues/1528
  [i915#1542]: https://gitlab.freedesktop.org/drm/intel/issues/1542
  [i915#155]: https://gitlab.freedesktop.org/drm/intel/issues/155
  [i915#1635]: https://gitlab.freedesktop.org/drm/intel/issues/1635
  [i915#180]: https://gitlab.freedesktop.org/drm/intel/issues/180
  [i915#1930]: https://gitlab.freedesktop.org/drm/intel/issues/1930
  [i915#1958]: https://gitlab.freedesktop.org/drm/intel/issues/1958
  [i915#198]: https://gitlab.freedesktop.org/drm/intel/issues/198
  [i915#1982]: https://gitlab.freedesktop.org/drm/intel/issues/1982
  [i915#265]: https://gitlab.freedesktop.org/drm/intel/issues/265
  [i915#454]: https://gitlab.freedesktop.org/drm/intel/issues/454
  [i915#54]: https://gitlab.freedesktop.org/drm/intel/issues/54
  [i915#588]: https://gitlab.freedesktop.org/drm/intel/issues/588
  [i915#658]: https://gitlab.freedesktop.org/drm/intel/issues/658
  [i915#69]: https://gitlab.freedesktop.org/drm/intel/issues/69
  [i915#79]: https://gitlab.freedesktop.org/drm/intel/issues/79
  [i915#82]: https://gitlab.freedesktop.org/drm/intel/issues/82
  [i915#93]: https://gitlab.freedesktop.org/drm/intel/issues/93
  [i915#95]: https://gitlab.freedesktop.org/drm/intel/issues/95


Participating hosts (11 -> 11)
------------------------------

  No changes in participating hosts


Build changes
-------------

  * Linux: CI_DRM_8602 -> Patchwork_17914

  CI-20190529: 20190529
  CI_DRM_8602: 8b5dcdfb116246ecc8e676961bb1cf82ec75e33f @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5699: 201da47cb57b8fadd9bc45be16b82617b32a2c01 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_17914: e9fc78f4bfec134476a0eed02ee2b2241c4f206b @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17914/index.html
_______________________________________________
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] drm/i915/gt: Incrementally check for rewinding
  2020-06-09 12:28 ` [Intel-gfx] " Chris Wilson
@ 2020-06-09 15:15   ` Chris Wilson
  -1 siblings, 0 replies; 18+ messages in thread
From: Chris Wilson @ 2020-06-09 15:15 UTC (permalink / raw)
  To: intel-gfx; +Cc: Mika Kuoppala, stable

Quoting Chris Wilson (2020-06-09 13:28:56)
> In commit 5ba32c7be81e ("drm/i915/execlists: Always force a context
> reload when rewinding RING_TAIL"), we placed the check for rewinding a
> context on actually submitting the next request in that context. This
> was so that we only had to check once, and could do so with precision
> avoiding as many forced restores as possible. For example, to ensure
> that we can resubmit the same request a couple of times, we include a
> small wa_tail such that on the next submission, the ring->tail will
> appear to move forwards when resubmitting the same request. This is very
> common as it will happen for every lite-restore to fill the second port
> after a context switch.
> 
> However, intel_ring_direction() is limited in precision to movements of
> upto half the ring size. The consequence being that if we tried to
> unwind many requests, we could exceed half the ring and flip the sense
> of the direction, so missing a force restore. As no request can be
> greater than half the ring (i.e. 2048 bytes in the smallest case), we
> can check for rollback incrementally. As we check against the tail that
> would be submitted, we do not lose any sensitivity and allow lite
> restores for the simple case. We still need to double check upon
> submitting the context, to allow for multiple preemptions and
> resubmissions.
> 
> Fixes: 5ba32c7be81e ("drm/i915/execlists: Always force a context reload when rewinding RING_TAIL")
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> Cc: <stable@vger.kernel.org> # v5.4+
> ---
>  drivers/gpu/drm/i915/gt/intel_engine_cs.c     |   4 +-
>  drivers/gpu/drm/i915/gt/intel_lrc.c           |  21 +++-
>  drivers/gpu/drm/i915/gt/intel_ring.c          |   4 +
>  drivers/gpu/drm/i915/gt/selftest_mocs.c       |  18 ++-
>  drivers/gpu/drm/i915/gt/selftest_ring.c       | 110 ++++++++++++++++++
>  .../drm/i915/selftests/i915_mock_selftests.h  |   1 +
>  6 files changed, 154 insertions(+), 4 deletions(-)
>  create mode 100644 drivers/gpu/drm/i915/gt/selftest_ring.c
> 
> diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> index e5141a897786..0a05301e00fb 100644
> --- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> +++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> @@ -646,7 +646,7 @@ static int engine_setup_common(struct intel_engine_cs *engine)
>  struct measure_breadcrumb {
>         struct i915_request rq;
>         struct intel_ring ring;
> -       u32 cs[1024];
> +       u32 cs[2048];
>  };
>  
>  static int measure_breadcrumb_dw(struct intel_context *ce)
> @@ -667,6 +667,8 @@ static int measure_breadcrumb_dw(struct intel_context *ce)
>  
>         frame->ring.vaddr = frame->cs;
>         frame->ring.size = sizeof(frame->cs);
> +       frame->ring.wrap =
> +               BITS_PER_TYPE(frame->ring.size) - ilog2(frame->ring.size);
>         frame->ring.effective_size = frame->ring.size;
>         intel_ring_update_space(&frame->ring);
>         frame->rq.ring = &frame->ring;
> diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c
> index a057f7a2a521..f66274e60bb6 100644
> --- a/drivers/gpu/drm/i915/gt/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
> @@ -1137,6 +1137,13 @@ __unwind_incomplete_requests(struct intel_engine_cs *engine)
>                         list_move(&rq->sched.link, pl);
>                         set_bit(I915_FENCE_FLAG_PQUEUE, &rq->fence.flags);
>  
> +                       /* Check for rollback incrementally */
> +                       if (intel_ring_direction(rq->ring,
> +                                                intel_ring_wrap(rq->ring,
> +                                                                rq->tail),
> +                                                rq->ring->tail) <= 0)
> +                               rq->context->lrc.desc |= CTX_DESC_FORCE_RESTORE;

We could be a bit more cheeky in that the problem only occurs if we
rollback far enough that there is a danger is mistaking the rollback for
a forward update.
-Chris

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

* Re: [Intel-gfx] [PATCH] drm/i915/gt: Incrementally check for rewinding
@ 2020-06-09 15:15   ` Chris Wilson
  0 siblings, 0 replies; 18+ messages in thread
From: Chris Wilson @ 2020-06-09 15:15 UTC (permalink / raw)
  To: intel-gfx; +Cc: stable

Quoting Chris Wilson (2020-06-09 13:28:56)
> In commit 5ba32c7be81e ("drm/i915/execlists: Always force a context
> reload when rewinding RING_TAIL"), we placed the check for rewinding a
> context on actually submitting the next request in that context. This
> was so that we only had to check once, and could do so with precision
> avoiding as many forced restores as possible. For example, to ensure
> that we can resubmit the same request a couple of times, we include a
> small wa_tail such that on the next submission, the ring->tail will
> appear to move forwards when resubmitting the same request. This is very
> common as it will happen for every lite-restore to fill the second port
> after a context switch.
> 
> However, intel_ring_direction() is limited in precision to movements of
> upto half the ring size. The consequence being that if we tried to
> unwind many requests, we could exceed half the ring and flip the sense
> of the direction, so missing a force restore. As no request can be
> greater than half the ring (i.e. 2048 bytes in the smallest case), we
> can check for rollback incrementally. As we check against the tail that
> would be submitted, we do not lose any sensitivity and allow lite
> restores for the simple case. We still need to double check upon
> submitting the context, to allow for multiple preemptions and
> resubmissions.
> 
> Fixes: 5ba32c7be81e ("drm/i915/execlists: Always force a context reload when rewinding RING_TAIL")
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> Cc: <stable@vger.kernel.org> # v5.4+
> ---
>  drivers/gpu/drm/i915/gt/intel_engine_cs.c     |   4 +-
>  drivers/gpu/drm/i915/gt/intel_lrc.c           |  21 +++-
>  drivers/gpu/drm/i915/gt/intel_ring.c          |   4 +
>  drivers/gpu/drm/i915/gt/selftest_mocs.c       |  18 ++-
>  drivers/gpu/drm/i915/gt/selftest_ring.c       | 110 ++++++++++++++++++
>  .../drm/i915/selftests/i915_mock_selftests.h  |   1 +
>  6 files changed, 154 insertions(+), 4 deletions(-)
>  create mode 100644 drivers/gpu/drm/i915/gt/selftest_ring.c
> 
> diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> index e5141a897786..0a05301e00fb 100644
> --- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> +++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> @@ -646,7 +646,7 @@ static int engine_setup_common(struct intel_engine_cs *engine)
>  struct measure_breadcrumb {
>         struct i915_request rq;
>         struct intel_ring ring;
> -       u32 cs[1024];
> +       u32 cs[2048];
>  };
>  
>  static int measure_breadcrumb_dw(struct intel_context *ce)
> @@ -667,6 +667,8 @@ static int measure_breadcrumb_dw(struct intel_context *ce)
>  
>         frame->ring.vaddr = frame->cs;
>         frame->ring.size = sizeof(frame->cs);
> +       frame->ring.wrap =
> +               BITS_PER_TYPE(frame->ring.size) - ilog2(frame->ring.size);
>         frame->ring.effective_size = frame->ring.size;
>         intel_ring_update_space(&frame->ring);
>         frame->rq.ring = &frame->ring;
> diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c
> index a057f7a2a521..f66274e60bb6 100644
> --- a/drivers/gpu/drm/i915/gt/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
> @@ -1137,6 +1137,13 @@ __unwind_incomplete_requests(struct intel_engine_cs *engine)
>                         list_move(&rq->sched.link, pl);
>                         set_bit(I915_FENCE_FLAG_PQUEUE, &rq->fence.flags);
>  
> +                       /* Check for rollback incrementally */
> +                       if (intel_ring_direction(rq->ring,
> +                                                intel_ring_wrap(rq->ring,
> +                                                                rq->tail),
> +                                                rq->ring->tail) <= 0)
> +                               rq->context->lrc.desc |= CTX_DESC_FORCE_RESTORE;

We could be a bit more cheeky in that the problem only occurs if we
rollback far enough that there is a danger is mistaking the rollback for
a forward update.
-Chris
_______________________________________________
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/gt: Incrementally check for rewinding
  2020-06-09 12:28 ` [Intel-gfx] " Chris Wilson
@ 2020-06-09 15:17   ` Chris Wilson
  -1 siblings, 0 replies; 18+ messages in thread
From: Chris Wilson @ 2020-06-09 15:17 UTC (permalink / raw)
  To: intel-gfx; +Cc: Chris Wilson, Mika Kuoppala, stable

In commit 5ba32c7be81e ("drm/i915/execlists: Always force a context
reload when rewinding RING_TAIL"), we placed the check for rewinding a
context on actually submitting the next request in that context. This
was so that we only had to check once, and could do so with precision
avoiding as many forced restores as possible. For example, to ensure
that we can resubmit the same request a couple of times, we include a
small wa_tail such that on the next submission, the ring->tail will
appear to move forwards when resubmitting the same request. This is very
common as it will happen for every lite-restore to fill the second port
after a context switch.

However, intel_ring_direction() is limited in precision to movements of
upto half the ring size. The consequence being that if we tried to
unwind many requests, we could exceed half the ring and flip the sense
of the direction, so missing a force restore. As no request can be
greater than half the ring (i.e. 2048 bytes in the smallest case), we
can check for rollback incrementally. As we check against the tail that
would be submitted, we do not lose any sensitivity and allow lite
restores for the simple case. We still need to double check upon
submitting the context, to allow for multiple preemptions and
resubmissions.

Fixes: 5ba32c7be81e ("drm/i915/execlists: Always force a context reload when rewinding RING_TAIL")
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Cc: <stable@vger.kernel.org> # v5.4+
---
 drivers/gpu/drm/i915/gt/intel_engine_cs.c     |   4 +-
 drivers/gpu/drm/i915/gt/intel_lrc.c           |  21 +++-
 drivers/gpu/drm/i915/gt/intel_ring.c          |   4 +
 drivers/gpu/drm/i915/gt/selftest_mocs.c       |  18 ++-
 drivers/gpu/drm/i915/gt/selftest_ring.c       | 110 ++++++++++++++++++
 .../drm/i915/selftests/i915_mock_selftests.h  |   1 +
 6 files changed, 154 insertions(+), 4 deletions(-)
 create mode 100644 drivers/gpu/drm/i915/gt/selftest_ring.c

diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
index e5141a897786..0a05301e00fb 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
@@ -646,7 +646,7 @@ static int engine_setup_common(struct intel_engine_cs *engine)
 struct measure_breadcrumb {
 	struct i915_request rq;
 	struct intel_ring ring;
-	u32 cs[1024];
+	u32 cs[2048];
 };
 
 static int measure_breadcrumb_dw(struct intel_context *ce)
@@ -667,6 +667,8 @@ static int measure_breadcrumb_dw(struct intel_context *ce)
 
 	frame->ring.vaddr = frame->cs;
 	frame->ring.size = sizeof(frame->cs);
+	frame->ring.wrap =
+		BITS_PER_TYPE(frame->ring.size) - ilog2(frame->ring.size);
 	frame->ring.effective_size = frame->ring.size;
 	intel_ring_update_space(&frame->ring);
 	frame->rq.ring = &frame->ring;
diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c
index a057f7a2a521..5f33342c15e2 100644
--- a/drivers/gpu/drm/i915/gt/intel_lrc.c
+++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
@@ -1137,6 +1137,13 @@ __unwind_incomplete_requests(struct intel_engine_cs *engine)
 			list_move(&rq->sched.link, pl);
 			set_bit(I915_FENCE_FLAG_PQUEUE, &rq->fence.flags);
 
+			/* Check in case rollback so far, we wrap [size/2] */
+			if (intel_ring_direction(rq->ring,
+						 intel_ring_wrap(rq->ring,
+								 rq->tail),
+						 rq->ring->tail) > 0)
+				rq->context->lrc.desc |= CTX_DESC_FORCE_RESTORE;
+
 			active = rq;
 		} else {
 			struct intel_engine_cs *owner = rq->context->engine;
@@ -1505,8 +1512,9 @@ static u64 execlists_update_context(struct i915_request *rq)
 	 * HW has a tendency to ignore us rewinding the TAIL to the end of
 	 * an earlier request.
 	 */
+	GEM_BUG_ON(ce->lrc_reg_state[CTX_RING_TAIL] != rq->ring->tail);
+	prev = rq->ring->tail;
 	tail = intel_ring_set_tail(rq->ring, rq->tail);
-	prev = ce->lrc_reg_state[CTX_RING_TAIL];
 	if (unlikely(intel_ring_direction(rq->ring, tail, prev) <= 0))
 		desc |= CTX_DESC_FORCE_RESTORE;
 	ce->lrc_reg_state[CTX_RING_TAIL] = tail;
@@ -4758,6 +4766,14 @@ static int gen12_emit_flush(struct i915_request *request, u32 mode)
 	return 0;
 }
 
+static void assert_request_valid(struct i915_request *rq)
+{
+	struct intel_ring *ring __maybe_unused = rq->ring;
+
+	/* Can we unwind this request without appearing to go forwards? */
+	GEM_BUG_ON(intel_ring_direction(ring, rq->wa_tail, rq->head) <= 0);
+}
+
 /*
  * Reserve space for 2 NOOPs at the end of each request to be
  * used as a workaround for not being allowed to do lite
@@ -4770,6 +4786,9 @@ static u32 *gen8_emit_wa_tail(struct i915_request *request, u32 *cs)
 	*cs++ = MI_NOOP;
 	request->wa_tail = intel_ring_offset(request, cs);
 
+	/* Check that entire request is less than half the ring */
+	assert_request_valid(request);
+
 	return cs;
 }
 
diff --git a/drivers/gpu/drm/i915/gt/intel_ring.c b/drivers/gpu/drm/i915/gt/intel_ring.c
index 8cda1b7e17ba..bdb324167ef3 100644
--- a/drivers/gpu/drm/i915/gt/intel_ring.c
+++ b/drivers/gpu/drm/i915/gt/intel_ring.c
@@ -315,3 +315,7 @@ int intel_ring_cacheline_align(struct i915_request *rq)
 	GEM_BUG_ON(rq->ring->emit & (CACHELINE_BYTES - 1));
 	return 0;
 }
+
+#if IS_ENABLED(CONFIG_DRM_I915_SELFTEST)
+#include "selftest_ring.c"
+#endif
diff --git a/drivers/gpu/drm/i915/gt/selftest_mocs.c b/drivers/gpu/drm/i915/gt/selftest_mocs.c
index 7bae64018ad9..b25eba50c88e 100644
--- a/drivers/gpu/drm/i915/gt/selftest_mocs.c
+++ b/drivers/gpu/drm/i915/gt/selftest_mocs.c
@@ -18,6 +18,20 @@ struct live_mocs {
 	void *vaddr;
 };
 
+static struct intel_context *mocs_context_create(struct intel_engine_cs *engine)
+{
+	struct intel_context *ce;
+
+	ce = intel_context_create(engine);
+	if (IS_ERR(ce))
+		return ce;
+
+	/* We build large requests to read the registers from the ring */
+	ce->ring = __intel_context_ring_size(SZ_16K);
+
+	return ce;
+}
+
 static int request_add_sync(struct i915_request *rq, int err)
 {
 	i915_request_get(rq);
@@ -301,7 +315,7 @@ static int live_mocs_clean(void *arg)
 	for_each_engine(engine, gt, id) {
 		struct intel_context *ce;
 
-		ce = intel_context_create(engine);
+		ce = mocs_context_create(engine);
 		if (IS_ERR(ce)) {
 			err = PTR_ERR(ce);
 			break;
@@ -395,7 +409,7 @@ static int live_mocs_reset(void *arg)
 	for_each_engine(engine, gt, id) {
 		struct intel_context *ce;
 
-		ce = intel_context_create(engine);
+		ce = mocs_context_create(engine);
 		if (IS_ERR(ce)) {
 			err = PTR_ERR(ce);
 			break;
diff --git a/drivers/gpu/drm/i915/gt/selftest_ring.c b/drivers/gpu/drm/i915/gt/selftest_ring.c
new file mode 100644
index 000000000000..2a8c534dc125
--- /dev/null
+++ b/drivers/gpu/drm/i915/gt/selftest_ring.c
@@ -0,0 +1,110 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright © 2020 Intel Corporation
+ */
+
+static struct intel_ring *mock_ring(unsigned long sz)
+{
+	struct intel_ring *ring;
+
+	ring = kzalloc(sizeof(*ring) + sz, GFP_KERNEL);
+	if (!ring)
+		return NULL;
+
+	kref_init(&ring->ref);
+	ring->size = sz;
+	ring->wrap = BITS_PER_TYPE(ring->size) - ilog2(sz);
+	ring->effective_size = sz;
+	ring->vaddr = (void *)(ring + 1);
+	atomic_set(&ring->pin_count, 1);
+
+	intel_ring_update_space(ring);
+
+	return ring;
+}
+
+static void mock_ring_free(struct intel_ring *ring)
+{
+	kfree(ring);
+}
+
+static int check_ring_direction(struct intel_ring *ring,
+				u32 next, u32 prev,
+				int expected)
+{
+	int result;
+
+	result = intel_ring_direction(ring, next, prev);
+	if (result < 0)
+		result = -1;
+	else if (result > 0)
+		result = 1;
+
+	if (result != expected) {
+		pr_err("intel_ring_direction(%u, %u):%d != %d\n",
+		       next, prev, result, expected);
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static int check_ring_step(struct intel_ring *ring, u32 x, u32 step)
+{
+	u32 prev = x, next = intel_ring_wrap(ring, x + step);
+	int err = 0;
+
+	err |= check_ring_direction(ring, next, next,  0);
+	err |= check_ring_direction(ring, prev, prev,  0);
+	err |= check_ring_direction(ring, next, prev,  1);
+	err |= check_ring_direction(ring, prev, next, -1);
+
+	return err;
+}
+
+static int check_ring_offset(struct intel_ring *ring, u32 x, u32 step)
+{
+	int err = 0;
+
+	err |= check_ring_step(ring, x, step);
+	err |= check_ring_step(ring, intel_ring_wrap(ring, x + 1), step);
+	err |= check_ring_step(ring, intel_ring_wrap(ring, x - 1), step);
+
+	return err;
+}
+
+static int igt_ring_direction(void *dummy)
+{
+	struct intel_ring *ring;
+	unsigned int half = 2048;
+	int step, err = 0;
+
+	ring = mock_ring(2 * half);
+	if (!ring)
+		return -ENOMEM;
+
+	GEM_BUG_ON(ring->size != 2 * half);
+
+	/* Precision of wrap detection is limited to ring->size / 2 */
+	for (step = 1; step < half; step <<= 1) {
+		err |= check_ring_offset(ring, 0, step);
+		err |= check_ring_offset(ring, half, step);
+	}
+	err |= check_ring_step(ring, 0, half - 64);
+
+	/* And check unwrapped handling for good measure */
+	err |= check_ring_offset(ring, 0, 2 * half + 64);
+	err |= check_ring_offset(ring, 3 * half, 1);
+
+	mock_ring_free(ring);
+	return err;
+}
+
+int intel_ring_mock_selftests(void)
+{
+	static const struct i915_subtest tests[] = {
+		SUBTEST(igt_ring_direction),
+	};
+
+	return i915_subtests(tests, NULL);
+}
diff --git a/drivers/gpu/drm/i915/selftests/i915_mock_selftests.h b/drivers/gpu/drm/i915/selftests/i915_mock_selftests.h
index 1929feba4e8e..3db34d3eea58 100644
--- a/drivers/gpu/drm/i915/selftests/i915_mock_selftests.h
+++ b/drivers/gpu/drm/i915/selftests/i915_mock_selftests.h
@@ -21,6 +21,7 @@ selftest(fence, i915_sw_fence_mock_selftests)
 selftest(scatterlist, scatterlist_mock_selftests)
 selftest(syncmap, i915_syncmap_mock_selftests)
 selftest(uncore, intel_uncore_mock_selftests)
+selftest(ring, intel_ring_mock_selftests)
 selftest(engine, intel_engine_cs_mock_selftests)
 selftest(timelines, intel_timeline_mock_selftests)
 selftest(requests, i915_request_mock_selftests)
-- 
2.20.1


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

* [Intel-gfx] [PATCH] drm/i915/gt: Incrementally check for rewinding
@ 2020-06-09 15:17   ` Chris Wilson
  0 siblings, 0 replies; 18+ messages in thread
From: Chris Wilson @ 2020-06-09 15:17 UTC (permalink / raw)
  To: intel-gfx; +Cc: stable, Chris Wilson

In commit 5ba32c7be81e ("drm/i915/execlists: Always force a context
reload when rewinding RING_TAIL"), we placed the check for rewinding a
context on actually submitting the next request in that context. This
was so that we only had to check once, and could do so with precision
avoiding as many forced restores as possible. For example, to ensure
that we can resubmit the same request a couple of times, we include a
small wa_tail such that on the next submission, the ring->tail will
appear to move forwards when resubmitting the same request. This is very
common as it will happen for every lite-restore to fill the second port
after a context switch.

However, intel_ring_direction() is limited in precision to movements of
upto half the ring size. The consequence being that if we tried to
unwind many requests, we could exceed half the ring and flip the sense
of the direction, so missing a force restore. As no request can be
greater than half the ring (i.e. 2048 bytes in the smallest case), we
can check for rollback incrementally. As we check against the tail that
would be submitted, we do not lose any sensitivity and allow lite
restores for the simple case. We still need to double check upon
submitting the context, to allow for multiple preemptions and
resubmissions.

Fixes: 5ba32c7be81e ("drm/i915/execlists: Always force a context reload when rewinding RING_TAIL")
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Cc: <stable@vger.kernel.org> # v5.4+
---
 drivers/gpu/drm/i915/gt/intel_engine_cs.c     |   4 +-
 drivers/gpu/drm/i915/gt/intel_lrc.c           |  21 +++-
 drivers/gpu/drm/i915/gt/intel_ring.c          |   4 +
 drivers/gpu/drm/i915/gt/selftest_mocs.c       |  18 ++-
 drivers/gpu/drm/i915/gt/selftest_ring.c       | 110 ++++++++++++++++++
 .../drm/i915/selftests/i915_mock_selftests.h  |   1 +
 6 files changed, 154 insertions(+), 4 deletions(-)
 create mode 100644 drivers/gpu/drm/i915/gt/selftest_ring.c

diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
index e5141a897786..0a05301e00fb 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
@@ -646,7 +646,7 @@ static int engine_setup_common(struct intel_engine_cs *engine)
 struct measure_breadcrumb {
 	struct i915_request rq;
 	struct intel_ring ring;
-	u32 cs[1024];
+	u32 cs[2048];
 };
 
 static int measure_breadcrumb_dw(struct intel_context *ce)
@@ -667,6 +667,8 @@ static int measure_breadcrumb_dw(struct intel_context *ce)
 
 	frame->ring.vaddr = frame->cs;
 	frame->ring.size = sizeof(frame->cs);
+	frame->ring.wrap =
+		BITS_PER_TYPE(frame->ring.size) - ilog2(frame->ring.size);
 	frame->ring.effective_size = frame->ring.size;
 	intel_ring_update_space(&frame->ring);
 	frame->rq.ring = &frame->ring;
diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c
index a057f7a2a521..5f33342c15e2 100644
--- a/drivers/gpu/drm/i915/gt/intel_lrc.c
+++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
@@ -1137,6 +1137,13 @@ __unwind_incomplete_requests(struct intel_engine_cs *engine)
 			list_move(&rq->sched.link, pl);
 			set_bit(I915_FENCE_FLAG_PQUEUE, &rq->fence.flags);
 
+			/* Check in case rollback so far, we wrap [size/2] */
+			if (intel_ring_direction(rq->ring,
+						 intel_ring_wrap(rq->ring,
+								 rq->tail),
+						 rq->ring->tail) > 0)
+				rq->context->lrc.desc |= CTX_DESC_FORCE_RESTORE;
+
 			active = rq;
 		} else {
 			struct intel_engine_cs *owner = rq->context->engine;
@@ -1505,8 +1512,9 @@ static u64 execlists_update_context(struct i915_request *rq)
 	 * HW has a tendency to ignore us rewinding the TAIL to the end of
 	 * an earlier request.
 	 */
+	GEM_BUG_ON(ce->lrc_reg_state[CTX_RING_TAIL] != rq->ring->tail);
+	prev = rq->ring->tail;
 	tail = intel_ring_set_tail(rq->ring, rq->tail);
-	prev = ce->lrc_reg_state[CTX_RING_TAIL];
 	if (unlikely(intel_ring_direction(rq->ring, tail, prev) <= 0))
 		desc |= CTX_DESC_FORCE_RESTORE;
 	ce->lrc_reg_state[CTX_RING_TAIL] = tail;
@@ -4758,6 +4766,14 @@ static int gen12_emit_flush(struct i915_request *request, u32 mode)
 	return 0;
 }
 
+static void assert_request_valid(struct i915_request *rq)
+{
+	struct intel_ring *ring __maybe_unused = rq->ring;
+
+	/* Can we unwind this request without appearing to go forwards? */
+	GEM_BUG_ON(intel_ring_direction(ring, rq->wa_tail, rq->head) <= 0);
+}
+
 /*
  * Reserve space for 2 NOOPs at the end of each request to be
  * used as a workaround for not being allowed to do lite
@@ -4770,6 +4786,9 @@ static u32 *gen8_emit_wa_tail(struct i915_request *request, u32 *cs)
 	*cs++ = MI_NOOP;
 	request->wa_tail = intel_ring_offset(request, cs);
 
+	/* Check that entire request is less than half the ring */
+	assert_request_valid(request);
+
 	return cs;
 }
 
diff --git a/drivers/gpu/drm/i915/gt/intel_ring.c b/drivers/gpu/drm/i915/gt/intel_ring.c
index 8cda1b7e17ba..bdb324167ef3 100644
--- a/drivers/gpu/drm/i915/gt/intel_ring.c
+++ b/drivers/gpu/drm/i915/gt/intel_ring.c
@@ -315,3 +315,7 @@ int intel_ring_cacheline_align(struct i915_request *rq)
 	GEM_BUG_ON(rq->ring->emit & (CACHELINE_BYTES - 1));
 	return 0;
 }
+
+#if IS_ENABLED(CONFIG_DRM_I915_SELFTEST)
+#include "selftest_ring.c"
+#endif
diff --git a/drivers/gpu/drm/i915/gt/selftest_mocs.c b/drivers/gpu/drm/i915/gt/selftest_mocs.c
index 7bae64018ad9..b25eba50c88e 100644
--- a/drivers/gpu/drm/i915/gt/selftest_mocs.c
+++ b/drivers/gpu/drm/i915/gt/selftest_mocs.c
@@ -18,6 +18,20 @@ struct live_mocs {
 	void *vaddr;
 };
 
+static struct intel_context *mocs_context_create(struct intel_engine_cs *engine)
+{
+	struct intel_context *ce;
+
+	ce = intel_context_create(engine);
+	if (IS_ERR(ce))
+		return ce;
+
+	/* We build large requests to read the registers from the ring */
+	ce->ring = __intel_context_ring_size(SZ_16K);
+
+	return ce;
+}
+
 static int request_add_sync(struct i915_request *rq, int err)
 {
 	i915_request_get(rq);
@@ -301,7 +315,7 @@ static int live_mocs_clean(void *arg)
 	for_each_engine(engine, gt, id) {
 		struct intel_context *ce;
 
-		ce = intel_context_create(engine);
+		ce = mocs_context_create(engine);
 		if (IS_ERR(ce)) {
 			err = PTR_ERR(ce);
 			break;
@@ -395,7 +409,7 @@ static int live_mocs_reset(void *arg)
 	for_each_engine(engine, gt, id) {
 		struct intel_context *ce;
 
-		ce = intel_context_create(engine);
+		ce = mocs_context_create(engine);
 		if (IS_ERR(ce)) {
 			err = PTR_ERR(ce);
 			break;
diff --git a/drivers/gpu/drm/i915/gt/selftest_ring.c b/drivers/gpu/drm/i915/gt/selftest_ring.c
new file mode 100644
index 000000000000..2a8c534dc125
--- /dev/null
+++ b/drivers/gpu/drm/i915/gt/selftest_ring.c
@@ -0,0 +1,110 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright © 2020 Intel Corporation
+ */
+
+static struct intel_ring *mock_ring(unsigned long sz)
+{
+	struct intel_ring *ring;
+
+	ring = kzalloc(sizeof(*ring) + sz, GFP_KERNEL);
+	if (!ring)
+		return NULL;
+
+	kref_init(&ring->ref);
+	ring->size = sz;
+	ring->wrap = BITS_PER_TYPE(ring->size) - ilog2(sz);
+	ring->effective_size = sz;
+	ring->vaddr = (void *)(ring + 1);
+	atomic_set(&ring->pin_count, 1);
+
+	intel_ring_update_space(ring);
+
+	return ring;
+}
+
+static void mock_ring_free(struct intel_ring *ring)
+{
+	kfree(ring);
+}
+
+static int check_ring_direction(struct intel_ring *ring,
+				u32 next, u32 prev,
+				int expected)
+{
+	int result;
+
+	result = intel_ring_direction(ring, next, prev);
+	if (result < 0)
+		result = -1;
+	else if (result > 0)
+		result = 1;
+
+	if (result != expected) {
+		pr_err("intel_ring_direction(%u, %u):%d != %d\n",
+		       next, prev, result, expected);
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static int check_ring_step(struct intel_ring *ring, u32 x, u32 step)
+{
+	u32 prev = x, next = intel_ring_wrap(ring, x + step);
+	int err = 0;
+
+	err |= check_ring_direction(ring, next, next,  0);
+	err |= check_ring_direction(ring, prev, prev,  0);
+	err |= check_ring_direction(ring, next, prev,  1);
+	err |= check_ring_direction(ring, prev, next, -1);
+
+	return err;
+}
+
+static int check_ring_offset(struct intel_ring *ring, u32 x, u32 step)
+{
+	int err = 0;
+
+	err |= check_ring_step(ring, x, step);
+	err |= check_ring_step(ring, intel_ring_wrap(ring, x + 1), step);
+	err |= check_ring_step(ring, intel_ring_wrap(ring, x - 1), step);
+
+	return err;
+}
+
+static int igt_ring_direction(void *dummy)
+{
+	struct intel_ring *ring;
+	unsigned int half = 2048;
+	int step, err = 0;
+
+	ring = mock_ring(2 * half);
+	if (!ring)
+		return -ENOMEM;
+
+	GEM_BUG_ON(ring->size != 2 * half);
+
+	/* Precision of wrap detection is limited to ring->size / 2 */
+	for (step = 1; step < half; step <<= 1) {
+		err |= check_ring_offset(ring, 0, step);
+		err |= check_ring_offset(ring, half, step);
+	}
+	err |= check_ring_step(ring, 0, half - 64);
+
+	/* And check unwrapped handling for good measure */
+	err |= check_ring_offset(ring, 0, 2 * half + 64);
+	err |= check_ring_offset(ring, 3 * half, 1);
+
+	mock_ring_free(ring);
+	return err;
+}
+
+int intel_ring_mock_selftests(void)
+{
+	static const struct i915_subtest tests[] = {
+		SUBTEST(igt_ring_direction),
+	};
+
+	return i915_subtests(tests, NULL);
+}
diff --git a/drivers/gpu/drm/i915/selftests/i915_mock_selftests.h b/drivers/gpu/drm/i915/selftests/i915_mock_selftests.h
index 1929feba4e8e..3db34d3eea58 100644
--- a/drivers/gpu/drm/i915/selftests/i915_mock_selftests.h
+++ b/drivers/gpu/drm/i915/selftests/i915_mock_selftests.h
@@ -21,6 +21,7 @@ selftest(fence, i915_sw_fence_mock_selftests)
 selftest(scatterlist, scatterlist_mock_selftests)
 selftest(syncmap, i915_syncmap_mock_selftests)
 selftest(uncore, intel_uncore_mock_selftests)
+selftest(ring, intel_ring_mock_selftests)
 selftest(engine, intel_engine_cs_mock_selftests)
 selftest(timelines, intel_timeline_mock_selftests)
 selftest(requests, i915_request_mock_selftests)
-- 
2.20.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

* [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915/gt: Incrementally check for rewinding (rev2)
  2020-06-09 12:28 ` [Intel-gfx] " Chris Wilson
                   ` (5 preceding siblings ...)
  (?)
@ 2020-06-09 15:37 ` Patchwork
  -1 siblings, 0 replies; 18+ messages in thread
From: Patchwork @ 2020-06-09 15:37 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx

== Series Details ==

Series: drm/i915/gt: Incrementally check for rewinding (rev2)
URL   : https://patchwork.freedesktop.org/series/78163/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
729184acbb94 drm/i915/gt: Incrementally check for rewinding
-:165: WARNING:FILE_PATH_CHANGES: added, moved or deleted file(s), does MAINTAINERS need updating?
#165: 
new file mode 100644

total: 0 errors, 1 warnings, 0 checks, 222 lines checked

_______________________________________________
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

* [Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915/gt: Incrementally check for rewinding (rev2)
  2020-06-09 12:28 ` [Intel-gfx] " Chris Wilson
                   ` (6 preceding siblings ...)
  (?)
@ 2020-06-09 15:58 ` Patchwork
  -1 siblings, 0 replies; 18+ messages in thread
From: Patchwork @ 2020-06-09 15:58 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx

== Series Details ==

Series: drm/i915/gt: Incrementally check for rewinding (rev2)
URL   : https://patchwork.freedesktop.org/series/78163/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_8603 -> Patchwork_17915
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17915/index.html

Known issues
------------

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

### IGT changes ###

#### Issues hit ####

  * igt@i915_module_load@reload:
    - fi-icl-guc:         [PASS][1] -> [DMESG-WARN][2] ([i915#1982])
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8603/fi-icl-guc/igt@i915_module_load@reload.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17915/fi-icl-guc/igt@i915_module_load@reload.html

  * igt@i915_pm_rpm@basic-pci-d3-state:
    - fi-bsw-kefka:       [PASS][3] -> [DMESG-WARN][4] ([i915#1982])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8603/fi-bsw-kefka/igt@i915_pm_rpm@basic-pci-d3-state.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17915/fi-bsw-kefka/igt@i915_pm_rpm@basic-pci-d3-state.html

  * igt@kms_busy@basic@flip:
    - fi-kbl-x1275:       [PASS][5] -> [DMESG-WARN][6] ([i915#62] / [i915#92] / [i915#95])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8603/fi-kbl-x1275/igt@kms_busy@basic@flip.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17915/fi-kbl-x1275/igt@kms_busy@basic@flip.html

  
#### Possible fixes ####

  * igt@kms_flip@basic-flip-vs-dpms@d-dsi1:
    - {fi-tgl-dsi}:       [DMESG-WARN][7] ([i915#1982]) -> [PASS][8] +1 similar issue
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8603/fi-tgl-dsi/igt@kms_flip@basic-flip-vs-dpms@d-dsi1.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17915/fi-tgl-dsi/igt@kms_flip@basic-flip-vs-dpms@d-dsi1.html

  * igt@kms_flip@basic-flip-vs-wf_vblank@c-edp1:
    - fi-icl-u2:          [DMESG-WARN][9] ([i915#1982]) -> [PASS][10]
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8603/fi-icl-u2/igt@kms_flip@basic-flip-vs-wf_vblank@c-edp1.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17915/fi-icl-u2/igt@kms_flip@basic-flip-vs-wf_vblank@c-edp1.html

  
#### Warnings ####

  * igt@kms_cursor_legacy@basic-busy-flip-before-cursor-legacy:
    - fi-kbl-x1275:       [DMESG-WARN][11] ([i915#62] / [i915#92] / [i915#95]) -> [DMESG-WARN][12] ([i915#62] / [i915#92]) +3 similar issues
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8603/fi-kbl-x1275/igt@kms_cursor_legacy@basic-busy-flip-before-cursor-legacy.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17915/fi-kbl-x1275/igt@kms_cursor_legacy@basic-busy-flip-before-cursor-legacy.html

  * igt@kms_flip@basic-flip-vs-wf_vblank@a-dp1:
    - fi-kbl-x1275:       [DMESG-WARN][13] ([i915#62] / [i915#92]) -> [DMESG-WARN][14] ([i915#62] / [i915#92] / [i915#95]) +3 similar issues
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8603/fi-kbl-x1275/igt@kms_flip@basic-flip-vs-wf_vblank@a-dp1.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17915/fi-kbl-x1275/igt@kms_flip@basic-flip-vs-wf_vblank@a-dp1.html

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [i915#1982]: https://gitlab.freedesktop.org/drm/intel/issues/1982
  [i915#62]: https://gitlab.freedesktop.org/drm/intel/issues/62
  [i915#92]: https://gitlab.freedesktop.org/drm/intel/issues/92
  [i915#95]: https://gitlab.freedesktop.org/drm/intel/issues/95


Participating hosts (49 -> 43)
------------------------------

  Additional (1): fi-kbl-7560u 
  Missing    (7): fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-ctg-p8600 fi-byt-clapper fi-bdw-samus 


Build changes
-------------

  * Linux: CI_DRM_8603 -> Patchwork_17915

  CI-20190529: 20190529
  CI_DRM_8603: 03f5a3d90ccfb2f1bb13e293a83d48a0b7da8af0 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5699: 201da47cb57b8fadd9bc45be16b82617b32a2c01 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_17915: 729184acbb949babd103b9a1ae59de9a6c4918b1 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

729184acbb94 drm/i915/gt: Incrementally check for rewinding

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17915/index.html
_______________________________________________
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

* [Intel-gfx] ✗ Fi.CI.IGT: failure for drm/i915/gt: Incrementally check for rewinding (rev2)
  2020-06-09 12:28 ` [Intel-gfx] " Chris Wilson
                   ` (7 preceding siblings ...)
  (?)
@ 2020-06-09 18:30 ` Patchwork
  -1 siblings, 0 replies; 18+ messages in thread
From: Patchwork @ 2020-06-09 18:30 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx

== Series Details ==

Series: drm/i915/gt: Incrementally check for rewinding (rev2)
URL   : https://patchwork.freedesktop.org/series/78163/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_8603_full -> Patchwork_17915_full
====================================================

Summary
-------

  **FAILURE**

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

  

Possible new issues
-------------------

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

### IGT changes ###

#### Possible regressions ####

  * igt@kms_cursor_crc@pipe-d-cursor-64x64-random:
    - shard-tglb:         [PASS][1] -> [TIMEOUT][2]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8603/shard-tglb5/igt@kms_cursor_crc@pipe-d-cursor-64x64-random.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17915/shard-tglb3/igt@kms_cursor_crc@pipe-d-cursor-64x64-random.html

  * igt@kms_flip@bo-too-big:
    - shard-tglb:         NOTRUN -> [TIMEOUT][3]
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17915/shard-tglb3/igt@kms_flip@bo-too-big.html

  
New tests
---------

  New tests have been introduced between CI_DRM_8603_full and Patchwork_17915_full:

### New IGT tests (1) ###

  * igt@i915_selftest@mock@ring:
    - Statuses : 7 pass(s)
    - Exec time: [0.10, 1.18] s

  

Known issues
------------

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

### IGT changes ###

#### Issues hit ####

  * igt@gem_ctx_persistence@engines-mixed-process@vcs0:
    - shard-glk:          [PASS][4] -> [FAIL][5] ([i915#1528])
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8603/shard-glk4/igt@gem_ctx_persistence@engines-mixed-process@vcs0.html
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17915/shard-glk8/igt@gem_ctx_persistence@engines-mixed-process@vcs0.html

  * igt@gem_exec_balancer@sliced:
    - shard-tglb:         [PASS][6] -> [TIMEOUT][7] ([i915#1936])
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8603/shard-tglb5/igt@gem_exec_balancer@sliced.html
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17915/shard-tglb3/igt@gem_exec_balancer@sliced.html

  * igt@gem_exec_whisper@basic-contexts-forked-all:
    - shard-tglb:         [PASS][8] -> [INCOMPLETE][9] ([i915#750])
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8603/shard-tglb1/igt@gem_exec_whisper@basic-contexts-forked-all.html
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17915/shard-tglb2/igt@gem_exec_whisper@basic-contexts-forked-all.html

  * igt@i915_module_load@reload-with-fault-injection:
    - shard-tglb:         [PASS][10] -> [DMESG-WARN][11] ([i915#402])
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8603/shard-tglb8/igt@i915_module_load@reload-with-fault-injection.html
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17915/shard-tglb2/igt@i915_module_load@reload-with-fault-injection.html

  * igt@i915_pm_rc6_residency@rc6-fence:
    - shard-hsw:          [PASS][12] -> [WARN][13] ([i915#1519])
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8603/shard-hsw7/igt@i915_pm_rc6_residency@rc6-fence.html
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17915/shard-hsw2/igt@i915_pm_rc6_residency@rc6-fence.html

  * igt@i915_suspend@fence-restore-tiled2untiled:
    - shard-apl:          [PASS][14] -> [DMESG-WARN][15] ([i915#180]) +1 similar issue
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8603/shard-apl1/igt@i915_suspend@fence-restore-tiled2untiled.html
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17915/shard-apl1/igt@i915_suspend@fence-restore-tiled2untiled.html

  * igt@kms_addfb_basic@bad-pitch-999:
    - shard-apl:          [PASS][16] -> [DMESG-WARN][17] ([i915#95]) +18 similar issues
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8603/shard-apl2/igt@kms_addfb_basic@bad-pitch-999.html
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17915/shard-apl7/igt@kms_addfb_basic@bad-pitch-999.html

  * igt@kms_big_fb@y-tiled-64bpp-rotate-180:
    - shard-glk:          [PASS][18] -> [DMESG-FAIL][19] ([i915#118] / [i915#95])
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8603/shard-glk4/igt@kms_big_fb@y-tiled-64bpp-rotate-180.html
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17915/shard-glk8/igt@kms_big_fb@y-tiled-64bpp-rotate-180.html

  * igt@kms_color@pipe-c-ctm-0-25:
    - shard-skl:          [PASS][20] -> [DMESG-WARN][21] ([i915#1982]) +4 similar issues
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8603/shard-skl9/igt@kms_color@pipe-c-ctm-0-25.html
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17915/shard-skl5/igt@kms_color@pipe-c-ctm-0-25.html

  * igt@kms_color@pipe-c-ctm-blue-to-red:
    - shard-kbl:          [PASS][22] -> [DMESG-WARN][23] ([i915#93] / [i915#95]) +1 similar issue
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8603/shard-kbl6/igt@kms_color@pipe-c-ctm-blue-to-red.html
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17915/shard-kbl6/igt@kms_color@pipe-c-ctm-blue-to-red.html

  * igt@kms_flip@2x-modeset-vs-vblank-race-interruptible@ab-vga1-hdmi-a1:
    - shard-hsw:          [PASS][24] -> [DMESG-WARN][25] ([i915#1982])
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8603/shard-hsw4/igt@kms_flip@2x-modeset-vs-vblank-race-interruptible@ab-vga1-hdmi-a1.html
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17915/shard-hsw8/igt@kms_flip@2x-modeset-vs-vblank-race-interruptible@ab-vga1-hdmi-a1.html

  * igt@kms_flip@flip-vs-suspend@c-dp1:
    - shard-kbl:          [PASS][26] -> [DMESG-WARN][27] ([i915#180]) +3 similar issues
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8603/shard-kbl6/igt@kms_flip@flip-vs-suspend@c-dp1.html
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17915/shard-kbl7/igt@kms_flip@flip-vs-suspend@c-dp1.html

  * igt@kms_flip@wf_vblank-ts-check-interruptible@a-dp1:
    - shard-kbl:          [PASS][28] -> [DMESG-WARN][29] ([i915#1982])
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8603/shard-kbl7/igt@kms_flip@wf_vblank-ts-check-interruptible@a-dp1.html
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17915/shard-kbl6/igt@kms_flip@wf_vblank-ts-check-interruptible@a-dp1.html

  * igt@kms_plane_alpha_blend@pipe-b-coverage-7efc:
    - shard-skl:          [PASS][30] -> [FAIL][31] ([fdo#108145] / [i915#265])
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8603/shard-skl7/igt@kms_plane_alpha_blend@pipe-b-coverage-7efc.html
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17915/shard-skl2/igt@kms_plane_alpha_blend@pipe-b-coverage-7efc.html

  * igt@kms_plane_scaling@pipe-b-scaler-with-clipping-clamping:
    - shard-iclb:         [PASS][32] -> [DMESG-WARN][33] ([i915#1982])
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8603/shard-iclb6/igt@kms_plane_scaling@pipe-b-scaler-with-clipping-clamping.html
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17915/shard-iclb3/igt@kms_plane_scaling@pipe-b-scaler-with-clipping-clamping.html

  * igt@kms_psr@psr2_basic:
    - shard-iclb:         [PASS][34] -> [SKIP][35] ([fdo#109441])
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8603/shard-iclb2/igt@kms_psr@psr2_basic.html
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17915/shard-iclb4/igt@kms_psr@psr2_basic.html

  * igt@perf@polling-parameterized:
    - shard-hsw:          [PASS][36] -> [FAIL][37] ([i915#1542])
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8603/shard-hsw1/igt@perf@polling-parameterized.html
   [37]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17915/shard-hsw6/igt@perf@polling-parameterized.html

  
#### Possible fixes ####

  * igt@gem_ctx_persistence@processes:
    - shard-skl:          [FAIL][38] ([i915#1528]) -> [PASS][39]
   [38]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8603/shard-skl1/igt@gem_ctx_persistence@processes.html
   [39]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17915/shard-skl3/igt@gem_ctx_persistence@processes.html

  * igt@gem_flink_basic@double-flink:
    - shard-kbl:          [DMESG-WARN][40] ([i915#93] / [i915#95]) -> [PASS][41]
   [40]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8603/shard-kbl2/igt@gem_flink_basic@double-flink.html
   [41]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17915/shard-kbl3/igt@gem_flink_basic@double-flink.html

  * igt@gem_sync@basic-store-each:
    - shard-apl:          [DMESG-WARN][42] ([i915#95]) -> [PASS][43] +18 similar issues
   [42]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8603/shard-apl7/igt@gem_sync@basic-store-each.html
   [43]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17915/shard-apl2/igt@gem_sync@basic-store-each.html

  * igt@gem_wait@await@vecs0:
    - shard-hsw:          [INCOMPLETE][44] ([i915#61]) -> [PASS][45]
   [44]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8603/shard-hsw4/igt@gem_wait@await@vecs0.html
   [45]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17915/shard-hsw4/igt@gem_wait@await@vecs0.html

  * igt@i915_module_load@reload:
    - shard-tglb:         [DMESG-WARN][46] ([i915#402]) -> [PASS][47]
   [46]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8603/shard-tglb5/igt@i915_module_load@reload.html
   [47]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17915/shard-tglb3/igt@i915_module_load@reload.html

  * igt@kms_atomic_transition@plane-use-after-nonblocking-unbind@pipe-a:
    - shard-apl:          [DMESG-WARN][48] ([i915#1982]) -> [PASS][49]
   [48]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8603/shard-apl2/igt@kms_atomic_transition@plane-use-after-nonblocking-unbind@pipe-a.html
   [49]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17915/shard-apl7/igt@kms_atomic_transition@plane-use-after-nonblocking-unbind@pipe-a.html

  * igt@kms_color@pipe-a-ctm-0-75:
    - shard-skl:          [DMESG-WARN][50] ([i915#1982]) -> [PASS][51] +3 similar issues
   [50]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8603/shard-skl9/igt@kms_color@pipe-a-ctm-0-75.html
   [51]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17915/shard-skl9/igt@kms_color@pipe-a-ctm-0-75.html

  * igt@kms_cursor_legacy@flip-vs-cursor-atomic:
    - shard-skl:          [FAIL][52] ([IGT#5]) -> [PASS][53]
   [52]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8603/shard-skl9/igt@kms_cursor_legacy@flip-vs-cursor-atomic.html
   [53]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17915/shard-skl10/igt@kms_cursor_legacy@flip-vs-cursor-atomic.html

  * igt@kms_flip@flip-vs-suspend@a-dp1:
    - shard-kbl:          [DMESG-WARN][54] ([i915#180]) -> [PASS][55] +5 similar issues
   [54]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8603/shard-kbl6/igt@kms_flip@flip-vs-suspend@a-dp1.html
   [55]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17915/shard-kbl7/igt@kms_flip@flip-vs-suspend@a-dp1.html

  * igt@kms_flip@flip-vs-suspend@b-dp1:
    - shard-apl:          [DMESG-WARN][56] ([i915#180]) -> [PASS][57] +1 similar issue
   [56]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8603/shard-apl8/igt@kms_flip@flip-vs-suspend@b-dp1.html
   [57]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17915/shard-apl2/igt@kms_flip@flip-vs-suspend@b-dp1.html

  * igt@kms_flip@plain-flip-ts-check-interruptible@b-edp1:
    - shard-skl:          [FAIL][58] ([i915#1928]) -> [PASS][59]
   [58]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8603/shard-skl9/igt@kms_flip@plain-flip-ts-check-interruptible@b-edp1.html
   [59]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17915/shard-skl5/igt@kms_flip@plain-flip-ts-check-interruptible@b-edp1.html

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-draw-blt:
    - shard-snb:          [SKIP][60] ([fdo#109271]) -> [PASS][61] +3 similar issues
   [60]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8603/shard-snb2/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-draw-blt.html
   [61]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17915/shard-snb2/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-draw-blt.html

  * igt@kms_hdr@bpc-switch-suspend:
    - shard-skl:          [FAIL][62] ([i915#1188]) -> [PASS][63]
   [62]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8603/shard-skl2/igt@kms_hdr@bpc-switch-suspend.html
   [63]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17915/shard-skl6/igt@kms_hdr@bpc-switch-suspend.html

  * igt@kms_psr@psr2_dpms:
    - shard-iclb:         [SKIP][64] ([fdo#109441]) -> [PASS][65]
   [64]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8603/shard-iclb7/igt@kms_psr@psr2_dpms.html
   [65]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17915/shard-iclb2/igt@kms_psr@psr2_dpms.html

  
#### Warnings ####

  * igt@kms_color@pipe-a-degamma:
    - shard-tglb:         [DMESG-FAIL][66] ([i915#1149] / [i915#402]) -> [FAIL][67] ([i915#1149] / [i915#1897])
   [66]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8603/shard-tglb3/igt@kms_color@pipe-a-degamma.html
   [67]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17915/shard-tglb8/igt@kms_color@pipe-a-degamma.html

  * igt@kms_content_protection@atomic:
    - shard-kbl:          [TIMEOUT][68] ([i915#1319]) -> [DMESG-FAIL][69] ([fdo#110321] / [i915#95])
   [68]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8603/shard-kbl3/igt@kms_content_protection@atomic.html
   [69]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17915/shard-kbl2/igt@kms_content_protection@atomic.html

  * igt@kms_content_protection@atomic-dpms:
    - shard-apl:          [TIMEOUT][70] ([i915#1319]) -> [TIMEOUT][71] ([i915#1319] / [i915#1635])
   [70]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8603/shard-apl2/igt@kms_content_protection@atomic-dpms.html
   [71]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17915/shard-apl6/igt@kms_content_protection@atomic-dpms.html

  * igt@kms_content_protection@lic:
    - shard-apl:          [TIMEOUT][72] ([i915#1319] / [i915#1635]) -> [TIMEOUT][73] ([i915#1319])
   [72]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8603/shard-apl4/igt@kms_content_protection@lic.html
   [73]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17915/shard-apl7/igt@kms_content_protection@lic.html
    - shard-kbl:          [TIMEOUT][74] ([i915#1319]) -> [TIMEOUT][75] ([i915#1319] / [i915#1958])
   [74]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8603/shard-kbl1/igt@kms_content_protection@lic.html
   [75]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17915/shard-kbl4/igt@kms_content_protection@lic.html

  * igt@kms_content_protection@srm:
    - shard-apl:          [TIMEOUT][76] ([i915#1319] / [i915#1635]) -> [FAIL][77] ([fdo#110321])
   [76]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8603/shard-apl7/igt@kms_content_protection@srm.html
   [77]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17915/shard-apl2/igt@kms_content_protection@srm.html

  
  [IGT#5]: https://gitlab.freedesktop.org/drm/igt-gpu-tools/issues/5
  [fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109441]: https://bugs.freedesktop.org/show_bug.cgi?id=109441
  [fdo#110321]: https://bugs.freedesktop.org/show_bug.cgi?id=110321
  [i915#1149]: https://gitlab.freedesktop.org/drm/intel/issues/1149
  [i915#118]: https://gitlab.freedesktop.org/drm/intel/issues/118
  [i915#1188]: https://gitlab.freedesktop.org/drm/intel/issues/1188
  [i915#1319]: https://gitlab.freedesktop.org/drm/intel/issues/1319
  [i915#1519]: https://gitlab.freedesktop.org/drm/intel/issues/1519
  [i915#1528]: https://gitlab.freedesktop.org/drm/intel/issues/1528
  [i915#1542]: https://gitlab.freedesktop.org/drm/intel/issues/1542
  [i915#1635]: https://gitlab.freedesktop.org/drm/intel/issues/1635
  [i915#180]: https://gitlab.freedesktop.org/drm/intel/issues/180
  [i915#1897]: https://gitlab.freedesktop.org/drm/intel/issues/1897
  [i915#1928]: https://gitlab.freedesktop.org/drm/intel/issues/1928
  [i915#1936]: https://gitlab.freedesktop.org/drm/intel/issues/1936
  [i915#1958]: https://gitlab.freedesktop.org/drm/intel/issues/1958
  [i915#1982]: https://gitlab.freedesktop.org/drm/intel/issues/1982
  [i915#265]: https://gitlab.freedesktop.org/drm/intel/issues/265
  [i915#402]: https://gitlab.freedesktop.org/drm/intel/issues/402
  [i915#61]: https://gitlab.freedesktop.org/drm/intel/issues/61
  [i915#750]: https://gitlab.freedesktop.org/drm/intel/issues/750
  [i915#93]: https://gitlab.freedesktop.org/drm/intel/issues/93
  [i915#95]: https://gitlab.freedesktop.org/drm/intel/issues/95


Participating hosts (11 -> 11)
------------------------------

  No changes in participating hosts


Build changes
-------------

  * Linux: CI_DRM_8603 -> Patchwork_17915

  CI-20190529: 20190529
  CI_DRM_8603: 03f5a3d90ccfb2f1bb13e293a83d48a0b7da8af0 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5699: 201da47cb57b8fadd9bc45be16b82617b32a2c01 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_17915: 729184acbb949babd103b9a1ae59de9a6c4918b1 @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17915/index.html
_______________________________________________
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: [Intel-gfx] [PATCH] drm/i915/gt: Incrementally check for rewinding
  2020-06-09 15:17   ` [Intel-gfx] " Chris Wilson
@ 2020-06-10  4:25     ` Chang, Bruce
  -1 siblings, 0 replies; 18+ messages in thread
From: Chang, Bruce @ 2020-06-10  4:25 UTC (permalink / raw)
  To: Chris Wilson, intel-gfx; +Cc: stable

On 6/9/2020 8:17 AM, Chris Wilson wrote:
> In commit 5ba32c7be81e ("drm/i915/execlists: Always force a context
> reload when rewinding RING_TAIL"), we placed the check for rewinding a
> context on actually submitting the next request in that context. This
> was so that we only had to check once, and could do so with precision
> avoiding as many forced restores as possible. For example, to ensure
> that we can resubmit the same request a couple of times, we include a
> small wa_tail such that on the next submission, the ring->tail will
> appear to move forwards when resubmitting the same request. This is very
> common as it will happen for every lite-restore to fill the second port
> after a context switch.
>
> However, intel_ring_direction() is limited in precision to movements of
> upto half the ring size. The consequence being that if we tried to
> unwind many requests, we could exceed half the ring and flip the sense
> of the direction, so missing a force restore. As no request can be
> greater than half the ring (i.e. 2048 bytes in the smallest case), we
> can check for rollback incrementally. As we check against the tail that
> would be submitted, we do not lose any sensitivity and allow lite
> restores for the simple case. We still need to double check upon
> submitting the context, to allow for multiple preemptions and
> resubmissions.
>
> Fixes: 5ba32c7be81e ("drm/i915/execlists: Always force a context reload when rewinding RING_TAIL")
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> Cc: <stable@vger.kernel.org> # v5.4+

Verified this has fixed the issue regarding the GPU hang with incomplete 
error state.

reviewed by: Bruce Chang <yu.bruce.chang@intel.com>

> ---
>   drivers/gpu/drm/i915/gt/intel_engine_cs.c     |   4 +-
>   drivers/gpu/drm/i915/gt/intel_lrc.c           |  21 +++-
>   drivers/gpu/drm/i915/gt/intel_ring.c          |   4 +
>   drivers/gpu/drm/i915/gt/selftest_mocs.c       |  18 ++-
>   drivers/gpu/drm/i915/gt/selftest_ring.c       | 110 ++++++++++++++++++
>   .../drm/i915/selftests/i915_mock_selftests.h  |   1 +
>   6 files changed, 154 insertions(+), 4 deletions(-)
>   create mode 100644 drivers/gpu/drm/i915/gt/selftest_ring.c
>
> diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> index e5141a897786..0a05301e00fb 100644
> --- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> +++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> @@ -646,7 +646,7 @@ static int engine_setup_common(struct intel_engine_cs *engine)
>   struct measure_breadcrumb {
>   	struct i915_request rq;
>   	struct intel_ring ring;
> -	u32 cs[1024];
> +	u32 cs[2048];
>   };
>   
>   static int measure_breadcrumb_dw(struct intel_context *ce)
> @@ -667,6 +667,8 @@ static int measure_breadcrumb_dw(struct intel_context *ce)
>   
>   	frame->ring.vaddr = frame->cs;
>   	frame->ring.size = sizeof(frame->cs);
> +	frame->ring.wrap =
> +		BITS_PER_TYPE(frame->ring.size) - ilog2(frame->ring.size);


Not sure if this  frame->ring.wrap being used anywhere


>   	frame->ring.effective_size = frame->ring.size;
>   	intel_ring_update_space(&frame->ring);
>   	frame->rq.ring = &frame->ring;
> diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c
> index a057f7a2a521..5f33342c15e2 100644
> --- a/drivers/gpu/drm/i915/gt/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
> @@ -1137,6 +1137,13 @@ __unwind_incomplete_requests(struct intel_engine_cs *engine)
>   			list_move(&rq->sched.link, pl);
>   			set_bit(I915_FENCE_FLAG_PQUEUE, &rq->fence.flags);
>   
> +			/* Check in case rollback so far, we wrap [size/2] */
> +			if (intel_ring_direction(rq->ring,
> +						 intel_ring_wrap(rq->ring,
> +								 rq->tail),
> +						 rq->ring->tail) > 0)
> +				rq->context->lrc.desc |= CTX_DESC_FORCE_RESTORE;
> +


minor: maybe just me, "so far" -> "too far"?


>   			active = rq;
>   		} else {
>   			struct intel_engine_cs *owner = rq->context->engine;
> @@ -1505,8 +1512,9 @@ static u64 execlists_update_context(struct i915_request *rq)
>   	 * HW has a tendency to ignore us rewinding the TAIL to the end of
>   	 * an earlier request.
>   	 */
> +	GEM_BUG_ON(ce->lrc_reg_state[CTX_RING_TAIL] != rq->ring->tail);
> +	prev = rq->ring->tail;
>   	tail = intel_ring_set_tail(rq->ring, rq->tail);
> -	prev = ce->lrc_reg_state[CTX_RING_TAIL];
>   	if (unlikely(intel_ring_direction(rq->ring, tail, prev) <= 0))
>   		desc |= CTX_DESC_FORCE_RESTORE;
>   	ce->lrc_reg_state[CTX_RING_TAIL] = tail;
> @@ -4758,6 +4766,14 @@ static int gen12_emit_flush(struct i915_request *request, u32 mode)
>   	return 0;
>   }
>   
> +static void assert_request_valid(struct i915_request *rq)
> +{
> +	struct intel_ring *ring __maybe_unused = rq->ring;
> +
> +	/* Can we unwind this request without appearing to go forwards? */
> +	GEM_BUG_ON(intel_ring_direction(ring, rq->wa_tail, rq->head) <= 0);
> +}
> +
>   /*
>    * Reserve space for 2 NOOPs at the end of each request to be
>    * used as a workaround for not being allowed to do lite
> @@ -4770,6 +4786,9 @@ static u32 *gen8_emit_wa_tail(struct i915_request *request, u32 *cs)
>   	*cs++ = MI_NOOP;
>   	request->wa_tail = intel_ring_offset(request, cs);
>   
> +	/* Check that entire request is less than half the ring */
> +	assert_request_valid(request);
> +
>   	return cs;
>   }
>   
> diff --git a/drivers/gpu/drm/i915/gt/intel_ring.c b/drivers/gpu/drm/i915/gt/intel_ring.c
> index 8cda1b7e17ba..bdb324167ef3 100644
> --- a/drivers/gpu/drm/i915/gt/intel_ring.c
> +++ b/drivers/gpu/drm/i915/gt/intel_ring.c
> @@ -315,3 +315,7 @@ int intel_ring_cacheline_align(struct i915_request *rq)
>   	GEM_BUG_ON(rq->ring->emit & (CACHELINE_BYTES - 1));
>   	return 0;
>   }
> +
> +#if IS_ENABLED(CONFIG_DRM_I915_SELFTEST)
> +#include "selftest_ring.c"
> +#endif
> diff --git a/drivers/gpu/drm/i915/gt/selftest_mocs.c b/drivers/gpu/drm/i915/gt/selftest_mocs.c
> index 7bae64018ad9..b25eba50c88e 100644
> --- a/drivers/gpu/drm/i915/gt/selftest_mocs.c
> +++ b/drivers/gpu/drm/i915/gt/selftest_mocs.c
> @@ -18,6 +18,20 @@ struct live_mocs {
>   	void *vaddr;
>   };
>   
> +static struct intel_context *mocs_context_create(struct intel_engine_cs *engine)
> +{
> +	struct intel_context *ce;
> +
> +	ce = intel_context_create(engine);
> +	if (IS_ERR(ce))
> +		return ce;
> +
> +	/* We build large requests to read the registers from the ring */
> +	ce->ring = __intel_context_ring_size(SZ_16K);
> +
> +	return ce;
> +}
> +
>   static int request_add_sync(struct i915_request *rq, int err)
>   {
>   	i915_request_get(rq);
> @@ -301,7 +315,7 @@ static int live_mocs_clean(void *arg)
>   	for_each_engine(engine, gt, id) {
>   		struct intel_context *ce;
>   
> -		ce = intel_context_create(engine);
> +		ce = mocs_context_create(engine);
>   		if (IS_ERR(ce)) {
>   			err = PTR_ERR(ce);
>   			break;
> @@ -395,7 +409,7 @@ static int live_mocs_reset(void *arg)
>   	for_each_engine(engine, gt, id) {
>   		struct intel_context *ce;
>   
> -		ce = intel_context_create(engine);
> +		ce = mocs_context_create(engine);
>   		if (IS_ERR(ce)) {
>   			err = PTR_ERR(ce);
>   			break;
> diff --git a/drivers/gpu/drm/i915/gt/selftest_ring.c b/drivers/gpu/drm/i915/gt/selftest_ring.c
> new file mode 100644
> index 000000000000..2a8c534dc125
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/gt/selftest_ring.c
> @@ -0,0 +1,110 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright © 2020 Intel Corporation
> + */
> +
> +static struct intel_ring *mock_ring(unsigned long sz)
> +{
> +	struct intel_ring *ring;
> +
> +	ring = kzalloc(sizeof(*ring) + sz, GFP_KERNEL);
> +	if (!ring)
> +		return NULL;
> +
> +	kref_init(&ring->ref);
> +	ring->size = sz;
> +	ring->wrap = BITS_PER_TYPE(ring->size) - ilog2(sz);
> +	ring->effective_size = sz;
> +	ring->vaddr = (void *)(ring + 1);
> +	atomic_set(&ring->pin_count, 1);
> +
> +	intel_ring_update_space(ring);
> +
> +	return ring;
> +}
> +
> +static void mock_ring_free(struct intel_ring *ring)
> +{
> +	kfree(ring);
> +}
> +
> +static int check_ring_direction(struct intel_ring *ring,
> +				u32 next, u32 prev,
> +				int expected)
> +{
> +	int result;
> +
> +	result = intel_ring_direction(ring, next, prev);
> +	if (result < 0)
> +		result = -1;
> +	else if (result > 0)
> +		result = 1;
> +
> +	if (result != expected) {
> +		pr_err("intel_ring_direction(%u, %u):%d != %d\n",
> +		       next, prev, result, expected);
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
> +static int check_ring_step(struct intel_ring *ring, u32 x, u32 step)
> +{
> +	u32 prev = x, next = intel_ring_wrap(ring, x + step);
> +	int err = 0;
> +
> +	err |= check_ring_direction(ring, next, next,  0);
> +	err |= check_ring_direction(ring, prev, prev,  0);
> +	err |= check_ring_direction(ring, next, prev,  1);
> +	err |= check_ring_direction(ring, prev, next, -1);
> +
> +	return err;
> +}
> +
> +static int check_ring_offset(struct intel_ring *ring, u32 x, u32 step)
> +{
> +	int err = 0;
> +
> +	err |= check_ring_step(ring, x, step);
> +	err |= check_ring_step(ring, intel_ring_wrap(ring, x + 1), step);
> +	err |= check_ring_step(ring, intel_ring_wrap(ring, x - 1), step);
> +
> +	return err;
> +}
> +
> +static int igt_ring_direction(void *dummy)
> +{
> +	struct intel_ring *ring;
> +	unsigned int half = 2048;
> +	int step, err = 0;
> +
> +	ring = mock_ring(2 * half);
> +	if (!ring)
> +		return -ENOMEM;
> +
> +	GEM_BUG_ON(ring->size != 2 * half);
> +
> +	/* Precision of wrap detection is limited to ring->size / 2 */
> +	for (step = 1; step < half; step <<= 1) {
> +		err |= check_ring_offset(ring, 0, step);
> +		err |= check_ring_offset(ring, half, step);
> +	}
> +	err |= check_ring_step(ring, 0, half - 64);
> +
> +	/* And check unwrapped handling for good measure */
> +	err |= check_ring_offset(ring, 0, 2 * half + 64);
> +	err |= check_ring_offset(ring, 3 * half, 1);
> +
> +	mock_ring_free(ring);
> +	return err;
> +}
> +
> +int intel_ring_mock_selftests(void)
> +{
> +	static const struct i915_subtest tests[] = {
> +		SUBTEST(igt_ring_direction),
> +	};
> +
> +	return i915_subtests(tests, NULL);
> +}
> diff --git a/drivers/gpu/drm/i915/selftests/i915_mock_selftests.h b/drivers/gpu/drm/i915/selftests/i915_mock_selftests.h
> index 1929feba4e8e..3db34d3eea58 100644
> --- a/drivers/gpu/drm/i915/selftests/i915_mock_selftests.h
> +++ b/drivers/gpu/drm/i915/selftests/i915_mock_selftests.h
> @@ -21,6 +21,7 @@ selftest(fence, i915_sw_fence_mock_selftests)
>   selftest(scatterlist, scatterlist_mock_selftests)
>   selftest(syncmap, i915_syncmap_mock_selftests)
>   selftest(uncore, intel_uncore_mock_selftests)
> +selftest(ring, intel_ring_mock_selftests)
>   selftest(engine, intel_engine_cs_mock_selftests)
>   selftest(timelines, intel_timeline_mock_selftests)
>   selftest(requests, i915_request_mock_selftests)

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

* Re: [Intel-gfx] [PATCH] drm/i915/gt: Incrementally check for rewinding
@ 2020-06-10  4:25     ` Chang, Bruce
  0 siblings, 0 replies; 18+ messages in thread
From: Chang, Bruce @ 2020-06-10  4:25 UTC (permalink / raw)
  To: Chris Wilson, intel-gfx; +Cc: stable

On 6/9/2020 8:17 AM, Chris Wilson wrote:
> In commit 5ba32c7be81e ("drm/i915/execlists: Always force a context
> reload when rewinding RING_TAIL"), we placed the check for rewinding a
> context on actually submitting the next request in that context. This
> was so that we only had to check once, and could do so with precision
> avoiding as many forced restores as possible. For example, to ensure
> that we can resubmit the same request a couple of times, we include a
> small wa_tail such that on the next submission, the ring->tail will
> appear to move forwards when resubmitting the same request. This is very
> common as it will happen for every lite-restore to fill the second port
> after a context switch.
>
> However, intel_ring_direction() is limited in precision to movements of
> upto half the ring size. The consequence being that if we tried to
> unwind many requests, we could exceed half the ring and flip the sense
> of the direction, so missing a force restore. As no request can be
> greater than half the ring (i.e. 2048 bytes in the smallest case), we
> can check for rollback incrementally. As we check against the tail that
> would be submitted, we do not lose any sensitivity and allow lite
> restores for the simple case. We still need to double check upon
> submitting the context, to allow for multiple preemptions and
> resubmissions.
>
> Fixes: 5ba32c7be81e ("drm/i915/execlists: Always force a context reload when rewinding RING_TAIL")
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> Cc: <stable@vger.kernel.org> # v5.4+

Verified this has fixed the issue regarding the GPU hang with incomplete 
error state.

reviewed by: Bruce Chang <yu.bruce.chang@intel.com>

> ---
>   drivers/gpu/drm/i915/gt/intel_engine_cs.c     |   4 +-
>   drivers/gpu/drm/i915/gt/intel_lrc.c           |  21 +++-
>   drivers/gpu/drm/i915/gt/intel_ring.c          |   4 +
>   drivers/gpu/drm/i915/gt/selftest_mocs.c       |  18 ++-
>   drivers/gpu/drm/i915/gt/selftest_ring.c       | 110 ++++++++++++++++++
>   .../drm/i915/selftests/i915_mock_selftests.h  |   1 +
>   6 files changed, 154 insertions(+), 4 deletions(-)
>   create mode 100644 drivers/gpu/drm/i915/gt/selftest_ring.c
>
> diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> index e5141a897786..0a05301e00fb 100644
> --- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> +++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> @@ -646,7 +646,7 @@ static int engine_setup_common(struct intel_engine_cs *engine)
>   struct measure_breadcrumb {
>   	struct i915_request rq;
>   	struct intel_ring ring;
> -	u32 cs[1024];
> +	u32 cs[2048];
>   };
>   
>   static int measure_breadcrumb_dw(struct intel_context *ce)
> @@ -667,6 +667,8 @@ static int measure_breadcrumb_dw(struct intel_context *ce)
>   
>   	frame->ring.vaddr = frame->cs;
>   	frame->ring.size = sizeof(frame->cs);
> +	frame->ring.wrap =
> +		BITS_PER_TYPE(frame->ring.size) - ilog2(frame->ring.size);


Not sure if this  frame->ring.wrap being used anywhere


>   	frame->ring.effective_size = frame->ring.size;
>   	intel_ring_update_space(&frame->ring);
>   	frame->rq.ring = &frame->ring;
> diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c
> index a057f7a2a521..5f33342c15e2 100644
> --- a/drivers/gpu/drm/i915/gt/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
> @@ -1137,6 +1137,13 @@ __unwind_incomplete_requests(struct intel_engine_cs *engine)
>   			list_move(&rq->sched.link, pl);
>   			set_bit(I915_FENCE_FLAG_PQUEUE, &rq->fence.flags);
>   
> +			/* Check in case rollback so far, we wrap [size/2] */
> +			if (intel_ring_direction(rq->ring,
> +						 intel_ring_wrap(rq->ring,
> +								 rq->tail),
> +						 rq->ring->tail) > 0)
> +				rq->context->lrc.desc |= CTX_DESC_FORCE_RESTORE;
> +


minor: maybe just me, "so far" -> "too far"?


>   			active = rq;
>   		} else {
>   			struct intel_engine_cs *owner = rq->context->engine;
> @@ -1505,8 +1512,9 @@ static u64 execlists_update_context(struct i915_request *rq)
>   	 * HW has a tendency to ignore us rewinding the TAIL to the end of
>   	 * an earlier request.
>   	 */
> +	GEM_BUG_ON(ce->lrc_reg_state[CTX_RING_TAIL] != rq->ring->tail);
> +	prev = rq->ring->tail;
>   	tail = intel_ring_set_tail(rq->ring, rq->tail);
> -	prev = ce->lrc_reg_state[CTX_RING_TAIL];
>   	if (unlikely(intel_ring_direction(rq->ring, tail, prev) <= 0))
>   		desc |= CTX_DESC_FORCE_RESTORE;
>   	ce->lrc_reg_state[CTX_RING_TAIL] = tail;
> @@ -4758,6 +4766,14 @@ static int gen12_emit_flush(struct i915_request *request, u32 mode)
>   	return 0;
>   }
>   
> +static void assert_request_valid(struct i915_request *rq)
> +{
> +	struct intel_ring *ring __maybe_unused = rq->ring;
> +
> +	/* Can we unwind this request without appearing to go forwards? */
> +	GEM_BUG_ON(intel_ring_direction(ring, rq->wa_tail, rq->head) <= 0);
> +}
> +
>   /*
>    * Reserve space for 2 NOOPs at the end of each request to be
>    * used as a workaround for not being allowed to do lite
> @@ -4770,6 +4786,9 @@ static u32 *gen8_emit_wa_tail(struct i915_request *request, u32 *cs)
>   	*cs++ = MI_NOOP;
>   	request->wa_tail = intel_ring_offset(request, cs);
>   
> +	/* Check that entire request is less than half the ring */
> +	assert_request_valid(request);
> +
>   	return cs;
>   }
>   
> diff --git a/drivers/gpu/drm/i915/gt/intel_ring.c b/drivers/gpu/drm/i915/gt/intel_ring.c
> index 8cda1b7e17ba..bdb324167ef3 100644
> --- a/drivers/gpu/drm/i915/gt/intel_ring.c
> +++ b/drivers/gpu/drm/i915/gt/intel_ring.c
> @@ -315,3 +315,7 @@ int intel_ring_cacheline_align(struct i915_request *rq)
>   	GEM_BUG_ON(rq->ring->emit & (CACHELINE_BYTES - 1));
>   	return 0;
>   }
> +
> +#if IS_ENABLED(CONFIG_DRM_I915_SELFTEST)
> +#include "selftest_ring.c"
> +#endif
> diff --git a/drivers/gpu/drm/i915/gt/selftest_mocs.c b/drivers/gpu/drm/i915/gt/selftest_mocs.c
> index 7bae64018ad9..b25eba50c88e 100644
> --- a/drivers/gpu/drm/i915/gt/selftest_mocs.c
> +++ b/drivers/gpu/drm/i915/gt/selftest_mocs.c
> @@ -18,6 +18,20 @@ struct live_mocs {
>   	void *vaddr;
>   };
>   
> +static struct intel_context *mocs_context_create(struct intel_engine_cs *engine)
> +{
> +	struct intel_context *ce;
> +
> +	ce = intel_context_create(engine);
> +	if (IS_ERR(ce))
> +		return ce;
> +
> +	/* We build large requests to read the registers from the ring */
> +	ce->ring = __intel_context_ring_size(SZ_16K);
> +
> +	return ce;
> +}
> +
>   static int request_add_sync(struct i915_request *rq, int err)
>   {
>   	i915_request_get(rq);
> @@ -301,7 +315,7 @@ static int live_mocs_clean(void *arg)
>   	for_each_engine(engine, gt, id) {
>   		struct intel_context *ce;
>   
> -		ce = intel_context_create(engine);
> +		ce = mocs_context_create(engine);
>   		if (IS_ERR(ce)) {
>   			err = PTR_ERR(ce);
>   			break;
> @@ -395,7 +409,7 @@ static int live_mocs_reset(void *arg)
>   	for_each_engine(engine, gt, id) {
>   		struct intel_context *ce;
>   
> -		ce = intel_context_create(engine);
> +		ce = mocs_context_create(engine);
>   		if (IS_ERR(ce)) {
>   			err = PTR_ERR(ce);
>   			break;
> diff --git a/drivers/gpu/drm/i915/gt/selftest_ring.c b/drivers/gpu/drm/i915/gt/selftest_ring.c
> new file mode 100644
> index 000000000000..2a8c534dc125
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/gt/selftest_ring.c
> @@ -0,0 +1,110 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright © 2020 Intel Corporation
> + */
> +
> +static struct intel_ring *mock_ring(unsigned long sz)
> +{
> +	struct intel_ring *ring;
> +
> +	ring = kzalloc(sizeof(*ring) + sz, GFP_KERNEL);
> +	if (!ring)
> +		return NULL;
> +
> +	kref_init(&ring->ref);
> +	ring->size = sz;
> +	ring->wrap = BITS_PER_TYPE(ring->size) - ilog2(sz);
> +	ring->effective_size = sz;
> +	ring->vaddr = (void *)(ring + 1);
> +	atomic_set(&ring->pin_count, 1);
> +
> +	intel_ring_update_space(ring);
> +
> +	return ring;
> +}
> +
> +static void mock_ring_free(struct intel_ring *ring)
> +{
> +	kfree(ring);
> +}
> +
> +static int check_ring_direction(struct intel_ring *ring,
> +				u32 next, u32 prev,
> +				int expected)
> +{
> +	int result;
> +
> +	result = intel_ring_direction(ring, next, prev);
> +	if (result < 0)
> +		result = -1;
> +	else if (result > 0)
> +		result = 1;
> +
> +	if (result != expected) {
> +		pr_err("intel_ring_direction(%u, %u):%d != %d\n",
> +		       next, prev, result, expected);
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
> +static int check_ring_step(struct intel_ring *ring, u32 x, u32 step)
> +{
> +	u32 prev = x, next = intel_ring_wrap(ring, x + step);
> +	int err = 0;
> +
> +	err |= check_ring_direction(ring, next, next,  0);
> +	err |= check_ring_direction(ring, prev, prev,  0);
> +	err |= check_ring_direction(ring, next, prev,  1);
> +	err |= check_ring_direction(ring, prev, next, -1);
> +
> +	return err;
> +}
> +
> +static int check_ring_offset(struct intel_ring *ring, u32 x, u32 step)
> +{
> +	int err = 0;
> +
> +	err |= check_ring_step(ring, x, step);
> +	err |= check_ring_step(ring, intel_ring_wrap(ring, x + 1), step);
> +	err |= check_ring_step(ring, intel_ring_wrap(ring, x - 1), step);
> +
> +	return err;
> +}
> +
> +static int igt_ring_direction(void *dummy)
> +{
> +	struct intel_ring *ring;
> +	unsigned int half = 2048;
> +	int step, err = 0;
> +
> +	ring = mock_ring(2 * half);
> +	if (!ring)
> +		return -ENOMEM;
> +
> +	GEM_BUG_ON(ring->size != 2 * half);
> +
> +	/* Precision of wrap detection is limited to ring->size / 2 */
> +	for (step = 1; step < half; step <<= 1) {
> +		err |= check_ring_offset(ring, 0, step);
> +		err |= check_ring_offset(ring, half, step);
> +	}
> +	err |= check_ring_step(ring, 0, half - 64);
> +
> +	/* And check unwrapped handling for good measure */
> +	err |= check_ring_offset(ring, 0, 2 * half + 64);
> +	err |= check_ring_offset(ring, 3 * half, 1);
> +
> +	mock_ring_free(ring);
> +	return err;
> +}
> +
> +int intel_ring_mock_selftests(void)
> +{
> +	static const struct i915_subtest tests[] = {
> +		SUBTEST(igt_ring_direction),
> +	};
> +
> +	return i915_subtests(tests, NULL);
> +}
> diff --git a/drivers/gpu/drm/i915/selftests/i915_mock_selftests.h b/drivers/gpu/drm/i915/selftests/i915_mock_selftests.h
> index 1929feba4e8e..3db34d3eea58 100644
> --- a/drivers/gpu/drm/i915/selftests/i915_mock_selftests.h
> +++ b/drivers/gpu/drm/i915/selftests/i915_mock_selftests.h
> @@ -21,6 +21,7 @@ selftest(fence, i915_sw_fence_mock_selftests)
>   selftest(scatterlist, scatterlist_mock_selftests)
>   selftest(syncmap, i915_syncmap_mock_selftests)
>   selftest(uncore, intel_uncore_mock_selftests)
> +selftest(ring, intel_ring_mock_selftests)
>   selftest(engine, intel_engine_cs_mock_selftests)
>   selftest(timelines, intel_timeline_mock_selftests)
>   selftest(requests, i915_request_mock_selftests)
_______________________________________________
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] drm/i915/gt: Incrementally check for rewinding
  2020-06-09 15:17   ` [Intel-gfx] " Chris Wilson
@ 2020-06-10 12:39     ` Mika Kuoppala
  -1 siblings, 0 replies; 18+ messages in thread
From: Mika Kuoppala @ 2020-06-10 12:39 UTC (permalink / raw)
  To: Chris Wilson, intel-gfx; +Cc: Chris Wilson, stable

Chris Wilson <chris@chris-wilson.co.uk> writes:

> In commit 5ba32c7be81e ("drm/i915/execlists: Always force a context
> reload when rewinding RING_TAIL"), we placed the check for rewinding a
> context on actually submitting the next request in that context. This
> was so that we only had to check once, and could do so with precision
> avoiding as many forced restores as possible. For example, to ensure
> that we can resubmit the same request a couple of times, we include a
> small wa_tail such that on the next submission, the ring->tail will
> appear to move forwards when resubmitting the same request. This is very
> common as it will happen for every lite-restore to fill the second port
> after a context switch.
>
> However, intel_ring_direction() is limited in precision to movements of
> upto half the ring size. The consequence being that if we tried to
> unwind many requests, we could exceed half the ring and flip the sense
> of the direction, so missing a force restore. As no request can be
> greater than half the ring (i.e. 2048 bytes in the smallest case), we
> can check for rollback incrementally. As we check against the tail that
> would be submitted, we do not lose any sensitivity and allow lite
> restores for the simple case. We still need to double check upon
> submitting the context, to allow for multiple preemptions and
> resubmissions.
>
> Fixes: 5ba32c7be81e ("drm/i915/execlists: Always force a context reload when rewinding RING_TAIL")
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> Cc: <stable@vger.kernel.org> # v5.4+
> ---
>  drivers/gpu/drm/i915/gt/intel_engine_cs.c     |   4 +-
>  drivers/gpu/drm/i915/gt/intel_lrc.c           |  21 +++-
>  drivers/gpu/drm/i915/gt/intel_ring.c          |   4 +
>  drivers/gpu/drm/i915/gt/selftest_mocs.c       |  18 ++-
>  drivers/gpu/drm/i915/gt/selftest_ring.c       | 110 ++++++++++++++++++
>  .../drm/i915/selftests/i915_mock_selftests.h  |   1 +
>  6 files changed, 154 insertions(+), 4 deletions(-)
>  create mode 100644 drivers/gpu/drm/i915/gt/selftest_ring.c
>
> diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> index e5141a897786..0a05301e00fb 100644
> --- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> +++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> @@ -646,7 +646,7 @@ static int engine_setup_common(struct intel_engine_cs *engine)
>  struct measure_breadcrumb {
>  	struct i915_request rq;
>  	struct intel_ring ring;
> -	u32 cs[1024];
> +	u32 cs[2048];
>  };
>  
>  static int measure_breadcrumb_dw(struct intel_context *ce)
> @@ -667,6 +667,8 @@ static int measure_breadcrumb_dw(struct intel_context *ce)
>  
>  	frame->ring.vaddr = frame->cs;
>  	frame->ring.size = sizeof(frame->cs);
> +	frame->ring.wrap =
> +		BITS_PER_TYPE(frame->ring.size) - ilog2(frame->ring.size);
>  	frame->ring.effective_size = frame->ring.size;
>  	intel_ring_update_space(&frame->ring);
>  	frame->rq.ring = &frame->ring;
> diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c
> index a057f7a2a521..5f33342c15e2 100644
> --- a/drivers/gpu/drm/i915/gt/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
> @@ -1137,6 +1137,13 @@ __unwind_incomplete_requests(struct intel_engine_cs *engine)
>  			list_move(&rq->sched.link, pl);
>  			set_bit(I915_FENCE_FLAG_PQUEUE, &rq->fence.flags);
>  
> +			/* Check in case rollback so far, we wrap [size/2] */

This could be ammended a little as why it is not always the case that
on the rewind the direction is not positive.

> +			if (intel_ring_direction(rq->ring,
> +						 intel_ring_wrap(rq->ring,
> +								 rq->tail),
> +						 rq->ring->tail) > 0)
> +				rq->context->lrc.desc |= CTX_DESC_FORCE_RESTORE;
> +
>  			active = rq;
>  		} else {
>  			struct intel_engine_cs *owner = rq->context->engine;
> @@ -1505,8 +1512,9 @@ static u64 execlists_update_context(struct i915_request *rq)
>  	 * HW has a tendency to ignore us rewinding the TAIL to the end of
>  	 * an earlier request.
>  	 */
> +	GEM_BUG_ON(ce->lrc_reg_state[CTX_RING_TAIL] != rq->ring->tail);
> +	prev = rq->ring->tail;
>  	tail = intel_ring_set_tail(rq->ring, rq->tail);
> -	prev = ce->lrc_reg_state[CTX_RING_TAIL];
>  	if (unlikely(intel_ring_direction(rq->ring, tail, prev) <= 0))
>  		desc |= CTX_DESC_FORCE_RESTORE;
>  	ce->lrc_reg_state[CTX_RING_TAIL] = tail;
> @@ -4758,6 +4766,14 @@ static int gen12_emit_flush(struct i915_request *request, u32 mode)
>  	return 0;
>  }
>  
> +static void assert_request_valid(struct i915_request *rq)
> +{
> +	struct intel_ring *ring __maybe_unused = rq->ring;
> +
> +	/* Can we unwind this request without appearing to go forwards? */
> +	GEM_BUG_ON(intel_ring_direction(ring, rq->wa_tail, rq->head) <= 0);

Chris explained in irc that as the wa_tail is reserved for next
resubmit, the nondirection is also possible.

> +}
> +
>  /*
>   * Reserve space for 2 NOOPs at the end of each request to be
>   * used as a workaround for not being allowed to do lite
> @@ -4770,6 +4786,9 @@ static u32 *gen8_emit_wa_tail(struct i915_request *request, u32 *cs)
>  	*cs++ = MI_NOOP;
>  	request->wa_tail = intel_ring_offset(request, cs);
>  
> +	/* Check that entire request is less than half the ring */
> +	assert_request_valid(request);

I was thinking about adding the check in the advance part but
that is too early. And also the tail validation is too early.

This is so tricky with the wrap handling. But it is easier to
stand behind the broad shoulders of the really appreciated
selftests.

Reviewed-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>

> +
>  	return cs;
>  }
>  
> diff --git a/drivers/gpu/drm/i915/gt/intel_ring.c b/drivers/gpu/drm/i915/gt/intel_ring.c
> index 8cda1b7e17ba..bdb324167ef3 100644
> --- a/drivers/gpu/drm/i915/gt/intel_ring.c
> +++ b/drivers/gpu/drm/i915/gt/intel_ring.c
> @@ -315,3 +315,7 @@ int intel_ring_cacheline_align(struct i915_request *rq)
>  	GEM_BUG_ON(rq->ring->emit & (CACHELINE_BYTES - 1));
>  	return 0;
>  }
> +
> +#if IS_ENABLED(CONFIG_DRM_I915_SELFTEST)
> +#include "selftest_ring.c"
> +#endif
> diff --git a/drivers/gpu/drm/i915/gt/selftest_mocs.c b/drivers/gpu/drm/i915/gt/selftest_mocs.c
> index 7bae64018ad9..b25eba50c88e 100644
> --- a/drivers/gpu/drm/i915/gt/selftest_mocs.c
> +++ b/drivers/gpu/drm/i915/gt/selftest_mocs.c
> @@ -18,6 +18,20 @@ struct live_mocs {
>  	void *vaddr;
>  };
>  
> +static struct intel_context *mocs_context_create(struct intel_engine_cs *engine)
> +{
> +	struct intel_context *ce;
> +
> +	ce = intel_context_create(engine);
> +	if (IS_ERR(ce))
> +		return ce;
> +
> +	/* We build large requests to read the registers from the ring */
> +	ce->ring = __intel_context_ring_size(SZ_16K);
> +
> +	return ce;
> +}
> +
>  static int request_add_sync(struct i915_request *rq, int err)
>  {
>  	i915_request_get(rq);
> @@ -301,7 +315,7 @@ static int live_mocs_clean(void *arg)
>  	for_each_engine(engine, gt, id) {
>  		struct intel_context *ce;
>  
> -		ce = intel_context_create(engine);
> +		ce = mocs_context_create(engine);
>  		if (IS_ERR(ce)) {
>  			err = PTR_ERR(ce);
>  			break;
> @@ -395,7 +409,7 @@ static int live_mocs_reset(void *arg)
>  	for_each_engine(engine, gt, id) {
>  		struct intel_context *ce;
>  
> -		ce = intel_context_create(engine);
> +		ce = mocs_context_create(engine);
>  		if (IS_ERR(ce)) {
>  			err = PTR_ERR(ce);
>  			break;
> diff --git a/drivers/gpu/drm/i915/gt/selftest_ring.c b/drivers/gpu/drm/i915/gt/selftest_ring.c
> new file mode 100644
> index 000000000000..2a8c534dc125
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/gt/selftest_ring.c
> @@ -0,0 +1,110 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright © 2020 Intel Corporation
> + */
> +
> +static struct intel_ring *mock_ring(unsigned long sz)
> +{
> +	struct intel_ring *ring;
> +
> +	ring = kzalloc(sizeof(*ring) + sz, GFP_KERNEL);
> +	if (!ring)
> +		return NULL;
> +
> +	kref_init(&ring->ref);
> +	ring->size = sz;
> +	ring->wrap = BITS_PER_TYPE(ring->size) - ilog2(sz);
> +	ring->effective_size = sz;
> +	ring->vaddr = (void *)(ring + 1);
> +	atomic_set(&ring->pin_count, 1);
> +
> +	intel_ring_update_space(ring);
> +
> +	return ring;
> +}
> +
> +static void mock_ring_free(struct intel_ring *ring)
> +{
> +	kfree(ring);
> +}
> +
> +static int check_ring_direction(struct intel_ring *ring,
> +				u32 next, u32 prev,
> +				int expected)
> +{
> +	int result;
> +
> +	result = intel_ring_direction(ring, next, prev);
> +	if (result < 0)
> +		result = -1;
> +	else if (result > 0)
> +		result = 1;
> +
> +	if (result != expected) {
> +		pr_err("intel_ring_direction(%u, %u):%d != %d\n",
> +		       next, prev, result, expected);
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
> +static int check_ring_step(struct intel_ring *ring, u32 x, u32 step)
> +{
> +	u32 prev = x, next = intel_ring_wrap(ring, x + step);
> +	int err = 0;
> +
> +	err |= check_ring_direction(ring, next, next,  0);
> +	err |= check_ring_direction(ring, prev, prev,  0);
> +	err |= check_ring_direction(ring, next, prev,  1);
> +	err |= check_ring_direction(ring, prev, next, -1);
> +
> +	return err;
> +}
> +
> +static int check_ring_offset(struct intel_ring *ring, u32 x, u32 step)
> +{
> +	int err = 0;
> +
> +	err |= check_ring_step(ring, x, step);
> +	err |= check_ring_step(ring, intel_ring_wrap(ring, x + 1), step);
> +	err |= check_ring_step(ring, intel_ring_wrap(ring, x - 1), step);
> +
> +	return err;
> +}
> +
> +static int igt_ring_direction(void *dummy)
> +{
> +	struct intel_ring *ring;
> +	unsigned int half = 2048;
> +	int step, err = 0;
> +
> +	ring = mock_ring(2 * half);
> +	if (!ring)
> +		return -ENOMEM;
> +
> +	GEM_BUG_ON(ring->size != 2 * half);
> +
> +	/* Precision of wrap detection is limited to ring->size / 2 */
> +	for (step = 1; step < half; step <<= 1) {
> +		err |= check_ring_offset(ring, 0, step);
> +		err |= check_ring_offset(ring, half, step);
> +	}
> +	err |= check_ring_step(ring, 0, half - 64);
> +
> +	/* And check unwrapped handling for good measure */
> +	err |= check_ring_offset(ring, 0, 2 * half + 64);
> +	err |= check_ring_offset(ring, 3 * half, 1);
> +
> +	mock_ring_free(ring);
> +	return err;
> +}
> +
> +int intel_ring_mock_selftests(void)
> +{
> +	static const struct i915_subtest tests[] = {
> +		SUBTEST(igt_ring_direction),
> +	};
> +
> +	return i915_subtests(tests, NULL);
> +}
> diff --git a/drivers/gpu/drm/i915/selftests/i915_mock_selftests.h b/drivers/gpu/drm/i915/selftests/i915_mock_selftests.h
> index 1929feba4e8e..3db34d3eea58 100644
> --- a/drivers/gpu/drm/i915/selftests/i915_mock_selftests.h
> +++ b/drivers/gpu/drm/i915/selftests/i915_mock_selftests.h
> @@ -21,6 +21,7 @@ selftest(fence, i915_sw_fence_mock_selftests)
>  selftest(scatterlist, scatterlist_mock_selftests)
>  selftest(syncmap, i915_syncmap_mock_selftests)
>  selftest(uncore, intel_uncore_mock_selftests)
> +selftest(ring, intel_ring_mock_selftests)
>  selftest(engine, intel_engine_cs_mock_selftests)
>  selftest(timelines, intel_timeline_mock_selftests)
>  selftest(requests, i915_request_mock_selftests)
> -- 
> 2.20.1

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

* Re: [Intel-gfx] [PATCH] drm/i915/gt: Incrementally check for rewinding
@ 2020-06-10 12:39     ` Mika Kuoppala
  0 siblings, 0 replies; 18+ messages in thread
From: Mika Kuoppala @ 2020-06-10 12:39 UTC (permalink / raw)
  To: Chris Wilson, intel-gfx; +Cc: stable, Chris Wilson

Chris Wilson <chris@chris-wilson.co.uk> writes:

> In commit 5ba32c7be81e ("drm/i915/execlists: Always force a context
> reload when rewinding RING_TAIL"), we placed the check for rewinding a
> context on actually submitting the next request in that context. This
> was so that we only had to check once, and could do so with precision
> avoiding as many forced restores as possible. For example, to ensure
> that we can resubmit the same request a couple of times, we include a
> small wa_tail such that on the next submission, the ring->tail will
> appear to move forwards when resubmitting the same request. This is very
> common as it will happen for every lite-restore to fill the second port
> after a context switch.
>
> However, intel_ring_direction() is limited in precision to movements of
> upto half the ring size. The consequence being that if we tried to
> unwind many requests, we could exceed half the ring and flip the sense
> of the direction, so missing a force restore. As no request can be
> greater than half the ring (i.e. 2048 bytes in the smallest case), we
> can check for rollback incrementally. As we check against the tail that
> would be submitted, we do not lose any sensitivity and allow lite
> restores for the simple case. We still need to double check upon
> submitting the context, to allow for multiple preemptions and
> resubmissions.
>
> Fixes: 5ba32c7be81e ("drm/i915/execlists: Always force a context reload when rewinding RING_TAIL")
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> Cc: <stable@vger.kernel.org> # v5.4+
> ---
>  drivers/gpu/drm/i915/gt/intel_engine_cs.c     |   4 +-
>  drivers/gpu/drm/i915/gt/intel_lrc.c           |  21 +++-
>  drivers/gpu/drm/i915/gt/intel_ring.c          |   4 +
>  drivers/gpu/drm/i915/gt/selftest_mocs.c       |  18 ++-
>  drivers/gpu/drm/i915/gt/selftest_ring.c       | 110 ++++++++++++++++++
>  .../drm/i915/selftests/i915_mock_selftests.h  |   1 +
>  6 files changed, 154 insertions(+), 4 deletions(-)
>  create mode 100644 drivers/gpu/drm/i915/gt/selftest_ring.c
>
> diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> index e5141a897786..0a05301e00fb 100644
> --- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> +++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> @@ -646,7 +646,7 @@ static int engine_setup_common(struct intel_engine_cs *engine)
>  struct measure_breadcrumb {
>  	struct i915_request rq;
>  	struct intel_ring ring;
> -	u32 cs[1024];
> +	u32 cs[2048];
>  };
>  
>  static int measure_breadcrumb_dw(struct intel_context *ce)
> @@ -667,6 +667,8 @@ static int measure_breadcrumb_dw(struct intel_context *ce)
>  
>  	frame->ring.vaddr = frame->cs;
>  	frame->ring.size = sizeof(frame->cs);
> +	frame->ring.wrap =
> +		BITS_PER_TYPE(frame->ring.size) - ilog2(frame->ring.size);
>  	frame->ring.effective_size = frame->ring.size;
>  	intel_ring_update_space(&frame->ring);
>  	frame->rq.ring = &frame->ring;
> diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c
> index a057f7a2a521..5f33342c15e2 100644
> --- a/drivers/gpu/drm/i915/gt/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
> @@ -1137,6 +1137,13 @@ __unwind_incomplete_requests(struct intel_engine_cs *engine)
>  			list_move(&rq->sched.link, pl);
>  			set_bit(I915_FENCE_FLAG_PQUEUE, &rq->fence.flags);
>  
> +			/* Check in case rollback so far, we wrap [size/2] */

This could be ammended a little as why it is not always the case that
on the rewind the direction is not positive.

> +			if (intel_ring_direction(rq->ring,
> +						 intel_ring_wrap(rq->ring,
> +								 rq->tail),
> +						 rq->ring->tail) > 0)
> +				rq->context->lrc.desc |= CTX_DESC_FORCE_RESTORE;
> +
>  			active = rq;
>  		} else {
>  			struct intel_engine_cs *owner = rq->context->engine;
> @@ -1505,8 +1512,9 @@ static u64 execlists_update_context(struct i915_request *rq)
>  	 * HW has a tendency to ignore us rewinding the TAIL to the end of
>  	 * an earlier request.
>  	 */
> +	GEM_BUG_ON(ce->lrc_reg_state[CTX_RING_TAIL] != rq->ring->tail);
> +	prev = rq->ring->tail;
>  	tail = intel_ring_set_tail(rq->ring, rq->tail);
> -	prev = ce->lrc_reg_state[CTX_RING_TAIL];
>  	if (unlikely(intel_ring_direction(rq->ring, tail, prev) <= 0))
>  		desc |= CTX_DESC_FORCE_RESTORE;
>  	ce->lrc_reg_state[CTX_RING_TAIL] = tail;
> @@ -4758,6 +4766,14 @@ static int gen12_emit_flush(struct i915_request *request, u32 mode)
>  	return 0;
>  }
>  
> +static void assert_request_valid(struct i915_request *rq)
> +{
> +	struct intel_ring *ring __maybe_unused = rq->ring;
> +
> +	/* Can we unwind this request without appearing to go forwards? */
> +	GEM_BUG_ON(intel_ring_direction(ring, rq->wa_tail, rq->head) <= 0);

Chris explained in irc that as the wa_tail is reserved for next
resubmit, the nondirection is also possible.

> +}
> +
>  /*
>   * Reserve space for 2 NOOPs at the end of each request to be
>   * used as a workaround for not being allowed to do lite
> @@ -4770,6 +4786,9 @@ static u32 *gen8_emit_wa_tail(struct i915_request *request, u32 *cs)
>  	*cs++ = MI_NOOP;
>  	request->wa_tail = intel_ring_offset(request, cs);
>  
> +	/* Check that entire request is less than half the ring */
> +	assert_request_valid(request);

I was thinking about adding the check in the advance part but
that is too early. And also the tail validation is too early.

This is so tricky with the wrap handling. But it is easier to
stand behind the broad shoulders of the really appreciated
selftests.

Reviewed-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>

> +
>  	return cs;
>  }
>  
> diff --git a/drivers/gpu/drm/i915/gt/intel_ring.c b/drivers/gpu/drm/i915/gt/intel_ring.c
> index 8cda1b7e17ba..bdb324167ef3 100644
> --- a/drivers/gpu/drm/i915/gt/intel_ring.c
> +++ b/drivers/gpu/drm/i915/gt/intel_ring.c
> @@ -315,3 +315,7 @@ int intel_ring_cacheline_align(struct i915_request *rq)
>  	GEM_BUG_ON(rq->ring->emit & (CACHELINE_BYTES - 1));
>  	return 0;
>  }
> +
> +#if IS_ENABLED(CONFIG_DRM_I915_SELFTEST)
> +#include "selftest_ring.c"
> +#endif
> diff --git a/drivers/gpu/drm/i915/gt/selftest_mocs.c b/drivers/gpu/drm/i915/gt/selftest_mocs.c
> index 7bae64018ad9..b25eba50c88e 100644
> --- a/drivers/gpu/drm/i915/gt/selftest_mocs.c
> +++ b/drivers/gpu/drm/i915/gt/selftest_mocs.c
> @@ -18,6 +18,20 @@ struct live_mocs {
>  	void *vaddr;
>  };
>  
> +static struct intel_context *mocs_context_create(struct intel_engine_cs *engine)
> +{
> +	struct intel_context *ce;
> +
> +	ce = intel_context_create(engine);
> +	if (IS_ERR(ce))
> +		return ce;
> +
> +	/* We build large requests to read the registers from the ring */
> +	ce->ring = __intel_context_ring_size(SZ_16K);
> +
> +	return ce;
> +}
> +
>  static int request_add_sync(struct i915_request *rq, int err)
>  {
>  	i915_request_get(rq);
> @@ -301,7 +315,7 @@ static int live_mocs_clean(void *arg)
>  	for_each_engine(engine, gt, id) {
>  		struct intel_context *ce;
>  
> -		ce = intel_context_create(engine);
> +		ce = mocs_context_create(engine);
>  		if (IS_ERR(ce)) {
>  			err = PTR_ERR(ce);
>  			break;
> @@ -395,7 +409,7 @@ static int live_mocs_reset(void *arg)
>  	for_each_engine(engine, gt, id) {
>  		struct intel_context *ce;
>  
> -		ce = intel_context_create(engine);
> +		ce = mocs_context_create(engine);
>  		if (IS_ERR(ce)) {
>  			err = PTR_ERR(ce);
>  			break;
> diff --git a/drivers/gpu/drm/i915/gt/selftest_ring.c b/drivers/gpu/drm/i915/gt/selftest_ring.c
> new file mode 100644
> index 000000000000..2a8c534dc125
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/gt/selftest_ring.c
> @@ -0,0 +1,110 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright © 2020 Intel Corporation
> + */
> +
> +static struct intel_ring *mock_ring(unsigned long sz)
> +{
> +	struct intel_ring *ring;
> +
> +	ring = kzalloc(sizeof(*ring) + sz, GFP_KERNEL);
> +	if (!ring)
> +		return NULL;
> +
> +	kref_init(&ring->ref);
> +	ring->size = sz;
> +	ring->wrap = BITS_PER_TYPE(ring->size) - ilog2(sz);
> +	ring->effective_size = sz;
> +	ring->vaddr = (void *)(ring + 1);
> +	atomic_set(&ring->pin_count, 1);
> +
> +	intel_ring_update_space(ring);
> +
> +	return ring;
> +}
> +
> +static void mock_ring_free(struct intel_ring *ring)
> +{
> +	kfree(ring);
> +}
> +
> +static int check_ring_direction(struct intel_ring *ring,
> +				u32 next, u32 prev,
> +				int expected)
> +{
> +	int result;
> +
> +	result = intel_ring_direction(ring, next, prev);
> +	if (result < 0)
> +		result = -1;
> +	else if (result > 0)
> +		result = 1;
> +
> +	if (result != expected) {
> +		pr_err("intel_ring_direction(%u, %u):%d != %d\n",
> +		       next, prev, result, expected);
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
> +static int check_ring_step(struct intel_ring *ring, u32 x, u32 step)
> +{
> +	u32 prev = x, next = intel_ring_wrap(ring, x + step);
> +	int err = 0;
> +
> +	err |= check_ring_direction(ring, next, next,  0);
> +	err |= check_ring_direction(ring, prev, prev,  0);
> +	err |= check_ring_direction(ring, next, prev,  1);
> +	err |= check_ring_direction(ring, prev, next, -1);
> +
> +	return err;
> +}
> +
> +static int check_ring_offset(struct intel_ring *ring, u32 x, u32 step)
> +{
> +	int err = 0;
> +
> +	err |= check_ring_step(ring, x, step);
> +	err |= check_ring_step(ring, intel_ring_wrap(ring, x + 1), step);
> +	err |= check_ring_step(ring, intel_ring_wrap(ring, x - 1), step);
> +
> +	return err;
> +}
> +
> +static int igt_ring_direction(void *dummy)
> +{
> +	struct intel_ring *ring;
> +	unsigned int half = 2048;
> +	int step, err = 0;
> +
> +	ring = mock_ring(2 * half);
> +	if (!ring)
> +		return -ENOMEM;
> +
> +	GEM_BUG_ON(ring->size != 2 * half);
> +
> +	/* Precision of wrap detection is limited to ring->size / 2 */
> +	for (step = 1; step < half; step <<= 1) {
> +		err |= check_ring_offset(ring, 0, step);
> +		err |= check_ring_offset(ring, half, step);
> +	}
> +	err |= check_ring_step(ring, 0, half - 64);
> +
> +	/* And check unwrapped handling for good measure */
> +	err |= check_ring_offset(ring, 0, 2 * half + 64);
> +	err |= check_ring_offset(ring, 3 * half, 1);
> +
> +	mock_ring_free(ring);
> +	return err;
> +}
> +
> +int intel_ring_mock_selftests(void)
> +{
> +	static const struct i915_subtest tests[] = {
> +		SUBTEST(igt_ring_direction),
> +	};
> +
> +	return i915_subtests(tests, NULL);
> +}
> diff --git a/drivers/gpu/drm/i915/selftests/i915_mock_selftests.h b/drivers/gpu/drm/i915/selftests/i915_mock_selftests.h
> index 1929feba4e8e..3db34d3eea58 100644
> --- a/drivers/gpu/drm/i915/selftests/i915_mock_selftests.h
> +++ b/drivers/gpu/drm/i915/selftests/i915_mock_selftests.h
> @@ -21,6 +21,7 @@ selftest(fence, i915_sw_fence_mock_selftests)
>  selftest(scatterlist, scatterlist_mock_selftests)
>  selftest(syncmap, i915_syncmap_mock_selftests)
>  selftest(uncore, intel_uncore_mock_selftests)
> +selftest(ring, intel_ring_mock_selftests)
>  selftest(engine, intel_engine_cs_mock_selftests)
>  selftest(timelines, intel_timeline_mock_selftests)
>  selftest(requests, i915_request_mock_selftests)
> -- 
> 2.20.1
_______________________________________________
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: [Intel-gfx] [PATCH] drm/i915/gt: Incrementally check for rewinding
  2020-06-10  4:25     ` Chang, Bruce
@ 2020-06-10 14:03       ` Chris Wilson
  -1 siblings, 0 replies; 18+ messages in thread
From: Chris Wilson @ 2020-06-10 14:03 UTC (permalink / raw)
  To: Chang, Bruce, intel-gfx; +Cc: stable

Quoting Chang, Bruce (2020-06-10 05:25:39)
> On 6/9/2020 8:17 AM, Chris Wilson wrote:
> > In commit 5ba32c7be81e ("drm/i915/execlists: Always force a context
> > reload when rewinding RING_TAIL"), we placed the check for rewinding a
> > context on actually submitting the next request in that context. This
> > was so that we only had to check once, and could do so with precision
> > avoiding as many forced restores as possible. For example, to ensure
> > that we can resubmit the same request a couple of times, we include a
> > small wa_tail such that on the next submission, the ring->tail will
> > appear to move forwards when resubmitting the same request. This is very
> > common as it will happen for every lite-restore to fill the second port
> > after a context switch.
> >
> > However, intel_ring_direction() is limited in precision to movements of
> > upto half the ring size. The consequence being that if we tried to
> > unwind many requests, we could exceed half the ring and flip the sense
> > of the direction, so missing a force restore. As no request can be
> > greater than half the ring (i.e. 2048 bytes in the smallest case), we
> > can check for rollback incrementally. As we check against the tail that
> > would be submitted, we do not lose any sensitivity and allow lite
> > restores for the simple case. We still need to double check upon
> > submitting the context, to allow for multiple preemptions and
> > resubmissions.
> >
> > Fixes: 5ba32c7be81e ("drm/i915/execlists: Always force a context reload when rewinding RING_TAIL")
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> > Cc: <stable@vger.kernel.org> # v5.4+
> 
> Verified this has fixed the issue regarding the GPU hang with incomplete 
> error state.

But it does not entirely... tgl b0 still has the issue of a lite restore
being processed while it is doing an [implicit] semaphore wait at just
the wrong time, dies (or something that looks suspiciously like that).
That can be reproduced without any preemption rollback, so I suspect a
placebo effect.
-Chris

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

* Re: [Intel-gfx] [PATCH] drm/i915/gt: Incrementally check for rewinding
@ 2020-06-10 14:03       ` Chris Wilson
  0 siblings, 0 replies; 18+ messages in thread
From: Chris Wilson @ 2020-06-10 14:03 UTC (permalink / raw)
  To: Chang, Bruce, intel-gfx; +Cc: stable

Quoting Chang, Bruce (2020-06-10 05:25:39)
> On 6/9/2020 8:17 AM, Chris Wilson wrote:
> > In commit 5ba32c7be81e ("drm/i915/execlists: Always force a context
> > reload when rewinding RING_TAIL"), we placed the check for rewinding a
> > context on actually submitting the next request in that context. This
> > was so that we only had to check once, and could do so with precision
> > avoiding as many forced restores as possible. For example, to ensure
> > that we can resubmit the same request a couple of times, we include a
> > small wa_tail such that on the next submission, the ring->tail will
> > appear to move forwards when resubmitting the same request. This is very
> > common as it will happen for every lite-restore to fill the second port
> > after a context switch.
> >
> > However, intel_ring_direction() is limited in precision to movements of
> > upto half the ring size. The consequence being that if we tried to
> > unwind many requests, we could exceed half the ring and flip the sense
> > of the direction, so missing a force restore. As no request can be
> > greater than half the ring (i.e. 2048 bytes in the smallest case), we
> > can check for rollback incrementally. As we check against the tail that
> > would be submitted, we do not lose any sensitivity and allow lite
> > restores for the simple case. We still need to double check upon
> > submitting the context, to allow for multiple preemptions and
> > resubmissions.
> >
> > Fixes: 5ba32c7be81e ("drm/i915/execlists: Always force a context reload when rewinding RING_TAIL")
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> > Cc: <stable@vger.kernel.org> # v5.4+
> 
> Verified this has fixed the issue regarding the GPU hang with incomplete 
> error state.

But it does not entirely... tgl b0 still has the issue of a lite restore
being processed while it is doing an [implicit] semaphore wait at just
the wrong time, dies (or something that looks suspiciously like that).
That can be reproduced without any preemption rollback, so I suspect a
placebo effect.
-Chris
_______________________________________________
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:[~2020-06-10 14:04 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-09 12:28 [PATCH] drm/i915/gt: Incrementally check for rewinding Chris Wilson
2020-06-09 12:28 ` [Intel-gfx] " Chris Wilson
2020-06-09 12:47 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for " Patchwork
2020-06-09 13:08 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
2020-06-09 14:10 ` [Intel-gfx] ✗ Fi.CI.IGT: failure " Patchwork
2020-06-09 15:15 ` [PATCH] " Chris Wilson
2020-06-09 15:15   ` [Intel-gfx] " Chris Wilson
2020-06-09 15:17 ` Chris Wilson
2020-06-09 15:17   ` [Intel-gfx] " Chris Wilson
2020-06-10  4:25   ` Chang, Bruce
2020-06-10  4:25     ` Chang, Bruce
2020-06-10 14:03     ` Chris Wilson
2020-06-10 14:03       ` Chris Wilson
2020-06-10 12:39   ` Mika Kuoppala
2020-06-10 12:39     ` [Intel-gfx] " Mika Kuoppala
2020-06-09 15:37 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915/gt: Incrementally check for rewinding (rev2) Patchwork
2020-06-09 15:58 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
2020-06-09 18:30 ` [Intel-gfx] ✗ Fi.CI.IGT: 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.