On Thu, Jul 11, 2013 at 12:23 AM, Ben Widawsky <ben at bwidawsk.net> wrote: > On Wed, Jul 10, 2013 at 07:05:52PM +0200, Daniel Vetter wrote: >> On Wed, Jul 10, 2013 at 09:37:10AM -0700, Ben Widawsky wrote: >> > On Tue, Jul 09, 2013 at 09:15:01AM +0200, Daniel Vetter wrote: >> > > On Mon, Jul 08, 2013 at 11:08:37PM -0700, Ben Widawsky wrote: >> > > > This patch was formerly known as: >> > > > "drm/i915: Create VMAs (part 3) - plumbing" >> > > > >> > > > This patch adds a VM argument, bind/unbind, and the object >> > > > offset/size/color getters/setters. It preserves the old ggtt helper >> > > > functions because things still need, and will continue to need them. >> > > > >> > > > Some code will still need to be ported over after this. >> > > > >> > > > v2: Fix purge to pick an object and unbind all vmas >> > > > This was doable because of the global bound list change. >> > > > >> > > > v3: With the commit to actually pin/unpin pages in place, there is no >> > > > longer a need to check if unbind succeeded before calling put_pages(). >> > > > Make put_pages only BUG() after checking pin count. >> > > > >> > > > v4: Rebased on top of the new hangcheck work by Mika >> > > > plumbed eb_destroy also >> > > > Many checkpatch related fixes >> > > > >> > > > v5: Very large rebase >> > > > >> > > > Signed-off-by: Ben Widawsky <ben at bwidawsk.net> >> > > >> > > This one is a rather large beast. Any chance we could split it into >> > > topics, e.g. convert execbuf code, convert shrinker code? Or does that get >> > > messy, fast? >> > > >> > >> > I've thought of this... >> > >> > The one solution I came up with is to have two bind/unbind functions >> > (similar to what I did with pin, and indeed it was my original plan with >> > pin), and do the set_caching one separately. >> > >> > I think it won't be too messy, just a lot of typing, as Keith likes to >> > say. >> > >> > However, my opinion was, since it's early in the merge cycle, we don't >> > yet have multiple VMs, and it's /mostly/ a copypasta kind of patch, it's >> > not a big deal. At a functional level too, I felt this made more sense. >> > >> > So I'll defer to your request on this and start splitting it up, unless >> > my email has changed your mind ;-). >> >> Well, my concern is mostly in reviewing since we need to think about each >> case and whether it makes sense to talk in therms of vma or objects in >> that function. And what exactly to test. >> >> If you've played around and concluded it'll be a mess then I don't think >> it'll help in reviewing. So pointless. > > I said I don't think it will be a mess, though I feel it won't really > help review too much. Can you take a crack and review and poke me if you > want me to try it. I'd rather not do it if I can avoid it, so I can try > to go back to my 15 patch maximum rule. > >> >> Still, there's a bunch of questions on this patch that we need to discuss >> ;-) > > Ready whenever. It's waiting for you in my first reply, just scroll down a bit ;-) -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch
