From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ben Widawsky Subject: Re: [PATCH] drm/i915: Split out GTT specific header file Date: Sun, 23 Mar 2014 10:49:12 -0700 Message-ID: <20140323174911.GC999@bwidawsk.net> References: <1395553641-18573-1-git-send-email-benjamin.widawsky@intel.com> <20140323152235.GJ26519@phenom.ffwll.local> <20140323174419.GB999@bwidawsk.net> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail.bwidawsk.net (bwidawsk.net [166.78.191.112]) by gabe.freedesktop.org (Postfix) with ESMTP id A79046E72E for ; Sun, 23 Mar 2014 10:49:26 -0700 (PDT) Content-Disposition: inline In-Reply-To: <20140323174419.GB999@bwidawsk.net> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: Daniel Vetter Cc: Intel GFX , Ben Widawsky List-Id: intel-gfx@lists.freedesktop.org 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 > > > > 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