subject:"\[Mesa\-dev\] \[PATCH\] intel\: Fix 3DSTATE

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

2018-05-02 Thread Kenneth Graunke

On Wednesday, May 2, 2018 9:50:33 AM PDT Lionel Landwerlin wrote:
> 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(, inst, p, 0, false);
> > +   while (gen_field_iterator_next()) {
> > +  if (outer.struct_desc != body)
> > + continue;
> >   
> > -   struct gen_field_iterator iter;
> > -   gen_field_iterator_init(, inst, p, 0, false);
> > -   while (gen_field_iterator_next()) {
> > -  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(, body, [outer.start_bit / 32],
> > +  0, false);
> > +
> > +  while (gen_field_iterator_next()) {
> > + int idx;
> > + if (sscanf(iter.name, "Read Length[%d]", ) == 1) {
> > +read_length[idx] = iter.raw_value;
> > + } else if (sscanf(iter.name, "Buffer[%d]", ) == 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 

FTR, I was confused by this suggestion so I talked to Lionel on IRC, he
was thinking was that we could print them out as we fetch the buffers...

But the iteration order is

   - Read Length[0]
   - Read Length[1]
   - Read Length[2]
   - Read Length[3]
   - Buffer[0]
   - Buffer[1]
   - Buffer[2]
   - Buffer[3]

So we'd at least need the read_length array, still.  We decided to
just leave it as it is.


signature.asc
Description: This is a digitally signed message part.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

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

2018-05-02 Thread 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(, inst, p, 0, false);
+   while (gen_field_iterator_next()) {
+  if (outer.struct_desc != body)
+ continue;
  
-   struct gen_field_iterator iter;

-   gen_field_iterator_init(, inst, p, 0, false);
-   while (gen_field_iterator_next()) {
-  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(, body, [outer.start_bit / 32],
+  0, false);
+
+  while (gen_field_iterator_next()) {
+ int idx;
+ if (sscanf(iter.name, "Read Length[%d]", ) == 1) {
+read_length[idx] = iter.raw_value;
+ } else if (sscanf(iter.name, "Buffer[%d]", ) == 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 


+  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);
+  }
 }
  }
  



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

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

2018-05-02 Thread Kenneth Graunke

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(, inst, p, 0, false);
+   while (gen_field_iterator_next()) {
+  if (outer.struct_desc != body)
+ continue;
 
-   struct gen_field_iterator iter;
-   gen_field_iterator_init(, inst, p, 0, false);
-   while (gen_field_iterator_next()) {
-  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(, body, [outer.start_bit / 32],
+  0, false);
+
+  while (gen_field_iterator_next()) {
+ int idx;
+ if (sscanf(iter.name, "Read Length[%d]", ) == 1) {
+read_length[idx] = iter.raw_value;
+ } else if (sscanf(iter.name, "Buffer[%d]", ) == 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;
+  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);
+  }
}
 }
 
-- 
2.17.0

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

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

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

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

3 matches

Site Navigation

Mail list logo

Footer information