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

[Ben Widawsky <ben@bwidawsk.net>]

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.

> 
> 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