On Thu, Apr 04, 2013 at 04:41:58PM -0700, Ben Widawsky wrote: > On Thu, Apr 04, 2013 at 04:41:46PM -0700, Ben Widawsky wrote: > > These patches implement full context reference counting. In the patch that > > actually adds the reference counting, I explain why I think Mika's reference > > counting isn't sufficient for me. Please see/respond to that patch if you > > disagree. > > > > Almost all of the actual work occurs in: > > "drm/i915: Store last context instead of the obj" > > > > This work is preliminary work for what I'm actually doing at the moment which > > is proper PPGTT support. The patches split off quite nicely here, so I'm > > submitting it for review separately. I believe these patches provide a superset > > of the functionality needed by Mika for ARB_Robustness. > > > > The primary reason I want to track context destruction is for PPGTT support > > we'd like to teardown ppgtt state when a context goes away, but can only do so > > when we're absolutely certain those PTEs are no longer needed. In my design, > > I've made a 1:1 relationship with context->ppgtt, and so refcounting the > > context makes sense there. The crux of the solution here is to store the > > context pointer in the object that backs it. I could probably use that alone to > > solve my problem, but I've gone a bit further and also stored the last context > > in the ring, instead of the last context object. I can't show code yet, but > > I believe there are a couple of other niceties to having the last context, and > > not an object. > > I sent the wrong version of this text. What I meant is, the crux of the > solution is reference counting + storing the context pointer in the > object. I believe I am not /required/ to store the last_context as I've > done. I've managed to convince myself storing the last context vs. the last context object really isn't important. Just the refcounting matters. Since we have refcounting though, I feel storing last_context makes a bit more sense, but I don't really care about that aspect. For PPGTT (at least for current gens) we have to store the current address space globally as opposed to per ring. > > > > > There is at least one sticking point in this patch series, which is the > > aforementioned storing of the context in the object backs the context. That > > patch has been rejected before. I'm open to other ways to handle this. > > At the very least, I require being able to teardown PPGTT when a context > > dies. > > > > For reference, here is the last time the patch was shot down: > > http://lists.freedesktop.org/archives/intel-gfx/2012-June/thread.html#18000 > > > > Ben Widawsky (7): > > drm/i915: Mark context switch likely > > drm/i915: Move context special case to get() > > drm/i915: Make object aware that it backs a context > > drm/i915: Better context messages > > drm/i915: Track context status > > drm/i915: Store last context instead of the obj > > drm/i915: Print all contexts in debugfs > > > > drivers/gpu/drm/i915/i915_debugfs.c | 30 +++++++- > > drivers/gpu/drm/i915/i915_drv.h | 10 ++- > > drivers/gpu/drm/i915/i915_gem.c | 2 + > > drivers/gpu/drm/i915/i915_gem_context.c | 127 ++++++++++++++++++++++---------- > > drivers/gpu/drm/i915/intel_ringbuffer.h | 2 +- > > 5 files changed, 129 insertions(+), 42 deletions(-) > > > > -- > > 1.8.2 > > > > -- > Ben Widawsky, Intel Open Source Technology Center > _______________________________________________ > Intel-gfx mailing list > Intel-gfx at lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Ben Widawsky, Intel Open Source Technology Center
