Re: [Linaro-mm-sig] [PATCH 6/7] drm/vgem: import support

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

On Wed, 22 Feb 2012 20:29:19 +0100, Ben Widawsky <ben@bwidawsk.net> wrote:
> dma-buf import support. The function definitely needs some cleanup.
> 
> When reading through this code, there are 3 cases to consider:
> 1. vgem exporter, vgem importer, same fd
> 2. vgem exporter, vgem importer, different fd
> 3. X expoter, vgem importer - not yet tested
> 
> See the comments in the code for detailed explanation.
> 
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Dave Airlie <airlied@redhat.com>
> Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
> ---
>  drivers/gpu/drm/vgem/vgem_dma_buf.c |  120 +++++++++++++++++++++++++++++++++++
>  1 files changed, 120 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/gpu/drm/vgem/vgem_dma_buf.c b/drivers/gpu/drm/vgem/vgem_dma_buf.c
> index eca9445..92c1823 100644
> --- a/drivers/gpu/drm/vgem/vgem_dma_buf.c
> +++ b/drivers/gpu/drm/vgem/vgem_dma_buf.c
> @@ -120,9 +120,129 @@ out_fd:
>  	return 0;
>  }
>  
> +/*
> + * Convert a dma-buf fd to a drm object handle, creating new object/handle as
> + * needed.
> + *
> + * There are 2 "interesting" cases we have to consider. The other, less interesting
> + * case is when importer == exporter, and drm_files are the same.
> + * vgem exporter
> + *   The original exporter may or may not still hold a reference to the
> + *   object by the time we reach here. Once we get a dma_buf reference though
> + *   we know the original object cannot go away. Next we grab the prime mutex
> + *   to prevent our lists from being screwed up from under us. We should next
> + *   find the object in our global dma_buf hash. To make everything cool
> + *   though we need to
> + *     create a handle for the importer
> + *     add the handle to the per file list
> + *     drop the dma_buf reference
> + *       the object can't go away due to us owning a file handle for it.
> + *   In the end there should be 2 handle references, and 1 dma-buf reference.
> + *
> + * other exporter
> + *   This case is very similar to the previous one. The primary difference is we
> + *   do not want to drop the dma_buf reference since we know nothing about the
> + *   reference counting from the exporter. So instead, we hold the dma_buf
> + *   reference, but can drop the object reference. In the end of this case there
> + *   should be 1 handle reference, and 1 dma-buf reference.
> + */
>  int vgem_prime_to_handle(struct drm_device *dev,
>  			 struct drm_file *file, int prime_fd,
>  			 uint32_t *handle)
>  {
> +	struct drm_vgem_file_private *file_priv = file->driver_priv;
> +	struct drm_vgem_gem_object *vobj = NULL;
> +	struct drm_gem_object *obj = NULL;
> +	struct dma_buf *dma_buf;
> +	struct dma_buf_attachment *attach = NULL;
> +	struct sg_table *sg = NULL;
> +	bool drop_dma_buf_ref = false;
> +	int ret;
> +
> +	dma_buf = dma_buf_get(prime_fd);
> +	if (IS_ERR(dma_buf))
> +		return PTR_ERR(dma_buf);
> +
> +	mutex_lock(&dev->prime_mutex);
> +	/* First check that we don't dup on this file */
> +	ret = drm_prime_lookup_fd_handle_mapping(&file_priv->prime, dma_buf,
> +						 handle);
> +	if (ret == 0) {
> +		DRM_DEBUG_PRIME("file_priv has an object for this dma_buf\n");
> +		dma_buf_put(dma_buf);
> +		mutex_unlock(&dev->prime_mutex);
> +		return 0;
> +	}
> +
> +	/* Now check if we've already created/imported this object */
> +	ret = drm_prime_lookup_obj(dev, dma_buf, &obj);
> +	if (ret == 0 && obj != NULL) {
> +		DRM_DEBUG_PRIME("driver has an object for this dma_buf\n");
> +		drop_dma_buf_ref = true;
> +		vobj = to_vgem_bo(obj);
> +		goto handle_create;
> +	}
> +
> +	DRM_DEBUG_PRIME("Creating a new object for dma_buf\n");
> +
> +	attach = dma_buf_attach(dma_buf, dev->dev);
> +	if (IS_ERR(attach)) {
> +		ret = PTR_ERR(attach);
> +		goto fail_put;
> +	}
> +
> +	sg = dma_buf_map_attachment(attach, DMA_BIDIRECTIONAL);
> +	if (IS_ERR(sg)) {
> +		ret = PTR_ERR(sg);
> +		goto fail_detach;
> +	}
> +
> +	vobj = kzalloc(sizeof(*vobj), GFP_KERNEL);
> +	if (vobj == NULL) {
> +		ret = -ENOMEM;
> +		goto fail_unmap;
> +	}
> +
> +	/* As a result of this mmap will not work -yet- */
> +	ret = drm_gem_private_object_init(dev, &vobj->base, dma_buf->size);
> +	if (ret) {
> +		kfree(vobj);
> +		ret = -ENOMEM;
> +		goto fail_unmap;
> +	}
> +
> +	obj = &vobj->base;
Don't we need to store the backing attachment and prime_fd on the newly
created vgem obj?

> +
> +handle_create:
> +	ret = drm_gem_handle_create(file, obj, handle);
> +	if (ret)
> +		return ret;
> +
> +	ret = drm_prime_insert_fd_handle_mapping(&file_priv->prime,
> +						 dma_buf, *handle);
> +	if (ret)
> +		goto fail_handle;
> +
> +	mutex_unlock(&dev->prime_mutex);
> +
> +	if (drop_dma_buf_ref) {
> +		/* This should mean we found this in the global hash */
> +		dma_buf_put(dma_buf);
> +	} else {
> +		/* Handle holds the reference for the object we created */
> +		drm_gem_object_unreference(obj);
> +	}
> +	return 0;
> +
> +fail_handle:
> +	drm_gem_object_handle_unreference_unlocked(obj);
> +fail_unmap:
> +	dma_buf_unmap_attachment(attach, sg);
> +fail_detach:
> +	dma_buf_detach(dma_buf, attach);
> +fail_put:
> +	dma_buf_put(dma_buf);
> +	mutex_unlock(&dev->prime_mutex);
> +
>  	return 0;
>  }

-- 
Chris Wilson, Intel Open Source Technology Centre