intel: gen-decoder: rework how we handle groups

The current way of handling groups doesn't seem to be able to handle
MI_LOAD_REGISTER_* with more than one register. This change reworks
the way we handle groups by building a traversal list on loading the
GENXML files.

Let's say you have

Instruction {
  Field0
  Field1
  Field2
  Group0 (count=2) {
    Field0-0
    Field0-1
  }
  Group1 (count=4) {
    Field1-0
    Field1-1
  }
}

We build of linked on load that goes :

Instruction -> Group0 -> Group1

All of those are gen_group structures, making the traversal trivial.
We just need to iterate groups for the right number of timers (count
field in genxml).

The more fancy case is when you have only a single group of unknown
size (count=0). In that case we keep on reading that group for as long
as we're within the DWordLength of that instruction.

Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Reviewed-by: Rafael Antognolli <rafael.antognolli@intel.com>

3 files changed, 161 insertions, 104 deletions

diff --git a/src/intel/common/gen_decoder.c b/src/intel/common/gen_decoder.c
index 8d0bf3cee6..ba3a5139ae 100644
--- a/src/intel/common/gen_decoder.c
+++ b/src/intel/common/gen_decoder.c
@@ -39,6 +39,8 @@
 
 #define XML_BUFFER_SIZE 4096
 
+#define MAX(a, b) ((a) < (b) ? (b) : (a))
+
 #define MAKE_GEN(major, minor) ( ((major) << 8) | (minor) )
 
 struct gen_spec {
@@ -68,9 +70,6 @@ struct parser_context {
    struct gen_group *group;
    struct gen_enum *enoom;
 
-   int nfields;
-   struct gen_field *fields[128];
-
    int nvalues;
    struct gen_value *values[256];
 
@@ -179,8 +178,32 @@ xzalloc(size_t s)
    return fail_on_null(zalloc(s));
 }
 
+static void
+get_group_offset_count(const char **atts, uint32_t *offset, uint32_t *count,
+                       uint32_t *size, bool *variable)
+{
+   char *p;
+   int i;
+
+   for (i = 0; atts[i]; i += 2) {
+      if (strcmp(atts[i], "count") == 0) {
+         *count = strtoul(atts[i + 1], &p, 0);
+         if (*count == 0)
+            *variable = true;
+      } else if (strcmp(atts[i], "start") == 0) {
+         *offset = strtoul(atts[i + 1], &p, 0);
+      } else if (strcmp(atts[i], "size") == 0) {
+         *size = strtoul(atts[i + 1], &p, 0);
+      }
+   }
+   return;
+}
+
 static struct gen_group *
-create_group(struct parser_context *ctx, const char *name, const char **atts)
+create_group(struct parser_context *ctx,
+             const char *name,
+             const char **atts,
+             struct gen_group *parent)
 {
    struct gen_group *group;
 
@@ -191,9 +214,17 @@ create_group(struct parser_context *ctx, const char *name, const char **atts)
    group->spec = ctx->spec;
    group->group_offset = 0;
    group->group_count = 0;
-   group->variable_offset = 0;
    group->variable = false;
 
+   if (parent) {
+      group->parent = parent;
+      get_group_offset_count(atts,
+                             &group->group_offset,
+                             &group->group_count,
+                             &group->group_size,
+                             &group->variable);
+   }
+
    return group;
 }
 
@@ -212,28 +243,6 @@ create_enum(struct parser_context *ctx, const char *name, const char **atts)
 }
 
 static void
-get_group_offset_count(struct parser_context *ctx, const char *name,
-                       const char **atts, uint32_t *offset, uint32_t *count,
-                       uint32_t *elem_size, bool *variable)
-{
-   char *p;
-   int i;
-
-   for (i = 0; atts[i]; i += 2) {
-      if (strcmp(atts[i], "count") == 0) {
-         *count = strtoul(atts[i + 1], &p, 0);
-         if (*count == 0)
-            *variable = true;
-      } else if (strcmp(atts[i], "start") == 0) {
-         *offset = strtoul(atts[i + 1], &p, 0);
-      } else if (strcmp(atts[i], "size") == 0) {
-         *elem_size = strtoul(atts[i + 1], &p, 0);
-      }
-   }
-   return;
-}
-
-static void
 get_register_offset(const char **atts, uint32_t *offset)
 {
    char *p;
@@ -325,7 +334,7 @@ string_to_type(struct parser_context *ctx, const char *s)
 }
 
 static struct gen_field *
-create_field(struct parser_context *ctx, const char **atts, int group_idx)
+create_field(struct parser_context *ctx, const char **atts)
 {
    struct gen_field *field;
    char *p;
@@ -335,21 +344,11 @@ create_field(struct parser_context *ctx, const char **atts, int group_idx)
 
    for (i = 0; atts[i]; i += 2) {
       if (strcmp(atts[i], "name") == 0)
-         if (ctx->group->elem_size == 0) {
-            field->name = xstrdup(atts[i + 1]);
-         } else {
-            field->name =
-               ralloc_asprintf(NULL, "%s[%d]", atts[i + 1], group_idx);
-         }
+         field->name = xstrdup(atts[i + 1]);
       else if (strcmp(atts[i], "start") == 0)
-         field->start = ctx->group->group_offset+strtoul(atts[i + 1], &p, 0);
+         field->start = strtoul(atts[i + 1], &p, 0);
       else if (strcmp(atts[i], "end") == 0) {
-         field->end = ctx->group->group_offset+strtoul(atts[i + 1], &p, 0);
-         if (ctx->group->elem_size > 0) {
-            ctx->group->group_offset = field->end+1;
-            if (ctx->group->variable)
-               ctx->group->variable_offset = ctx->group->group_offset;
-         }
+         field->end = strtoul(atts[i + 1], &p, 0);
       } else if (strcmp(atts[i], "type") == 0)
          field->type = string_to_type(ctx, atts[i + 1]);
       else if (strcmp(atts[i], "default") == 0 &&
@@ -378,6 +377,21 @@ create_value(struct parser_context *ctx, const char **atts)
 }
 
 static void
+create_and_append_field(struct parser_context *ctx,
+                        const char **atts)
+{
+   if (ctx->group->nfields == ctx->group->fields_size) {
+      ctx->group->fields_size = MAX(ctx->group->fields_size * 2, 2);
+      ctx->group->fields =
+         (struct gen_field **) realloc(ctx->group->fields,
+                                       sizeof(ctx->group->fields[0]) *
+                                       ctx->group->fields_size);
+   }
+
+   ctx->group->fields[ctx->group->nfields++] = create_field(ctx, atts);
+}
+
+static void
 start_element(void *data, const char *element_name, const char **atts)
 {
    struct parser_context *ctx = data;
@@ -411,23 +425,27 @@ start_element(void *data, const char *element_name, const char **atts)
       ctx->spec->gen = MAKE_GEN(major, minor);
    } else if (strcmp(element_name, "instruction") == 0 ||
               strcmp(element_name, "struct") == 0) {
-      ctx->group = create_group(ctx, name, atts);
+      ctx->group = create_group(ctx, name, atts, NULL);
    } else if (strcmp(element_name, "register") == 0) {
-      ctx->group = create_group(ctx, name, atts);
+      ctx->group = create_group(ctx, name, atts, NULL);
       get_register_offset(atts, &ctx->group->register_offset);
    } else if (strcmp(element_name, "group") == 0) {
-      get_group_offset_count(ctx, name, atts, &ctx->group->group_offset,
-                             &ctx->group->group_count, &ctx->group->elem_size,
-                             &ctx->group->variable);
+      struct gen_group *previous_group = ctx->group;
+      while (previous_group->next)
+         previous_group = previous_group->next;
+
+      struct gen_group *group = create_group(ctx, "", atts, ctx->group);
+      previous_group->next = group;
+      ctx->group = group;
    } else if (strcmp(element_name, "field") == 0) {
-      for (int g = 0; g < MAX2(ctx->group->group_count, 1); g++) {
-         ctx->fields[ctx->nfields++] = create_field(ctx, atts, g);
-      }
+      create_and_append_field(ctx, atts);
    } else if (strcmp(element_name, "enum") == 0) {
       ctx->enoom = create_enum(ctx, name, atts);
    } else if (strcmp(element_name, "value") == 0) {
       ctx->values[ctx->nvalues++] = create_value(ctx, atts);
+      assert(ctx->nvalues < ARRAY_SIZE(ctx->values));
    }
+
 }
 
 static void
@@ -439,14 +457,9 @@ end_element(void *data, const char *name)
    if (strcmp(name, "instruction") == 0 ||
        strcmp(name, "struct") == 0 ||
        strcmp(name, "register") == 0) {
-      size_t size = ctx->nfields * sizeof(ctx->fields[0]);
       struct gen_group *group = ctx->group;
 
-      group->fields = xzalloc(size);
-      group->nfields = ctx->nfields;
-      memcpy(group->fields, ctx->fields, size);
-      ctx->nfields = 0;
-      ctx->group = NULL;
+      ctx->group = ctx->group->parent;
 
       for (int i = 0; i < group->nfields; i++) {
          if (group->fields[i]->start >= 16 &&
@@ -465,12 +478,15 @@ end_element(void *data, const char *name)
          spec->structs[spec->nstructs++] = group;
       else if (strcmp(name, "register") == 0)
          spec->registers[spec->nregisters++] = group;
+
+      assert(spec->ncommands < ARRAY_SIZE(spec->commands));
+      assert(spec->nstructs < ARRAY_SIZE(spec->structs));
+      assert(spec->nregisters < ARRAY_SIZE(spec->registers));
    } else if (strcmp(name, "group") == 0) {
-      ctx->group->group_offset = 0;
-      ctx->group->group_count = 0;
+      ctx->group = ctx->group->parent;
    } else if (strcmp(name, "field") == 0) {
-      assert(ctx->nfields > 0);
-      struct gen_field *field = ctx->fields[ctx->nfields - 1];
+      assert(ctx->group->nfields > 0);
+      struct gen_field *field = ctx->group->fields[ctx->group->nfields - 1];
       size_t size = ctx->nvalues * sizeof(ctx->values[0]);
       field->inline_enum.values = xzalloc(size);
       field->inline_enum.nvalues = ctx->nvalues;
@@ -756,12 +772,11 @@ gen_field_iterator_init(struct gen_field_iterator *iter,
                         const uint32_t *p,
                         bool print_colors)
 {
+   memset(iter, 0, sizeof(*iter));
+
    iter->group = group;
    iter->p = p;
-   iter->i = 0;
    iter->print_colors = print_colors;
-   iter->repeat = false;
-   iter->addr_inc = 0;
 }
 
 static const char *
@@ -775,6 +790,70 @@ gen_get_enum_name(struct gen_enum *e, uint64_t value)
    return NULL;
 }
 
+static bool
+iter_more_fields(const struct gen_field_iterator *iter)
+{
+   return iter->field_iter < iter->group->nfields;
+}
+
+static uint32_t
+iter_group_offset_bits(const struct gen_field_iterator *iter,
+                       uint32_t group_iter)
+{
+   return iter->group->group_offset + (group_iter * iter->group->group_size);
+}
+
+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);
+   } else {
+      return (iter->group_iter + 1) < iter->group->group_count ||
+         iter->group->next != NULL;
+   }
+}
+
+static void
+iter_advance_group(struct gen_field_iterator *iter)
+{
+   if (iter->group->variable)
+      iter->group_iter++;
+   else {
+      if ((iter->group_iter + 1) < iter->group->group_count) {
+         iter->group_iter++;
+      } else {
+         iter->group = iter->group->next;
+         iter->group_iter = 0;
+      }
+   }
+
+   iter->field_iter = 0;
+}
+
+static bool
+iter_advance_field(struct gen_field_iterator *iter)
+{
+   while (!iter_more_fields(iter)) {
+      if (!iter_more_groups(iter))
+         return false;
+
+      iter_advance_group(iter);
+   }
+
+   iter->field = iter->group->fields[iter->field_iter++];
+   if (iter->field->name)
+       strncpy(iter->name, iter->field->name, sizeof(iter->name));
+   else
+      memset(iter->name, 0, sizeof(iter->name));
+   iter->dword = iter_group_offset_bits(iter, iter->group_iter) / 32 +
+      iter->field->start / 32;
+   iter->struct_desc = NULL;
+
+   return true;
+}
+
 bool
 gen_field_iterator_next(struct gen_field_iterator *iter)
 {
@@ -783,22 +862,8 @@ gen_field_iterator_next(struct gen_field_iterator *iter)
       float f;
    } v;
 
-   if (iter->i == iter->group->nfields) {
-      if (iter->group->group_size > 0) {
-         int iter_length = iter->group->elem_size;
-
-         iter->group->group_size -= iter_length / 32;
-         iter->addr_inc += iter_length;
-         iter->dword = (iter->field->start + iter->addr_inc) / 32;
-         return true;
-      }
+   if (!iter_advance_field(iter))
       return false;
-   }
-
-   iter->field = iter->group->fields[iter->i++];
-   iter->name = iter->field->name;
-   iter->dword = iter->field->start / 32;
-   iter->struct_desc = NULL;
 
    if ((iter->field->end - iter->field->start) > 32)
       v.qw = ((uint64_t) iter->p[iter->dword+1] << 32) | iter->p[iter->dword];
@@ -864,6 +929,12 @@ gen_field_iterator_next(struct gen_field_iterator *iter)
    }
    }
 
+   if (strlen(iter->group->name) == 0) {
+      int length = strlen(iter->name);
+      snprintf(iter->name + length, sizeof(iter->name) - length,
+               "[%i]", iter->group_iter);
+   }
+
    if (enum_name) {
       int length = strlen(iter->value);
       snprintf(iter->value + length, sizeof(iter->value) - length,
@@ -912,7 +983,6 @@ 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;
-            print_dword_header(outfile, &iter, struct_offset);
             gen_print_group(outfile, iter.struct_desc, struct_offset,
                             &p[iter.dword], color);
          }
diff --git a/src/intel/common/gen_decoder.h b/src/intel/common/gen_decoder.h
index 4f4295ff95..cfc9f2e3f1 100644
--- a/src/intel/common/gen_decoder.h
+++ b/src/intel/common/gen_decoder.h
@@ -53,28 +53,33 @@ struct gen_enum *gen_spec_find_enum(struct gen_spec *spec, const char *name);
 
 struct gen_field_iterator {
    struct gen_group *group;
-   const char *name;
+   char name[128];
    char value[128];
    struct gen_group *struct_desc;
    const uint32_t *p;
    int dword; /**< current field starts at &p[dword] */
-   int i;
+
+   int field_iter;
+   int group_iter;
+
    struct gen_field *field;
    bool print_colors;
-   bool repeat;
-   uint32_t addr_inc;
 };
 
 struct gen_group {
    struct gen_spec *spec;
    char *name;
-   int nfields;
+
    struct gen_field **fields;
+   uint32_t nfields;
+   uint32_t fields_size;
+
    uint32_t group_offset, group_count;
-   uint32_t elem_size;
-   uint32_t variable_offset;
-   bool variable;
    uint32_t group_size;
+   bool variable;
+
+   struct gen_group *parent;
+   struct gen_group *next;
 
    uint32_t opcode_mask;
    uint32_t opcode;
diff --git a/src/intel/tools/aubinator.c b/src/intel/tools/aubinator.c
index 53b2a27fa9..f481fbe007 100644
--- a/src/intel/tools/aubinator.c
+++ b/src/intel/tools/aubinator.c
@@ -95,23 +95,6 @@ valid_offset(uint32_t offset)
    return offset < gtt_end;
 }
 
-/**
- * Set group variable size for groups with count="0".
- *
- * By default the group size is fixed and not needed because the struct
- * describing a group knows the number of elements. However, for groups with
- * count="0" we have a variable number of elements, and the struct describing
- * the group only includes one of them. So we calculate the remaining size of
- * the group based on the size we get here, and the offset after the last
- * element added to the group.
- */
-static void
-group_set_size(struct gen_group *strct, uint32_t size)
-{
-   if (strct->variable && strct->elem_size)
-      strct->group_size = size - (strct->variable_offset / 32);
-}
-
 static void
 decode_group(struct gen_group *strct, const uint32_t *p, int starting_dword)
 {
@@ -740,7 +723,6 @@ parse_commands(struct gen_spec *spec, uint32_t *cmds, int size, int engine)
               gen_group_get_name(inst), reset_color);
 
       if (option_full_decode) {
-         group_set_size(inst, length);
          decode_group(inst, p, 0);
 
          for (i = 0; i < ARRAY_LENGTH(custom_handlers); i++) {