diff mbox

[v3,9/9] drm/i915/error: Capture WA ctx batch in error state

Message ID 1456502817-10048-1-git-send-email-arun.siluvery@linux.intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

arun.siluvery@linux.intel.com Feb. 26, 2016, 4:06 p.m. UTC
From Gen8 onwards we apply ctx workarounds using special batch buffers that
execute during save/restore, good to have them in error state.

v2: use wa_ctx->size and print only size values (Mika)

v3: simplify conditions when recording and freeing object (Chris)

Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Mika Kuoppala <mika.kuoppala@intel.com>
Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com>
Signed-off-by: Arun Siluvery <arun.siluvery@linux.intel.com>
---

Applied r-b received for v2, this version just simplifies conditions so I
am assuming it still holds.

 drivers/gpu/drm/i915/i915_drv.h       |  2 +-
 drivers/gpu/drm/i915/i915_gpu_error.c | 27 +++++++++++++++++++++++++++
 2 files changed, 28 insertions(+), 1 deletion(-)

Comments

Tvrtko Ursulin March 1, 2016, 10:34 a.m. UTC | #1
On 26/02/16 16:06, Arun Siluvery wrote:
>  From Gen8 onwards we apply ctx workarounds using special batch buffers that
> execute during save/restore, good to have them in error state.
>
> v2: use wa_ctx->size and print only size values (Mika)
>
> v3: simplify conditions when recording and freeing object (Chris)

Tried to merge it but it doesn't apply. I think you need to rebase and 
resend.

Plus checkpatch is not fully happy so please consider the below as well:

> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Mika Kuoppala <mika.kuoppala@intel.com>
> Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com>
> Signed-off-by: Arun Siluvery <arun.siluvery@linux.intel.com>
> ---
>
> Applied r-b received for v2, this version just simplifies conditions so I
> am assuming it still holds.
>
>   drivers/gpu/drm/i915/i915_drv.h       |  2 +-
>   drivers/gpu/drm/i915/i915_gpu_error.c | 27 +++++++++++++++++++++++++++
>   2 files changed, 28 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index e8a47af..b1ba039 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -568,7 +568,7 @@ struct drm_i915_error_state {
>   			bool is_ppgtt;
>   			int page_count;
>   			u32 *pages[0];
> -		} *req_ringbuffer, *hw_ringbuffer, *batchbuffer, *wa_batchbuffer, *ctx, *hws_page;
> +		} *req_ringbuffer, *hw_ringbuffer, *batchbuffer, *wa_batchbuffer, *ctx, *hws_page, *wa_ctx;

Line over 80 chars, not sure how much we care.

>
>   		struct drm_i915_error_request {
>   			u64 ctx_desc;
> diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c
> index f426538..f48788e 100644
> --- a/drivers/gpu/drm/i915/i915_gpu_error.c
> +++ b/drivers/gpu/drm/i915/i915_gpu_error.c
> @@ -659,6 +659,27 @@ int i915_error_state_to_str(struct drm_i915_error_state_buf *m,
>   			}
>   		}
>
> +		if ((obj = error->ring[i].wa_ctx)) {

Assignment in if condition is a check patch error.

> +			u64 wa_ctx_offset = obj->gtt_offset;
> +			u32 *wa_ctx_page = &obj->pages[0][0];
> +			struct intel_engine_cs *ring = &dev_priv->ring[RCS];
> +			u32 wa_ctx_size = (ring->wa_ctx.indirect_ctx.size +
> +					   ring->wa_ctx.per_ctx.size);
> +
> +			err_printf(m, "%s --- WA Ctx batch buffer = 0x%08llx\n",
> +				   dev_priv->ring[i].name, wa_ctx_offset);
> +			offset = 0;
> +			for (elt = 0; elt < wa_ctx_size; elt += 4) {
> +				err_printf(m, "[%04x] %08x %08x %08x %08x\n",
> +					   offset,
> +					   wa_ctx_page[elt],
> +					   wa_ctx_page[elt+1],
> +					   wa_ctx_page[elt+2],
> +					   wa_ctx_page[elt+3]);

Here it suggests spaces around '+'.

> +				offset += 16;
> +			}
> +		}
> +
>   		if ((obj = error->ring[i].ctx)) {
>   			err_printf(m, "%s --- HW Context = 0x%08x\n",
>   				   dev_priv->ring[i].name,
> @@ -752,6 +773,7 @@ static void i915_error_state_free(struct kref *error_ref)
>   		i915_error_object_free(error->ring[i].hws_page);
>   		i915_error_object_free(error->ring[i].ctx);
>   		kfree(error->ring[i].requests);
> +		i915_error_object_free(error->ring[i].wa_ctx);
>   	}
>
>   	i915_error_object_free(error->semaphore_obj);
> @@ -1267,6 +1289,11 @@ static void i915_gem_record_rings(struct drm_device *dev,
>   		error->ring[i].hws_page =
>   			i915_error_ggtt_object_create(dev_priv, ring->status_page.obj);
>
> +		if (ring->wa_ctx.obj) {
> +			error->ring[i].wa_ctx =
> +				i915_error_ggtt_object_create(dev_priv, ring->wa_ctx.obj);

And this is another line over 80-char warning.

> +		}
> +
>   		i915_gem_record_active_context(ring, error, &error->ring[i]);
>
>   		count = 0;
>

Regards,

Tvrtko
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index e8a47af..b1ba039 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -568,7 +568,7 @@  struct drm_i915_error_state {
 			bool is_ppgtt;
 			int page_count;
 			u32 *pages[0];
-		} *req_ringbuffer, *hw_ringbuffer, *batchbuffer, *wa_batchbuffer, *ctx, *hws_page;
+		} *req_ringbuffer, *hw_ringbuffer, *batchbuffer, *wa_batchbuffer, *ctx, *hws_page, *wa_ctx;
 
 		struct drm_i915_error_request {
 			u64 ctx_desc;
diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c
index f426538..f48788e 100644
--- a/drivers/gpu/drm/i915/i915_gpu_error.c
+++ b/drivers/gpu/drm/i915/i915_gpu_error.c
@@ -659,6 +659,27 @@  int i915_error_state_to_str(struct drm_i915_error_state_buf *m,
 			}
 		}
 
+		if ((obj = error->ring[i].wa_ctx)) {
+			u64 wa_ctx_offset = obj->gtt_offset;
+			u32 *wa_ctx_page = &obj->pages[0][0];
+			struct intel_engine_cs *ring = &dev_priv->ring[RCS];
+			u32 wa_ctx_size = (ring->wa_ctx.indirect_ctx.size +
+					   ring->wa_ctx.per_ctx.size);
+
+			err_printf(m, "%s --- WA Ctx batch buffer = 0x%08llx\n",
+				   dev_priv->ring[i].name, wa_ctx_offset);
+			offset = 0;
+			for (elt = 0; elt < wa_ctx_size; elt += 4) {
+				err_printf(m, "[%04x] %08x %08x %08x %08x\n",
+					   offset,
+					   wa_ctx_page[elt],
+					   wa_ctx_page[elt+1],
+					   wa_ctx_page[elt+2],
+					   wa_ctx_page[elt+3]);
+				offset += 16;
+			}
+		}
+
 		if ((obj = error->ring[i].ctx)) {
 			err_printf(m, "%s --- HW Context = 0x%08x\n",
 				   dev_priv->ring[i].name,
@@ -752,6 +773,7 @@  static void i915_error_state_free(struct kref *error_ref)
 		i915_error_object_free(error->ring[i].hws_page);
 		i915_error_object_free(error->ring[i].ctx);
 		kfree(error->ring[i].requests);
+		i915_error_object_free(error->ring[i].wa_ctx);
 	}
 
 	i915_error_object_free(error->semaphore_obj);
@@ -1267,6 +1289,11 @@  static void i915_gem_record_rings(struct drm_device *dev,
 		error->ring[i].hws_page =
 			i915_error_ggtt_object_create(dev_priv, ring->status_page.obj);
 
+		if (ring->wa_ctx.obj) {
+			error->ring[i].wa_ctx =
+				i915_error_ggtt_object_create(dev_priv, ring->wa_ctx.obj);
+		}
+
 		i915_gem_record_active_context(ring, error, &error->ring[i]);
 
 		count = 0;