Re: [Mesa-dev] [PATCH 1/4] intel/decoder: Clean up field iteration and fix sub-dword fields

[Lionel Landwerlin]

The whole series is :

Reviewed-by: Lionel Landwerlin <lionel.g.landwer...@intel.com>

Thanks for the great cleanup!

-
Lionel

On 24/08/2018 22:40, Jason Ekstrand wrote:

First of all, setting iter->name in advance_field is unnecessary because
it gets set by gen_decode_field which gets called immediately after
gen_decode_field in the one call-site.  Second, we weren't properly
initializing start_bit and end_bit in the initial condition of
gen_field_iterator_next so the first field of a struct would get printed
wrong if it doesn't start on the first bit.  This is fixed by adding a
iter_start_field helper which sets the field and also sets up the other
bits we need.  This fixes decoding of 3DSTATE_SBE_SWIZ.
---
  src/intel/common/gen_decoder.c | 32 ++++++++++++++++----------------
  1 file changed, 16 insertions(+), 16 deletions(-)

diff --git a/src/intel/common/gen_decoder.c b/src/intel/common/gen_decoder.c
index 39da3cadbf8..e9dabeae653 100644
--- a/src/intel/common/gen_decoder.c
+++ b/src/intel/common/gen_decoder.c
@@ -812,6 +812,18 @@ iter_more_groups(const struct gen_field_iterator *iter)
     }
  }

+static void

+iter_start_field(struct gen_field_iterator *iter, struct gen_field *field)
+{
+   iter->field = field;
+
+   int group_member_offset = iter_group_offset_bits(iter, iter->group_iter);
+
+   iter->start_bit = group_member_offset + iter->field->start;
+   iter->end_bit = group_member_offset + iter->field->end;
+   iter->struct_desc = NULL;
+}
+
  static void
  iter_advance_group(struct gen_field_iterator *iter)
  {
@@ -826,32 +838,20 @@ iter_advance_group(struct gen_field_iterator *iter)
        }
     }

- iter->field = iter->group->fields;

+   iter_start_field(iter, iter->group->fields);
  }

static bool

  iter_advance_field(struct gen_field_iterator *iter)
  {
     if (iter_more_fields(iter)) {
-      iter->field = iter->field->next;
+      iter_start_field(iter, iter->field->next);
     } else {
        if (!iter_more_groups(iter))
           return false;

iter_advance_group(iter);

     }
-
-   if (iter->field->name)
-      snprintf(iter->name, sizeof(iter->name), "%s", iter->field->name);
-   else
-      memset(iter->name, 0, sizeof(iter->name));
-
-   int group_member_offset = iter_group_offset_bits(iter, iter->group_iter);
-
-   iter->start_bit = group_member_offset + iter->field->start;
-   iter->end_bit = group_member_offset + iter->field->end;
-   iter->struct_desc = NULL;
-
     return true;
  }

@@ -1006,9 +1006,9 @@ gen_field_iterator_next(struct gen_field_iterator *iter)

     /* Initial condition */
     if (!iter->field) {
        if (iter->group->fields)
-         iter->field = iter->group->fields;
+         iter_start_field(iter, iter->group->fields);
        else
-         iter->field = iter->group->next->fields;
+         iter_start_field(iter, iter->group->next->fields);

bool result = iter_decode_field(iter);

        if (iter->p_end)

_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev