[Intel-gfx] [PATCH v3] intel: New libdrm interface to create unbound wc user mappings for objects

[Chris Wilson]

On Wed, Dec 03, 2014 at 02:13:58PM +0000, Damien Lespiau wrote:
> On Tue, Oct 28, 2014 at 06:39:27PM +0530, akash.goel at intel.com wrote:
> > @@ -1126,6 +1136,8 @@ static void drm_intel_gem_bo_purge_vma_cache(drm_intel_bufmgr_gem *bufmgr_gem)
> >  
> >  	/* We may need to evict a few entries in order to create new mmaps */
> >  	limit = bufmgr_gem->vma_max - 2*bufmgr_gem->vma_open;
> > +	if (bufmgr_gem->has_ext_mmap)
> > +		limit -= bufmgr_gem->vma_open;
> >  	if (limit < 0)
> >  		limit = 0;
> >  
> 
> Aren't we being overly aggressive in purging the vma cache with this new
> code? I guess it depends on the workload but I don't think the WC
> mapping is likely to be used in addition to the other two often enough
> to warrant a max - 3 * open;

And if they do, you've just exceeded their limit. And it conceivably
that you will have bo with all 3 vma mappings, though I only expect 2 to
be active in any single lifetime. The patch is correct regarding the code
as it exists, you may want to argue for finer tracking of vma_open and
vma_cached, but that would require a different approach.
 
> > +	/* Now move it to the GTT domain so that the GPU and CPU
> > +	 * caches are flushed and the GPU isn't actively using the
> > +	 * buffer.
> > +	 *
> > +	 * The domain change is done even for the objects which
> > +	 * are not bounded. For them first the pages are acquired,
> > +	 * before the domain change.
> > +	 */
> > +	VG_CLEAR(set_domain);
> > +	set_domain.handle = bo_gem->gem_handle;
> > +	set_domain.read_domains = I915_GEM_DOMAIN_GTT;
> > +	set_domain.write_domain = I915_GEM_DOMAIN_GTT;
> > +	ret = drmIoctl(bufmgr_gem->fd,
> > +		       DRM_IOCTL_I915_GEM_SET_DOMAIN,
> > +		       &set_domain);
> > +	if (ret != 0) {
> > +		DBG("%s:%d: Error setting domain %d: %s\n",
> > +		    __FILE__, __LINE__, bo_gem->gem_handle,
> > +		    strerror(errno));
> > +	}
> 
> Why move the buffer to the GTT domain and not the CPU domain here?

There are 3 cache domains: GPU, CPU and NONE (aka GTT). The purpose of
mmap(wc) is to be able to access the buffer without going through the CPU
cache (and it should be out of the GPU cache to maintain coherency). The
NONE cache domain is exactly what we want, or else you end up in clflush
misery.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre