All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/5] Some minor CSB/execlist stuff
@ 2016-01-05 18:30 Ben Widawsky
  2016-01-05 18:30 ` [PATCH 1/5] drm/i915: Cleanup some of the CSB handling Ben Widawsky
                   ` (5 more replies)
  0 siblings, 6 replies; 14+ messages in thread
From: Ben Widawsky @ 2016-01-05 18:30 UTC (permalink / raw)
  To: Intel GFX; +Cc: Rodrigo Vivi, Ben Widawsky

While doing some debug in simulation, I came across a handful of patches which I
think are beneficial today. Mostly this just has some minor cleanups and error
state additions. They're pretty optional, though I have a private branch that
depends on some of this stuff, so it'd be nice to land as much as possible - but
I'll live without it.

Ben Widawsky (5):
  drm/i915: Cleanup some of the CSB handling
  drm/i915: change WARN to ERROR in CSB count
  drm/i915: Extract CSB status read
  drm/i915: Add basic execlist info to error state
  drm/i915: Use CSB helper in debugfs

 drivers/gpu/drm/i915/i915_debugfs.c   |  9 ++++-----
 drivers/gpu/drm/i915/i915_drv.h       |  7 ++++++-
 drivers/gpu/drm/i915/i915_gpu_error.c | 23 +++++++++++++++++++++
 drivers/gpu/drm/i915/intel_lrc.c      | 38 ++++++++++++++++++++++++++---------
 drivers/gpu/drm/i915/intel_lrc.h      | 22 ++++++++++++++++++--
 5 files changed, 81 insertions(+), 18 deletions(-)

-- 
2.6.4

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

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

* [PATCH 1/5] drm/i915: Cleanup some of the CSB handling
  2016-01-05 18:30 [PATCH 0/5] Some minor CSB/execlist stuff Ben Widawsky
@ 2016-01-05 18:30 ` Ben Widawsky
  2016-01-06 15:08   ` Michel Thierry
  2016-01-05 18:30 ` [PATCH 2/5] drm/i915: Change WARN to ERROR in CSB count Ben Widawsky
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 14+ messages in thread
From: Ben Widawsky @ 2016-01-05 18:30 UTC (permalink / raw)
  To: Intel GFX; +Cc: Ben Widawsky

I think this patch is a worthwhile cleanup even if it might look only marginally
useful. It gets more useful in upcoming patches and for handling of future GEN
platforms.

The only non-mechanical part of this is the removal of the extra & operation on
the ring->next_context_status_buffer. This is safe because right above this, we
already did a modulus operation.

Signed-off-by: Ben Widawsky <benjamin.widawsky@intel.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c |  6 +++---
 drivers/gpu/drm/i915/intel_lrc.c    | 15 +++++++++------
 drivers/gpu/drm/i915/intel_lrc.h    | 18 ++++++++++++++++--
 3 files changed, 28 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 0fc38bb..3b05bd1 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -2092,13 +2092,13 @@ static int i915_execlists(struct seq_file *m, void *data)
 		seq_printf(m, "\tStatus pointer: 0x%08X\n", status_pointer);
 
 		read_pointer = ring->next_context_status_buffer;
-		write_pointer = status_pointer & 0x07;
+		write_pointer = GEN8_CSB_WRITE_PTR(status_pointer);
 		if (read_pointer > write_pointer)
-			write_pointer += 6;
+			write_pointer += GEN8_CSB_ENTRIES;
 		seq_printf(m, "\tRead pointer: 0x%08X, write pointer 0x%08X\n",
 			   read_pointer, write_pointer);
 
-		for (i = 0; i < 6; i++) {
+		for (i = 0; i < GEN8_CSB_ENTRIES; i++) {
 			status = I915_READ(RING_CONTEXT_STATUS_BUF_LO(ring, i));
 			ctx_id = I915_READ(RING_CONTEXT_STATUS_BUF_HI(ring, i));
 
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 8096c6a..7fb2035 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -516,7 +516,7 @@ void intel_lrc_irq_handler(struct intel_engine_cs *ring)
 	status_pointer = I915_READ(RING_CONTEXT_STATUS_PTR(ring));
 
 	read_pointer = ring->next_context_status_buffer;
-	write_pointer = status_pointer & GEN8_CSB_PTR_MASK;
+	write_pointer = GEN8_CSB_WRITE_PTR(status_pointer);
 	if (read_pointer > write_pointer)
 		write_pointer += GEN8_CSB_ENTRIES;
 
@@ -559,10 +559,11 @@ void intel_lrc_irq_handler(struct intel_engine_cs *ring)
 	WARN(submit_contexts > 2, "More than two context complete events?\n");
 	ring->next_context_status_buffer = write_pointer % GEN8_CSB_ENTRIES;
 
+	/* Update the read pointer to the old write pointer. Manual ringbuffer
+	 * management ftw </sarcasm> */
 	I915_WRITE(RING_CONTEXT_STATUS_PTR(ring),
-		   _MASKED_FIELD(GEN8_CSB_PTR_MASK << 8,
-				 ((u32)ring->next_context_status_buffer &
-				  GEN8_CSB_PTR_MASK) << 8));
+		   _MASKED_FIELD(GEN8_CSB_READ_PTR_MASK,
+				 ring->next_context_status_buffer << 8));
 }
 
 static int execlists_context_queue(struct drm_i915_gem_request *request)
@@ -1506,9 +1507,11 @@ static int gen8_init_common_ring(struct intel_engine_cs *ring)
 	 *      | Suspend-to-idle (freeze) | Suspend-to-RAM (mem) |
 	 * BDW  | CSB regs not reset       | CSB regs reset       |
 	 * CHT  | CSB regs not reset       | CSB regs not reset   |
+	 * SKL  |         ?                |         ?            |
+	 * BXT  |         ?                |         ?            |
 	 */
-	next_context_status_buffer_hw = (I915_READ(RING_CONTEXT_STATUS_PTR(ring))
-						   & GEN8_CSB_PTR_MASK);
+	next_context_status_buffer_hw =
+		GEN8_CSB_WRITE_PTR(I915_READ(RING_CONTEXT_STATUS_PTR(ring)));
 
 	/*
 	 * When the CSB registers are reset (also after power-up / gpu reset),
diff --git a/drivers/gpu/drm/i915/intel_lrc.h b/drivers/gpu/drm/i915/intel_lrc.h
index ae90f86..de41ad6 100644
--- a/drivers/gpu/drm/i915/intel_lrc.h
+++ b/drivers/gpu/drm/i915/intel_lrc.h
@@ -25,8 +25,6 @@
 #define _INTEL_LRC_H_
 
 #define GEN8_LR_CONTEXT_ALIGN 4096
-#define GEN8_CSB_ENTRIES 6
-#define GEN8_CSB_PTR_MASK 0x07
 
 /* Execlists regs */
 #define RING_ELSP(ring)				_MMIO((ring)->mmio_base + 0x230)
@@ -40,6 +38,22 @@
 #define RING_CONTEXT_STATUS_BUF_HI(ring, i)	_MMIO((ring)->mmio_base + 0x370 + (i) * 8 + 4)
 #define RING_CONTEXT_STATUS_PTR(ring)		_MMIO((ring)->mmio_base + 0x3a0)
 
+/* The docs specify that the write pointer wraps around after 5h, "After status
+ * is written out to the last available status QW at offset 5h, this pointer
+ * wraps to 0."
+ *
+ * Therefore, one must infer than even though there are 3 bits available, 6 and
+ * 7 appear to be * reserved.
+ */
+#define GEN8_CSB_ENTRIES 6
+#define GEN8_CSB_PTR_MASK 0x7
+#define GEN8_CSB_READ_PTR_MASK (GEN8_CSB_PTR_MASK << 8)
+#define GEN8_CSB_WRITE_PTR_MASK (GEN8_CSB_PTR_MASK << 0)
+#define GEN8_CSB_WRITE_PTR(csb_status) \
+	(((csb_status) & GEN8_CSB_WRITE_PTR_MASK) >> 0)
+#define GEN8_CSB_READ_PTR(csb_status) \
+	(((csb_status) & GEN8_CSB_READ_PTR_MASK) >> 8)
+
 /* Logical Rings */
 int intel_logical_ring_alloc_request_extras(struct drm_i915_gem_request *request);
 int intel_logical_ring_reserve_space(struct drm_i915_gem_request *request);
-- 
2.6.4

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

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

* [PATCH 2/5] drm/i915: Change WARN to ERROR in CSB count
  2016-01-05 18:30 [PATCH 0/5] Some minor CSB/execlist stuff Ben Widawsky
  2016-01-05 18:30 ` [PATCH 1/5] drm/i915: Cleanup some of the CSB handling Ben Widawsky
@ 2016-01-05 18:30 ` Ben Widawsky
  2016-01-06 15:09   ` Michel Thierry
  2016-01-05 18:30 ` [PATCH 3/5] drm/i915: Extract CSB status read Ben Widawsky
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 14+ messages in thread
From: Ben Widawsky @ 2016-01-05 18:30 UTC (permalink / raw)
  To: Intel GFX; +Cc: Ben Widawsky

There is no point in emitting a WARN since the backtrace will always be the
same. Errors have actually become easier to spot given the large number of WARNs
which exist today in modesetting paths.

Signed-off-by: Ben Widawsky <benjamin.widawsky@intel.com>
---
 drivers/gpu/drm/i915/intel_lrc.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 7fb2035..14affaa 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -556,7 +556,9 @@ void intel_lrc_irq_handler(struct intel_engine_cs *ring)
 
 	spin_unlock(&ring->execlist_lock);
 
-	WARN(submit_contexts > 2, "More than two context complete events?\n");
+	if (unlikely(submit_contexts > 2))
+		DRM_ERROR("More than two context complete events?\n");
+
 	ring->next_context_status_buffer = write_pointer % GEN8_CSB_ENTRIES;
 
 	/* Update the read pointer to the old write pointer. Manual ringbuffer
-- 
2.6.4

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

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

* [PATCH 3/5] drm/i915: Extract CSB status read
  2016-01-05 18:30 [PATCH 0/5] Some minor CSB/execlist stuff Ben Widawsky
  2016-01-05 18:30 ` [PATCH 1/5] drm/i915: Cleanup some of the CSB handling Ben Widawsky
  2016-01-05 18:30 ` [PATCH 2/5] drm/i915: Change WARN to ERROR in CSB count Ben Widawsky
@ 2016-01-05 18:30 ` Ben Widawsky
  2016-01-06 15:09   ` Michel Thierry
  2016-01-07 14:34   ` Daniel Vetter
  2016-01-05 18:30 ` [PATCH 4/5] drm/i915: Add basic execlist info to error state Ben Widawsky
                   ` (2 subsequent siblings)
  5 siblings, 2 replies; 14+ messages in thread
From: Ben Widawsky @ 2016-01-05 18:30 UTC (permalink / raw)
  To: Intel GFX; +Cc: Ben Widawsky

This is a useful thing to have around as a function because the mechanism may
change in the future.

There is a net increase in LOC here, and it will continue to be the case on GEN8
and GEN9 - but future GENs may have an alternate mechanism for doing this.

Signed-off-by: Ben Widawsky <benjamin.widawsky@intel.com>
---
 drivers/gpu/drm/i915/intel_lrc.c | 19 ++++++++++++++++---
 1 file changed, 16 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 14affaa..23839ff 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -496,6 +496,19 @@ static bool execlists_check_remove_request(struct intel_engine_cs *ring,
 	return false;
 }
 
+static void get_context_status(struct intel_engine_cs *ring,
+			       u8 read_pointer,
+			       u32 *status, u32 *context_id)
+{
+	struct drm_i915_private *dev_priv = ring->dev->dev_private;
+
+	if (WARN_ON(read_pointer >= GEN8_CSB_ENTRIES))
+		return;
+
+	*status = I915_READ(RING_CONTEXT_STATUS_BUF_LO(ring, read_pointer));
+	*context_id = I915_READ(RING_CONTEXT_STATUS_BUF_HI(ring, read_pointer));
+}
+
 /**
  * intel_lrc_irq_handler() - handle Context Switch interrupts
  * @ring: Engine Command Streamer to handle.
@@ -523,9 +536,9 @@ void intel_lrc_irq_handler(struct intel_engine_cs *ring)
 	spin_lock(&ring->execlist_lock);
 
 	while (read_pointer < write_pointer) {
-		read_pointer++;
-		status = I915_READ(RING_CONTEXT_STATUS_BUF_LO(ring, read_pointer % GEN8_CSB_ENTRIES));
-		status_id = I915_READ(RING_CONTEXT_STATUS_BUF_HI(ring, read_pointer % GEN8_CSB_ENTRIES));
+
+		get_context_status(ring, ++read_pointer % GEN8_CSB_ENTRIES,
+				   &status, &status_id);
 
 		if (status & GEN8_CTX_STATUS_IDLE_ACTIVE)
 			continue;
-- 
2.6.4

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

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

* [PATCH 4/5] drm/i915: Add basic execlist info to error state
  2016-01-05 18:30 [PATCH 0/5] Some minor CSB/execlist stuff Ben Widawsky
                   ` (2 preceding siblings ...)
  2016-01-05 18:30 ` [PATCH 3/5] drm/i915: Extract CSB status read Ben Widawsky
@ 2016-01-05 18:30 ` Ben Widawsky
  2016-01-06 15:10   ` Michel Thierry
  2016-01-05 18:30 ` [PATCH 5/5] drm/i915: Use CSB helper in debugfs Ben Widawsky
  2016-01-06  8:20 ` ✗ warning: Fi.CI.BAT Patchwork
  5 siblings, 1 reply; 14+ messages in thread
From: Ben Widawsky @ 2016-01-05 18:30 UTC (permalink / raw)
  To: Intel GFX; +Cc: Ben Widawsky

Sample output:
...
  waiting: yes
  ring->head: 0x00000000
  ring->tail: 0x00000c50
  ring->next_context_status_buffer: 0x5
  CSB Pointer: 0x00000405
    Context 0 Status: 0x0000000000000001
    Context 1 Status: 0x0000009d00000018
    Context 2 Status: 0x0000000000000001
    Context 3 Status: 0x0000000100000018
    Context 4 Status: 0x0000000000000001
    Context 5 Status: 0x0000009d00000018
  hangcheck: hung [40]
bsd command stream:
  START: 0x00039000
  HEAD:  0x00000018
...

Signed-off-by: Ben Widawsky <benjamin.widawsky@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h       |  7 ++++++-
 drivers/gpu/drm/i915/i915_gpu_error.c | 23 +++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_lrc.c      | 10 +++++-----
 drivers/gpu/drm/i915/intel_lrc.h      |  4 ++++
 4 files changed, 38 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index c6dd4db..c79e869 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -526,6 +526,7 @@ struct drm_i915_error_state {
 		u32 cpu_ring_tail;
 
 		u32 semaphore_seqno[I915_NUM_RINGS - 1];
+		u32 semaphore_mboxes[I915_NUM_RINGS - 1];
 
 		/* Register state */
 		u32 start;
@@ -545,7 +546,11 @@ struct drm_i915_error_state {
 		u32 fault_reg;
 		u64 faddr;
 		u32 rc_psmi; /* sleep state */
-		u32 semaphore_mboxes[I915_NUM_RINGS - 1];
+
+		/* execlist state */
+		u32 csb_ptr;
+		u8 next_context_status_buffer;
+		u64 context_status[GEN8_CSB_ENTRIES];
 
 		struct drm_i915_error_object {
 			int page_count;
diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c
index 06ca408..20a5daa 100644
--- a/drivers/gpu/drm/i915/i915_gpu_error.c
+++ b/drivers/gpu/drm/i915/i915_gpu_error.c
@@ -301,6 +301,18 @@ static void i915_ring_error_state(struct drm_i915_error_state_buf *m,
 	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);
+
+	if (i915.enable_execlists) {
+		int j;
+		err_printf(m, "  ring->next_context_status_buffer: 0x%d\n",
+			      ring->next_context_status_buffer);
+		err_printf(m, "  CSB Pointer: 0x%08x\n", ring->csb_ptr);
+		for (j = 0; j < GEN8_CSB_ENTRIES; j++) {
+			err_printf(m, "    Context %d Status: 0x%016llx\n",
+				           j, ring->context_status[j]);
+		}
+	}
+
 	err_printf(m, "  hangcheck: %s [%d]\n",
 		   hangcheck_action_to_str(ring->hangcheck_action),
 		   ring->hangcheck_score);
@@ -1042,6 +1054,8 @@ static void i915_gem_record_rings(struct drm_device *dev,
 		}
 
 		if (i915.enable_execlists) {
+			int j;
+
 			/* TODO: This is only a small fix to keep basic error
 			 * capture working, but we need to add more information
 			 * for it to be useful (e.g. dump the context being
@@ -1051,6 +1065,15 @@ static void i915_gem_record_rings(struct drm_device *dev,
 				rbuf = request->ctx->engine[ring->id].ringbuf;
 			else
 				rbuf = ring->default_context->engine[ring->id].ringbuf;
+
+			error->ring[i].csb_ptr = I915_READ(RING_CONTEXT_STATUS_PTR(ring));
+			error->ring[i].next_context_status_buffer = ring->next_context_status_buffer;
+			for (j = 0; j < GEN8_CSB_ENTRIES; j++) {
+				u32 status, id;
+				intel_lrc_get_context_status(ring, j, &status, &id);
+				error->ring[i].context_status[j] = ((__u64)id<<32)|(__u64)status;
+			}
+
 		} else
 			rbuf = ring->buffer;
 
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 23839ff..a118146 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -496,9 +496,9 @@ static bool execlists_check_remove_request(struct intel_engine_cs *ring,
 	return false;
 }
 
-static void get_context_status(struct intel_engine_cs *ring,
-			       u8 read_pointer,
-			       u32 *status, u32 *context_id)
+void intel_lrc_get_context_status(struct intel_engine_cs *ring,
+				  u8 read_pointer,
+				  u32 *status, u32 *context_id)
 {
 	struct drm_i915_private *dev_priv = ring->dev->dev_private;
 
@@ -537,8 +537,8 @@ void intel_lrc_irq_handler(struct intel_engine_cs *ring)
 
 	while (read_pointer < write_pointer) {
 
-		get_context_status(ring, ++read_pointer % GEN8_CSB_ENTRIES,
-				   &status, &status_id);
+		intel_lrc_get_context_status(ring, ++read_pointer % GEN8_CSB_ENTRIES,
+					     &status, &status_id);
 
 		if (status & GEN8_CTX_STATUS_IDLE_ACTIVE)
 			continue;
diff --git a/drivers/gpu/drm/i915/intel_lrc.h b/drivers/gpu/drm/i915/intel_lrc.h
index de41ad6..82c87f9 100644
--- a/drivers/gpu/drm/i915/intel_lrc.h
+++ b/drivers/gpu/drm/i915/intel_lrc.h
@@ -118,4 +118,8 @@ u32 intel_execlists_ctx_id(struct drm_i915_gem_object *ctx_obj);
 void intel_lrc_irq_handler(struct intel_engine_cs *ring);
 void intel_execlists_retire_requests(struct intel_engine_cs *ring);
 
+void intel_lrc_get_context_status(struct intel_engine_cs *ring,
+				  u8 read_pointer,
+				  u32 *status, u32 *context_id);
+
 #endif /* _INTEL_LRC_H_ */
-- 
2.6.4

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

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

* [PATCH 5/5] drm/i915: Use CSB helper in debugfs
  2016-01-05 18:30 [PATCH 0/5] Some minor CSB/execlist stuff Ben Widawsky
                   ` (3 preceding siblings ...)
  2016-01-05 18:30 ` [PATCH 4/5] drm/i915: Add basic execlist info to error state Ben Widawsky
@ 2016-01-05 18:30 ` Ben Widawsky
  2016-01-06 15:10   ` Michel Thierry
  2016-01-06  8:20 ` ✗ warning: Fi.CI.BAT Patchwork
  5 siblings, 1 reply; 14+ messages in thread
From: Ben Widawsky @ 2016-01-05 18:30 UTC (permalink / raw)
  To: Intel GFX; +Cc: Ben Widawsky

Since we extracted it for use in error state, we may as well use it in debugfs
too.

Signed-off-by: Ben Widawsky <benjamin.widawsky@intel.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 3b05bd1..0b829fa 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -2099,8 +2099,7 @@ static int i915_execlists(struct seq_file *m, void *data)
 			   read_pointer, write_pointer);
 
 		for (i = 0; i < GEN8_CSB_ENTRIES; i++) {
-			status = I915_READ(RING_CONTEXT_STATUS_BUF_LO(ring, i));
-			ctx_id = I915_READ(RING_CONTEXT_STATUS_BUF_HI(ring, i));
+			intel_lrc_get_context_status(ring, i, &status, &ctx_id);
 
 			seq_printf(m, "\tStatus buffer %d: 0x%08X, context: %u\n",
 				   i, status, ctx_id);
-- 
2.6.4

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

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

* ✗ warning: Fi.CI.BAT
  2016-01-05 18:30 [PATCH 0/5] Some minor CSB/execlist stuff Ben Widawsky
                   ` (4 preceding siblings ...)
  2016-01-05 18:30 ` [PATCH 5/5] drm/i915: Use CSB helper in debugfs Ben Widawsky
@ 2016-01-06  8:20 ` Patchwork
  5 siblings, 0 replies; 14+ messages in thread
From: Patchwork @ 2016-01-06  8:20 UTC (permalink / raw)
  To: Ben Widawsky; +Cc: intel-gfx

== Summary ==

Built on bc303261a81a96298b2f9e02734aeaa0a25421a6 drm-intel-nightly: 2016y-01m-05d-16h-47m-54s UTC integration manifest

Test gem_storedw_loop:
        Subgroup basic-render:
                pass       -> DMESG-WARN (skl-i5k-2) UNSTABLE
                dmesg-warn -> PASS       (skl-i7k-2) UNSTABLE
Test kms_flip:
        Subgroup basic-flip-vs-modeset:
                pass       -> DMESG-WARN (skl-i5k-2) UNSTABLE
Test kms_pipe_crc_basic:
        Subgroup read-crc-pipe-b:
                dmesg-warn -> PASS       (skl-i5k-2) UNSTABLE
                dmesg-warn -> PASS       (snb-dellxps) UNSTABLE
Test kms_setmode:
        Subgroup basic-clone-single-crtc:
                pass       -> DMESG-WARN (snb-dellxps)
Test pm_rpm:
        Subgroup basic-rte:
                pass       -> DMESG-WARN (byt-nuc) UNSTABLE

bdw-nuci7        total:132  pass:121  dwarn:2   dfail:0   fail:0   skip:9  
bdw-ultra        total:132  pass:124  dwarn:2   dfail:0   fail:0   skip:6  
bsw-nuc-2        total:135  pass:114  dwarn:1   dfail:0   fail:0   skip:20 
byt-nuc          total:135  pass:120  dwarn:2   dfail:0   fail:0   skip:13 
hsw-brixbox      total:135  pass:126  dwarn:2   dfail:0   fail:0   skip:7  
hsw-gt2          total:135  pass:130  dwarn:1   dfail:0   fail:0   skip:4  
ilk-hp8440p      total:135  pass:100  dwarn:0   dfail:0   fail:0   skip:35 
ivb-t430s        total:135  pass:127  dwarn:2   dfail:0   fail:0   skip:6  
skl-i5k-2        total:135  pass:123  dwarn:4   dfail:0   fail:0   skip:8  
skl-i7k-2        total:135  pass:125  dwarn:2   dfail:0   fail:0   skip:8  
snb-dellxps      total:135  pass:121  dwarn:2   dfail:0   fail:0   skip:12 
snb-x220t        total:135  pass:121  dwarn:2   dfail:0   fail:1   skip:11 

Results at /archive/results/CI_IGT_test/Patchwork_1091/

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

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

* Re: [PATCH 1/5] drm/i915: Cleanup some of the CSB handling
  2016-01-05 18:30 ` [PATCH 1/5] drm/i915: Cleanup some of the CSB handling Ben Widawsky
@ 2016-01-06 15:08   ` Michel Thierry
  0 siblings, 0 replies; 14+ messages in thread
From: Michel Thierry @ 2016-01-06 15:08 UTC (permalink / raw)
  To: Ben Widawsky, Intel GFX

On 1/5/2016 6:30 PM, Ben Widawsky wrote:
> I think this patch is a worthwhile cleanup even if it might look only marginally
> useful. It gets more useful in upcoming patches and for handling of future GEN
> platforms.
>
> The only non-mechanical part of this is the removal of the extra & operation on
> the ring->next_context_status_buffer. This is safe because right above this, we
> already did a modulus operation.
>
> Signed-off-by: Ben Widawsky <benjamin.widawsky@intel.com>
> ---
>   drivers/gpu/drm/i915/i915_debugfs.c |  6 +++---
>   drivers/gpu/drm/i915/intel_lrc.c    | 15 +++++++++------
>   drivers/gpu/drm/i915/intel_lrc.h    | 18 ++++++++++++++++--
>   3 files changed, 28 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index 0fc38bb..3b05bd1 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -2092,13 +2092,13 @@ static int i915_execlists(struct seq_file *m, void *data)
>                  seq_printf(m, "\tStatus pointer: 0x%08X\n", status_pointer);
>
>                  read_pointer = ring->next_context_status_buffer;
> -               write_pointer = status_pointer & 0x07;
> +               write_pointer = GEN8_CSB_WRITE_PTR(status_pointer);
>                  if (read_pointer > write_pointer)
> -                       write_pointer += 6;
> +                       write_pointer += GEN8_CSB_ENTRIES;
>                  seq_printf(m, "\tRead pointer: 0x%08X, write pointer 0x%08X\n",
>                             read_pointer, write_pointer);
>
> -               for (i = 0; i < 6; i++) {
> +               for (i = 0; i < GEN8_CSB_ENTRIES; i++) {
>                          status = I915_READ(RING_CONTEXT_STATUS_BUF_LO(ring, i));
>                          ctx_id = I915_READ(RING_CONTEXT_STATUS_BUF_HI(ring, i));
>
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> index 8096c6a..7fb2035 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -516,7 +516,7 @@ void intel_lrc_irq_handler(struct intel_engine_cs *ring)
>          status_pointer = I915_READ(RING_CONTEXT_STATUS_PTR(ring));
>
>          read_pointer = ring->next_context_status_buffer;
> -       write_pointer = status_pointer & GEN8_CSB_PTR_MASK;
> +       write_pointer = GEN8_CSB_WRITE_PTR(status_pointer);
>          if (read_pointer > write_pointer)
>                  write_pointer += GEN8_CSB_ENTRIES;
>
> @@ -559,10 +559,11 @@ void intel_lrc_irq_handler(struct intel_engine_cs *ring)
>          WARN(submit_contexts > 2, "More than two context complete events?\n");
>          ring->next_context_status_buffer = write_pointer % GEN8_CSB_ENTRIES;
>
> +       /* Update the read pointer to the old write pointer. Manual ringbuffer
> +        * management ftw </sarcasm> */
>          I915_WRITE(RING_CONTEXT_STATUS_PTR(ring),
> -                  _MASKED_FIELD(GEN8_CSB_PTR_MASK << 8,
> -                                ((u32)ring->next_context_status_buffer &
> -                                 GEN8_CSB_PTR_MASK) << 8));
> +                  _MASKED_FIELD(GEN8_CSB_READ_PTR_MASK,
> +                                ring->next_context_status_buffer << 8));
>   }
>
>   static int execlists_context_queue(struct drm_i915_gem_request *request)
> @@ -1506,9 +1507,11 @@ static int gen8_init_common_ring(struct intel_engine_cs *ring)
>           *      | Suspend-to-idle (freeze) | Suspend-to-RAM (mem) |
>           * BDW  | CSB regs not reset       | CSB regs reset       |
>           * CHT  | CSB regs not reset       | CSB regs not reset   |
> +        * SKL  |         ?                |         ?            |
> +        * BXT  |         ?                |         ?            |
>           */
> -       next_context_status_buffer_hw = (I915_READ(RING_CONTEXT_STATUS_PTR(ring))
> -                                                  & GEN8_CSB_PTR_MASK);
> +       next_context_status_buffer_hw =
> +               GEN8_CSB_WRITE_PTR(I915_READ(RING_CONTEXT_STATUS_PTR(ring)));
>
>          /*
>           * When the CSB registers are reset (also after power-up / gpu reset),
> diff --git a/drivers/gpu/drm/i915/intel_lrc.h b/drivers/gpu/drm/i915/intel_lrc.h
> index ae90f86..de41ad6 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.h
> +++ b/drivers/gpu/drm/i915/intel_lrc.h
> @@ -25,8 +25,6 @@
>   #define _INTEL_LRC_H_
>
>   #define GEN8_LR_CONTEXT_ALIGN 4096
> -#define GEN8_CSB_ENTRIES 6
> -#define GEN8_CSB_PTR_MASK 0x07
>
>   /* Execlists regs */
>   #define RING_ELSP(ring)                                _MMIO((ring)->mmio_base + 0x230)
> @@ -40,6 +38,22 @@
>   #define RING_CONTEXT_STATUS_BUF_HI(ring, i)    _MMIO((ring)->mmio_base + 0x370 + (i) * 8 + 4)
>   #define RING_CONTEXT_STATUS_PTR(ring)          _MMIO((ring)->mmio_base + 0x3a0)
>
> +/* The docs specify that the write pointer wraps around after 5h, "After status
> + * is written out to the last available status QW at offset 5h, this pointer
> + * wraps to 0."
> + *
> + * Therefore, one must infer than even though there are 3 bits available, 6 and
> + * 7 appear to be * reserved.

I always see 7 (b111) after boot/reset, but before any execlist has been 
submitted.

> + */
> +#define GEN8_CSB_ENTRIES 6
> +#define GEN8_CSB_PTR_MASK 0x7
> +#define GEN8_CSB_READ_PTR_MASK (GEN8_CSB_PTR_MASK << 8)
> +#define GEN8_CSB_WRITE_PTR_MASK (GEN8_CSB_PTR_MASK << 0)
> +#define GEN8_CSB_WRITE_PTR(csb_status) \
> +       (((csb_status) & GEN8_CSB_WRITE_PTR_MASK) >> 0)
> +#define GEN8_CSB_READ_PTR(csb_status) \
> +       (((csb_status) & GEN8_CSB_READ_PTR_MASK) >> 8)
> +
>   /* Logical Rings */
>   int intel_logical_ring_alloc_request_extras(struct drm_i915_gem_request *request);
>   int intel_logical_ring_reserve_space(struct drm_i915_gem_request *request);
> --
> 2.6.4

Reviewed-by: Michel Thierry <michel.thierry@intel.com>

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

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

* Re: [PATCH 2/5] drm/i915: Change WARN to ERROR in CSB count
  2016-01-05 18:30 ` [PATCH 2/5] drm/i915: Change WARN to ERROR in CSB count Ben Widawsky
@ 2016-01-06 15:09   ` Michel Thierry
  0 siblings, 0 replies; 14+ messages in thread
From: Michel Thierry @ 2016-01-06 15:09 UTC (permalink / raw)
  To: Ben Widawsky, Intel GFX

On 1/5/2016 6:30 PM, Ben Widawsky wrote:
> There is no point in emitting a WARN since the backtrace will always be the
> same. Errors have actually become easier to spot given the large number of WARNs
> which exist today in modesetting paths.
>
> Signed-off-by: Ben Widawsky <benjamin.widawsky@intel.com>
> ---
>   drivers/gpu/drm/i915/intel_lrc.c | 4 +++-
>   1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> index 7fb2035..14affaa 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -556,7 +556,9 @@ void intel_lrc_irq_handler(struct intel_engine_cs *ring)
>
>          spin_unlock(&ring->execlist_lock);
>
> -       WARN(submit_contexts > 2, "More than two context complete events?\n");
> +       if (unlikely(submit_contexts > 2))
> +               DRM_ERROR("More than two context complete events?\n");
> +
>          ring->next_context_status_buffer = write_pointer % GEN8_CSB_ENTRIES;
>
>          /* Update the read pointer to the old write pointer. Manual ringbuffer

And it's unlikely we'd recover without some kind of GPU reset...

Reviewed-by: Michel Thierry <michel.thierry@intel.com>

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

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

* Re: [PATCH 3/5] drm/i915: Extract CSB status read
  2016-01-05 18:30 ` [PATCH 3/5] drm/i915: Extract CSB status read Ben Widawsky
@ 2016-01-06 15:09   ` Michel Thierry
  2016-01-06 15:15     ` Chris Wilson
  2016-01-07 14:34   ` Daniel Vetter
  1 sibling, 1 reply; 14+ messages in thread
From: Michel Thierry @ 2016-01-06 15:09 UTC (permalink / raw)
  To: Ben Widawsky, Intel GFX

On 1/5/2016 6:30 PM, Ben Widawsky wrote:
> This is a useful thing to have around as a function because the mechanism may
> change in the future.
>
> There is a net increase in LOC here, and it will continue to be the case on GEN8
> and GEN9 - but future GENs may have an alternate mechanism for doing this.
>
> Signed-off-by: Ben Widawsky <benjamin.widawsky@intel.com>
> ---
>   drivers/gpu/drm/i915/intel_lrc.c | 19 ++++++++++++++++---
>   1 file changed, 16 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> index 14affaa..23839ff 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -496,6 +496,19 @@ static bool execlists_check_remove_request(struct intel_engine_cs *ring,
>          return false;
>   }
>
> +static void get_context_status(struct intel_engine_cs *ring,
> +                              u8 read_pointer,
> +                              u32 *status, u32 *context_id)
> +{
> +       struct drm_i915_private *dev_priv = ring->dev->dev_private;
> +
> +       if (WARN_ON(read_pointer >= GEN8_CSB_ENTRIES))
> +               return;
> +
> +       *status = I915_READ(RING_CONTEXT_STATUS_BUF_LO(ring, read_pointer));
> +       *context_id = I915_READ(RING_CONTEXT_STATUS_BUF_HI(ring, read_pointer));
> +}
> +
>   /**
>    * intel_lrc_irq_handler() - handle Context Switch interrupts
>    * @ring: Engine Command Streamer to handle.
> @@ -523,9 +536,9 @@ void intel_lrc_irq_handler(struct intel_engine_cs *ring)
>          spin_lock(&ring->execlist_lock);
>
>          while (read_pointer < write_pointer) {
> -               read_pointer++;
> -               status = I915_READ(RING_CONTEXT_STATUS_BUF_LO(ring, read_pointer % GEN8_CSB_ENTRIES));
> -               status_id = I915_READ(RING_CONTEXT_STATUS_BUF_HI(ring, read_pointer % GEN8_CSB_ENTRIES));
> +
> +               get_context_status(ring, ++read_pointer % GEN8_CSB_ENTRIES,
> +                                  &status, &status_id);
>
>                  if (status & GEN8_CTX_STATUS_IDLE_ACTIVE)
>                          continue;
> --
> 2.6.4

Future GENs may need a completely new lrc_irq_handler anyway.

Reviewed-by: Michel Thierry <michel.thierry@intel.com>

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

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

* Re: [PATCH 4/5] drm/i915: Add basic execlist info to error state
  2016-01-05 18:30 ` [PATCH 4/5] drm/i915: Add basic execlist info to error state Ben Widawsky
@ 2016-01-06 15:10   ` Michel Thierry
  0 siblings, 0 replies; 14+ messages in thread
From: Michel Thierry @ 2016-01-06 15:10 UTC (permalink / raw)
  To: Ben Widawsky, Intel GFX

On 1/5/2016 6:30 PM, Ben Widawsky wrote:
> Sample output:
> ...
>    waiting: yes
>    ring->head: 0x00000000
>    ring->tail: 0x00000c50
>    ring->next_context_status_buffer: 0x5
>    CSB Pointer: 0x00000405
>      Context 0 Status: 0x0000000000000001
>      Context 1 Status: 0x0000009d00000018
>      Context 2 Status: 0x0000000000000001
>      Context 3 Status: 0x0000000100000018
>      Context 4 Status: 0x0000000000000001
>      Context 5 Status: 0x0000009d00000018

There's another patch floating that does more less the same, plus also 
decodes the CSB events 
(http://patchwork.freedesktop.org/patch/msgid/1448278932-31551-8-git-send-email-John.C.Harrison@Intel.com).

It's too much to ask to combine them, but at least adding a blank space 
in the context status (upper/lower_32_bits) will make it more readable.

>    hangcheck: hung [40]
> bsd command stream:
>    START: 0x00039000
>    HEAD:  0x00000018
> ...
>
> Signed-off-by: Ben Widawsky <benjamin.widawsky@intel.com>
> ---
>   drivers/gpu/drm/i915/i915_drv.h       |  7 ++++++-
>   drivers/gpu/drm/i915/i915_gpu_error.c | 23 +++++++++++++++++++++++
>   drivers/gpu/drm/i915/intel_lrc.c      | 10 +++++-----
>   drivers/gpu/drm/i915/intel_lrc.h      |  4 ++++
>   4 files changed, 38 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index c6dd4db..c79e869 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -526,6 +526,7 @@ struct drm_i915_error_state {
>                  u32 cpu_ring_tail;
>
>                  u32 semaphore_seqno[I915_NUM_RINGS - 1];
> +               u32 semaphore_mboxes[I915_NUM_RINGS - 1];
>
>                  /* Register state */
>                  u32 start;
> @@ -545,7 +546,11 @@ struct drm_i915_error_state {
>                  u32 fault_reg;
>                  u64 faddr;
>                  u32 rc_psmi; /* sleep state */
> -               u32 semaphore_mboxes[I915_NUM_RINGS - 1];
> +
> +               /* execlist state */
> +               u32 csb_ptr;
> +               u8 next_context_status_buffer;
> +               u64 context_status[GEN8_CSB_ENTRIES];
>
>                  struct drm_i915_error_object {
>                          int page_count;
> diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c
> index 06ca408..20a5daa 100644
> --- a/drivers/gpu/drm/i915/i915_gpu_error.c
> +++ b/drivers/gpu/drm/i915/i915_gpu_error.c
> @@ -301,6 +301,18 @@ static void i915_ring_error_state(struct drm_i915_error_state_buf *m,
>          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);
> +
> +       if (i915.enable_execlists) {
> +               int j;
> +               err_printf(m, "  ring->next_context_status_buffer: 0x%d\n",
> +                             ring->next_context_status_buffer);
> +               err_printf(m, "  CSB Pointer: 0x%08x\n", ring->csb_ptr);
> +               for (j = 0; j < GEN8_CSB_ENTRIES; j++) {
> +                       err_printf(m, "    Context %d Status: 0x%016llx\n",
> +                                          j, ring->context_status[j]);
> +               }
> +       }
> +
>          err_printf(m, "  hangcheck: %s [%d]\n",
>                     hangcheck_action_to_str(ring->hangcheck_action),
>                     ring->hangcheck_score);
> @@ -1042,6 +1054,8 @@ static void i915_gem_record_rings(struct drm_device *dev,
>                  }
>
>                  if (i915.enable_execlists) {
> +                       int j;
> +
>                          /* TODO: This is only a small fix to keep basic error
>                           * capture working, but we need to add more information
>                           * for it to be useful (e.g. dump the context being
> @@ -1051,6 +1065,15 @@ static void i915_gem_record_rings(struct drm_device *dev,
>                                  rbuf = request->ctx->engine[ring->id].ringbuf;
>                          else
>                                  rbuf = ring->default_context->engine[ring->id].ringbuf;
> +
> +                       error->ring[i].csb_ptr = I915_READ(RING_CONTEXT_STATUS_PTR(ring));
> +                       error->ring[i].next_context_status_buffer = ring->next_context_status_buffer;
> +                       for (j = 0; j < GEN8_CSB_ENTRIES; j++) {
> +                               u32 status, id;
> +                               intel_lrc_get_context_status(ring, j, &status, &id);
> +                               error->ring[i].context_status[j] = ((__u64)id<<32)|(__u64)status;
> +                       }
> +
>                  } else
>                          rbuf = ring->buffer;
>
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> index 23839ff..a118146 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -496,9 +496,9 @@ static bool execlists_check_remove_request(struct intel_engine_cs *ring,
>          return false;
>   }
>
> -static void get_context_status(struct intel_engine_cs *ring,
> -                              u8 read_pointer,
> -                              u32 *status, u32 *context_id)
> +void intel_lrc_get_context_status(struct intel_engine_cs *ring,
> +                                 u8 read_pointer,
> +                                 u32 *status, u32 *context_id)
>   {
>          struct drm_i915_private *dev_priv = ring->dev->dev_private;
>
> @@ -537,8 +537,8 @@ void intel_lrc_irq_handler(struct intel_engine_cs *ring)
>
>          while (read_pointer < write_pointer) {
>
> -               get_context_status(ring, ++read_pointer % GEN8_CSB_ENTRIES,
> -                                  &status, &status_id);
> +               intel_lrc_get_context_status(ring, ++read_pointer % GEN8_CSB_ENTRIES,
> +                                            &status, &status_id);
>
>                  if (status & GEN8_CTX_STATUS_IDLE_ACTIVE)
>                          continue;
> diff --git a/drivers/gpu/drm/i915/intel_lrc.h b/drivers/gpu/drm/i915/intel_lrc.h
> index de41ad6..82c87f9 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.h
> +++ b/drivers/gpu/drm/i915/intel_lrc.h
> @@ -118,4 +118,8 @@ u32 intel_execlists_ctx_id(struct drm_i915_gem_object *ctx_obj);
>   void intel_lrc_irq_handler(struct intel_engine_cs *ring);
>   void intel_execlists_retire_requests(struct intel_engine_cs *ring);
>
> +void intel_lrc_get_context_status(struct intel_engine_cs *ring,
> +                                 u8 read_pointer,
> +                                 u32 *status, u32 *context_id);
> +
>   #endif /* _INTEL_LRC_H_ */
> --
> 2.6.4
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH 5/5] drm/i915: Use CSB helper in debugfs
  2016-01-05 18:30 ` [PATCH 5/5] drm/i915: Use CSB helper in debugfs Ben Widawsky
@ 2016-01-06 15:10   ` Michel Thierry
  0 siblings, 0 replies; 14+ messages in thread
From: Michel Thierry @ 2016-01-06 15:10 UTC (permalink / raw)
  To: Ben Widawsky, Intel GFX

On 1/5/2016 6:30 PM, Ben Widawsky wrote:
> Since we extracted it for use in error state, we may as well use it in debugfs
> too.
>
> Signed-off-by: Ben Widawsky <benjamin.widawsky@intel.com>

Reviewed-by: Michel Thierry <michel.thierry@intel.com>

(depends on patch 4/5, where I have a small request)

> ---
>   drivers/gpu/drm/i915/i915_debugfs.c | 3 +--
>   1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index 3b05bd1..0b829fa 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -2099,8 +2099,7 @@ static int i915_execlists(struct seq_file *m, void *data)
>                             read_pointer, write_pointer);
>
>                  for (i = 0; i < GEN8_CSB_ENTRIES; i++) {
> -                       status = I915_READ(RING_CONTEXT_STATUS_BUF_LO(ring, i));
> -                       ctx_id = I915_READ(RING_CONTEXT_STATUS_BUF_HI(ring, i));
> +                       intel_lrc_get_context_status(ring, i, &status, &ctx_id);
>
>                          seq_printf(m, "\tStatus buffer %d: 0x%08X, context: %u\n",
>                                     i, status, ctx_id);
> --
> 2.6.4
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH 3/5] drm/i915: Extract CSB status read
  2016-01-06 15:09   ` Michel Thierry
@ 2016-01-06 15:15     ` Chris Wilson
  0 siblings, 0 replies; 14+ messages in thread
From: Chris Wilson @ 2016-01-06 15:15 UTC (permalink / raw)
  To: Michel Thierry; +Cc: Intel GFX, Ben Widawsky

On Wed, Jan 06, 2016 at 03:09:41PM +0000, Michel Thierry wrote:
> On 1/5/2016 6:30 PM, Ben Widawsky wrote:
> >This is a useful thing to have around as a function because the mechanism may
> >change in the future.
> >
> >There is a net increase in LOC here, and it will continue to be the case on GEN8
> >and GEN9 - but future GENs may have an alternate mechanism for doing this.
> >
> >Signed-off-by: Ben Widawsky <benjamin.widawsky@intel.com>
> >---
> >  drivers/gpu/drm/i915/intel_lrc.c | 19 ++++++++++++++++---
> >  1 file changed, 16 insertions(+), 3 deletions(-)
> >
> >diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> >index 14affaa..23839ff 100644
> >--- a/drivers/gpu/drm/i915/intel_lrc.c
> >+++ b/drivers/gpu/drm/i915/intel_lrc.c
> >@@ -496,6 +496,19 @@ static bool execlists_check_remove_request(struct intel_engine_cs *ring,
> >         return false;
> >  }
> >
> >+static void get_context_status(struct intel_engine_cs *ring,
> >+                              u8 read_pointer,
> >+                              u32 *status, u32 *context_id)
> >+{
> >+       struct drm_i915_private *dev_priv = ring->dev->dev_private;
> >+
> >+       if (WARN_ON(read_pointer >= GEN8_CSB_ENTRIES))
> >+               return;
> >+
> >+       *status = I915_READ(RING_CONTEXT_STATUS_BUF_LO(ring, read_pointer));
> >+       *context_id = I915_READ(RING_CONTEXT_STATUS_BUF_HI(ring, read_pointer));
> >+}
> >+
> >  /**
> >   * intel_lrc_irq_handler() - handle Context Switch interrupts
> >   * @ring: Engine Command Streamer to handle.
> >@@ -523,9 +536,9 @@ void intel_lrc_irq_handler(struct intel_engine_cs *ring)
> >         spin_lock(&ring->execlist_lock);
> >
> >         while (read_pointer < write_pointer) {
> >-               read_pointer++;
> >-               status = I915_READ(RING_CONTEXT_STATUS_BUF_LO(ring, read_pointer % GEN8_CSB_ENTRIES));
> >-               status_id = I915_READ(RING_CONTEXT_STATUS_BUF_HI(ring, read_pointer % GEN8_CSB_ENTRIES));
> >+
> >+               get_context_status(ring, ++read_pointer % GEN8_CSB_ENTRIES,
> >+                                  &status, &status_id);
> >
> >                 if (status & GEN8_CTX_STATUS_IDLE_ACTIVE)
> >                         continue;
> >--
> >2.6.4
> 
> Future GENs may need a completely new lrc_irq_handler anyway.
s/Future/Current/
-Chris

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

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

* Re: [PATCH 3/5] drm/i915: Extract CSB status read
  2016-01-05 18:30 ` [PATCH 3/5] drm/i915: Extract CSB status read Ben Widawsky
  2016-01-06 15:09   ` Michel Thierry
@ 2016-01-07 14:34   ` Daniel Vetter
  1 sibling, 0 replies; 14+ messages in thread
From: Daniel Vetter @ 2016-01-07 14:34 UTC (permalink / raw)
  To: Ben Widawsky; +Cc: Intel GFX

On Tue, Jan 05, 2016 at 10:30:07AM -0800, Ben Widawsky wrote:
> This is a useful thing to have around as a function because the mechanism may
> change in the future.
> 
> There is a net increase in LOC here, and it will continue to be the case on GEN8
> and GEN9 - but future GENs may have an alternate mechanism for doing this.
> 
> Signed-off-by: Ben Widawsky <benjamin.widawsky@intel.com>

Merged up to this one to dinq.

Thanks, Daniel

> ---
>  drivers/gpu/drm/i915/intel_lrc.c | 19 ++++++++++++++++---
>  1 file changed, 16 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> index 14affaa..23839ff 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -496,6 +496,19 @@ static bool execlists_check_remove_request(struct intel_engine_cs *ring,
>  	return false;
>  }
>  
> +static void get_context_status(struct intel_engine_cs *ring,
> +			       u8 read_pointer,
> +			       u32 *status, u32 *context_id)
> +{
> +	struct drm_i915_private *dev_priv = ring->dev->dev_private;
> +
> +	if (WARN_ON(read_pointer >= GEN8_CSB_ENTRIES))
> +		return;
> +
> +	*status = I915_READ(RING_CONTEXT_STATUS_BUF_LO(ring, read_pointer));
> +	*context_id = I915_READ(RING_CONTEXT_STATUS_BUF_HI(ring, read_pointer));
> +}
> +
>  /**
>   * intel_lrc_irq_handler() - handle Context Switch interrupts
>   * @ring: Engine Command Streamer to handle.
> @@ -523,9 +536,9 @@ void intel_lrc_irq_handler(struct intel_engine_cs *ring)
>  	spin_lock(&ring->execlist_lock);
>  
>  	while (read_pointer < write_pointer) {
> -		read_pointer++;
> -		status = I915_READ(RING_CONTEXT_STATUS_BUF_LO(ring, read_pointer % GEN8_CSB_ENTRIES));
> -		status_id = I915_READ(RING_CONTEXT_STATUS_BUF_HI(ring, read_pointer % GEN8_CSB_ENTRIES));
> +
> +		get_context_status(ring, ++read_pointer % GEN8_CSB_ENTRIES,
> +				   &status, &status_id);
>  
>  		if (status & GEN8_CTX_STATUS_IDLE_ACTIVE)
>  			continue;
> -- 
> 2.6.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

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

end of thread, other threads:[~2016-01-07 14:34 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-01-05 18:30 [PATCH 0/5] Some minor CSB/execlist stuff Ben Widawsky
2016-01-05 18:30 ` [PATCH 1/5] drm/i915: Cleanup some of the CSB handling Ben Widawsky
2016-01-06 15:08   ` Michel Thierry
2016-01-05 18:30 ` [PATCH 2/5] drm/i915: Change WARN to ERROR in CSB count Ben Widawsky
2016-01-06 15:09   ` Michel Thierry
2016-01-05 18:30 ` [PATCH 3/5] drm/i915: Extract CSB status read Ben Widawsky
2016-01-06 15:09   ` Michel Thierry
2016-01-06 15:15     ` Chris Wilson
2016-01-07 14:34   ` Daniel Vetter
2016-01-05 18:30 ` [PATCH 4/5] drm/i915: Add basic execlist info to error state Ben Widawsky
2016-01-06 15:10   ` Michel Thierry
2016-01-05 18:30 ` [PATCH 5/5] drm/i915: Use CSB helper in debugfs Ben Widawsky
2016-01-06 15:10   ` Michel Thierry
2016-01-06  8:20 ` ✗ warning: Fi.CI.BAT 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.