On Tue, Sep 20, 2016 at 12:35 PM, Sirisha Gandikota
wrote:
> From: Sirisha Gandikota
>
> 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
> ---
> 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
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
