[Mesa-dev] [PATCH 31/33] intel: decoder: decouple decoding from memory pointers
Lionel Landwerlin
lionel.g.landwerlin at intel.com
Tue Oct 31 22:01:04 UTC 2017
On 31/10/17 20:54, Scott D Phillips wrote:
> 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.
First, thanks for you time reviewing this!
I should have stated that in patch 33 I introduce a sparse memory object
that isn't contiguous.
It's based on the data structure described here :
https://en.wikipedia.org/wiki/Hash_array_mapped_trie
The idea is to split the memory into chunks of 4Kb but still make it
look like it's a 64bit address space.
The trie structure allows for reuse of pages at different point in time
without having an actual copy of the whole address space.
Like a couple of pages might have been written by relocations associated
to the first batch buffer, then 10 batches later you override them.
The amount of memory we need to allocate for storing 2 snapshots is just
the modified pages (+ ~12 nodes in the trie but those are less than
300bytes).
That allows the UI to decode 2 batches at the same time as well as all
the associated memory with a small cost.
>
>> 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