On Thu, Sep 6, 2018 at 5:35 AM Lionel Landwerlin <
lionel.g.landwer...@intel.com> wrote:
> Some commands like MI_BATCH_BUFFER_START have this indicator.
>
> Signed-off-by: Lionel Landwerlin <lionel.g.landwer...@intel.com>
> ---
> src/intel/common/gen_batch_decoder.c | 34 ++++++++++--------
> src/intel/common/gen_decoder.h | 3 +-
> src/intel/tools/aubinator.c | 23 ++++++++----
> src/intel/tools/aubinator_error_decode.c | 2 +-
> src/intel/tools/aubinator_viewer.cpp | 4 +--
> src/intel/tools/aubinator_viewer.h | 4 +--
> src/intel/tools/aubinator_viewer_decoder.cpp | 35 +++++++++++--------
> src/mesa/drivers/dri/i965/intel_batchbuffer.c | 2 +-
> 8 files changed, 65 insertions(+), 42 deletions(-)
>
> diff --git a/src/intel/common/gen_batch_decoder.c
> b/src/intel/common/gen_batch_decoder.c
> index 13daffeeedd..50c031e9164 100644
> --- a/src/intel/common/gen_batch_decoder.c
> +++ b/src/intel/common/gen_batch_decoder.c
> @@ -33,6 +33,7 @@ gen_batch_decode_ctx_init(struct gen_batch_decode_ctx
> *ctx,
> FILE *fp, enum gen_batch_decode_flags flags,
> const char *xml_path,
> struct gen_batch_decode_bo (*get_bo)(void *,
> + bool,
> uint64_t),
> unsigned (*get_state_size)(void *, uint32_t),
> void *user_data)
> @@ -76,7 +77,7 @@ ctx_print_group(struct gen_batch_decode_ctx *ctx,
> }
>
> static struct gen_batch_decode_bo
> -ctx_get_bo(struct gen_batch_decode_ctx *ctx, uint64_t addr)
> +ctx_get_bo(struct gen_batch_decode_ctx *ctx, bool ppgtt, uint64_t addr)
> {
> if (gen_spec_get_gen(ctx->spec) >= gen_make_gen(8,0)) {
> /* On Broadwell and above, we have 48-bit addresses which consume
> two
> @@ -88,7 +89,7 @@ ctx_get_bo(struct gen_batch_decode_ctx *ctx, uint64_t
> addr)
> addr &= (~0ull >> 16);
> }
>
> - struct gen_batch_decode_bo bo = ctx->get_bo(ctx->user_data, addr);
> + struct gen_batch_decode_bo bo = ctx->get_bo(ctx->user_data, ppgtt,
> addr);
>
> if (gen_spec_get_gen(ctx->spec) >= gen_make_gen(8,0))
> bo.addr &= (~0ull >> 16);
> @@ -128,7 +129,7 @@ ctx_disassemble_program(struct gen_batch_decode_ctx
> *ctx,
> uint32_t ksp, const char *type)
> {
> uint64_t addr = ctx->instruction_base + ksp;
> - struct gen_batch_decode_bo bo = ctx_get_bo(ctx, addr);
> + struct gen_batch_decode_bo bo = ctx_get_bo(ctx, true, addr);
> if (!bo.map)
> return;
>
> @@ -230,7 +231,7 @@ dump_binding_table(struct gen_batch_decode_ctx *ctx,
> uint32_t offset, int count)
> }
>
> struct gen_batch_decode_bo bind_bo =
> - ctx_get_bo(ctx, ctx->surface_base + offset);
> + ctx_get_bo(ctx, true, ctx->surface_base + offset);
>
> if (bind_bo.map == NULL) {
> fprintf(ctx->fp, " binding table unavailable\n");
> @@ -243,7 +244,7 @@ dump_binding_table(struct gen_batch_decode_ctx *ctx,
> uint32_t offset, int count)
> continue;
>
> uint64_t addr = ctx->surface_base + pointers[i];
> - struct gen_batch_decode_bo bo = ctx_get_bo(ctx, addr);
> + struct gen_batch_decode_bo bo = ctx_get_bo(ctx, true, addr);
> uint32_t size = strct->dw_length * 4;
>
> if (pointers[i] % 32 != 0 ||
> @@ -266,7 +267,7 @@ dump_samplers(struct gen_batch_decode_ctx *ctx,
> uint32_t offset, int count)
> count = update_count(ctx, offset, strct->dw_length, 4);
>
> uint64_t state_addr = ctx->dynamic_base + offset;
> - struct gen_batch_decode_bo bo = ctx_get_bo(ctx, state_addr);
> + struct gen_batch_decode_bo bo = ctx_get_bo(ctx, true, state_addr);
> const void *state_map = bo.map;
>
> if (state_map == NULL) {
> @@ -309,7 +310,7 @@ handle_media_interface_descriptor_load(struct
> gen_batch_decode_ctx *ctx,
> }
>
> uint64_t desc_addr = ctx->dynamic_base + descriptor_offset;
> - struct gen_batch_decode_bo bo = ctx_get_bo(ctx, desc_addr);
> + struct gen_batch_decode_bo bo = ctx_get_bo(ctx, true, desc_addr);
> const void *desc_map = bo.map;
>
> if (desc_map == NULL) {
> @@ -378,7 +379,7 @@ handle_3dstate_vertex_buffers(struct
> gen_batch_decode_ctx *ctx,
> } else if (strcmp(vbs_iter.name, "Buffer Pitch") == 0) {
> pitch = vbs_iter.raw_value;
> } else if (strcmp(vbs_iter.name, "Buffer Starting Address") ==
> 0) {
> - vb = ctx_get_bo(ctx, vbs_iter.raw_value);
> + vb = ctx_get_bo(ctx, true, vbs_iter.raw_value);
> } else if (strcmp(vbs_iter.name, "Buffer Size") == 0) {
> vb_size = vbs_iter.raw_value;
> ready = true;
> @@ -430,7 +431,7 @@ handle_3dstate_index_buffer(struct
> gen_batch_decode_ctx *ctx,
> if (strcmp(iter.name, "Index Format") == 0) {
> format = iter.raw_value;
> } else if (strcmp(iter.name, "Buffer Starting Address") == 0) {
> - ib = ctx_get_bo(ctx, iter.raw_value);
> + ib = ctx_get_bo(ctx, true, iter.raw_value);
> } else if (strcmp(iter.name, "Buffer Size") == 0) {
> ib_size = iter.raw_value;
> }
> @@ -588,7 +589,7 @@ decode_3dstate_constant(struct gen_batch_decode_ctx
> *ctx, const uint32_t *p)
> if (read_length[i] == 0)
> continue;
>
> - struct gen_batch_decode_bo buffer = ctx_get_bo(ctx,
> read_addr[i]);
> + struct gen_batch_decode_bo buffer = ctx_get_bo(ctx, true,
> read_addr[i]);
> if (!buffer.map) {
> fprintf(ctx->fp, "constant buffer %d unavailable\n", i);
> continue;
> @@ -654,7 +655,7 @@ decode_dynamic_state_pointers(struct
> gen_batch_decode_ctx *ctx,
> }
>
> uint64_t state_addr = ctx->dynamic_base + state_offset;
> - struct gen_batch_decode_bo bo = ctx_get_bo(ctx, state_addr);
> + struct gen_batch_decode_bo bo = ctx_get_bo(ctx, true, state_addr);
> const void *state_map = bo.map;
>
> if (state_map == NULL) {
> @@ -858,21 +859,26 @@ gen_print_batch(struct gen_batch_decode_ctx *ctx,
> }
>
> if (strcmp(inst_name, "MI_BATCH_BUFFER_START") == 0) {
> - struct gen_batch_decode_bo next_batch = {};
> + uint64_t next_batch_addr;
> + bool ppgtt = false;
> bool second_level;
> struct gen_field_iterator iter;
> gen_field_iterator_init(&iter, inst, p, 0, false);
> while (gen_field_iterator_next(&iter)) {
> if (strcmp(iter.name, "Batch Buffer Start Address") == 0) {
> - next_batch = ctx_get_bo(ctx, iter.raw_value);
> + next_batch_addr = iter.raw_value;
> } else if (strcmp(iter.name, "Second Level Batch Buffer") ==
> 0) {
> second_level = iter.raw_value;
> + } else if (strcmp(iter.name, "Address Space Indicator") ==
> 0) {
> + ppgtt = iter.raw_value;
> }
> }
>
> + struct gen_batch_decode_bo next_batch = ctx_get_bo(ctx, ppgtt,
> next_batch_addr);
> +
> if (next_batch.map == NULL) {
> fprintf(ctx->fp, "Secondary batch at 0x%08"PRIx64"
> unavailable\n",
> - next_batch.addr);
> + next_batch_addr);
>
Why did this have to change? It either doesn't matter or it's subtle and
very important.
> } else {
> gen_print_batch(ctx, next_batch.map, next_batch.size,
> next_batch.addr);
> diff --git a/src/intel/common/gen_decoder.h
> b/src/intel/common/gen_decoder.h
> index a80c50b6647..ac1e199005e 100644
> --- a/src/intel/common/gen_decoder.h
> +++ b/src/intel/common/gen_decoder.h
> @@ -209,7 +209,7 @@ struct gen_batch_decode_ctx {
> * If the given address is inside a buffer, the map pointer should be
> * offset accordingly so it points at the data corresponding to
> address.
> */
> - struct gen_batch_decode_bo (*get_bo)(void *user_data, uint64_t
> address);
> + struct gen_batch_decode_bo (*get_bo)(void *user_data, bool ppgtt,
> uint64_t address);
> unsigned (*get_state_size)(void *user_data,
> uint32_t
> offset_from_dynamic_state_base_addr);
> void *user_data;
> @@ -232,6 +232,7 @@ void gen_batch_decode_ctx_init(struct
> gen_batch_decode_ctx *ctx,
> FILE *fp, enum gen_batch_decode_flags
> flags,
> const char *xml_path,
> struct gen_batch_decode_bo (*get_bo)(void
> *,
> + bool,
>
> uint64_t),
>
> unsigned (*get_state_size)(void *,
> uint32_t),
> diff --git a/src/intel/tools/aubinator.c b/src/intel/tools/aubinator.c
> index 55672fa073c..55519f13e5e 100644
> --- a/src/intel/tools/aubinator.c
> +++ b/src/intel/tools/aubinator.c
> @@ -118,6 +118,15 @@ aubinator_init(void *user_data, int aub_pci_id, const
> char *app_name)
> fprintf(outfile, "\n");
> }
>
> +static struct gen_batch_decode_bo
> +get_bo(void *user_data, bool ppgtt, uint64_t addr)
> +{
> + if (ppgtt)
> + return aub_mem_get_ppgtt_bo(user_data, addr);
> + else
> + return aub_mem_get_ggtt_bo(user_data, addr);
> +}
> +
> static void
> handle_execlist_write(void *user_data, enum gen_engine engine, uint64_t
> context_descriptor)
> {
> @@ -140,11 +149,7 @@ handle_execlist_write(void *user_data, enum
> gen_engine engine, uint64_t context_
> assert(ring_bo.size > 0);
> void *commands = (uint8_t *)ring_bo.map + (ring_buffer_start -
> ring_bo.addr);
>
> - if (context_descriptor & 0x100 /* ppgtt */) {
> - batch_ctx.get_bo = aub_mem_get_ppgtt_bo;
> - } else {
> - batch_ctx.get_bo = aub_mem_get_ggtt_bo;
> - }
> + batch_ctx.get_bo = get_bo;
>
Is this going to break older platforms? There are a lot of hard-coded
"true"s above and I'd hate to cause problems on platforms that don't have a
pptgg.
>
> (void)engine; /* TODO */
> gen_print_batch(&batch_ctx, commands, ring_buffer_tail -
> ring_buffer_head,
> @@ -152,12 +157,18 @@ handle_execlist_write(void *user_data, enum
> gen_engine engine, uint64_t context_
> aub_mem_clear_bo_maps(&mem);
> }
>
> +static struct gen_batch_decode_bo
> +get_legacy_bo(void *user_data, bool ppgtt, uint64_t addr)
> +{
> + return aub_mem_get_ggtt_bo(user_data, addr);
> +}
> +
> static void
> handle_ring_write(void *user_data, enum gen_engine engine,
> const void *data, uint32_t data_len)
> {
> batch_ctx.user_data = &mem;
> - batch_ctx.get_bo = aub_mem_get_ggtt_bo;
> + batch_ctx.get_bo = get_legacy_bo;
>
> gen_print_batch(&batch_ctx, data, data_len, 0);
>
> diff --git a/src/intel/tools/aubinator_error_decode.c
> b/src/intel/tools/aubinator_error_decode.c
> index d7f9d9f2af1..8cf4909f528 100644
> --- a/src/intel/tools/aubinator_error_decode.c
> +++ b/src/intel/tools/aubinator_error_decode.c
> @@ -385,7 +385,7 @@ static int ascii85_decode(const char *in, uint32_t
> **out, bool inflate)
> }
>
> static struct gen_batch_decode_bo
> -get_gen_batch_bo(void *user_data, uint64_t address)
> +get_gen_batch_bo(void *user_data, bool ppgtt, uint64_t address)
> {
> for (int s = 0; s < num_sections; s++) {
> if (sections[s].gtt_offset <= address &&
> diff --git a/src/intel/tools/aubinator_viewer.cpp
> b/src/intel/tools/aubinator_viewer.cpp
> index e29bccb192a..c96f9a9cea3 100644
> --- a/src/intel/tools/aubinator_viewer.cpp
> +++ b/src/intel/tools/aubinator_viewer.cpp
> @@ -673,11 +673,11 @@ batch_edit_address(void *user_data, uint64_t
> address, uint32_t len)
> }
>
> static struct gen_batch_decode_bo
> -batch_get_bo(void *user_data, uint64_t address)
> +batch_get_bo(void *user_data, bool ppgtt, uint64_t address)
> {
> struct batch_window *window = (struct batch_window *) user_data;
>
> - if (window->uses_ppgtt)
> + if (window->uses_ppgtt && ppgtt)
> return aub_mem_get_ppgtt_bo(&window->mem, address);
> else
> return aub_mem_get_ggtt_bo(&window->mem, address);
> diff --git a/src/intel/tools/aubinator_viewer.h
> b/src/intel/tools/aubinator_viewer.h
> index 4a030efc0d0..cdbd9c98f39 100644
> --- a/src/intel/tools/aubinator_viewer.h
> +++ b/src/intel/tools/aubinator_viewer.h
> @@ -56,7 +56,7 @@ struct aub_decode_urb_stage_state {
> };
>
> struct aub_viewer_decode_ctx {
> - struct gen_batch_decode_bo (*get_bo)(void *user_data, uint64_t
> address);
> + struct gen_batch_decode_bo (*get_bo)(void *user_data, bool ppgtt,
> uint64_t address);
> unsigned (*get_state_size)(void *user_data,
> uint32_t
> offset_from_dynamic_state_base_addr);
>
> @@ -86,7 +86,7 @@ void aub_viewer_decode_ctx_init(struct
> aub_viewer_decode_ctx *ctx,
> struct aub_viewer_decode_cfg *decode_cfg,
> struct gen_spec *spec,
> struct gen_disasm *disasm,
> - struct gen_batch_decode_bo (*get_bo)(void
> *, uint64_t),
> + struct gen_batch_decode_bo (*get_bo)(void
> *, bool, uint64_t),
> unsigned (*get_state_size)(void *,
> uint32_t),
> void *user_data);
>
> diff --git a/src/intel/tools/aubinator_viewer_decoder.cpp
> b/src/intel/tools/aubinator_viewer_decoder.cpp
> index c3a1de30a18..58d4f0fa5d7 100644
> --- a/src/intel/tools/aubinator_viewer_decoder.cpp
> +++ b/src/intel/tools/aubinator_viewer_decoder.cpp
> @@ -33,7 +33,7 @@ aub_viewer_decode_ctx_init(struct aub_viewer_decode_ctx
> *ctx,
> struct aub_viewer_decode_cfg *decode_cfg,
> struct gen_spec *spec,
> struct gen_disasm *disasm,
> - struct gen_batch_decode_bo (*get_bo)(void *,
> uint64_t),
> + struct gen_batch_decode_bo (*get_bo)(void *,
> bool, uint64_t),
> unsigned (*get_state_size)(void *, uint32_t),
> void *user_data)
> {
> @@ -86,7 +86,7 @@ aub_viewer_print_group(struct aub_viewer_decode_ctx *ctx,
> }
>
> static struct gen_batch_decode_bo
> -ctx_get_bo(struct aub_viewer_decode_ctx *ctx, uint64_t addr)
> +ctx_get_bo(struct aub_viewer_decode_ctx *ctx, bool ppgtt, uint64_t addr)
> {
> if (gen_spec_get_gen(ctx->spec) >= gen_make_gen(8,0)) {
> /* On Broadwell and above, we have 48-bit addresses which consume
> two
> @@ -98,7 +98,7 @@ ctx_get_bo(struct aub_viewer_decode_ctx *ctx, uint64_t
> addr)
> addr &= (~0ull >> 16);
> }
>
> - struct gen_batch_decode_bo bo = ctx->get_bo(ctx->user_data, addr);
> + struct gen_batch_decode_bo bo = ctx->get_bo(ctx->user_data, ppgtt,
> addr);
>
> if (gen_spec_get_gen(ctx->spec) >= gen_make_gen(8,0))
> bo.addr &= (~0ull >> 16);
> @@ -138,7 +138,7 @@ ctx_disassemble_program(struct aub_viewer_decode_ctx
> *ctx,
> uint32_t ksp, const char *type)
> {
> uint64_t addr = ctx->instruction_base + ksp;
> - struct gen_batch_decode_bo bo = ctx_get_bo(ctx, addr);
> + struct gen_batch_decode_bo bo = ctx_get_bo(ctx, true, addr);
> if (!bo.map) {
> ImGui::TextColored(ctx->cfg->missing_color, "Shader unavailable");
> return;
> @@ -188,7 +188,7 @@ dump_binding_table(struct aub_viewer_decode_ctx *ctx,
> uint32_t offset, int count
> }
>
> struct gen_batch_decode_bo bind_bo =
> - ctx_get_bo(ctx, ctx->surface_base + offset);
> + ctx_get_bo(ctx, true, ctx->surface_base + offset);
>
> if (bind_bo.map == NULL) {
> ImGui::TextColored(ctx->cfg->missing_color,
> @@ -203,7 +203,7 @@ dump_binding_table(struct aub_viewer_decode_ctx *ctx,
> uint32_t offset, int count
> continue;
>
> uint64_t addr = ctx->surface_base + pointers[i];
> - struct gen_batch_decode_bo bo = ctx_get_bo(ctx, addr);
> + struct gen_batch_decode_bo bo = ctx_get_bo(ctx, true, addr);
> uint32_t size = strct->dw_length * 4;
>
> if (pointers[i] % 32 != 0 ||
> @@ -227,7 +227,7 @@ dump_samplers(struct aub_viewer_decode_ctx *ctx,
> uint32_t offset, int count)
> count = update_count(ctx, offset, strct->dw_length, 4);
>
> uint64_t state_addr = ctx->dynamic_base + offset;
> - struct gen_batch_decode_bo bo = ctx_get_bo(ctx, state_addr);
> + struct gen_batch_decode_bo bo = ctx_get_bo(ctx, true, state_addr);
> const uint8_t *state_map = (const uint8_t *) bo.map;
>
> if (state_map == NULL) {
> @@ -271,7 +271,7 @@ handle_media_interface_descriptor_load(struct
> aub_viewer_decode_ctx *ctx,
> }
>
> uint64_t desc_addr = ctx->dynamic_base + descriptor_offset;
> - struct gen_batch_decode_bo bo = ctx_get_bo(ctx, desc_addr);
> + struct gen_batch_decode_bo bo = ctx_get_bo(ctx, true, desc_addr);
> const uint32_t *desc_map = (const uint32_t *) bo.map;
>
> if (desc_map == NULL) {
> @@ -343,7 +343,7 @@ handle_3dstate_vertex_buffers(struct
> aub_viewer_decode_ctx *ctx,
> pitch = vbs_iter.raw_value;
> } else if (strcmp(vbs_iter.name, "Buffer Starting Address") ==
> 0) {
> buffer_addr = vbs_iter.raw_value;
> - vb = ctx_get_bo(ctx, buffer_addr);
> + vb = ctx_get_bo(ctx, true, buffer_addr);
> } else if (strcmp(vbs_iter.name, "Buffer Size") == 0) {
> vb_size = vbs_iter.raw_value;
> ready = true;
> @@ -395,7 +395,7 @@ handle_3dstate_index_buffer(struct
> aub_viewer_decode_ctx *ctx,
> format = iter.raw_value;
> } else if (strcmp(iter.name, "Buffer Starting Address") == 0) {
> buffer_addr = iter.raw_value;
> - ib = ctx_get_bo(ctx, buffer_addr);
> + ib = ctx_get_bo(ctx, true, buffer_addr);
> } else if (strcmp(iter.name, "Buffer Size") == 0) {
> ib_size = iter.raw_value;
> }
> @@ -546,7 +546,7 @@ decode_3dstate_constant(struct aub_viewer_decode_ctx
> *ctx,
> if (read_length[i] == 0)
> continue;
>
> - struct gen_batch_decode_bo buffer = ctx_get_bo(ctx,
> read_addr[i]);
> + struct gen_batch_decode_bo buffer = ctx_get_bo(ctx, true,
> read_addr[i]);
> if (!buffer.map) {
> ImGui::TextColored(ctx->cfg->missing_color,
> "constant buffer %d unavailable
> addr=0x%08" PRIx64,
> @@ -620,7 +620,7 @@ decode_dynamic_state_pointers(struct
> aub_viewer_decode_ctx *ctx,
> }
>
> uint64_t state_addr = ctx->dynamic_base + state_offset;
> - struct gen_batch_decode_bo bo = ctx_get_bo(ctx, state_addr);
> + struct gen_batch_decode_bo bo = ctx_get_bo(ctx, true, state_addr);
> const uint8_t *state_map = (const uint8_t *) bo.map;
>
> if (state_map == NULL) {
> @@ -922,22 +922,27 @@ aub_viewer_render_batch(struct aub_viewer_decode_ctx
> *ctx,
> }
>
> if (strcmp(inst_name, "MI_BATCH_BUFFER_START") == 0) {
> - struct gen_batch_decode_bo next_batch = {};
> + uint64_t next_batch_addr;
> + bool ppgtt = false;
> bool second_level;
> struct gen_field_iterator iter;
> gen_field_iterator_init(&iter, inst, p, 0, false);
> while (gen_field_iterator_next(&iter)) {
> if (strcmp(iter.name, "Batch Buffer Start Address") == 0) {
> - next_batch = ctx_get_bo(ctx, iter.raw_value);
> + next_batch_addr = iter.raw_value;
> } else if (strcmp(iter.name, "Second Level Batch Buffer") ==
> 0) {
> second_level = iter.raw_value;
> + } else if (strcmp(iter.name, "Address Space Indicator") ==
> 0) {
> + ppgtt = iter.raw_value;
> }
> }
>
> + struct gen_batch_decode_bo next_batch = ctx_get_bo(ctx, ppgtt,
> next_batch_addr);
> +
> if (next_batch.map == NULL) {
> ImGui::TextColored(ctx->cfg->missing_color,
> "Secondary batch at 0x%08" PRIx64 "
> unavailable",
> - next_batch.addr);
> + next_batch_addr);
> } else {
> aub_viewer_render_batch(ctx, next_batch.map, next_batch.size,
> next_batch.addr);
> diff --git a/src/mesa/drivers/dri/i965/intel_batchbuffer.c
> b/src/mesa/drivers/dri/i965/intel_batchbuffer.c
> index 65d2c64e319..7ab9da663da 100644
> --- a/src/mesa/drivers/dri/i965/intel_batchbuffer.c
> +++ b/src/mesa/drivers/dri/i965/intel_batchbuffer.c
> @@ -79,7 +79,7 @@ dump_validation_list(struct intel_batchbuffer *batch)
> }
>
> static struct gen_batch_decode_bo
> -decode_get_bo(void *v_brw, uint64_t address)
> +decode_get_bo(void *v_brw, bool ppgtt, uint64_t address)
> {
> struct brw_context *brw = v_brw;
> struct intel_batchbuffer *batch = &brw->batch;
> --
> 2.19.0.rc1
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
