[Mesa-dev] [PATCH] intel: Fix 3DSTATE_CONSTANT buffer decoding.

[Lionel Landwerlin]

On 02/05/18 17:45, Kenneth Graunke wrote:
> First, this was iterating over the 3DSTATE_CONSTANT_* instruction
> but trying to process fields of the 3DSTATE_CONSTANT_BODY substructure.
>
> Secondly, the fields have been called Buffer[0] and Read Length[0],
> for a while now, and we were not handling the subscripts correctly.
> ---
>   src/intel/common/gen_batch_decoder.c | 40 +++++++++++++++++-----------
>   1 file changed, 25 insertions(+), 15 deletions(-)
>
> diff --git a/src/intel/common/gen_batch_decoder.c b/src/intel/common/gen_batch_decoder.c
> index dd78e07827e..c059b194974 100644
> --- a/src/intel/common/gen_batch_decoder.c
> +++ b/src/intel/common/gen_batch_decoder.c
> @@ -543,31 +543,41 @@ static void
>   decode_3dstate_constant(struct gen_batch_decode_ctx *ctx, const uint32_t *p)
>   {
>      struct gen_group *inst = gen_spec_find_instruction(ctx->spec, p);
> +   struct gen_group *body =
> +      gen_spec_find_struct(ctx->spec, "3DSTATE_CONSTANT_BODY");
>   
>      uint32_t read_length[4];
>      struct gen_batch_decode_bo buffer[4];
>      memset(buffer, 0, sizeof(buffer));
>   
> -   int rlidx = 0, bidx = 0;
> +   struct gen_field_iterator outer;
> +   gen_field_iterator_init(&outer, inst, p, 0, false);
> +   while (gen_field_iterator_next(&outer)) {
> +      if (outer.struct_desc != body)
> +         continue;
>   
> -   struct gen_field_iterator iter;
> -   gen_field_iterator_init(&iter, inst, p, 0, false);
> -   while (gen_field_iterator_next(&iter)) {
> -      if (strcmp(iter.name, "Read Length") == 0) {
> -         read_length[rlidx++] = iter.raw_value;
> -      } else if (strcmp(iter.name, "Buffer") == 0) {
> -         buffer[bidx++] = ctx_get_bo(ctx, iter.raw_value);
> +      struct gen_field_iterator iter;
> +      gen_field_iterator_init(&iter, body, &outer.p[outer.start_bit / 32],
> +                              0, false);
> +
> +      while (gen_field_iterator_next(&iter)) {
> +         int idx;
> +         if (sscanf(iter.name, "Read Length[%d]", &idx) == 1) {
> +            read_length[idx] = iter.raw_value;
> +         } else if (sscanf(iter.name, "Buffer[%d]", &idx) == 1) {
> +            buffer[idx] = ctx_get_bo(ctx, iter.raw_value);
> +         }
>         }
> -   }
>   
> -   for (int i = 0; i < 4; i++) {
> -      if (read_length[i] == 0 || buffer[i].map == NULL)
> -         continue;

I'm kind of thinking we could get rid of the read_length[4] buffer[4] 
arrays and just put that into the outer loop.
What do you think?

Either way this is :

Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin at intel.com>

> +      for (int i = 0; i < 4; i++) {
> +         if (read_length[i] == 0 || buffer[i].map == NULL)
> +            continue;
>   
> -      unsigned size = read_length[i] * 32;
> -      fprintf(ctx->fp, "constant buffer %d, size %u\n", i, size);
> +         unsigned size = read_length[i] * 32;
> +         fprintf(ctx->fp, "constant buffer %d, size %u\n", i, size);
>   
> -      ctx_print_buffer(ctx, buffer[i], size, 0);
> +         ctx_print_buffer(ctx, buffer[i], size, 0);
> +      }
>      }
>   }
>