Re: [PATCH 1/8] drm/i915: Print the batchbuffer offset next to BBADDR in error state

[Joonas Lahtinen <joonas.lahtinen@linux.intel.com>]

On to, 2016-06-09 at 12:29 +0100, Chris Wilson wrote:
> It is useful when looking at captured error states to check the recorded
> BBADDR register (the address of the last batchbuffer instruction loaded)
> against the expected offset of the batch buffer, and so do a quick check
> that (a) the capture is true or (b) HEAD hasn't wandered off into the
> badlands.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>  drivers/gpu/drm/i915/i915_drv.h       |  1 +
>  drivers/gpu/drm/i915/i915_gpu_error.c | 12 +++++++++++-
>  2 files changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 48cf9dfbe4ac..b333cf4923bc 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -541,6 +541,7 @@ struct drm_i915_error_state {
>  		struct drm_i915_error_object {
>  			int page_count;
>  			u64 gtt_offset;
> +			u64 gtt_size;
>  			u32 *pages[0];
>  		} *ringbuffer, *batchbuffer, *wa_batchbuffer, *ctx, *hws_page;
>  
> diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c
> index 76d63e047fae..ab0824b1ce6d 100644
> --- a/drivers/gpu/drm/i915/i915_gpu_error.c
> +++ b/drivers/gpu/drm/i915/i915_gpu_error.c
> @@ -249,6 +249,13 @@ static void i915_ring_error_state(struct drm_i915_error_state_buf *m,
>  	err_printf(m, "  IPEIR: 0x%08x\n", ring->ipeir);
>  	err_printf(m, "  IPEHR: 0x%08x\n", ring->ipehr);
>  	err_printf(m, "  INSTDONE: 0x%08x\n", ring->instdone);
> +	if (ring->batchbuffer) {
> +		u64 start = ring->batchbuffer->gtt_offset;
> +		u64 end = start + ring->batchbuffer->gtt_size;
> +		err_printf(m, "  batch: [0x%08x %08x, 0x%08x %08x]\n",
> +			   upper_32_bits(start), lower_32_bits(start),
> +			   upper_32_bits(end), lower_32_bits(end));

I'm not very fond of this 32/32 split done repeatedly and manually, but
it seems to be used already.

Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>

> +	}
>  	if (INTEL_INFO(dev)->gen >= 4) {
>  		err_printf(m, "  BBADDR: 0x%08x %08x\n", (u32)(ring->bbaddr>>32), (u32)ring->bbaddr);
>  		err_printf(m, "  BB_STATE: 0x%08x\n", ring->bbstate);
> @@ -655,7 +662,10 @@ i915_error_object_create(struct drm_i915_private *dev_priv,
>  	if (dst == NULL)
>  		return NULL;
>  
> -	reloc_offset = dst->gtt_offset = vma->node.start;
> +	dst->gtt_offset = vma->node.start;
> +	dst->gtt_size = vma->node.size;
> +
> +	reloc_offset = dst->gtt_offset;

This is completely unrelated fixup. You could split it with my R-b in
both.

>  	use_ggtt = (src->cache_level == I915_CACHE_NONE &&
>  		   (vma->bound & GLOBAL_BIND) &&
>  		   reloc_offset + num_pages * PAGE_SIZE <= ggtt->mappable_end);
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx