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

[Kristian Høgsberg]

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.
>
> Anyway, that's why it is the way it is.  Feel free to blame me for it :)

That sounds very reasonable. Sirisha, Thanks for getting the tool
ready to send out!

Kristian

>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>