[Intel-gfx] [PATCH 2/2] drm/i915/dmabuf: Pin pages as we flush for CPU access

[Chris Wilson]

On Fri, May 06, 2016 at 08:35:17PM +0100, Dave Gordon wrote:
> On 06/05/16 15:36, Chris Wilson wrote:
> >As a precautionary defensive measure against changes to the core (where
> >we stop changing domains when unbinding), we want to ensure that the pages
> >are available when we want to start CPU access, otherwise we will not
> >perform the requisite clflushes to make the contents coherent for the user.
> >
> >In contrast, we do not have to worry about ensuring the pages are
> >available when flushing after CPU access, as any subsequent use that
> >requires the pages in the GTT domain will be perform the flush
> >themselves.
> >
> >The tricky part is that we can not force the behaviour within
> >set-to-cpu-domain as that currently needs to be callable from within the
> >put_pages cleanup path, thus we place the burden on the caller.
> >
> >Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> >---
> >  drivers/gpu/drm/i915/i915_gem_dmabuf.c | 8 ++++++++
> >  1 file changed, 8 insertions(+)
> >
> >diff --git a/drivers/gpu/drm/i915/i915_gem_dmabuf.c b/drivers/gpu/drm/i915/i915_gem_dmabuf.c
> >index 80bbe43a2e92..6d898a201a46 100644
> >--- a/drivers/gpu/drm/i915/i915_gem_dmabuf.c
> >+++ b/drivers/gpu/drm/i915/i915_gem_dmabuf.c
> >@@ -182,7 +182,15 @@ static int i915_gem_begin_cpu_access(struct dma_buf *dma_buf, enum dma_data_dire
> >  	if (ret)
> >  		return ret;
> >
> >+	ret = i915_gem_object_get_pages(obj);
> >+	if (ret)
> >+		goto err_unlock;
> >+
> >+	i915_gem_object_pin_pages(obj);
> >  	ret = i915_gem_object_set_to_cpu_domain(obj, write);
> >+	i915_gem_object_unpin_pages(obj);
> >+
> >+err_unlock:
> >  	mutex_unlock(&dev->struct_mutex);
> >  	return ret;
> >  }
> 
> So, will these two patches fix the anomalous
> setting-dirty-while-not-pinned behaviour of set-to-gtt/set-to-cpu?

Only by coincidence for set-domain-ioctl with DOMAIN_GTT.

The actual issue I am trying to prevent is missing clflushes when the
object is unbound but the pages are still being tracked... Though now
that appears to be entirely an artifact of removing the struct_mutex
lock rather than my earlier thought that was the lockless patch in
conjuction with the old patch to stop fudging the domains on unbind.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre