diff mbox

[10/13] drm/i915/bdw: collect semaphore error state

Message ID 1392877166-9195-10-git-send-email-benjamin.widawsky@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Ben Widawsky Feb. 20, 2014, 6:19 a.m. UTC
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)

v4: Fix semaphore offset calculation during error state collection
(Ville)

Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
---
 drivers/gpu/drm/i915/i915_drv.h         |  1 +
 drivers/gpu/drm/i915/i915_gpu_error.c   | 52 ++++++++++++++++++++++++++++++---
 drivers/gpu/drm/i915/intel_ringbuffer.h | 12 ++++----
 3 files changed, 55 insertions(+), 10 deletions(-)

Comments

Ville Syrjala Feb. 24, 2014, 1:08 p.m. UTC | #1
On Wed, Feb 19, 2014 at 10:19:23PM -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)
> 
> v4: Fix semaphore offset calculation during error state collection
> (Ville)
> 
> Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
> ---
>  drivers/gpu/drm/i915/i915_drv.h         |  1 +
>  drivers/gpu/drm/i915/i915_gpu_error.c   | 52 ++++++++++++++++++++++++++++++---
>  drivers/gpu/drm/i915/intel_ringbuffer.h | 12 ++++----
>  3 files changed, 55 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 028ce5a..9b86874 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -322,6 +322,7 @@ struct drm_i915_error_state {
>  	u64 fence[I915_MAX_NUM_FENCES];
>  	struct intel_overlay_error_state *overlay;
>  	struct intel_display_error_state *display;
> +	struct drm_i915_error_object *semaphore_obj;
>  
>  	struct drm_i915_error_ring {
>  		bool valid;
> diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c
> index 2b64493..25fc2ea 100644
> --- a/drivers/gpu/drm/i915/i915_gpu_error.c
> +++ b/drivers/gpu/drm/i915/i915_gpu_error.c
> @@ -310,6 +310,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) {
> @@ -360,8 +361,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,
> @@ -436,6 +435,19 @@ int i915_error_state_to_str(struct drm_i915_error_state_buf *m,
>  		}
>  	}
>  
> +	obj = error->semaphore_obj;
> +	if (obj) {

Still won't pass under Chris's consitency radar.

> +		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]);
> +		}
> +	}
> +
>  	if (error->overlay)
>  		intel_overlay_print_error_state(m, error->overlay);
>  
> @@ -506,6 +518,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);
> @@ -797,6 +810,33 @@ i915_error_first_batchbuffer(struct drm_i915_private *dev_priv,
>  	return NULL;
>  }
>  
> +static void gen8_record_semaphore_state(struct drm_i915_private *dev_priv,
> +					struct drm_i915_error_state *error,
> +					struct intel_ring_buffer *ring,
> +					struct drm_i915_error_ring *ering)
> +{
> +	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) & PAGE_MASK;

(... & PAGE_MASK) / 4;

> +		u32 *tmp = error->semaphore_obj->pages[0];
> +
> +		ering->semaphore_mboxes[i] = tmp[signal_offset];
> +		ering->semaphore_seqno[i] = ring->semaphore.sync_seqno[i];
> +	}
> +}
> +
>  static void gen6_record_semaphore_state(struct drm_i915_private *dev_priv,
>  					struct intel_ring_buffer *ring,
>  					struct drm_i915_error_ring *ering)
> @@ -814,6 +854,7 @@ static void gen6_record_semaphore_state(struct drm_i915_private *dev_priv,
>  }
>  
>  static void i915_record_ring_state(struct drm_device *dev,
> +				   struct drm_i915_error_state *error,
>  				   struct intel_ring_buffer *ring,
>  				   struct drm_i915_error_ring *ering)
>  {
> @@ -822,7 +863,10 @@ static void i915_record_ring_state(struct drm_device *dev,
>  	if (INTEL_INFO(dev)->gen >= 6) {
>  		ering->rc_psmi = I915_READ(ring->mmio_base + 0x50);
>  		ering->fault_reg = I915_READ(RING_FAULT_REG(ring));
> -		gen6_record_semaphore_state(dev_priv, ring, ering);
> +		if (INTEL_INFO(dev)->gen >= 8)
> +			gen8_record_semaphore_state(dev_priv, error, ring, ering);
> +		else
> +			gen6_record_semaphore_state(dev_priv, ring, ering);
>  	}
>  
>  	if (INTEL_INFO(dev)->gen >= 4) {
> @@ -948,7 +992,7 @@ static void i915_gem_record_rings(struct drm_device *dev,
>  
>  		error->ring[i].valid = true;
>  
> -		i915_record_ring_state(dev, ring, &error->ring[i]);
> +		i915_record_ring_state(dev, error, ring, &error->ring[i]);
>  
>  		error->ring[i].batchbuffer =
>  			i915_error_first_batchbuffer(dev_priv, ring);
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
> index 5d56c95..01aad54 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.signal_ggtt[ring->id] = MI_SEMAPHORE_SYNC_INVALID; \
>  	} while(0)
>  
> -- 
> 1.9.0
Ben Widawsky April 29, 2014, 9:43 p.m. UTC | #2
On Mon, Feb 24, 2014 at 03:08:04PM +0200, Ville Syrjälä wrote:
> On Wed, Feb 19, 2014 at 10:19:23PM -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)
> > 
> > v4: Fix semaphore offset calculation during error state collection
> > (Ville)
> > 
> > Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
> > ---
> >  drivers/gpu/drm/i915/i915_drv.h         |  1 +
> >  drivers/gpu/drm/i915/i915_gpu_error.c   | 52 ++++++++++++++++++++++++++++++---
> >  drivers/gpu/drm/i915/intel_ringbuffer.h | 12 ++++----
> >  3 files changed, 55 insertions(+), 10 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> > index 028ce5a..9b86874 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -322,6 +322,7 @@ struct drm_i915_error_state {
> >  	u64 fence[I915_MAX_NUM_FENCES];
> >  	struct intel_overlay_error_state *overlay;
> >  	struct intel_display_error_state *display;
> > +	struct drm_i915_error_object *semaphore_obj;
> >  
> >  	struct drm_i915_error_ring {
> >  		bool valid;
> > diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c
> > index 2b64493..25fc2ea 100644
> > --- a/drivers/gpu/drm/i915/i915_gpu_error.c
> > +++ b/drivers/gpu/drm/i915/i915_gpu_error.c
> > @@ -310,6 +310,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) {
> > @@ -360,8 +361,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,
> > @@ -436,6 +435,19 @@ int i915_error_state_to_str(struct drm_i915_error_state_buf *m,
> >  		}
> >  	}
> >  
> > +	obj = error->semaphore_obj;
> > +	if (obj) {
> 
> Still won't pass under Chris's consitency radar.
> 
> > +		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]);
> > +		}
> > +	}
> > +
> >  	if (error->overlay)
> >  		intel_overlay_print_error_state(m, error->overlay);
> >  
> > @@ -506,6 +518,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);
> > @@ -797,6 +810,33 @@ i915_error_first_batchbuffer(struct drm_i915_private *dev_priv,
> >  	return NULL;
> >  }
> >  
> > +static void gen8_record_semaphore_state(struct drm_i915_private *dev_priv,
> > +					struct drm_i915_error_state *error,
> > +					struct intel_ring_buffer *ring,
> > +					struct drm_i915_error_ring *ering)
> > +{
> > +	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) & PAGE_MASK;
> 
> (... & PAGE_MASK) / 4;
> 

Nice catch.

> > +		u32 *tmp = error->semaphore_obj->pages[0];
> > +
> > +		ering->semaphore_mboxes[i] = tmp[signal_offset];
> > +		ering->semaphore_seqno[i] = ring->semaphore.sync_seqno[i];
> > +	}
> > +}
> > +
> >  static void gen6_record_semaphore_state(struct drm_i915_private *dev_priv,
> >  					struct intel_ring_buffer *ring,
> >  					struct drm_i915_error_ring *ering)
> > @@ -814,6 +854,7 @@ static void gen6_record_semaphore_state(struct drm_i915_private *dev_priv,
> >  }
> >  
> >  static void i915_record_ring_state(struct drm_device *dev,
> > +				   struct drm_i915_error_state *error,
> >  				   struct intel_ring_buffer *ring,
> >  				   struct drm_i915_error_ring *ering)
> >  {
> > @@ -822,7 +863,10 @@ static void i915_record_ring_state(struct drm_device *dev,
> >  	if (INTEL_INFO(dev)->gen >= 6) {
> >  		ering->rc_psmi = I915_READ(ring->mmio_base + 0x50);
> >  		ering->fault_reg = I915_READ(RING_FAULT_REG(ring));
> > -		gen6_record_semaphore_state(dev_priv, ring, ering);
> > +		if (INTEL_INFO(dev)->gen >= 8)
> > +			gen8_record_semaphore_state(dev_priv, error, ring, ering);
> > +		else
> > +			gen6_record_semaphore_state(dev_priv, ring, ering);
> >  	}
> >  
> >  	if (INTEL_INFO(dev)->gen >= 4) {
> > @@ -948,7 +992,7 @@ static void i915_gem_record_rings(struct drm_device *dev,
> >  
> >  		error->ring[i].valid = true;
> >  
> > -		i915_record_ring_state(dev, ring, &error->ring[i]);
> > +		i915_record_ring_state(dev, error, ring, &error->ring[i]);
> >  
> >  		error->ring[i].batchbuffer =
> >  			i915_error_first_batchbuffer(dev_priv, ring);
> > diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
> > index 5d56c95..01aad54 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.signal_ggtt[ring->id] = MI_SEMAPHORE_SYNC_INVALID; \
> >  	} while(0)
> >  
> > -- 
> > 1.9.0
> 
> -- 
> Ville Syrjälä
> Intel OTC
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 028ce5a..9b86874 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -322,6 +322,7 @@  struct drm_i915_error_state {
 	u64 fence[I915_MAX_NUM_FENCES];
 	struct intel_overlay_error_state *overlay;
 	struct intel_display_error_state *display;
+	struct drm_i915_error_object *semaphore_obj;
 
 	struct drm_i915_error_ring {
 		bool valid;
diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c
index 2b64493..25fc2ea 100644
--- a/drivers/gpu/drm/i915/i915_gpu_error.c
+++ b/drivers/gpu/drm/i915/i915_gpu_error.c
@@ -310,6 +310,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) {
@@ -360,8 +361,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,
@@ -436,6 +435,19 @@  int i915_error_state_to_str(struct drm_i915_error_state_buf *m,
 		}
 	}
 
+	obj = error->semaphore_obj;
+	if (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]);
+		}
+	}
+
 	if (error->overlay)
 		intel_overlay_print_error_state(m, error->overlay);
 
@@ -506,6 +518,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);
@@ -797,6 +810,33 @@  i915_error_first_batchbuffer(struct drm_i915_private *dev_priv,
 	return NULL;
 }
 
+static void gen8_record_semaphore_state(struct drm_i915_private *dev_priv,
+					struct drm_i915_error_state *error,
+					struct intel_ring_buffer *ring,
+					struct drm_i915_error_ring *ering)
+{
+	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) & PAGE_MASK;
+		u32 *tmp = error->semaphore_obj->pages[0];
+
+		ering->semaphore_mboxes[i] = tmp[signal_offset];
+		ering->semaphore_seqno[i] = ring->semaphore.sync_seqno[i];
+	}
+}
+
 static void gen6_record_semaphore_state(struct drm_i915_private *dev_priv,
 					struct intel_ring_buffer *ring,
 					struct drm_i915_error_ring *ering)
@@ -814,6 +854,7 @@  static void gen6_record_semaphore_state(struct drm_i915_private *dev_priv,
 }
 
 static void i915_record_ring_state(struct drm_device *dev,
+				   struct drm_i915_error_state *error,
 				   struct intel_ring_buffer *ring,
 				   struct drm_i915_error_ring *ering)
 {
@@ -822,7 +863,10 @@  static void i915_record_ring_state(struct drm_device *dev,
 	if (INTEL_INFO(dev)->gen >= 6) {
 		ering->rc_psmi = I915_READ(ring->mmio_base + 0x50);
 		ering->fault_reg = I915_READ(RING_FAULT_REG(ring));
-		gen6_record_semaphore_state(dev_priv, ring, ering);
+		if (INTEL_INFO(dev)->gen >= 8)
+			gen8_record_semaphore_state(dev_priv, error, ring, ering);
+		else
+			gen6_record_semaphore_state(dev_priv, ring, ering);
 	}
 
 	if (INTEL_INFO(dev)->gen >= 4) {
@@ -948,7 +992,7 @@  static void i915_gem_record_rings(struct drm_device *dev,
 
 		error->ring[i].valid = true;
 
-		i915_record_ring_state(dev, ring, &error->ring[i]);
+		i915_record_ring_state(dev, error, ring, &error->ring[i]);
 
 		error->ring[i].batchbuffer =
 			i915_error_first_batchbuffer(dev_priv, ring);
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
index 5d56c95..01aad54 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.signal_ggtt[ring->id] = MI_SEMAPHORE_SYNC_INVALID; \
 	} while(0)