Re: [PATCH] staging/ion: Add support to get ion handle from dma buf

[Laura Abbott <labbott@redhat.com>]

On 01/05/2016 05:03 AM, Rohit kumar 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.
>
> Signed-off-by: Rohit kumar <rohit.kr@samsung.com>
> ---
> Currently, ION is the memory manager for graphics in android. However,
> in other linux platforms such as Tizen, 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.
>

Is this the same patch that was sent on 12/29? In general it's best to
wait a bit longer before resending, especially with lots of people
being off for the holidays. Please also tag your patch with [RESEND]
so it's easier to tell that this is the same patch being sent again.

This is also a good explanation that should be included in the commit
text as well. It gives a much more thorough explanation why this
API is needed. The substance of the patch looks okay to me.

Thanks,
Laura

>   drivers/staging/android/ion/ion.c |   21 +++++++++++++++------
>   drivers/staging/android/ion/ion.h |   20 ++++++++++++++++----
>   2 files changed, 31 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/staging/android/ion/ion.c b/drivers/staging/android/ion/ion.c
> index e237e9f..5509716 100644
> --- a/drivers/staging/android/ion/ion.c
> +++ b/drivers/staging/android/ion/ion.c
> @@ -1151,16 +1151,13 @@ 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) {
> @@ -1199,6 +1196,18 @@ end:
>   }
>   EXPORT_SYMBOL(ion_import_dma_buf);
>
> +struct ion_handle *ion_import_dma_buf_fd(struct ion_client *client, int fd)
> +{
> +	struct dma_buf *dmabuf;
> +
> +	dmabuf = dma_buf_get(fd);
> +	if (IS_ERR(dmabuf))
> +		return ERR_CAST(dmabuf);
> +
> +	return ion_import_dma_buf(client, dmabuf);
> +}
> +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 +1315,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 */
>