From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Vetter Subject: Re: [PATCH 1/2] [v3] drm/i915: reference count for i915_hw_contexts Date: Wed, 3 Apr 2013 18:37:24 +0200 Message-ID: <20130403163724.GN2228@phenom.ffwll.local> References: <1363276337-12509-4-git-send-email-mika.kuoppala@intel.com> <1364942743-6041-1-git-send-email-ben@bwidawsk.net> <20130403012700.GA7837@bwidawsk.net> <20130403115610.GB3877@cantiga.alporthouse.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail-ee0-f54.google.com (mail-ee0-f54.google.com [74.125.83.54]) by gabe.freedesktop.org (Postfix) with ESMTP id A728CE614D for ; Wed, 3 Apr 2013 09:34:28 -0700 (PDT) Received: by mail-ee0-f54.google.com with SMTP id e51so733330eek.27 for ; Wed, 03 Apr 2013 09:34:27 -0700 (PDT) Content-Disposition: inline In-Reply-To: <20130403115610.GB3877@cantiga.alporthouse.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org Errors-To: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org To: Chris Wilson , Ben Widawsky , intel-gfx@lists.freedesktop.org, Daniel Vetter , Mika Kuoppala , Mika Kuoppala List-Id: intel-gfx@lists.freedesktop.org On Wed, Apr 03, 2013 at 12:56:11PM +0100, Chris Wilson wrote: > On Tue, Apr 02, 2013 at 06:27:00PM -0700, Ben Widawsky wrote: > > On Tue, Apr 02, 2013 at 03:45:42PM -0700, Ben Widawsky wrote: > > > From: Mika Kuoppala > > > > > > In preparation to do analysis of which context was > > > guilty of gpu hung, store kreffed context pointer > > > into request struct. > > > > > > This allows us to inspect contexts when gpu is reset > > > even if those contexts would already be released > > > by userspace. > > > > > > v2: track i915_hw_context pointers instead of using ctx_ids > > > (from Chris Wilson) > > > > > > v3 (Ben): Get rid of do_release() and handle refcounting more compactly. > > > (recommended by Chris) > > > > > > Signed-off-by: Mika Kuoppala > > > Signed-off-by: Ben Widawsky > > > > Now I remember why my version of reference counting was so much more > > complicated. In my case, I want to keep the last context around instead > > of the last context object. To do this we can't do a kref_put until > > we've switched to the next context (similar to how we manage the context > > object). I want to do this since the context stores the PPGTT which will > > currently be in use. I need to switch PDEs at context switch time. > > This seems feasible using requests and a callback from retire. The > alternative is something hairy like intel_overlay, hence my desire for > keeping all ring operations as a i915_gem_request. As long as the request object keeps a ref while the request is still oustanding and the ring itself keeps a ref to whatever is the currently last scheduled context, everything should work out fine. So I don't think we need to jump through any complicated hoops here. One quick bikeshed on the patch itself though: I'd like to see some static inlines for kref_get/put on contexts ... -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch