Message-id: <20160201132916.0d822f60@rohitk-ubuntu.sisodomain.com>
Date:	Mon, 01 Feb 2016 13:29:16 +0530
From:	Rohit <rohit.kr@...sung.com>
To:	Rohit kumar <rohit.kr@...sung.com>
Cc:	gregkh@...uxfoundation.org, arve@...roid.com,
	riandrews@...roid.com, labbott@...hat.com,
	dan.carpenter@...cle.com, gioh.kim@....com,
	sumit.semwal@...aro.org, mitchelh@...eaurora.org,
	paul.gortmaker@...driver.com, linux@...musvillemoes.dk,
	shawn.lin@...k-chips.com, sriram@...irs.net.in,
	devel@...verdev.osuosl.org, linux-kernel@...r.kernel.org,
	pintu.k@...sung.com, vishnu.ps@...sung.com, sreenathd@...sung.com,
	pintu_agarwal@...oo.com, me.rohit@...e.com, cpgs@...sung.com
Subject: Re: [PATCHv3 1/1] staging/ion: Add support to get ion handle from dma
 buf

Kindly review the patch. Patchv2 had refcount issue which is fixed here.

On Tue, 12 Jan 2016 09:31:46 +0530
Rohit kumar <rohit.kr@...sung.com> wrote:

> Currently we can only import dma buf fd's to get ion_handle.
> Adding support to import dma buf handles to support kernel
> specific use cases.
> 
> An example use case is in linux platforms such as Tizen, in which
> DRM-GEM is used for buffer management for graphics. It has gem_handle
> corresponding to a buffer and uses gem_name for sharing the buffer
> with other processes. However,it also uses dma_buf fd for 3d
> operations. For wayland, there are multiple calls for gem_handle to
> dma_buf fd conversion. So, we store dma_buf associated with buffer.
> But, there is no api for getting ion_handle from dma_buf. This patch
> exposes api to retrieve the ion handle from dma_buf for similar use
> cases. With this patch, we can integrate ION within DRM-GEM for
> buffer management and dma_buf sharing.
> 
> Signed-off-by: Rohit kumar <rohit.kr@...sung.com>
> ---
> v2: Updated commit message with use case explanation, as suggested by
>     Laura Abbott<labbott@...hat.com>
> v3: Fixed dmabuf refcount issue in ion_import_dma_buf. dma_buf_put()
>     was being called without dma_buf_get(), so moving it to
>     ion_import_dma_buf_fd().
> 
>  drivers/staging/android/ion/ion.c |   26 ++++++++++++++++++--------
>  drivers/staging/android/ion/ion.h |   20 ++++++++++++++++----
>  2 files changed, 34 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/staging/android/ion/ion.c
> b/drivers/staging/android/ion/ion.c index e237e9f..7e86e18 100644
> --- a/drivers/staging/android/ion/ion.c
> +++ b/drivers/staging/android/ion/ion.c
> @@ -1151,22 +1151,18 @@ int ion_share_dma_buf_fd(struct ion_client
> *client, struct ion_handle *handle) }
>  EXPORT_SYMBOL(ion_share_dma_buf_fd);
>  
> -struct ion_handle *ion_import_dma_buf(struct ion_client *client, int
> fd) +struct ion_handle *ion_import_dma_buf(struct ion_client *client,
> +				      struct dma_buf *dmabuf)
>  {
> -	struct dma_buf *dmabuf;
>  	struct ion_buffer *buffer;
>  	struct ion_handle *handle;
>  	int ret;
>  
> -	dmabuf = dma_buf_get(fd);
> -	if (IS_ERR(dmabuf))
> -		return ERR_CAST(dmabuf);
>  	/* if this memory came from ion */
>  
>  	if (dmabuf->ops != &dma_buf_ops) {
>  		pr_err("%s: can not import dmabuf from another
> exporter\n", __func__);
> -		dma_buf_put(dmabuf);
>  		return ERR_PTR(-EINVAL);
>  	}
>  	buffer = dmabuf->priv;
> @@ -1194,11 +1190,25 @@ struct ion_handle *ion_import_dma_buf(struct
> ion_client *client, int fd) }
>  
>  end:
> -	dma_buf_put(dmabuf);
>  	return handle;
>  }
>  EXPORT_SYMBOL(ion_import_dma_buf);
>  
> +struct ion_handle *ion_import_dma_buf_fd(struct ion_client *client,
> int fd) +{
> +	struct dma_buf *dmabuf;
> +	struct ion_handle *handle;
> +
> +	dmabuf = dma_buf_get(fd);
> +	if (IS_ERR(dmabuf))
> +		return ERR_CAST(dmabuf);
> +
> +	handle = ion_import_dma_buf(client, dmabuf);
> +	dma_buf_put(dmabuf);
> +	return handle;
> +}
> +EXPORT_SYMBOL(ion_import_dma_buf_fd);
> +
>  static int ion_sync_for_device(struct ion_client *client, int fd)
>  {
>  	struct dma_buf *dmabuf;
> @@ -1306,7 +1316,7 @@ static long ion_ioctl(struct file *filp,
> unsigned int cmd, unsigned long arg) {
>  		struct ion_handle *handle;
>  
> -		handle = ion_import_dma_buf(client, data.fd.fd);
> +		handle = ion_import_dma_buf_fd(client, data.fd.fd);
>  		if (IS_ERR(handle))
>  			ret = PTR_ERR(handle);
>  		else
> diff --git a/drivers/staging/android/ion/ion.h
> b/drivers/staging/android/ion/ion.h index b860c5f..a1331fc 100644
> --- a/drivers/staging/android/ion/ion.h
> +++ b/drivers/staging/android/ion/ion.h
> @@ -192,14 +192,26 @@ struct dma_buf *ion_share_dma_buf(struct
> ion_client *client, int ion_share_dma_buf_fd(struct ion_client
> *client, struct ion_handle *handle); 
>  /**
> - * ion_import_dma_buf() - given an dma-buf fd from the ion exporter
> get handle
> + * ion_import_dma_buf() - get ion_handle from dma-buf
> + * @client:	the client
> + * @dmabuf:	the dma-buf
> + *
> + * Get the ion_buffer associated with the dma-buf and return the
> ion_handle.
> + * If no ion_handle exists for this buffer, return newly created
> ion_handle.
> + * If dma-buf from another exporter is passed, return
> ERR_PTR(-EINVAL)
> + */
> +struct ion_handle *ion_import_dma_buf(struct ion_client *client,
> +				      struct dma_buf *dmabuf);
> +
> +/**
> + * ion_import_dma_buf_fd() - given a dma-buf fd from the ion
> exporter get handle
>   * @client:	the client
>   * @fd:		the dma-buf fd
>   *
> - * Given an dma-buf fd that was allocated through ion via
> ion_share_dma_buf,
> - * import that fd and return a handle representing it.  If a dma-buf
> from
> + * Given an dma-buf fd that was allocated through ion via
> ion_share_dma_buf_fd,
> + * import that fd and return a handle representing it. If a dma-buf
> from
>   * another exporter is passed in this function will return
> ERR_PTR(-EINVAL) */
> -struct ion_handle *ion_import_dma_buf(struct ion_client *client, int
> fd); +struct ion_handle *ion_import_dma_buf_fd(struct ion_client
> *client, int fd); 
>  #endif /* _LINUX_ION_H */

Powered by blists - more mailing lists