All of lore.kernel.org
 help / color / mirror / Atom feed
* [CI-ping 1/9] drm/i915: Include engine->last_submitted_seqno in GPU error state
@ 2016-04-07  6:29 Chris Wilson
  2016-04-07  6:29 ` [CI-ping 2/9] drm/i915: On GPU reset, set the HWS breadcrumb to the last seqno Chris Wilson
                   ` (9 more replies)
  0 siblings, 10 replies; 13+ messages in thread
From: Chris Wilson @ 2016-04-07  6:29 UTC (permalink / raw)
  To: intel-gfx

It's useful to look at the last seqno submitted on a particular engine
and compare it against the HWS value to check for irregularities.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Reviewed-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c   | 6 ++++--
 drivers/gpu/drm/i915/i915_drv.h       | 1 +
 drivers/gpu/drm/i915/i915_gpu_error.c | 2 ++
 3 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index a2e3af02c292..906e5424e488 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1363,8 +1363,10 @@ static int i915_hangcheck_info(struct seq_file *m, void *unused)
 
 	for_each_engine_id(engine, dev_priv, id) {
 		seq_printf(m, "%s:\n", engine->name);
-		seq_printf(m, "\tseqno = %x [current %x]\n",
-			   engine->hangcheck.seqno, seqno[id]);
+		seq_printf(m, "\tseqno = %x [current %x, last %x]\n",
+			   engine->hangcheck.seqno,
+			   seqno[id],
+			   engine->last_submitted_seqno);
 		seq_printf(m, "\tACTHD = 0x%08llx [current 0x%08llx]\n",
 			   (long long)engine->hangcheck.acthd,
 			   (long long)acthd[id]);
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index a1f78f275c55..c7ce452d35ad 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -495,6 +495,7 @@ struct drm_i915_error_state {
 		u32 cpu_ring_head;
 		u32 cpu_ring_tail;
 
+		u32 last_seqno;
 		u32 semaphore_seqno[I915_NUM_ENGINES - 1];
 
 		/* Register state */
diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c
index 9b55409d91b3..9463cb4e9323 100644
--- a/drivers/gpu/drm/i915/i915_gpu_error.c
+++ b/drivers/gpu/drm/i915/i915_gpu_error.c
@@ -296,6 +296,7 @@ static void i915_ring_error_state(struct drm_i915_error_state_buf *m,
 		}
 	}
 	err_printf(m, "  seqno: 0x%08x\n", ring->seqno);
+	err_printf(m, "  last_seqno: 0x%08x\n", ring->last_seqno);
 	err_printf(m, "  waiting: %s\n", yesno(ring->waiting));
 	err_printf(m, "  ring->head: 0x%08x\n", ring->cpu_ring_head);
 	err_printf(m, "  ring->tail: 0x%08x\n", ring->cpu_ring_tail);
@@ -931,6 +932,7 @@ static void i915_record_ring_state(struct drm_device *dev,
 	ering->instpm = I915_READ(RING_INSTPM(engine->mmio_base));
 	ering->seqno = engine->get_seqno(engine, false);
 	ering->acthd = intel_ring_get_active_head(engine);
+	ering->last_seqno = engine->last_submitted_seqno;
 	ering->start = I915_READ_START(engine);
 	ering->head = I915_READ_HEAD(engine);
 	ering->tail = I915_READ_TAIL(engine);
-- 
2.8.0.rc3

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

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

* [CI-ping 2/9] drm/i915: On GPU reset, set the HWS breadcrumb to the last seqno
  2016-04-07  6:29 [CI-ping 1/9] drm/i915: Include engine->last_submitted_seqno in GPU error state Chris Wilson
@ 2016-04-07  6:29 ` Chris Wilson
  2016-04-07  6:29 ` [CI-ping 3/9] drm/i915: Remove unneeded drm_device pointer from intel_ring_init_seqno() Chris Wilson
                   ` (8 subsequent siblings)
  9 siblings, 0 replies; 13+ messages in thread
From: Chris Wilson @ 2016-04-07  6:29 UTC (permalink / raw)
  To: intel-gfx

After the GPU reset and we discard all of the incomplete requests, mark
the GPU as having advanced to the last_submitted_seqno (as having
completed the requests and ready for fresh work). The impact of this is
negligible, as all the requests will be considered completed by this
point, it just brings the HWS into line with expectations for external
viewers.

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

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index b342f672f391..05711e7f9fb8 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2882,6 +2882,8 @@ static void i915_gem_reset_engine_cleanup(struct drm_i915_private *dev_priv,
 		buffer->last_retired_head = buffer->tail;
 		intel_ring_update_space(buffer);
 	}
+
+	intel_ring_init_seqno(engine, engine->last_submitted_seqno);
 }
 
 void i915_gem_reset(struct drm_device *dev)
-- 
2.8.0.rc3

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

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

* [CI-ping 3/9] drm/i915: Remove unneeded drm_device pointer from intel_ring_init_seqno()
  2016-04-07  6:29 [CI-ping 1/9] drm/i915: Include engine->last_submitted_seqno in GPU error state Chris Wilson
  2016-04-07  6:29 ` [CI-ping 2/9] drm/i915: On GPU reset, set the HWS breadcrumb to the last seqno Chris Wilson
@ 2016-04-07  6:29 ` Chris Wilson
  2016-04-07  6:29 ` [CI-ping 4/9] drm/i915: Move the hw semaphore initialisation from GEM to the engine Chris Wilson
                   ` (7 subsequent siblings)
  9 siblings, 0 replies; 13+ messages in thread
From: Chris Wilson @ 2016-04-07  6:29 UTC (permalink / raw)
  To: intel-gfx

We only use drm_i915_private within the function, so delete the unneeded
drm_device local.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_ringbuffer.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 2e864b7a528b..371f4c1fc33c 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -2550,13 +2550,12 @@ int intel_ring_cacheline_align(struct drm_i915_gem_request *req)
 
 void intel_ring_init_seqno(struct intel_engine_cs *engine, u32 seqno)
 {
-	struct drm_device *dev = engine->dev;
-	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct drm_i915_private *dev_priv = to_i915(engine->dev);
 
-	if (INTEL_INFO(dev)->gen == 6 || INTEL_INFO(dev)->gen == 7) {
+	if (INTEL_INFO(dev_priv)->gen == 6 || INTEL_INFO(dev_priv)->gen == 7) {
 		I915_WRITE(RING_SYNC_0(engine->mmio_base), 0);
 		I915_WRITE(RING_SYNC_1(engine->mmio_base), 0);
-		if (HAS_VEBOX(dev))
+		if (HAS_VEBOX(dev_priv))
 			I915_WRITE(RING_SYNC_2(engine->mmio_base), 0);
 	}
 
-- 
2.8.0.rc3

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

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

* [CI-ping 4/9] drm/i915: Move the hw semaphore initialisation from GEM to the engine
  2016-04-07  6:29 [CI-ping 1/9] drm/i915: Include engine->last_submitted_seqno in GPU error state Chris Wilson
  2016-04-07  6:29 ` [CI-ping 2/9] drm/i915: On GPU reset, set the HWS breadcrumb to the last seqno Chris Wilson
  2016-04-07  6:29 ` [CI-ping 3/9] drm/i915: Remove unneeded drm_device pointer from intel_ring_init_seqno() Chris Wilson
@ 2016-04-07  6:29 ` Chris Wilson
  2016-04-07  6:29 ` [CI-ping 5/9] drm/i915: Refactor gen8 semaphore offset calculation Chris Wilson
                   ` (6 subsequent siblings)
  9 siblings, 0 replies; 13+ messages in thread
From: Chris Wilson @ 2016-04-07  6:29 UTC (permalink / raw)
  To: intel-gfx

Since we are setting engine local values that are tied to the hardware,
move it out of i915_gem_init_seqno() into the intel_ring_init_seqno()
backend, next to where the other hw semaphore registers are written.

v2: Make the explanatory comment about always resetting the semaphores to
0 irrespective of the value of the reset seqno.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Reviewed-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_gem.c         |  8 ++------
 drivers/gpu/drm/i915/intel_ringbuffer.c | 11 +++++++++++
 2 files changed, 13 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 05711e7f9fb8..7506e850913e 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2468,7 +2468,7 @@ i915_gem_init_seqno(struct drm_device *dev, u32 seqno)
 {
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct intel_engine_cs *engine;
-	int ret, j;
+	int ret;
 
 	/* Carefully retire all requests without writing to the rings */
 	for_each_engine(engine, dev_priv) {
@@ -2479,13 +2479,9 @@ i915_gem_init_seqno(struct drm_device *dev, u32 seqno)
 	i915_gem_retire_requests(dev);
 
 	/* Finally reset hw state */
-	for_each_engine(engine, dev_priv) {
+	for_each_engine(engine, dev_priv)
 		intel_ring_init_seqno(engine, seqno);
 
-		for (j = 0; j < ARRAY_SIZE(engine->semaphore.sync_seqno); j++)
-			engine->semaphore.sync_seqno[j] = 0;
-	}
-
 	return 0;
 }
 
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 371f4c1fc33c..69852ac4018d 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -2552,14 +2552,25 @@ void intel_ring_init_seqno(struct intel_engine_cs *engine, u32 seqno)
 {
 	struct drm_i915_private *dev_priv = to_i915(engine->dev);
 
+	/* Our semaphore implementation is strictly monotonic (i.e. we proceed
+	 * so long as the semaphore value in the register/page is greater
+	 * than the sync value), so whenever we reset the seqno,
+	 * so long as we reset the tracking semaphore value to 0, it will
+	 * always be before the next request's seqno. If we don't reset
+	 * the semaphore value, then when the seqno moves backwards all
+	 * future waits will complete instantly (causing rendering corruption).
+	 */
 	if (INTEL_INFO(dev_priv)->gen == 6 || INTEL_INFO(dev_priv)->gen == 7) {
 		I915_WRITE(RING_SYNC_0(engine->mmio_base), 0);
 		I915_WRITE(RING_SYNC_1(engine->mmio_base), 0);
 		if (HAS_VEBOX(dev_priv))
 			I915_WRITE(RING_SYNC_2(engine->mmio_base), 0);
 	}
+	memset(engine->semaphore.sync_seqno, 0,
+	       sizeof(engine->semaphore.sync_seqno));
 
 	engine->set_seqno(engine, seqno);
+
 	engine->hangcheck.seqno = seqno;
 }
 
-- 
2.8.0.rc3

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

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

* [CI-ping 5/9] drm/i915: Refactor gen8 semaphore offset calculation
  2016-04-07  6:29 [CI-ping 1/9] drm/i915: Include engine->last_submitted_seqno in GPU error state Chris Wilson
                   ` (2 preceding siblings ...)
  2016-04-07  6:29 ` [CI-ping 4/9] drm/i915: Move the hw semaphore initialisation from GEM to the engine Chris Wilson
@ 2016-04-07  6:29 ` Chris Wilson
  2016-04-07  6:29 ` [CI-ping 6/9] drm/i915: Reset semaphore page for gen8 Chris Wilson
                   ` (5 subsequent siblings)
  9 siblings, 0 replies; 13+ messages in thread
From: Chris Wilson @ 2016-04-07  6:29 UTC (permalink / raw)
  To: intel-gfx

We reuse the same calculation into two macros, and I want to add a third
user. Time to refactor.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_ringbuffer.h | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
index 18074ab55f61..98eadfa79116 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.h
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
@@ -52,16 +52,15 @@ struct  intel_hw_status_page {
 /* seqno size is actually only a uint32, but since we plan to use MI_FLUSH_DW to
  * do the writes, and that must have qw aligned offsets, simply pretend it's 8b.
  */
-#define i915_semaphore_seqno_size sizeof(uint64_t)
+#define gen8_semaphore_seqno_size sizeof(uint64_t)
+#define GEN8_SEMAPHORE_OFFSET(__from, __to)			     \
+	(((__from) * I915_NUM_ENGINES  + (__to)) * gen8_semaphore_seqno_size)
 #define GEN8_SIGNAL_OFFSET(__ring, to)			     \
 	(i915_gem_obj_ggtt_offset(dev_priv->semaphore_obj) + \
-	((__ring)->id * I915_NUM_ENGINES * i915_semaphore_seqno_size) +	\
-	(i915_semaphore_seqno_size * (to)))
-
+	 GEN8_SEMAPHORE_OFFSET((__ring)->id, (to)))
 #define GEN8_WAIT_OFFSET(__ring, from)			     \
 	(i915_gem_obj_ggtt_offset(dev_priv->semaphore_obj) + \
-	((from) * I915_NUM_ENGINES * i915_semaphore_seqno_size) + \
-	(i915_semaphore_seqno_size * (__ring)->id))
+	 GEN8_SEMAPHORE_OFFSET(from, (__ring)->id))
 
 #define GEN8_RING_SEMAPHORE_INIT(e) do { \
 	if (!dev_priv->semaphore_obj) { \
-- 
2.8.0.rc3

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

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

* [CI-ping 6/9] drm/i915: Reset semaphore page for gen8
  2016-04-07  6:29 [CI-ping 1/9] drm/i915: Include engine->last_submitted_seqno in GPU error state Chris Wilson
                   ` (3 preceding siblings ...)
  2016-04-07  6:29 ` [CI-ping 5/9] drm/i915: Refactor gen8 semaphore offset calculation Chris Wilson
@ 2016-04-07  6:29 ` Chris Wilson
  2016-04-07  6:29 ` [CI-ping 7/9] drm/i915: Reset engine->last_submitted_seqno Chris Wilson
                   ` (4 subsequent siblings)
  9 siblings, 0 replies; 13+ messages in thread
From: Chris Wilson @ 2016-04-07  6:29 UTC (permalink / raw)
  To: intel-gfx

An oversight is that when we wrap the seqno, we need to reset the hw
semaphore counters to 0. We did this for gen6 and gen7 and forgot to do
so for the new implementation required for gen8 (legacy).

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

diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 69852ac4018d..0e6490074011 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -2566,6 +2566,14 @@ void intel_ring_init_seqno(struct intel_engine_cs *engine, u32 seqno)
 		if (HAS_VEBOX(dev_priv))
 			I915_WRITE(RING_SYNC_2(engine->mmio_base), 0);
 	}
+	if (dev_priv->semaphore_obj) {
+		struct drm_i915_gem_object *obj = dev_priv->semaphore_obj;
+		struct page *page = i915_gem_object_get_dirty_page(obj, 0);
+		void *semaphores = kmap(page);
+		memset(semaphores + GEN8_SEMAPHORE_OFFSET(engine->id, 0),
+		       0, I915_NUM_ENGINES * gen8_semaphore_seqno_size);
+		kunmap(page);
+	}
 	memset(engine->semaphore.sync_seqno, 0,
 	       sizeof(engine->semaphore.sync_seqno));
 
-- 
2.8.0.rc3

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

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

* [CI-ping 7/9] drm/i915: Reset engine->last_submitted_seqno
  2016-04-07  6:29 [CI-ping 1/9] drm/i915: Include engine->last_submitted_seqno in GPU error state Chris Wilson
                   ` (4 preceding siblings ...)
  2016-04-07  6:29 ` [CI-ping 6/9] drm/i915: Reset semaphore page for gen8 Chris Wilson
@ 2016-04-07  6:29 ` Chris Wilson
  2016-04-07  6:29 ` [CI-ping 8/9] drm/i915: Apply a mb between emitting the request and hangcheck Chris Wilson
                   ` (3 subsequent siblings)
  9 siblings, 0 replies; 13+ messages in thread
From: Chris Wilson @ 2016-04-07  6:29 UTC (permalink / raw)
  To: intel-gfx

When we change the current seqno, we also need to remember to reset the
last_submitted_seqno for the engine.

Testcase: igt/gem_exec_whisper
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Reviewed-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_ringbuffer.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 0e6490074011..6b4952031e30 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -2578,6 +2578,7 @@ void intel_ring_init_seqno(struct intel_engine_cs *engine, u32 seqno)
 	       sizeof(engine->semaphore.sync_seqno));
 
 	engine->set_seqno(engine, seqno);
+	engine->last_submitted_seqno = seqno;
 
 	engine->hangcheck.seqno = seqno;
 }
-- 
2.8.0.rc3

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

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

* [CI-ping 8/9] drm/i915: Apply a mb between emitting the request and hangcheck
  2016-04-07  6:29 [CI-ping 1/9] drm/i915: Include engine->last_submitted_seqno in GPU error state Chris Wilson
                   ` (5 preceding siblings ...)
  2016-04-07  6:29 ` [CI-ping 7/9] drm/i915: Reset engine->last_submitted_seqno Chris Wilson
@ 2016-04-07  6:29 ` Chris Wilson
  2016-04-07  6:29 ` [CI-ping 9/9] drm/i915: Simplify check for idleness in hangcheck Chris Wilson
                   ` (2 subsequent siblings)
  9 siblings, 0 replies; 13+ messages in thread
From: Chris Wilson @ 2016-04-07  6:29 UTC (permalink / raw)
  To: intel-gfx

Seal the request and mark it as pending execution before we submit it to
hardware. We assume that the actual submission cannot fail (that
guarantee is provided by preallocating space in the request for the
submission). As we may inspect this state without holding any locks
during hangcheck we should apply a barrier to ensure that we do
not see a more recent value in the HWS than we are tracking.

Based on a patch by Mika Kuoppala.

Suggested-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_gem.c | 39 ++++++++++++++++++++++-----------------
 drivers/gpu/drm/i915/i915_irq.c |  3 ++-
 2 files changed, 24 insertions(+), 18 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 7506e850913e..5a65a7663b88 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2575,6 +2575,28 @@ void __i915_add_request(struct drm_i915_gem_request *request,
 		WARN(ret, "*_ring_flush_all_caches failed: %d!\n", ret);
 	}
 
+	trace_i915_gem_request_add(request);
+
+	request->head = request_start;
+
+	/* Whilst this request exists, batch_obj will be on the
+	 * active_list, and so will hold the active reference. Only when this
+	 * request is retired will the the batch_obj be moved onto the
+	 * inactive_list and lose its active reference. Hence we do not need
+	 * to explicitly hold another reference here.
+	 */
+	request->batch_obj = obj;
+
+	/* Seal the request and mark it as pending execution. Note that
+	 * we may inspect this state, without holding any locks, during
+	 * hangcheck. Hence we apply the barrier to ensure that we do not
+	 * see a more recent value in the hws than we are tracking.
+	 */
+	request->emitted_jiffies = jiffies;
+	request->previous_seqno = engine->last_submitted_seqno;
+	smp_store_mb(engine->last_submitted_seqno, request->seqno);
+	list_add_tail(&request->list, &engine->request_list);
+
 	/* Record the position of the start of the request so that
 	 * should we detect the updated seqno part-way through the
 	 * GPU processing the request, we never over-estimate the
@@ -2592,23 +2614,6 @@ void __i915_add_request(struct drm_i915_gem_request *request,
 	/* Not allowed to fail! */
 	WARN(ret, "emit|add_request failed: %d!\n", ret);
 
-	request->head = request_start;
-
-	/* Whilst this request exists, batch_obj will be on the
-	 * active_list, and so will hold the active reference. Only when this
-	 * request is retired will the the batch_obj be moved onto the
-	 * inactive_list and lose its active reference. Hence we do not need
-	 * to explicitly hold another reference here.
-	 */
-	request->batch_obj = obj;
-
-	request->emitted_jiffies = jiffies;
-	request->previous_seqno = engine->last_submitted_seqno;
-	engine->last_submitted_seqno = request->seqno;
-	list_add_tail(&request->list, &engine->request_list);
-
-	trace_i915_gem_request_add(request);
-
 	i915_queue_hangcheck(engine->dev);
 
 	queue_delayed_work(dev_priv->wq,
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 5bec13844800..a5eb77d1f8cb 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -2806,7 +2806,8 @@ static bool
 ring_idle(struct intel_engine_cs *engine, u32 seqno)
 {
 	return (list_empty(&engine->request_list) ||
-		i915_seqno_passed(seqno, engine->last_submitted_seqno));
+		i915_seqno_passed(seqno,
+				  READ_ONCE(engine->last_submitted_seqno)));
 }
 
 static bool
-- 
2.8.0.rc3

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

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

* [CI-ping 9/9] drm/i915: Simplify check for idleness in hangcheck
  2016-04-07  6:29 [CI-ping 1/9] drm/i915: Include engine->last_submitted_seqno in GPU error state Chris Wilson
                   ` (6 preceding siblings ...)
  2016-04-07  6:29 ` [CI-ping 8/9] drm/i915: Apply a mb between emitting the request and hangcheck Chris Wilson
@ 2016-04-07  6:29 ` Chris Wilson
  2016-04-07 15:06 ` [CI-ping 1/9] drm/i915: Include engine->last_submitted_seqno in GPU error state Chris Wilson
  2016-04-08  6:49 ` ✗ Fi.CI.BAT: failure for series starting with [CI-ping,1/9] " Patchwork
  9 siblings, 0 replies; 13+ messages in thread
From: Chris Wilson @ 2016-04-07  6:29 UTC (permalink / raw)
  To: intel-gfx

Having fixed the tracking of the engine's last_submitted_seqno, we can
now rely on it for detecting when the engine is idle (and not have to
touch the requests pointer).

Testcase: igt/gem_exec_whisper
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_irq.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index a5eb77d1f8cb..5007cde8d4c3 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -2805,9 +2805,8 @@ static void gen8_disable_vblank(struct drm_device *dev, unsigned int pipe)
 static bool
 ring_idle(struct intel_engine_cs *engine, u32 seqno)
 {
-	return (list_empty(&engine->request_list) ||
-		i915_seqno_passed(seqno,
-				  READ_ONCE(engine->last_submitted_seqno)));
+	return i915_seqno_passed(seqno,
+				 READ_ONCE(engine->last_submitted_seqno));
 }
 
 static bool
-- 
2.8.0.rc3

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

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

* Re: [CI-ping 1/9] drm/i915: Include engine->last_submitted_seqno in GPU error state
  2016-04-07  6:29 [CI-ping 1/9] drm/i915: Include engine->last_submitted_seqno in GPU error state Chris Wilson
                   ` (7 preceding siblings ...)
  2016-04-07  6:29 ` [CI-ping 9/9] drm/i915: Simplify check for idleness in hangcheck Chris Wilson
@ 2016-04-07 15:06 ` Chris Wilson
  2016-04-07 16:11   ` Marius Vlad
  2016-04-08  6:49 ` ✗ Fi.CI.BAT: failure for series starting with [CI-ping,1/9] " Patchwork
  9 siblings, 1 reply; 13+ messages in thread
From: Chris Wilson @ 2016-04-07 15:06 UTC (permalink / raw)
  To: intel-gfx, Tomi Sarvela

On Thu, Apr 07, 2016 at 07:29:10AM +0100, Chris Wilson wrote:
> It's useful to look at the last seqno submitted on a particular engine
> and compare it against the HWS value to check for irregularities.

CI, what have I done to offend you?
-Chris

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

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

* Re: [CI-ping 1/9] drm/i915: Include engine->last_submitted_seqno in GPU error state
  2016-04-07 15:06 ` [CI-ping 1/9] drm/i915: Include engine->last_submitted_seqno in GPU error state Chris Wilson
@ 2016-04-07 16:11   ` Marius Vlad
  2016-04-08  6:56     ` Tomi Sarvela
  0 siblings, 1 reply; 13+ messages in thread
From: Marius Vlad @ 2016-04-07 16:11 UTC (permalink / raw)
  To: Chris Wilson, intel-gfx, Tomi Sarvela, Mika Kuoppala, Joonas Lahtinen


[-- Attachment #1.1: Type: text/plain, Size: 702 bytes --]

There are some results on PW: https://patchwork.freedesktop.org/series/5400/

Fi CI seems to be a little bit late on the results....

On Thu, Apr 07, 2016 at 04:06:49PM +0100, Chris Wilson wrote:
> On Thu, Apr 07, 2016 at 07:29:10AM +0100, Chris Wilson wrote:
> > It's useful to look at the last seqno submitted on a particular engine
> > and compare it against the HWS value to check for irregularities.
> 
> CI, what have I done to offend you?
> -Chris
> 
> -- 
> Chris Wilson, Intel Open Source Technology Centre
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

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

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

* ✗ Fi.CI.BAT: failure for series starting with [CI-ping,1/9] drm/i915: Include engine->last_submitted_seqno in GPU error state
  2016-04-07  6:29 [CI-ping 1/9] drm/i915: Include engine->last_submitted_seqno in GPU error state Chris Wilson
                   ` (8 preceding siblings ...)
  2016-04-07 15:06 ` [CI-ping 1/9] drm/i915: Include engine->last_submitted_seqno in GPU error state Chris Wilson
@ 2016-04-08  6:49 ` Patchwork
  9 siblings, 0 replies; 13+ messages in thread
From: Patchwork @ 2016-04-08  6:49 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx

== Series Details ==

Series: series starting with [CI-ping,1/9] drm/i915: Include engine->last_submitted_seqno in GPU error state
URL   : https://patchwork.freedesktop.org/series/5400/
State : failure

== Summary ==

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

Test drv_hangman:
        Subgroup error-state-basic:
                fail       -> PASS       (ilk-hp8440p)
Test gem_ctx_param_basic:
        Subgroup root-set:
                pass       -> DMESG-WARN (bsw-nuc-2)
Test gem_exec_basic:
        Subgroup basic-blt:
                pass       -> SKIP       (bsw-nuc-2)
        Subgroup gtt-vebox:
                pass       -> SKIP       (bsw-nuc-2)
Test gem_exec_nop:
        Subgroup basic:
                pass       -> SKIP       (bsw-nuc-2)
Test gem_exec_store:
        Subgroup basic-blt:
                pass       -> SKIP       (bsw-nuc-2)
        Subgroup basic-render:
                pass       -> SKIP       (bsw-nuc-2)
Test gem_exec_suspend:
        Subgroup basic-s3:
                pass       -> SKIP       (bsw-nuc-2)
Test gem_mmap_gtt:
        Subgroup basic-read-no-prefault:
                pass       -> DMESG-WARN (bsw-nuc-2)
        Subgroup basic-write-no-prefault:
                pass       -> DMESG-WARN (bsw-nuc-2)
Test gem_pread:
        Subgroup basic:
                pass       -> DMESG-WARN (bsw-nuc-2)
Test gem_ringfill:
        Subgroup basic-default-interruptible:
                skip       -> PASS       (bsw-nuc-2)
Test gem_sync:
        Subgroup basic-all:
                dmesg-fail -> PASS       (bsw-nuc-2)
        Subgroup basic-bsd:
                skip       -> DMESG-WARN (bsw-nuc-2)
        Subgroup basic-render:
                incomplete -> PASS       (bdw-nuci7) UNSTABLE
Test gem_tiled_blits:
        Subgroup basic:
                pass       -> DMESG-FAIL (bsw-nuc-2)
Test kms_addfb_basic:
        Subgroup bad-pitch-0:
                pass       -> DMESG-WARN (bsw-nuc-2)
        Subgroup bad-pitch-128:
                pass       -> DMESG-WARN (bsw-nuc-2)
        Subgroup bad-pitch-256:
                pass       -> DMESG-WARN (bsw-nuc-2)
        Subgroup basic-y-tiled:
                pass       -> DMESG-WARN (bsw-nuc-2)
        Subgroup unused-modifier:
                pass       -> DMESG-WARN (bsw-nuc-2)
Test kms_flip:
        Subgroup basic-flip-vs-dpms:
                incomplete -> DMESG-FAIL (bsw-nuc-2)
Test kms_force_connector_basic:
        Subgroup force-connector-state:
                skip       -> PASS       (ivb-t430s)
Test pm_rpm:
        Subgroup basic-pci-d3-state:
                pass       -> DMESG-WARN (bsw-nuc-2)
        Subgroup basic-rte:
                pass       -> DMESG-WARN (byt-nuc) UNSTABLE

bdw-nuci7        total:196  pass:184  dwarn:0   dfail:0   fail:0   skip:12 
bdw-ultra        total:196  pass:175  dwarn:0   dfail:0   fail:0   skip:21 
bsw-nuc-2        total:82   pass:13   dwarn:33  dfail:6   fail:0   skip:29 
byt-nuc          total:196  pass:160  dwarn:1   dfail:0   fail:0   skip:35 
hsw-brixbox      total:196  pass:174  dwarn:0   dfail:0   fail:0   skip:22 
hsw-gt2          total:196  pass:179  dwarn:0   dfail:0   fail:0   skip:17 
ilk-hp8440p      total:196  pass:132  dwarn:0   dfail:0   fail:0   skip:64 
ivb-t430s        total:196  pass:171  dwarn:0   dfail:0   fail:0   skip:25 
skl-i7k-2        total:196  pass:173  dwarn:0   dfail:0   fail:0   skip:23 
skl-nuci5        total:196  pass:185  dwarn:0   dfail:0   fail:0   skip:11 
snb-dellxps      total:196  pass:162  dwarn:0   dfail:0   fail:0   skip:34 
snb-x220t        total:196  pass:162  dwarn:0   dfail:0   fail:1   skip:33 

Results at /archive/results/CI_IGT_test/Patchwork_1823/

6b0bf94b75318abeab4c95fbf0507bc54d8959be drm-intel-nightly: 2016y-04m-07d-08h-29m-47s UTC integration manifest
aa9ee20123b9ebbbcb9d809ac0cff57122941a5e drm/i915: Simplify check for idleness in hangcheck
536ef38e2acf85b2ffb5426e1b2b7469c0cc2ca5 drm/i915: Apply a mb between emitting the request and hangcheck
64142bced59e9d5345e1edb50da17b8aeda5f9f0 drm/i915: Reset engine->last_submitted_seqno
9cde3ae26dd1c1244de85b05ac0a25c259114ebc drm/i915: Reset semaphore page for gen8
7ff73843656ccf84ad1c44a578a9598a3fdf35f3 drm/i915: Refactor gen8 semaphore offset calculation
4154724f8b67525c15af943d38e5301b8a34c95b drm/i915: Move the hw semaphore initialisation from GEM to the engine
4fd936cac3f79f0774bba57563b0846560a069b3 drm/i915: Remove unneeded drm_device pointer from intel_ring_init_seqno()
5050572357090e02ddb222fe461263dfd0a06d7a drm/i915: On GPU reset, set the HWS breadcrumb to the last seqno
5e8ef0ed4a466f9e12bded033d1dd5bf90cc020a drm/i915: Include engine->last_submitted_seqno in GPU error state

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

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

* Re: [CI-ping 1/9] drm/i915: Include engine->last_submitted_seqno in GPU error state
  2016-04-07 16:11   ` Marius Vlad
@ 2016-04-08  6:56     ` Tomi Sarvela
  0 siblings, 0 replies; 13+ messages in thread
From: Tomi Sarvela @ 2016-04-08  6:56 UTC (permalink / raw)
  To: Marius Vlad; +Cc: intel-gfx


[-- Attachment #1.1: Type: text/plain, Size: 1207 bytes --]

Yesterday CI got lagged for a bit while I was tuning timeouts in host connectivity. We're 
now using nmap (instead of ping) to get better results over firewalls and long 
distances. Change sparked from connecting French DUT to our Jenkins.

The results for this series were run, but not posted for some reason, so I redid that. 
Lots of noise from BSW-NUC, first failed tests were in order:

dmesg-fail: igt/gem_cpu_reloc/basic        
dmesg-fail: igt/kms_flip/basic-flip-vs-dpms               
dmesg-fail: igt/gem_render_tiled_blits/basic              

https://patchwork.freedesktop.org/series/5400/[1] 

Tomi


On Thursday 07 April 2016 19:11:30 Marius Vlad wrote:
> There are some results on PW: https://patchwork.freedesktop.org/series/5400/
> 
> Fi CI seems to be a little bit late on the results....
> 
> On Thu, Apr 07, 2016 at 04:06:49PM +0100, Chris Wilson wrote:
> > On Thu, Apr 07, 2016 at 07:29:10AM +0100, Chris Wilson wrote:
> > > It's useful to look at the last seqno submitted on a particular engine
> > > and compare it against the HWS value to check for irregularities.
> > 
> > CI, what have I done to offend you?
> > -Chris


--------
[1] https://patchwork.freedesktop.org/series/5400/

[-- Attachment #1.2: Type: text/html, Size: 5830 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

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

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

end of thread, other threads:[~2016-04-08  6:56 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-04-07  6:29 [CI-ping 1/9] drm/i915: Include engine->last_submitted_seqno in GPU error state Chris Wilson
2016-04-07  6:29 ` [CI-ping 2/9] drm/i915: On GPU reset, set the HWS breadcrumb to the last seqno Chris Wilson
2016-04-07  6:29 ` [CI-ping 3/9] drm/i915: Remove unneeded drm_device pointer from intel_ring_init_seqno() Chris Wilson
2016-04-07  6:29 ` [CI-ping 4/9] drm/i915: Move the hw semaphore initialisation from GEM to the engine Chris Wilson
2016-04-07  6:29 ` [CI-ping 5/9] drm/i915: Refactor gen8 semaphore offset calculation Chris Wilson
2016-04-07  6:29 ` [CI-ping 6/9] drm/i915: Reset semaphore page for gen8 Chris Wilson
2016-04-07  6:29 ` [CI-ping 7/9] drm/i915: Reset engine->last_submitted_seqno Chris Wilson
2016-04-07  6:29 ` [CI-ping 8/9] drm/i915: Apply a mb between emitting the request and hangcheck Chris Wilson
2016-04-07  6:29 ` [CI-ping 9/9] drm/i915: Simplify check for idleness in hangcheck Chris Wilson
2016-04-07 15:06 ` [CI-ping 1/9] drm/i915: Include engine->last_submitted_seqno in GPU error state Chris Wilson
2016-04-07 16:11   ` Marius Vlad
2016-04-08  6:56     ` Tomi Sarvela
2016-04-08  6:49 ` ✗ Fi.CI.BAT: failure for series starting with [CI-ping,1/9] " 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.