[Mesa-dev] [PATCH] intel: Fix 3DSTATE_CONSTANT buffer decoding.
Kenneth Graunke
kenneth at whitecape.org
Wed May 2 17:12:14 UTC 2018
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(&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>
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.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: This is a digitally signed message part.
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20180502/9464bd2e/attachment.sig>
More information about the mesa-dev
mailing list