[Intel-gfx] [PATCH 06/12] drm/i915: Use the new vm [un]bind functions

[Daniel Vetter]

On Fri, Jul 26, 2013 at 02:48:32PM -0700, Ben Widawsky wrote:
> On Tue, Jul 23, 2013 at 06:54:43PM +0200, Daniel Vetter wrote:
> > On Sun, Jul 21, 2013 at 07:08:13PM -0700, Ben Widawsky wrote:
> > > Building on the last patch which created the new function pointers in
> > > the VM for bind/unbind, here we actually put those new function pointers
> > > to use.
> > > 
> > > Split out as a separate patch to aid in review. I'm fine with squashing
> > > into the previous patch if people request it.
> > > 
> > > v2: Updated to address the smart ggtt which can do aliasing as needed
> > > Make sure we bind to global gtt when mappable and fenceable. I thought
> > > we could get away without this initialy, but we cannot.
> > > 
> > > Signed-off-by: Ben Widawsky <ben at bwidawsk.net>
> > 
> > Meta review on the patch split: If you create new functions in a prep
> > patch, then switch and then kill the old functions it's much harder to
> > review whether any unwanted functional changes have been introduced.
> > Reviewers have to essentially keep both the old and new code open and
> > compare by hand.  And generally the really hard regression in gem have
> > been due to such deeply-hidden accidental changes, and we frankly don't
> > yet have the test coverage to just gloss over this.
> > 
> > If you instead first prepare the existing functions by changing the
> > arguments and logic, and then once everything is in place switch over to
> > vfuncs in the 2nd patch changes will be in-place. In-place changes are
> > much easier to review since diff compresses away unchanged parts.
> > 
> > Second reason for this approach is that the functions stay at the same
> > place in the source code file, which reduces the amount of spurious
> > conflicts when rebasing a large set of patches around such changes ...
> > 
> > I need to ponder this more.
> > -Daniel
> 
> ping

Keep it in mind for next time around. I think my general approach is
easier on reviewers ... but hey, vacation!
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch