On Tue, Sep 20, 2016 at 12:35 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. > > 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 | 40 ++++++++++++++++++++++++++++++++-------- > 1 file changed, 32 insertions(+), 8 deletions(-) > > diff --git a/src/intel/tools/decoder.c b/src/intel/tools/decoder.c > index b5f557c..bea9f22 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 dw; uint64_t qw? > float f; > } v; > > @@ -500,7 +518,13 @@ 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.dw = ((uint64_t) iter->p[index+1] << 32 ) | iter->p[index]; Omit the white space after 32. > + else > + v.dw = iter->p[index]; > + > switch (f->type.kind) { > case GEN_TYPE_UNKNOWN: > case GEN_TYPE_INT: > -- > 2.7.4 > > _______________________________________________ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/mesa-dev
With above comments fixed: 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