On Sun, Feb 03, 2013 at 12:22:25PM +0000, Chris Wilson wrote: > On Sun, Feb 03, 2013 at 10:29:15AM +0100, Jesse Barnes wrote: > > On Sat, 2 Feb 2013 16:07:52 -0800 > > Ben Widawsky <ben at bwidawsk.net> wrote: > > > > > This is my second attempt at winning approval for the series. First one > > > was: https://patchwork.kernel.org/patch/1493131/ > > > > > > In spending the time to rework this tool, I've begun to lose my belief > > > in some of the original motivations I had. Even if you don't want to > > > review, but just like (or dislike) what you see, I'd appreciate such > > > comments. > > > > I'd like to see it land in i-g-t. Having the regs defined in a text or > > xml file is an improvement over what we have today, and is easier to > > extend. At first the advantage of reg_dumper was that it parsed out > > the bitfields of the various regs. But we didn't keep up with that, > > and also haven't kept up with the regs on new platforms as well as we > > could. Text files would make that easier, and xml files might bring > > back the bit field parsing, which would be extra nice. > > Completely agree. For me the big improvement would be being able to use > the bspec register names or our internal approximation thereof rather > than having to loop up the actual addresses every time. > > Having the name database available in python should make building > integrated little snippets to parse traces which are also python > accessible. > -Chris It's really nice to get support from you. A mix of fever and staring at the same code too long can really make someone go crazy. Still, a few concerns left from me, one of which I accidentally left out of the description. - Someone needs to give me a yes or no on the m4 extension macros. This will block any pushing. - The build kind of sucks on Arch because of Arch's choices regarding python libraries. To build this on Arch, you must run something like: ./autogen.sh PYTHON_LDFLAGS="-L/usr/lib/python3.3 -lpython3.3m" I really don't like autogen not working out of the box. Perhaps I need to add an AC_ flag to default this tool to off? What do others think? Does it work properly on other distros? How to handle this? - Ideally, I'd like someone to send me some fixes for valleyview definitions if they're needed. I am not sure. Jesse, if you can send me a list of DPIO offsets to read, I'll add the appropriate patch. (It can wait until you get back). -- Ben Widawsky, Intel Open Source Technology Center
