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

[Jason Ekstrand]

On Wed, Aug 10, 2016 at 11:18 PM, Kenneth Graunke <kenneth at whitecape.org>
wrote:

> 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.
>

That's fine.  I don't care *that* much either way.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20160811/25fc66c2/attachment.html>