[Mesa-dev] [PATCH 2/2] aubinator: Fix the tool to correctly decode the DWords

[Kenneth Graunke]

On Wednesday, August 10, 2016 2:22:29 PM PDT Jason Ekstrand wrote:
> On Tue, Aug 9, 2016 at 4:52 PM, Sirisha Gandikota <
> sirisha.gandikota at intel.com> wrote:
> 
> > From: Sirisha Gandikota <Sirisha.Gandikota at intel.com>
> >
> > Several fixes have been added as part of this as listed below:
> >
> > 1) Fix the mask and add disassembler handling for STATE_DS, STATE_HS
> > as the mask returned wrong values of the fields.
> >
> > 2) Fix the GEN_TYPE_ADDRESS/GEN_TYPE_OFFSET decoding - the address/
> > offset were handled the same way as the other fields and that gives
> > the wrong values for the address/offset.
> >
> > 3) Decode nested/recurssive structures - Many packets contain nested
> > structures, ex: 3DSATE_SO_BUFFER, STATE_BASE_ADDRESS, etc contain MOC
> > structures. Previously, the aubinator printed 1 if there was a MOC
> > structure. Now we decode the entire structure and print out its fields.
> >
> > 4) Print out the DWord address along with its hex value - For a better
> > clarity of information, it is helpful to print both the address and
> > hex value of the DWord along with the DWord count. Since the DWord0
> > contains the instruction code and the instruction length, it is
> > unnecessary to print the decoded values for DWord0. This information
> > is already available from the DWord hex value.
> >
> > 5) Decode the <group> and the corresponding fields in the group- The
> > <group> tag can have fields of several types including structures. A
> > group can contain one or more number of fields and this has be correctly
> > decoded. Previously, aubinator did not decode the groups or the
> > fields/structures inside them. Now we decode the <group> in the
> > instructions and structures where the fields in it repeat for any number
> > of times specified.
> >
> 
> One comment for now:  Usually, when making a bunch of changes like this, we
> try and split each functional change into its own patch.  That way it's
> more clear what's going on.  This is brand new code, so a couple giant
> patches may be ok, but having it split up would be nicer. :)  Git has some
> tools that make this less painful than it might look and I'm available to
> help if you'd like.
> 
> I'll try and get around to actually playing with it soon.
> --Jason

Sirisha asked me whether she should split things up, or squash everything
together.  I suggested that she may as well squash everything together
for the initial import (but obviously any future changes should follow
the normal procedure).

My reasoning was:

- There was already one giant commit from Kristian importing most of
  the tool (patch 1 here).  I didn't think it was worth time trying
  to split that out into an artificial history.  And...when developing
  a tool from scratch...what parts go where?

- I figured reviewing the tool as a whole would be easier than a series
  of patches that gave an incomplete picture.

- There's nothing to bisect across or anything that might regress.

But I did think it was important to preserve authorship information,
so both Kristian and Sirisha got proper credit for their work.

Anyway, that's why it is the way it is.  Feel free to blame me for it :)
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part.
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20160810/a5f33d18/attachment.sig>