> -----Original Message----- > From: Daniel Vetter [mailto:daniel.vetter@xxxxxxxx] On Behalf Of Daniel > Vetter > Sent: Tuesday, November 18, 2014 2:33 PM > To: Daniel, Thomas > Cc: Daniel Vetter; intel-gfx@xxxxxxxxxxxxxxxxxxxxx > Subject: Re: [PATCH v5 3/4] drm/i915/bdw: Pin the context > backing objects to GGTT on-demand > > On Tue, Nov 18, 2014 at 10:48:09AM +0000, Daniel, Thomas wrote: > > > -----Original Message----- > > > From: Intel-gfx [mailto:intel-gfx-bounces@xxxxxxxxxxxxxxxxxxxxx] On > > > Behalf Of Daniel, Thomas > > > Sent: Tuesday, November 18, 2014 9:28 AM > > > To: Daniel Vetter > > > Cc: intel-gfx@xxxxxxxxxxxxxxxxxxxxx > > > Subject: Re: [PATCH v5 3/4] drm/i915/bdw: Pin the > > > context backing objects to GGTT on-demand > > > > > > > -----Original Message----- > > > > From: Daniel Vetter [mailto:daniel.vetter@xxxxxxxx] On Behalf Of > > > > Daniel Vetter > > > > Sent: Monday, November 17, 2014 6:09 PM > > > > To: Daniel, Thomas > > > > Cc: intel-gfx@xxxxxxxxxxxxxxxxxxxxx > > > > Subject: Re: [PATCH v5 3/4] drm/i915/bdw: Pin the > > > > context backing objects to GGTT on-demand > > > > > > > > On Thu, Nov 13, 2014 at 10:28:10AM +0000, Thomas Daniel wrote: > > > > > diff --git a/drivers/gpu/drm/i915/i915_drv.h > > > > > b/drivers/gpu/drm/i915/i915_drv.h index 059330c..3c7299d 100644 > > > > > --- a/drivers/gpu/drm/i915/i915_drv.h > > > > > +++ b/drivers/gpu/drm/i915/i915_drv.h > > > > > @@ -655,6 +655,7 @@ struct intel_context { > > > > > struct { > > > > > struct drm_i915_gem_object *state; > > > > > struct intel_ringbuffer *ringbuf; > > > > > + int unpin_count; > > > > > > > > Pinning is already refcounted. Why this additional refcount? > > > > > > The vma.pin_count is only allocated 4 bits of storage. If this > > > restriction can be lifted then I can use that. > > Those 4 bits are good enough for legacy contexts, so I wonder a bit what's so > massively different for execlist contexts. With execlists, in order to dynamically unpin the LRC backing object and ring buffer object when not required we take a reference for each execlist request that uses them (remember that the execlist request lifecycle is currently different from the execbuffer request). This can be a lot, especially in some of the less sane i-g-t tests. > > Actually I just tried to implement this, it causes a problem for patch > > 4 of this set as the unpin_count is also used for the ringbuffer > > object which has an ioremap as well as a ggtt pin. > > Yeah, ioremap needs to be redone every time we pin/unpin. But on sane > archs it's almost no overhead really. And if this does start to matter (shudder > for 32bit kernels on gen8) then we can fix it ... Hm, so the CPU vaddr of the ring buffer will move around as more requests reference it which I suppose is not a problem. We will use a lot of address space (again, especially with the i-g-t stress tests which can submit tens of thousands of requests in a very short space of time). What would the fix be? An extra reference count for the ioremap? Looks familiar :) I still think it's best to keep the context unpin_count for execlists mode. Thomas. > -Daniel > -- > Daniel Vetter > Software Engineer, Intel Corporation > +41 (0) 79 365 57 48 - http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
