diff mbox

[2/2] drm/i915: Include the last semaphore sync point in the error-state

Message ID 1354036014-10911-2-git-send-email-chris@chris-wilson.co.uk (mailing list archive)
State New, archived
Headers show

Commit Message

Chris Wilson Nov. 27, 2012, 5:06 p.m. UTC 
Should be useful to know what the driver thought the other ring's seqno
was when it last used a semaphore.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_debugfs.c |   10 ++++++----
 drivers/gpu/drm/i915/i915_drv.h     |    1 +
 drivers/gpu/drm/i915/i915_irq.c     |    2 ++
 3 files changed, 9 insertions(+), 4 deletions(-)

Comments

Ben Widawsky Nov. 27, 2012, 8:54 p.m. UTC | #1 
On Tue, 27 Nov 2012 17:06:54 +0000
Chris Wilson <chris@chris-wilson.co.uk> wrote:

> Should be useful to know what the driver thought the other ring's seqno
> was when it last used a semaphore.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>

Note to Daniel: It would be nice to work on merging VECS and then adding
this IMO

> ---
>  drivers/gpu/drm/i915/i915_debugfs.c |   10 ++++++----
>  drivers/gpu/drm/i915/i915_drv.h     |    1 +
>  drivers/gpu/drm/i915/i915_irq.c     |    2 ++
>  3 files changed, 9 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index c52758c..5359f376 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -657,10 +657,12 @@ static void i915_ring_error_state(struct seq_file *m,
>  	if (INTEL_INFO(dev)->gen >= 6) {
>  		seq_printf(m, "  RC PSMI: 0x%08x\n", error->rc_psmi[ring]);
>  		seq_printf(m, "  FAULT_REG: 0x%08x\n", error->fault_reg[ring]);
> -		seq_printf(m, "  SYNC_0: 0x%08x\n",
> -			   error->semaphore_mboxes[ring][0]);
> -		seq_printf(m, "  SYNC_1: 0x%08x\n",
> -			   error->semaphore_mboxes[ring][1]);
> +		seq_printf(m, "  SYNC_0: 0x%08x [last synced 0x%08x]\n",
> +			   error->semaphore_mboxes[ring][0],
> +			   error->semaphore_seqno[ring][0]);
> +		seq_printf(m, "  SYNC_1: 0x%08x [last synced 0x%08x]\n",
> +			   error->semaphore_mboxes[ring][1],
> +			   error->semaphore_seqno[ring][1]);
>  	}
>  	seq_printf(m, "  seqno: 0x%08x\n", error->seqno[ring]);
>  	seq_printf(m, "  waiting: %s\n", yesno(error->waiting[ring]));
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 23afc21..476aa94 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -199,6 +199,7 @@ struct drm_i915_error_state {
>  	u32 instdone[I915_NUM_RINGS];
>  	u32 acthd[I915_NUM_RINGS];
>  	u32 semaphore_mboxes[I915_NUM_RINGS][I915_NUM_RINGS - 1];
> +	u32 semaphore_seqno[I915_NUM_RINGS][I915_NUM_RINGS - 1];
>  	u32 rc_psmi[I915_NUM_RINGS]; /* sleep state */
>  	/* our own tracking of ring head and tail */
>  	u32 cpu_ring_head[I915_NUM_RINGS];
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index bfb6c51..1f40090 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -1156,6 +1156,8 @@ static void i915_record_ring_state(struct drm_device *dev,
>  			= I915_READ(RING_SYNC_0(ring->mmio_base));
>  		error->semaphore_mboxes[ring->id][1]
>  			= I915_READ(RING_SYNC_1(ring->mmio_base));
> +		error->semaphore_seqno[ring->id][0] = ring->sync_seqno[0];
> +		error->semaphore_seqno[ring->id][1] = ring->sync_seqno[1];
>  	}
>  
>  	if (INTEL_INFO(dev)->gen >= 4) {
Chris Wilson Nov. 27, 2012, 9:04 p.m. UTC | #2 
On Tue, 27 Nov 2012 12:54:35 -0800, Ben Widawsky <ben@bwidawsk.net> wrote:
> On Tue, 27 Nov 2012 17:06:54 +0000
> Chris Wilson <chris@chris-wilson.co.uk> wrote:
> 
> > Should be useful to know what the driver thought the other ring's seqno
> > was when it last used a semaphore.
> > 
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> 
> Note to Daniel: It would be nice to work on merging VECS and then adding
> this IMO

Note to Mika, you need to apply this patch asap. :-p
-Chris
Mika Kuoppala Nov. 29, 2012, 9:16 a.m. UTC | #3 
On Tue, 27 Nov 2012 17:06:54 +0000, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> Should be useful to know what the driver thought the other ring's seqno
> was when it last used a semaphore.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>  drivers/gpu/drm/i915/i915_debugfs.c |   10 ++++++----
>  drivers/gpu/drm/i915/i915_drv.h     |    1 +
>  drivers/gpu/drm/i915/i915_irq.c     |    2 ++
>  3 files changed, 9 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index c52758c..5359f376 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -657,10 +657,12 @@ static void i915_ring_error_state(struct seq_file *m,
>  	if (INTEL_INFO(dev)->gen >= 6) {
>  		seq_printf(m, "  RC PSMI: 0x%08x\n", error->rc_psmi[ring]);
>  		seq_printf(m, "  FAULT_REG: 0x%08x\n", error->fault_reg[ring]);
> -		seq_printf(m, "  SYNC_0: 0x%08x\n",
> -			   error->semaphore_mboxes[ring][0]);
> -		seq_printf(m, "  SYNC_1: 0x%08x\n",
> -			   error->semaphore_mboxes[ring][1]);
> +		seq_printf(m, "  SYNC_0: 0x%08x [last synced 0x%08x]\n",
> +			   error->semaphore_mboxes[ring][0],
> +			   error->semaphore_seqno[ring][0]);
> +		seq_printf(m, "  SYNC_1: 0x%08x [last synced 0x%08x]\n",
> +			   error->semaphore_mboxes[ring][1],
> +			   error->semaphore_seqno[ring][1]);
>  	}
>  	seq_printf(m, "  seqno: 0x%08x\n", error->seqno[ring]);
>  	seq_printf(m, "  waiting: %s\n", yesno(error->waiting[ring]));
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 23afc21..476aa94 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -199,6 +199,7 @@ struct drm_i915_error_state {
>  	u32 instdone[I915_NUM_RINGS];
>  	u32 acthd[I915_NUM_RINGS];
>  	u32 semaphore_mboxes[I915_NUM_RINGS][I915_NUM_RINGS - 1];
> +	u32 semaphore_seqno[I915_NUM_RINGS][I915_NUM_RINGS - 1];
>  	u32 rc_psmi[I915_NUM_RINGS]; /* sleep state */
>  	/* our own tracking of ring head and tail */
>  	u32 cpu_ring_head[I915_NUM_RINGS];
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index bfb6c51..1f40090 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -1156,6 +1156,8 @@ static void i915_record_ring_state(struct drm_device *dev,
>  			= I915_READ(RING_SYNC_0(ring->mmio_base));
>  		error->semaphore_mboxes[ring->id][1]
>  			= I915_READ(RING_SYNC_1(ring->mmio_base));
> +		error->semaphore_seqno[ring->id][0] = ring->sync_seqno[0];
> +		error->semaphore_seqno[ring->id][1] = ring->sync_seqno[1];
>  	}
>  
>  	if (INTEL_INFO(dev)->gen >= 4) {
> -- 
> 1.7.10.4

This patch was important for catching a seqno wrap problem in i915_gem_object_sync().

Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com>
Daniel Vetter Nov. 29, 2012, 10:01 a.m. UTC | #4 
On Thu, Nov 29, 2012 at 11:16:20AM +0200, Mika Kuoppala wrote:
> On Tue, 27 Nov 2012 17:06:54 +0000, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> > Should be useful to know what the driver thought the other ring's seqno
> > was when it last used a semaphore.
> > 
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > ---
> >  drivers/gpu/drm/i915/i915_debugfs.c |   10 ++++++----
> >  drivers/gpu/drm/i915/i915_drv.h     |    1 +
> >  drivers/gpu/drm/i915/i915_irq.c     |    2 ++
> >  3 files changed, 9 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> > index c52758c..5359f376 100644
> > --- a/drivers/gpu/drm/i915/i915_debugfs.c
> > +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> > @@ -657,10 +657,12 @@ static void i915_ring_error_state(struct seq_file *m,
> >  	if (INTEL_INFO(dev)->gen >= 6) {
> >  		seq_printf(m, "  RC PSMI: 0x%08x\n", error->rc_psmi[ring]);
> >  		seq_printf(m, "  FAULT_REG: 0x%08x\n", error->fault_reg[ring]);
> > -		seq_printf(m, "  SYNC_0: 0x%08x\n",
> > -			   error->semaphore_mboxes[ring][0]);
> > -		seq_printf(m, "  SYNC_1: 0x%08x\n",
> > -			   error->semaphore_mboxes[ring][1]);
> > +		seq_printf(m, "  SYNC_0: 0x%08x [last synced 0x%08x]\n",
> > +			   error->semaphore_mboxes[ring][0],
> > +			   error->semaphore_seqno[ring][0]);
> > +		seq_printf(m, "  SYNC_1: 0x%08x [last synced 0x%08x]\n",
> > +			   error->semaphore_mboxes[ring][1],
> > +			   error->semaphore_seqno[ring][1]);
> >  	}
> >  	seq_printf(m, "  seqno: 0x%08x\n", error->seqno[ring]);
> >  	seq_printf(m, "  waiting: %s\n", yesno(error->waiting[ring]));
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> > index 23afc21..476aa94 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -199,6 +199,7 @@ struct drm_i915_error_state {
> >  	u32 instdone[I915_NUM_RINGS];
> >  	u32 acthd[I915_NUM_RINGS];
> >  	u32 semaphore_mboxes[I915_NUM_RINGS][I915_NUM_RINGS - 1];
> > +	u32 semaphore_seqno[I915_NUM_RINGS][I915_NUM_RINGS - 1];
> >  	u32 rc_psmi[I915_NUM_RINGS]; /* sleep state */
> >  	/* our own tracking of ring head and tail */
> >  	u32 cpu_ring_head[I915_NUM_RINGS];
> > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> > index bfb6c51..1f40090 100644
> > --- a/drivers/gpu/drm/i915/i915_irq.c
> > +++ b/drivers/gpu/drm/i915/i915_irq.c
> > @@ -1156,6 +1156,8 @@ static void i915_record_ring_state(struct drm_device *dev,
> >  			= I915_READ(RING_SYNC_0(ring->mmio_base));
> >  		error->semaphore_mboxes[ring->id][1]
> >  			= I915_READ(RING_SYNC_1(ring->mmio_base));
> > +		error->semaphore_seqno[ring->id][0] = ring->sync_seqno[0];
> > +		error->semaphore_seqno[ring->id][1] = ring->sync_seqno[1];
> >  	}
> >  
> >  	if (INTEL_INFO(dev)->gen >= 4) {
> > -- 
> > 1.7.10.4
> 
> This patch was important for catching a seqno wrap problem in i915_gem_object_sync().
> 
> Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com>

Applied both patches to dinq.
-Daniel
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index c52758c..5359f376 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -657,10 +657,12 @@  static void i915_ring_error_state(struct seq_file *m,
 	if (INTEL_INFO(dev)->gen >= 6) {
 		seq_printf(m, "  RC PSMI: 0x%08x\n", error->rc_psmi[ring]);
 		seq_printf(m, "  FAULT_REG: 0x%08x\n", error->fault_reg[ring]);
-		seq_printf(m, "  SYNC_0: 0x%08x\n",
-			   error->semaphore_mboxes[ring][0]);
-		seq_printf(m, "  SYNC_1: 0x%08x\n",
-			   error->semaphore_mboxes[ring][1]);
+		seq_printf(m, "  SYNC_0: 0x%08x [last synced 0x%08x]\n",
+			   error->semaphore_mboxes[ring][0],
+			   error->semaphore_seqno[ring][0]);
+		seq_printf(m, "  SYNC_1: 0x%08x [last synced 0x%08x]\n",
+			   error->semaphore_mboxes[ring][1],
+			   error->semaphore_seqno[ring][1]);
 	}
 	seq_printf(m, "  seqno: 0x%08x\n", error->seqno[ring]);
 	seq_printf(m, "  waiting: %s\n", yesno(error->waiting[ring]));
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 23afc21..476aa94 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -199,6 +199,7 @@  struct drm_i915_error_state {
 	u32 instdone[I915_NUM_RINGS];
 	u32 acthd[I915_NUM_RINGS];
 	u32 semaphore_mboxes[I915_NUM_RINGS][I915_NUM_RINGS - 1];
+	u32 semaphore_seqno[I915_NUM_RINGS][I915_NUM_RINGS - 1];
 	u32 rc_psmi[I915_NUM_RINGS]; /* sleep state */
 	/* our own tracking of ring head and tail */
 	u32 cpu_ring_head[I915_NUM_RINGS];
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index bfb6c51..1f40090 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -1156,6 +1156,8 @@  static void i915_record_ring_state(struct drm_device *dev,
 			= I915_READ(RING_SYNC_0(ring->mmio_base));
 		error->semaphore_mboxes[ring->id][1]
 			= I915_READ(RING_SYNC_1(ring->mmio_base));
+		error->semaphore_seqno[ring->id][0] = ring->sync_seqno[0];
+		error->semaphore_seqno[ring->id][1] = ring->sync_seqno[1];
 	}
 
 	if (INTEL_INFO(dev)->gen >= 4) {