Hey folks, Putting previous work aside, I have to agree with Ben about getting the user to provide the absolute register offset - the adding of the 0x180000 into the igt tool patch below was based on some prior work we had (some internal ULTs we have running with this tool). This makes more sense considering the fact that there are some registers that have the same B-Spec register offset for both Render and Display - except one needs the 180000 offset and the other doesn't - i.e. for those cases, u cant silently just add the 0x180000 by simply inspecting what offset it is. Actually, I emailed about this previously and I remember that in that thread our team's incarnation of igt would have an additional input param for intel-reg-read/write to explicitly say if we want to dictate it as a display register or not (which is a NO-OP for non-VLV). But I see that got lost from the patch. Ben, Daniel, please let us the final verdict - if the 0x180000 should NOT be added, we'll re-do the patches and require use explicit addition of the 180000 (and probably need to re-do our internal ULTs in some near future). However, WRT to the xml file parsing - this patch has no relationship whatsoever to that. I agree that XML file parsing is a good idea but this patch is about enabling VLV support of existing i-g-t functions. On the xml concept, should we perhaps consider chipset HW abstraction for the kernel driver too??? i.e. the registers header files?? (i915_regs.h --> PIPEA_STATUS_GEN vs ivlv_regs.h --> PIPEA_STATUS_VLV for example - with the 180000 already added to the latter)? ...alan -----Original Message----- From: Ben Widawsky [mailto:ben at bwidawsk.net] Sent: Wednesday, January 30, 2013 9:13 AM To: Vetter, Daniel Cc: Barnes, Jesse; intel-gfx at lists.freedesktop.org; Cheah, Vincent Beng Keat; Ung, Teng En; Teres Alexis, Alan Previn; Widawsky, Benjamin Subject: Re: intel-gpu-tools patches for read/write MMIO On Tue, Jan 29, 2013 at 09:15:22PM +0100, Daniel Vetter wrote: > On 29/01/2013 21:01, Jesse Barnes wrote: > >Can you just post them externally tointel-gfx at lists.freedesktop.org? > >It's best to use git send-email to do it, that way the changelogs are > >preserved and posted to the ml along with the patches. > Public intel-gfx is already on the cc list, just in case you get the > urge to spill some secrets ;-) > >Not sure if there's a bunch of duplication between the two, but you > >could split them up a bit. > > > >I still don't like the idea of silently adding the display offset on > >vlv; these are just debug tools and the developer should get the > >absolute offset they asked for no matter what. > On that topic of silently adding display offset - with Ville's kernel > work we'll have switched away completely from such tricks in the > kernel. So I think i-g-t shouldn't automatically add the offset. > > Which essentially just leaves us with intel_reg_dumper. Now for that > I'm somewhat hopefully that we will be able to (eventually) dump > registers using the bspec xml sources (there should be bspec xmls > around for just the open-source approved parts). In the meantime, > can't we just adjust the relevant offsets of the register blocks? > IIrc their all somewhat usefully grouped together, so this would > amount to adding a quick function to add the offset to a given table > (put keep all the names) and then feed the adjusted table to the > dumper functions ... > -Daniel As we discussed in private, even if we get to the point of having bspec xml, we would still want a tool similar to the one that was proposed for parsing the XML (as opposed to the text). Reg dumper as has been mentioned in several threads is pretty inflexible, and a pain to modify for person use. As we also discussed in private, I'd like Jesse to either fight or not for this because I don't think he has to butt heads with you enough. -- Ben Widawsky, Intel Open Source Technology Center
