Re: [PATCH] drm/i915: Split out GTT specific header file

[Ben Widawsky <ben@bwidawsk.net>]

On Sun, Mar 23, 2014 at 10:44:19AM -0700, Ben Widawsky wrote:
> On Sun, Mar 23, 2014 at 04:22:35PM +0100, Daniel Vetter wrote:
> > On Sat, Mar 22, 2014 at 10:47:21PM -0700, Ben Widawsky wrote:
> > > This file contains all necessary defines, prototypes and typesdefs for
> > > manipulating GEN graphics address translation (this does not include the
> > > legacy AGP driver)
> > > 
> > > Reiterating the comment in the header,
> > > "Please try to maintain the following order within this file unless it
> > > makes sense to do otherwise. From top to bottom:
> > > 1. typedefs
> > > 2. #defines, and macros
> > > 3. structure definitions
> > > 4. function prototypes
> > > 
> > > Within each section, please try to order by generation in ascending
> > > order, from top to bottom (ie. GEN6 on the top, GEN8 on the bottom)."
> > > 
> > > I've made some minor cleanups, and fixed a couple of typos while here -
> > > but there should be no functional changes.
> > > 
> > > The purpose of the patch is to reduce clutter in our main header file,
> > > making room for new growth, and make documentation of our interfaces
> > > easier by splitting things out.
> > > 
> > > With a little more work, like making i915_gtt a pointer, we could
> > > potentially completely isolate this header from i915_drv.h. At the
> > > moment however, I don't think it's worth the effort.
> > > 
> > > Personally, I would have liked to put the PTE encoding functions in this
> > > file too, but I didn't want to rock the boat too much.
> > 
> > Why would we split those out? They shouldn't be used outside of
> > i915_gem_gtt.c, so a forward decl (if needed at all) should be all we
> > need.
> 
> iirc it was in the x86 header file, which I was trying to emulate as
> much as possible. In there case though, they have less platform
> specificity.
> 
s/there/that

> > 
> > Note that i915_gem_gtt.c is a pretty big mess with hw spcific stuff and
> > generic code confusingly interleaved. I've had patches around to fix this
> > up and remove all the forward decls and stuff back in Oct last year, but
> > nuked them again.
> 
> Yeah. It's actually not *that* bad. It's mostly hw specific stuff. We
> could potentially move a few things over to gem.c, and feel okay about
> it I think.
> 
> > 
> > > A similar patch has been in use on my machine for some time. This exact
> > > patch though has only been compile tested.
> > > 
> > > Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
> > 
> > Queued for -next, thanks for the patch.
> > -Daniel
> 
> Awesome, thanks. I'll rebase my stuff now.
> 
> [snip]
> 
> 
> -- 
> Ben Widawsky, Intel Open Source Technology Center
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ben Widawsky, Intel Open Source Technology Center