[PATCH] drm/prime: add exported buffers to current fprivs imported buffer list

[PATCH] drm/prime: add exported buffers to current fprivs imported buffer list

[Dave Airlie]

From: Dave Airlie <airlied@redhat.com>

If userspace attempts to import a buffer it exported on the same device,
we need to return the same GEM handle for it, not a new handle pointing
at the same GEM object.

Signed-off-by: Dave Airlie <airlied@redhat.com>
---
 drivers/gpu/drm/drm_gem.c   |   16 ++++++++++++++--
 drivers/gpu/drm/drm_prime.c |   12 ++++++++++++
 2 files changed, 26 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
index 1ab29a7..9cc3761 100644
--- a/drivers/gpu/drm/drm_gem.c
+++ b/drivers/gpu/drm/drm_gem.c
@@ -233,9 +233,15 @@ drm_gem_handle_delete(struct drm_file *filp, u32 handle)
 	idr_remove(&filp->object_idr, handle);
 	spin_unlock(&filp->table_lock);
 
-	if (obj->import_attach)
+	if (obj->import_attach) {
 		drm_prime_remove_imported_buf_handle(&filp->prime,
 				obj->import_attach->dmabuf);
+	}
+	if (obj->export_dma_buf) {
+		drm_prime_remove_imported_buf_handle(&filp->prime,
+				obj->export_dma_buf);
+		obj->export_dma_buf = NULL;
+	}
 
 	if (dev->driver->gem_close_object)
 		dev->driver->gem_close_object(obj, filp);
@@ -530,9 +536,15 @@ drm_gem_object_release_handle(int id, void *ptr, void *data)
 	struct drm_gem_object *obj = ptr;
 	struct drm_device *dev = obj->dev;
 
-	if (obj->import_attach)
+	if (obj->import_attach) {
 		drm_prime_remove_imported_buf_handle(&file_priv->prime,
 				obj->import_attach->dmabuf);
+	}
+	if (obj->export_dma_buf) {
+		drm_prime_remove_imported_buf_handle(&file_priv->prime,
+				obj->export_dma_buf);
+		obj->export_dma_buf = NULL;
+	}
 
 	if (dev->driver->gem_close_object)
 		dev->driver->gem_close_object(obj, file_priv);
diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
index c35e585..c06f0f0 100644
--- a/drivers/gpu/drm/drm_prime.c
+++ b/drivers/gpu/drm/drm_prime.c
@@ -68,6 +68,7 @@ int drm_gem_prime_handle_to_fd(struct drm_device *dev,
 {
 	struct drm_gem_object *obj;
 	void *buf;
+	int ret;
 
 	obj = drm_gem_object_lookup(dev, file_priv, handle);
 	if (!obj)
@@ -100,6 +101,17 @@ int drm_gem_prime_handle_to_fd(struct drm_device *dev,
 		obj->export_dma_buf = buf;
 		*prime_fd = dma_buf_fd(buf, flags);
 	}
+	/* if we've exported this buffer the cheat and add it to the import list
+	 * so we get the correct handle back
+	 */
+	ret = drm_prime_add_imported_buf_handle(&file_priv->prime,
+			obj->export_dma_buf, handle);
+	if (ret) {
+		drm_gem_object_unreference_unlocked(obj);
+		mutex_unlock(&file_priv->prime.lock);
+		return ret;
+	}
+
 	mutex_unlock(&file_priv->prime.lock);
 	return 0;
 }
-- 
1.7.6

Re: [PATCH] drm/prime: add exported buffers to current fprivs imported buffer list

[Chris Wilson]

On Mon, 21 May 2012 08:25:14 +0100, Dave Airlie <airlied@gmail.com> wrote:
> From: Dave Airlie <airlied@redhat.com>
> 
> If userspace attempts to import a buffer it exported on the same device,
> we need to return the same GEM handle for it, not a new handle pointing
> at the same GEM object.
> 
> Signed-off-by: Dave Airlie <airlied@redhat.com>
> ---
>  drivers/gpu/drm/drm_gem.c   |   16 ++++++++++++++--
>  drivers/gpu/drm/drm_prime.c |   12 ++++++++++++
>  2 files changed, 26 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
> index 1ab29a7..9cc3761 100644
> --- a/drivers/gpu/drm/drm_gem.c
> +++ b/drivers/gpu/drm/drm_gem.c
> @@ -233,9 +233,15 @@ drm_gem_handle_delete(struct drm_file *filp, u32 handle)
>  	idr_remove(&filp->object_idr, handle);
>  	spin_unlock(&filp->table_lock);
>  
> -	if (obj->import_attach)
> +	if (obj->import_attach) {
>  		drm_prime_remove_imported_buf_handle(&filp->prime,
>  				obj->import_attach->dmabuf);
> +	}
> +	if (obj->export_dma_buf) {
> +		drm_prime_remove_imported_buf_handle(&filp->prime,
> +				obj->export_dma_buf);
> +		obj->export_dma_buf = NULL;

Any particular reason for nullifying the pointer here and nowhere else
along this free path?

> +	}
>  
>  	if (dev->driver->gem_close_object)
>  		dev->driver->gem_close_object(obj, filp);
> @@ -530,9 +536,15 @@ drm_gem_object_release_handle(int id, void *ptr, void *data)
>  	struct drm_gem_object *obj = ptr;
>  	struct drm_device *dev = obj->dev;
>  
> -	if (obj->import_attach)
> +	if (obj->import_attach) {
>  		drm_prime_remove_imported_buf_handle(&file_priv->prime,
>  				obj->import_attach->dmabuf);
> +	}
> +	if (obj->export_dma_buf) {
> +		drm_prime_remove_imported_buf_handle(&file_priv->prime,
> +				obj->export_dma_buf);
> +		obj->export_dma_buf = NULL;
> +	}

Strongly suggests wrapping this into a small function to undo prime for
a handle and avoiding the duplicate code.

>  
>  	if (dev->driver->gem_close_object)
>  		dev->driver->gem_close_object(obj, file_priv);
> diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
> index c35e585..c06f0f0 100644
> --- a/drivers/gpu/drm/drm_prime.c
> +++ b/drivers/gpu/drm/drm_prime.c
> @@ -68,6 +68,7 @@ int drm_gem_prime_handle_to_fd(struct drm_device *dev,
>  {
>  	struct drm_gem_object *obj;
>  	void *buf;
> +	int ret;
>  
>  	obj = drm_gem_object_lookup(dev, file_priv, handle);
>  	if (!obj)
> @@ -100,6 +101,17 @@ int drm_gem_prime_handle_to_fd(struct drm_device *dev,
>  		obj->export_dma_buf = buf;
>  		*prime_fd = dma_buf_fd(buf, flags);
>  	}
> +	/* if we've exported this buffer the cheat and add it to the import list
> +	 * so we get the correct handle back
> +	 */
s/the/then/
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre