Re: [Intel-gfx] [PATCH] i915: add dmabuf/prime buffer sharing support.

On Thu, 10 May 2012 15:11:15 +0200, Daniel Vetter daniel.vetter@ffwll.ch wrote:

From: Dave Airlie airlied@redhat.com

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.

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 | 12 ++- drivers/gpu/drm/i915/i915_gem_dmabuf.c | 149 ++++++++++++++++++++++++++++++++ drivers/gpu/drm/i915/i915_gem_gtt.c | 17 +++- 6 files changed, 193 insertions(+), 7 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 8b8bbc7..7b7ecb8 100644 --- a/drivers/gpu/drm/i915/Makefile +++ b/drivers/gpu/drm/i915/Makefile @@ -37,7 +37,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
  
  

i915-$(CONFIG_COMPAT) += i915_ioc32.o

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index 1ccfc23..ac13c2c 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -1034,7 +1034,7 @@ static struct drm_driver driver = { */ .driver_features = DRIVER_USE_AGP | DRIVER_REQUIRE_AGP | /* DRIVER_USE_MTRR |*/

•    DRIVER_HAVE_IRQ | DRIVER_IRQ_SHARED | DRIVER_GEM,
  

•    DRIVER_HAVE_IRQ | DRIVER_IRQ_SHARED | DRIVER_GEM | DRIVER_PRIME,
  

  .load = i915_driver_load, .unload = i915_driver_unload, .open = i915_driver_open,

@@ -1057,6 +1057,12 @@ static struct drm_driver driver = { .gem_init_object = i915_gem_init_object, .gem_free_object = i915_gem_free_object, .gem_vm_ops = &i915_gem_vm_ops,

• .prime_handle_to_fd = drm_gem_prime_handle_to_fd,
• .prime_fd_to_handle = drm_gem_prime_fd_to_handle,
• .gem_prime_export = i915_gem_prime_export,
• .gem_prime_import = i915_gem_prime_import,

Maybe a .gem_prime_ops = &i915_gem_prime_ops.

• .dumb_create = i915_gem_dumb_create, .dumb_map_offset = i915_gem_mmap_gtt, .dumb_destroy = i915_gem_dumb_destroy,

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index e03a4f8..751f25c 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -935,6 +935,8 @@ struct drm_i915_gem_object { struct scatterlist *sg_list; int num_sg;

• /* prime dma-buf support */
• struct sg_table *sg_table;

This looks like a kludge which with a little more work could integrate neatly into the existing sg code without having to add aditional special cases.

+struct sg_table *i915_gem_map_dma_buf(struct dma_buf_attachment *attachment,

• 		      enum dma_data_direction dir)
  

+{

• struct drm_i915_gem_object *obj = attachment->dmabuf->priv;
• struct drm_device *dev = obj->base.dev;
• int npages = obj->base.size / PAGE_SIZE;
• struct sg_table *sg = NULL;
• int ret;
• int nents;
• ret = i915_mutex_lock_interruptible(dev);
• if (ret)

• return NULL;
  

Not impressed by the lack of error reporting through this interface.

+struct drm_gem_object *i915_gem_prime_import(struct drm_device *dev,

• 		struct dma_buf *dma_buf)
  

+{

• struct dma_buf_attachment *attach;
• struct sg_table *sg;
• struct drm_i915_gem_object *obj;
• int npages;
• int size;
• int ret;
• /* is this one of own objects? */
• if (dma_buf->ops == &i915_dmabuf_ops) {

• obj = dma_buf->priv;
  

• /* is it from our device? */
  

• if (obj->base.dev == dev) {
  

• 	drm_gem_object_reference(&obj->base);
  

• 	return &obj->base;
  

• }
  

• }
• /* need to attach */
• attach = dma_buf_attach(dma_buf, dev->dev);
• if (IS_ERR(attach))

• return ERR_PTR(-EINVAL);
  

Return the original error. -Chris