[Mesa-dev] [PATCH 31/33] intel: decoder: decouple decoding from memory pointers
Scott D Phillips
scott.d.phillips at intel.com
Tue Oct 31 20:54:39 UTC 2017
Lionel Landwerlin <lionel.g.landwerlin at intel.com> writes:
> We want to introduce a reader interface for accessing memory, so that
> later on we can use different ways of storing the content of the GTT
> address space that don't involve a pointer to a linear buffer.
I'm kinda sceptical that this is the best way to achieve what you want
here. It strikes me as code that we'll look at in a year and wonder
what's going on.
If I'm understanding, it seems like the essence of what you're going for
here is in the one place where you're using the sub_struct_reader. Maybe
instead of plumbing the reader object through everywhere, you can add a
callback just in gen_print_group for fixing up offsets to pointers, and
then leave everywhere else assuming contiguous memory blocks as today.
> Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin at intel.com>
> ---
> src/intel/common/gen_decoder.c | 75 ++++++++++++++++++++-------
> src/intel/common/gen_decoder.h | 24 +++++++--
> src/intel/tools/aubinator.c | 7 ++-
> src/mesa/drivers/dri/i965/intel_batchbuffer.c | 26 +++++++---
> 4 files changed, 101 insertions(+), 31 deletions(-)
>
> diff --git a/src/intel/common/gen_decoder.c b/src/intel/common/gen_decoder.c
> index 098ff472b37..c3fa150a6ea 100644
> --- a/src/intel/common/gen_decoder.c
> +++ b/src/intel/common/gen_decoder.c
> @@ -807,12 +807,18 @@ iter_group_offset_bits(const struct gen_field_iterator *iter,
> return iter->group->group_offset + (group_iter * iter->group->group_size);
> }
>
> +uint32_t gen_read_dword_from_pointer(void *user_data, uint32_t dword_offset)
> +{
> + return ((uint32_t *) user_data)[dword_offset];
> +}
> +
> static bool
> iter_more_groups(const struct gen_field_iterator *iter)
> {
> if (iter->group->variable) {
> return iter_group_offset_bits(iter, iter->group_iter + 1) <
> - (gen_group_get_length(iter->group, iter->p) * 32);
> + (gen_group_get_length(iter->group,
> + gen_read_dword(iter->reader, 0)) * 32);
> } else {
> return (iter->group_iter + 1) < iter->group->group_count ||
> iter->group->next != NULL;
> @@ -856,17 +862,20 @@ iter_advance_field(struct gen_field_iterator *iter)
>
> static uint64_t
> iter_decode_field_raw(struct gen_field *field,
> - const uint32_t *p,
> - const uint32_t *end)
> + uint32_t dword_offset,
> + uint32_t dword_end,
> + const struct gen_dword_reader *reader)
> {
> uint64_t qw = 0;
>
> if ((field->end - field->start) > 32) {
> - if ((p + 1) < end)
> - qw = ((uint64_t) p[1]) << 32;
> - qw |= p[0];
> + if ((dword_offset + 1) < dword_end) {
> + qw = gen_read_dword(reader, dword_offset + 1);
> + qw <<= 32;
> + }
> + qw |= gen_read_dword(reader, dword_offset);
> } else
> - qw = p[0];
> + qw = gen_read_dword(reader, dword_offset);
>
> qw = field_value(qw, field->start, field->end);
>
> @@ -895,8 +904,8 @@ iter_decode_field(struct gen_field_iterator *iter)
>
> memset(&v, 0, sizeof(v));
>
> - v.qw = iter_decode_field_raw(iter->field,
> - &iter->p[iter->dword], iter->end);
> + v.qw = iter_decode_field_raw(iter->field, iter->dword,
> + iter->dword_end, iter->reader);
>
> const char *enum_name = NULL;
>
> @@ -966,7 +975,7 @@ iter_decode_field(struct gen_field_iterator *iter)
> void
> gen_field_iterator_init(struct gen_field_iterator *iter,
> struct gen_group *group,
> - const uint32_t *p,
> + const struct gen_dword_reader *reader,
> bool print_colors)
> {
> memset(iter, 0, sizeof(*iter));
> @@ -976,8 +985,9 @@ gen_field_iterator_init(struct gen_field_iterator *iter,
> iter->field = group->fields;
> else
> iter->field = group->next->fields;
> - iter->p = p;
> - iter->end = &p[gen_group_get_length(iter->group, p[0])];
> + iter->reader = reader;
> + iter->dword_end = gen_group_get_length(iter->group,
> + gen_read_dword(reader, 0));
> iter->print_colors = print_colors;
>
> iter_decode_field(iter);
> @@ -997,10 +1007,12 @@ gen_field_iterator_next(struct gen_field_iterator *iter)
> static void
> print_dword_header(FILE *outfile,
> struct gen_field_iterator *iter,
> - uint64_t offset, uint32_t dword)
> + uint64_t offset,
> + uint32_t dword)
> {
> fprintf(outfile, "0x%08"PRIx64": 0x%08x : Dword %d\n",
> - offset + 4 * dword, iter->p[dword], dword);
> + offset + 4 * dword,
> + gen_read_dword(iter->reader, dword), dword);
> }
>
> bool
> @@ -1018,21 +1030,38 @@ gen_field_is_header(struct gen_field *field)
> }
>
> void gen_field_decode(struct gen_field *field,
> - const uint32_t *p, const uint32_t *end,
> + const struct gen_dword_reader *reader,
> union gen_field_value *value)
> {
> + uint32_t length = gen_group_get_length(field->parent,
> + gen_read_dword(reader, 0));
> uint32_t dword = field->start / 32;
> - value->u64 = iter_decode_field_raw(field, &p[dword], end);
> + value->u64 = iter_decode_field_raw(field, dword, length, reader);
> +}
> +
> +struct sub_struct_reader {
> + struct gen_dword_reader base;
> + const struct gen_dword_reader *reader;
> + uint32_t struct_offset;
> +};
> +
> +static uint32_t
> +read_struct_dword(void *user_data, uint32_t dword_offset)
> +{
> + struct sub_struct_reader *reader = user_data;
> + return gen_read_dword(reader->reader, reader->struct_offset + dword_offset);
> }
>
> void
> gen_print_group(FILE *outfile, struct gen_group *group,
> - uint64_t offset, const uint32_t *p, bool color)
> + uint64_t offset,
> + const struct gen_dword_reader *reader,
> + bool color)
> {
> struct gen_field_iterator iter;
> int last_dword = -1;
>
> - gen_field_iterator_init(&iter, group, p, color);
> + gen_field_iterator_init(&iter, group, reader, color);
> do {
> if (last_dword != iter.dword) {
> for (int i = last_dword + 1; i <= iter.dword; i++)
> @@ -1043,8 +1072,16 @@ gen_print_group(FILE *outfile, struct gen_group *group,
> fprintf(outfile, " %s: %s\n", iter.name, iter.value);
> if (iter.struct_desc) {
> uint64_t struct_offset = offset + 4 * iter.dword;
> + struct sub_struct_reader struct_reader = {
> + .base = {
> + .user_data = &struct_reader,
> + .read = read_struct_dword,
> + },
> + .reader = reader,
> + .struct_offset = 4 * iter.dword,
> + };
> gen_print_group(outfile, iter.struct_desc, struct_offset,
> - &p[iter.dword], color);
> + &struct_reader.base, color);
> }
> }
> } while (gen_field_iterator_next(&iter));
> diff --git a/src/intel/common/gen_decoder.h b/src/intel/common/gen_decoder.h
> index b6cb735753d..0fbe385c71c 100644
> --- a/src/intel/common/gen_decoder.h
> +++ b/src/intel/common/gen_decoder.h
> @@ -39,6 +39,19 @@ struct gen_group;
> struct gen_field;
> union gen_field_value;
>
> +struct gen_dword_reader {
> + void *user_data;
> + uint32_t (*read)(void *user_data, uint32_t dword_offset);
> +};
> +
> +uint32_t gen_read_dword_from_pointer(void *user_data, uint32_t dword_offset);
> +
> +static inline uint32_t
> +gen_read_dword(const struct gen_dword_reader *reader, uint32_t dword_offset)
> +{
> + return reader->read(reader->user_data, dword_offset);
> +}
> +
> static inline uint32_t gen_make_gen(uint32_t major, uint32_t minor)
> {
> return (major << 8) | minor;
> @@ -63,7 +76,7 @@ struct gen_field *gen_group_find_field(struct gen_group *group, const char *name
>
> bool gen_field_is_header(struct gen_field *field);
> void gen_field_decode(struct gen_field *field,
> - const uint32_t *p, const uint32_t *end,
> + const struct gen_dword_reader *reader,
> union gen_field_value *value);
>
> struct gen_field_iterator {
> @@ -71,9 +84,9 @@ struct gen_field_iterator {
> char name[128];
> char value[128];
> struct gen_group *struct_desc;
> - const uint32_t *p;
> - const uint32_t *end;
> + const struct gen_dword_reader *reader;
> int dword; /**< current field starts at &p[dword] */
> + int dword_end;
>
> int group_iter;
>
> @@ -174,14 +187,15 @@ struct gen_field {
>
> void gen_field_iterator_init(struct gen_field_iterator *iter,
> struct gen_group *group,
> - const uint32_t *p,
> + const struct gen_dword_reader *reader,
> bool print_colors);
>
> bool gen_field_iterator_next(struct gen_field_iterator *iter);
>
> void gen_print_group(FILE *out,
> struct gen_group *group,
> - uint64_t offset, const uint32_t *p,
> + uint64_t offset,
> + const struct gen_dword_reader *reader,
> bool color);
>
> #ifdef __cplusplus
> diff --git a/src/intel/tools/aubinator.c b/src/intel/tools/aubinator.c
> index abbebbb462f..0354a32cf43 100644
> --- a/src/intel/tools/aubinator.c
> +++ b/src/intel/tools/aubinator.c
> @@ -99,8 +99,13 @@ static void
> decode_group(struct gen_group *strct, const uint32_t *p, int starting_dword)
> {
> uint64_t offset = option_print_offsets ? (void *) p - gtt : 0;
> + struct gen_dword_reader reader = {
> + .user_data = (void *) p,
> + .read = gen_read_dword_from_pointer,
> + };
>
> - gen_print_group(outfile, strct, offset, p, option_color == COLOR_ALWAYS);
> + gen_print_group(outfile, strct, offset, &reader,
> + option_color == COLOR_ALWAYS);
> }
>
> static void
> diff --git a/src/mesa/drivers/dri/i965/intel_batchbuffer.c b/src/mesa/drivers/dri/i965/intel_batchbuffer.c
> index 0f6759d55aa..065bc249732 100644
> --- a/src/mesa/drivers/dri/i965/intel_batchbuffer.c
> +++ b/src/mesa/drivers/dri/i965/intel_batchbuffer.c
> @@ -405,8 +405,11 @@ decode_struct(struct brw_context *brw, struct gen_spec *spec,
> return;
>
> fprintf(stderr, "%s\n", struct_name);
> - gen_print_group(stderr, group, gtt_offset + offset,
> - &data[offset / 4], color);
> + struct gen_dword_reader reader = {
> + .read = gen_read_dword_from_pointer,
> + .user_data = &data[offset / 4],
> + };
> + gen_print_group(stderr, group, gtt_offset + offset, &reader, color);
> }
>
> static void
> @@ -422,8 +425,11 @@ decode_structs(struct brw_context *brw, struct gen_spec *spec,
> int entries = brw_state_batch_size(brw, offset) / struct_size;
> for (int i = 0; i < entries; i++) {
> fprintf(stderr, "%s %d\n", struct_name, i);
> - gen_print_group(stderr, group, gtt_offset + offset,
> - &data[(offset + i * struct_size) / 4], color);
> + struct gen_dword_reader reader = {
> + .read = gen_read_dword_from_pointer,
> + .user_data = &data[(offset + i * struct_size) / 4],
> + };
> + gen_print_group(stderr, group, gtt_offset + offset, &reader, color);
> }
> }
>
> @@ -468,7 +474,11 @@ do_batch_dump(struct brw_context *brw)
> fprintf(stderr, "%s0x%08"PRIx64": 0x%08x: %-80s%s\n", header_color,
> offset, p[0], gen_group_get_name(inst), reset_color);
>
> - gen_print_group(stderr, inst, offset, p, color);
> + struct gen_dword_reader reader = {
> + .read = gen_read_dword_from_pointer,
> + .user_data = p,
> + };
> + gen_print_group(stderr, inst, offset, &reader, color);
>
> switch (gen_group_get_opcode(inst) >> 16) {
> case _3DSTATE_PIPELINED_POINTERS:
> @@ -509,8 +519,12 @@ do_batch_dump(struct brw_context *brw)
> uint32_t *bt_pointers = &state[bt_offset / 4];
> for (int i = 0; i < bt_entries; i++) {
> fprintf(stderr, "SURFACE_STATE - BTI = %d\n", i);
> + struct gen_dword_reader reader = {
> + .read = gen_read_dword_from_pointer,
> + .user_data = &state[bt_pointers[i] / 4],
> + };
> gen_print_group(stderr, group, state_gtt_offset + bt_pointers[i],
> - &state[bt_pointers[i] / 4], color);
> + &reader, color);
> }
> break;
> }
> --
> 2.15.0.rc2
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
More information about the mesa-dev
mailing list