On Tue, Sep 20, 2016 at 3:59 PM, Sirisha Gandikota <sirisha.gandik...@intel.com> wrote: > From: Sirisha Gandikota <sirisha.gandik...@intel.com> > > Fixed the way the values that span two Dwords are decoded. > Based on the start and end indices of the field, the Dwords > are fetched and decoded accordingly. > > v2: rename dw to qw in gen_field_iterator_next > and remove extra white space (Anuj) > > v3: change all instances of dw to qw (Anuj) > > Earlier, 64-bit fields (such as most pointers on Gen8+) > weren't decoded correctly. gen_field_iterator_next seemed > to walk one DWord at a time, sets v.dw, and then passes it > to field(). So, even though field() takes a uint64_t, we're > passing it a uint32_t (which gets promoted, so the top 32 > bits will always be zero). This seems pretty bogus... (Ken) > > Signed-off-by: Sirisha Gandikota <sirisha.gandik...@intel.com> > --- > src/intel/tools/decoder.c | 50 > +++++++++++++++++++++++++++++++++++------------ > 1 file changed, 37 insertions(+), 13 deletions(-) > > diff --git a/src/intel/tools/decoder.c b/src/intel/tools/decoder.c > index b5f557c..be3558b 100644 > --- a/src/intel/tools/decoder.c > +++ b/src/intel/tools/decoder.c > @@ -191,6 +191,26 @@ get_register_offset(const char **atts, uint32_t *offset) > return; > } > > +static void > +get_start_end_pos(int *start, int *end) > +{ > + /* start value has to be mod with 32 as we need the relative > + * start position in the first DWord. For the end position, add > + * the length of the field to the start position to get the > + * relative postion in the 64 bit address. > + */ > + if (*end - *start > 32) { > + int len = *end - *start; > + *start = *start % 32; > + *end = *start + len; > + } else { > + *start = *start % 32; > + *end = *end % 32; > + } > + > + return; > +} > + > static inline uint64_t > mask(int start, int end) > { > @@ -204,18 +224,16 @@ mask(int start, int end) > static inline uint64_t > field(uint64_t value, int start, int end) > { > - /* The field values are obtained from the DWord, > - * Hence, get the relative start and end positions > - * by doing a %32 on the start and end positions > - */ > - return (value & mask(start % 32, end % 32)) >> (start % 32); > + get_start_end_pos(&start, &end); > + return (value & mask(start, end)) >> (start); > } > > static inline uint64_t > field_address(uint64_t value, int start, int end) > { > /* no need to right shift for address/offset */ > - return (value & mask(start % 32, end % 32)); > + get_start_end_pos(&start, &end); > + return (value & mask(start, end)); > } > > static struct gen_type > @@ -491,7 +509,7 @@ gen_field_iterator_next(struct gen_field_iterator *iter) > { > struct gen_field *f; > union { > - uint32_t dw; > + uint64_t qw; > float f; > } v; > > @@ -500,20 +518,26 @@ gen_field_iterator_next(struct gen_field_iterator *iter) > > f = iter->group->fields[iter->i++]; > iter->name = f->name; > - v.dw = iter->p[f->start / 32]; > + int index = f->start / 32; > + > + if ((f->end - f->start) > 32) > + v.qw = ((uint64_t) iter->p[index+1] << 32) | iter->p[index]; > + else > + v.qw = iter->p[index]; > + > switch (f->type.kind) { > case GEN_TYPE_UNKNOWN: > case GEN_TYPE_INT: > snprintf(iter->value, sizeof(iter->value), > - "%ld", field(v.dw, f->start, f->end)); > + "%ld", field(v.qw, f->start, f->end)); > break; > case GEN_TYPE_UINT: > snprintf(iter->value, sizeof(iter->value), > - "%lu", field(v.dw, f->start, f->end)); > + "%lu", field(v.qw, f->start, f->end)); > break; > case GEN_TYPE_BOOL: > snprintf(iter->value, sizeof(iter->value), > - "%s", field(v.dw, f->start, f->end) ? "true" : "false"); > + "%s", field(v.qw, f->start, f->end) ? "true" : "false"); > break; > case GEN_TYPE_FLOAT: > snprintf(iter->value, sizeof(iter->value), "%f", v.f); > @@ -521,7 +545,7 @@ gen_field_iterator_next(struct gen_field_iterator *iter) > case GEN_TYPE_ADDRESS: > case GEN_TYPE_OFFSET: > snprintf(iter->value, sizeof(iter->value), > - "0x%08lx", field_address(v.dw, f->start, f->end)); > + "0x%08lx", field_address(v.qw, f->start, f->end)); > break; > case GEN_TYPE_STRUCT: > snprintf(iter->value, sizeof(iter->value), > @@ -529,7 +553,7 @@ gen_field_iterator_next(struct gen_field_iterator *iter) > break; > case GEN_TYPE_UFIXED: > snprintf(iter->value, sizeof(iter->value), > - "%f", (float) field(v.dw, f->start, f->end) / (1 << > f->type.f)); > + "%f", (float) field(v.qw, f->start, f->end) / (1 << > f->type.f)); > break; > case GEN_TYPE_SFIXED: > /* FIXME: Sign extend extracted field. */ > -- > 2.7.4 > > _______________________________________________ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Reviewed-by: Anuj Phogat <anuj.pho...@gmail.com> _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev