Re: [PATCH 0/7] [RFC] Context reference counting

[Ben Widawsky <ben@bwidawsk.net>]

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@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ben Widawsky, Intel Open Source Technology Center