Re: [PATCH 6/7] i915: add dmabuf/prime buffer sharing support.

[Chris Wilson <chris@chris-wilson.co.uk>]

On Tue, 22 May 2012 13:50:03 +0100, Dave Airlie <airlied@gmail.com> wrote:
> From: Daniel Vetter <daniel.vetter@ffwll.ch>
> 
> This adds handle->fd and fd->handle support to i915, this is to allow
> for offloading of rendering in one direction and outputs in the other.
> 
> v2 from Daniel Vetter:
> - fixup conflicts with the prepare/finish gtt prep work.
> - implement ppgtt binding support.
> 
> Note that we have squat i-g-t testcoverage for any of the lifetime and
> access rules dma_buf/prime support brings along. And there are quite a
> few intricate situations here.
> 
> Also note that the integration with the existing code is a bit
> hackish, especially around get_gtt_pages and put_gtt_pages. It imo
> would be easier with the prep code from Chris Wilson's unbound series,
> but that is for 3.6.
> 
> Also note that I didn't bother to put the new prepare/finish gtt hooks
> to good use by moving the dma_buf_map/unmap_attachment calls in there
> (like we've originally planned for).
> 
> Last but not least this patch is only compile-tested, but I've changed
> very little compared to Dave Airlie's version. So there's a decent
> chance v2 on drm-next works as well as v1 on 3.4-rc.
> 
> v3: Right when I've hit sent I've noticed that I've screwed up one
> obj->sg_list (for dmar support) and obj->sg_table (for prime support)
> disdinction. We should be able to merge these 2 paths, but that's
> material for another patch.
> 
> v4: fix the error reporting bugs pointed out by ickle.
> 
> v5: fix another error, and stop non-gtt mmaps on shared objects
> stop pread/pwrite on imported objects, add fake kmap

Everything up to this point looks reasonable wrt i915. We obviously need
to lift a few of the restrictions for i915 introduced here and protect
dma-buf objects from the shrinker/truncate.  In fact we need to add a
guard to i915_gem_object_truncate in this patch to prevent one such
explosion. Wrt to the shrinker and the leak of exported pages, that
should be managed by the unbound page tracking which is queued for
review this cycle (right Daniel? ;). That just leaves fixing up
pread/pwrite and fixing the other guards that will interact sorely with
stolen (also non-shmem backed) objects.
-Chris

> Signed-off-by: Dave Airlie <airlied@redhat.com>
> Signed-Off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
>  drivers/gpu/drm/i915/Makefile          |    3 +-
>  drivers/gpu/drm/i915/i915_drv.c        |    8 ++-
>  drivers/gpu/drm/i915/i915_drv.h        |   11 ++
>  drivers/gpu/drm/i915/i915_gem.c        |   37 +++++++-
>  drivers/gpu/drm/i915/i915_gem_dmabuf.c |  171 ++++++++++++++++++++++++++++++++
>  drivers/gpu/drm/i915/i915_gem_gtt.c    |   15 +++-
>  6 files changed, 239 insertions(+), 6 deletions(-)
>  create mode 100644 drivers/gpu/drm/i915/i915_gem_dmabuf.c
> 
> diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
> index 0ca7f76..2e9268d 100644
> --- a/drivers/gpu/drm/i915/Makefile
> +++ b/drivers/gpu/drm/i915/Makefile
> @@ -38,7 +38,8 @@ i915-y := i915_drv.o i915_dma.o i915_irq.o \
>  	  dvo_ch7017.o \
>  	  dvo_ivch.o \
>  	  dvo_tfp410.o \
> -	  dvo_sil164.o
> +	  dvo_sil164.o \
> +	  i915_gem_dmabuf.o

Since we need another chunk to this patch, can you please add this file
in the i915_gem_*.c block.

-- 
Chris Wilson, Intel Open Source Technology Centre