On Tue, 20 Mar 2012 19:43:04 +0100 Daniel Vetter <daniel at ffwll.ch> wrote: > > new, range specific i915_read/write routines, e.g. i915_read_gt, > > i915_read_display to make forcewake and register block moves easier > > to handle > > I'm not convinced whether this is a great idea if applied all over the > code. For specific cases where a block moves it's imo better to just add a > mmio base for that (like we're doing with the ring ctrl regs, which move > around quite a bit over the various rings and generations). Obviously > adding a small helper is good, but imo we should name it a bit more > specific (if possible). So the problem with VLV is that it has a CedarTrail like display (so some new registers, lots of moved bits, etc) but moved to 0x180000. Not everything has moved, just enough to be painful. For example, the PLLs and some interrupt regs are still in the low range, but pipe & display plane regs have moved. So it's not as simple as doing a single offset and applying it everywhere. We need to only apply it for certain regs, which means touching a bunch of read/write accesses that don't already use a wrapper. And for the ones that use a wrapper, like PIPECONF, we'd need a dev_priv->display_mmio_offset or something? I'm not happy with any solution here, but definitely don't want to upstream my current hack (a new IS_DISPLAYREG() check in read/write that adds the offset if needed). > > I'm open to suggestions on how to fix i915_reg.h; it's becoming quite a > > beast. Our goal to be to make it easy to add new definitions while > > also making it easy to not accidentally use old an incorrect > > definitions on a new platform. > > Close your eyes and just keep on adding gunk. Imo i915_reg.h is pretty > much a write-once file, and cscope can still keep up with the definitions. > So not a pain point for me. > > > Then obviously within those files there's lots of room for improvement, > > for example in i9xx mode setting we still have some pretty massive > > functions that need to be split (I have patches to do that). > > > > Thoughts? It may also make sense to split some of our port specific > > files where they differ enough from previous platforms. E.g. g4x DP vs > > ironlake+... > > I've just talked about this a bit with Eugeni in the context of Haswell, > and I think we might want to hold of for that code until we move output > stuff all over the place. Yeah I don't want to make HSW any harder than necessary; we can put off splits there. -- Jesse Barnes, Intel Open Source Technology Center -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 836 bytes Desc: not available URL: <http://lists.freedesktop.org/archives/intel-gfx/attachments/20120320/26db51d9/attachment.pgp>
