[Intel-gfx] [PATCH 11/13] drm/i915/bdw: collect semaphore error state

Ben Widawsky ben at bwidawsk.net
Wed Feb 12 01:23:39 CET 2014


On Thu, Jan 30, 2014 at 04:53:32PM +0200, Ville Syrjälä wrote:
> On Wed, Jan 29, 2014 at 11:55:31AM -0800, Ben Widawsky wrote:
> > Since the semaphore information is in an object, just dump it, and let
> > the user parse it later.
> > 
> > NOTE: The page being used for the semaphores are incoherent with the
> > CPU. No matter what I do, I cannot figure out a way to read anything but
> > 0s. Note that the semaphore waits are indeed working.
> > 
> > v2: Don't print signal, and wait (they should be the same). Instead,
> > print sync_seqno (Chris)
> > 
> > v3: Free the semaphore error object (Chris)
> > 
> > Signed-off-by: Ben Widawsky <ben at bwidawsk.net>
> > ---
> >  drivers/gpu/drm/i915/i915_drv.h         |  1 +
> >  drivers/gpu/drm/i915/i915_gpu_error.c   | 47 ++++++++++++++++++++++++++++++---
> >  drivers/gpu/drm/i915/intel_ringbuffer.h | 12 ++++-----
> >  3 files changed, 51 insertions(+), 9 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> > index f521059..b08e6eb 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -313,6 +313,7 @@ struct drm_i915_error_state {
> >  	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];
> > +	struct drm_i915_error_object *semaphore_obj;
> >  	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_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c
> > index efaad96..d6afc01 100644
> > --- a/drivers/gpu/drm/i915/i915_gpu_error.c
> > +++ b/drivers/gpu/drm/i915/i915_gpu_error.c
> > @@ -297,6 +297,7 @@ int i915_error_state_to_str(struct drm_i915_error_state_buf *m,
> >  	struct drm_device *dev = error_priv->dev;
> >  	drm_i915_private_t *dev_priv = dev->dev_private;
> >  	struct drm_i915_error_state *error = error_priv->error;
> > +	struct drm_i915_error_object *obj;
> >  	int i, j, page, offset, elt;
> >  
> >  	if (!error) {
> > @@ -345,8 +346,6 @@ int i915_error_state_to_str(struct drm_i915_error_state_buf *m,
> >  				    error->pinned_bo_count[0]);
> >  
> >  	for (i = 0; i < ARRAY_SIZE(error->ring); i++) {
> > -		struct drm_i915_error_object *obj;
> > -
> >  		if ((obj = error->ring[i].batchbuffer)) {
> >  			err_printf(m, "%s --- gtt_offset = 0x%08x\n",
> >  				   dev_priv->ring[i].name,
> > @@ -421,6 +420,19 @@ int i915_error_state_to_str(struct drm_i915_error_state_buf *m,
> >  		}
> >  	}
> >  
> > +	obj = error->semaphore_obj;
> > +	if (obj) {
> 
> Chris will come along and change this to
> 
> if ((obj = error->semaphore_obj))
> 
> 
> > +		err_printf(m, "Semaphore page = 0x%08x\n", obj->gtt_offset);
> > +		for (elt = 0; elt < PAGE_SIZE/16; elt += 4) {
> > +			err_printf(m, "[%04x] %08x %08x %08x %08x\n",
> > +				   elt * 4,
> > +				   obj->pages[0][elt],
> > +				   obj->pages[0][elt+1],
> > +				   obj->pages[0][elt+2],
> > +				   obj->pages[0][elt+3]);
> > +		}
> 
> That'll be the third copy of this page dumping code. Time to refactor?
> 

I did enough refactoring in this series - but acked.

> > +	}
> > +
> >  	if (error->overlay)
> >  		intel_overlay_print_error_state(m, error->overlay);
> >  
> > @@ -491,6 +503,7 @@ static void i915_error_state_free(struct kref *error_ref)
> >  		kfree(error->ring[i].requests);
> >  	}
> >  
> > +	i915_error_object_free(error->semaphore_obj);
> >  	kfree(error->active_bo);
> >  	kfree(error->overlay);
> >  	kfree(error->display);
> > @@ -772,6 +785,31 @@ static void gen6_record_semaphore_state(struct drm_i915_private *dev_priv,
> >  	}
> >  }
> >  
> > +static void gen8_record_semaphore_state(struct drm_i915_private *dev_priv,
> > +					struct drm_i915_error_state *error,
> > +					struct intel_ring_buffer *ring)
> > +{
> > +	struct intel_ring_buffer *useless;
> > +	int i;
> > +
> > +	if (!i915_semaphore_is_enabled(dev_priv->dev))
> > +		return;
> > +
> > +	if (!error->semaphore_obj)
> > +		error->semaphore_obj =
> > +			i915_error_object_create(dev_priv,
> > +						 dev_priv->semaphore_obj,
> > +						 &dev_priv->gtt.base);
> > +
> > +	for_each_ring(useless, dev_priv, i) {
> > +		u16 signal_offset = GEN8_SIGNAL_OFFSET(ring, i) / 4;
> 
> GEN8_SIGNAL_OFFSET() returns the full ggtt offset.
> 

Fixed locally. I'll wait until you respond to one of my earlier mails
and then resend.

> > +		u32 *tmp = error->semaphore_obj->pages[0];
> > +
> > +		error->semaphore_mboxes[ring->id][i] = tmp[signal_offset];
> > +		error->semaphore_seqno[ring->id][i] = ring->semaphore.sync_seqno[i];
> > +	}
> > +}
> > +
> >  static void i915_record_ring_state(struct drm_device *dev,
> >  				   struct drm_i915_error_state *error,
> >  				   struct intel_ring_buffer *ring)
> > @@ -781,7 +819,10 @@ static void i915_record_ring_state(struct drm_device *dev,
> >  	if (INTEL_INFO(dev)->gen >= 6) {
> >  		error->rc_psmi[ring->id] = I915_READ(ring->mmio_base + 0x50);
> >  		error->fault_reg[ring->id] = I915_READ(RING_FAULT_REG(ring));
> > -		gen6_record_semaphore_state(dev_priv, error, ring);
> > +		if (INTEL_INFO(dev)->gen >= 8)
> > +			gen8_record_semaphore_state(dev_priv, error, ring);
> > +		else
> > +			gen6_record_semaphore_state(dev_priv, error, ring);
> >  	}
> >  
> >  	if (INTEL_INFO(dev)->gen >= 4) {
> > diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
> > index ed55370..4ca2789 100644
> > --- a/drivers/gpu/drm/i915/intel_ringbuffer.h
> > +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
> > @@ -37,9 +37,9 @@ struct  intel_hw_status_page {
> >   * 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_SIGNAL_OFFSET(to) \
> > +#define GEN8_SIGNAL_OFFSET(__ring, to) \
> >  	(i915_gem_obj_ggtt_offset(dev_priv->semaphore_obj) + \
> > -	(ring->id * I915_NUM_RINGS * i915_semaphore_seqno_size) + \
> > +	((__ring)->id * I915_NUM_RINGS * i915_semaphore_seqno_size) + \
> >  	(i915_semaphore_seqno_size * (to)))
> >  
> >  #define GEN8_WAIT_OFFSET(__ring, from) \
> > @@ -51,10 +51,10 @@ struct  intel_hw_status_page {
> >  	if (!dev_priv->semaphore_obj) { \
> >  		break; \
> >  	} \
> > -	ring->semaphore.signal_ggtt[RCS] = GEN8_SIGNAL_OFFSET(RCS); \
> > -	ring->semaphore.signal_ggtt[VCS] = GEN8_SIGNAL_OFFSET(VCS); \
> > -	ring->semaphore.signal_ggtt[BCS] = GEN8_SIGNAL_OFFSET(BCS); \
> > -	ring->semaphore.signal_ggtt[VECS] = GEN8_SIGNAL_OFFSET(VECS); \
> > +	ring->semaphore.signal_ggtt[RCS] = GEN8_SIGNAL_OFFSET(ring, RCS); \
> > +	ring->semaphore.signal_ggtt[VCS] = GEN8_SIGNAL_OFFSET(ring, VCS); \
> > +	ring->semaphore.signal_ggtt[BCS] = GEN8_SIGNAL_OFFSET(ring, BCS); \
> > +	ring->semaphore.signal_ggtt[VECS] = GEN8_SIGNAL_OFFSET(ring, VECS); \
> >  	ring->semaphore.mbox[RCS] = GEN8_WAIT_OFFSET(ring, RCS); \
> >  	ring->semaphore.mbox[VCS] = GEN8_WAIT_OFFSET(ring, VCS); \
> >  	ring->semaphore.mbox[BCS] = GEN8_WAIT_OFFSET(ring, BCS); \
> > -- 
> > 1.8.5.3
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx at lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Ville Syrjälä
> Intel OTC

-- 
Ben Widawsky, Intel Open Source Technology Center



More information about the Intel-gfx mailing list