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

[Ben Widawsky <ben@bwidawsk.net>]

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.

> 
> 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