Re: [PATCH 1/2] aubdump: Fix GTT setup for Gen8+

[Jason Ekstrand <jason@xxxxxxxxxxxxxx>]

> Gen8+ have 64 bit GTT entries, so we need to allocate twice as much
> space for the GTT table in order to cover the same number of GTT
> pages.  Fixes sporadic page-fault crashes on the simulator.
> ---
>  tools/aubdump.c | 21 ++++++++++++++++-----
>  1 file changed, 16 insertions(+), 5 deletions(-)
>
> diff --git a/tools/aubdump.c b/tools/aubdump.c
> index 30dc742..d0774c1 100644
> --- a/tools/aubdump.c
> +++ b/tools/aubdump.c
> @@ -54,7 +54,6 @@ static char *filename;
>  static FILE *file;
>  static int gen = 0;
>  static int verbose = 0;
> -static const uint32_t gtt_size = 0x10000;
>  static bool device_override;
>  static uint32_t device;
> 
> @@ -149,9 +148,18 @@ data_out(const void *data, size_t size)
>      fwrite(data, 1, size, file);
>  }
> 
> +static uint32_t
> +gtt_size(void)
> +{
> +    /* Enough for 64MB assuming 4kB pages. */

This disagrees with a (obviously wrong) comment further down that says we allow 256 MB.

> +    const unsigned entries = 0x4000;
> +    return entries * (gen >= 8 ? 8 : 4);
> +}
> +
>  static void
>  write_header(void)
>  {
> +    const unsigned gtt_entry_size = gen >= 8 ? 8 : 4;

It might be a good idea to make this a helper instead of repeating it here and above.

Other than those two comments, it looks good to me.  Sadly, I can't say I know the aub format well enough to give it a full r-b. :-(

>      uint32_t entry = 0x200003;
> 
>      /* Start with a (required) version packet. */
> @@ -171,11 +179,14 @@ write_header(void)
>            AUB_TRACE_TYPE_NOTYPE | AUB_TRACE_OP_DATA_WRITE);
>      dword_out(0); /* subtype */
>      dword_out(0); /* offset */
> -    dword_out(gtt_size); /* size */
> +    dword_out(gtt_size()); /* size */
>      if (gen >= 8)
>          dword_out(0);
> -    for (uint32_t i = 0; i < gtt_size; i += 4, entry += 0x1000)
> -        dword_out(entry);
> +    for (uint32_t i = 0; i * gtt_entry_size < gtt_size(); i++) {
> +        dword_out(entry + 0x1000 * i);
> +        if (gen >= 8)
> +            dword_out(0);
> +    }
>  }
> 
>  /**
> @@ -332,7 +343,7 @@ dump_execbuffer2(int fd, struct drm_i915_gem_execbuffer2 *execbuffer2)
>      struct drm_i915_gem_exec_object2 *exec_objects =
>          (struct drm_i915_gem_exec_object2 *) (uintptr_t) execbuffer2->buffers_ptr;
>      uint32_t ring_flag = execbuffer2->flags & I915_EXEC_RING_MASK;
> -    uint32_t offset = gtt_size;
> +    uint32_t offset = gtt_size();
>      struct drm_i915_gem_exec_object2 *obj;
>      struct bo *bo, *batch_bo;
>      void *data;
> --
> 2.10.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx