On Wed, Aug 10, 2016 at 11:18 PM, Kenneth Graunke <kenn...@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.gandik...@intel.com> wrote: >> >> > From: Sirisha Gandikota <sirisha.gandik...@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@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/mesa-dev > _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
