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@xxxxxxxxxxxx> > > > > 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 _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
