Re: [RFCv2 PATCH 5/9] v4l: vb2: add buffer exporting via dmabuf

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

Hi Tomasz,

Thanks for the patch.

On Tuesday 13 March 2012 11:17:03 Tomasz Stanislawski wrote:
> This patch adds extension to videobuf2-core. It allow to export a mmap
> buffer as a file descriptor.
> 
> Signed-off-by: Tomasz Stanislawski <t.stanislaws@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> ---
>  drivers/media/video/videobuf2-core.c |   64
> ++++++++++++++++++++++++++++++++++ include/media/videobuf2-core.h       |  
>  2 +
>  2 files changed, 66 insertions(+), 0 deletions(-)
> 
> diff --git a/drivers/media/video/videobuf2-core.c
> b/drivers/media/video/videobuf2-core.c index e7df560..41c4bf8 100644
> --- a/drivers/media/video/videobuf2-core.c
> +++ b/drivers/media/video/videobuf2-core.c
> @@ -1553,6 +1553,70 @@ int vb2_dqbuf(struct vb2_queue *q, struct v4l2_buffer
> *b, bool nonblocking) }
>  EXPORT_SYMBOL_GPL(vb2_dqbuf);
> 
> +static int __find_plane_by_offset(struct vb2_queue *q, unsigned long off,
> +			unsigned int *_buffer, unsigned int *_plane);
> +

Could you please move __find_plane_by_offset() up or move vb2_expbuf() down to 
avoid the forward declaration ? The later might make more sense, you could 
declare vb2_expbuf() right after vb2_mmap() (here and in videobuf2-core.h), 
both functions perform similar tasks.

> +/**
> + * vb2_expbuf() - Export a buffer as a file descriptor
> + * @q:		videobuf2 queue
> + * @b:		export buffer structure passed from userspace to vidioc_expbuf
> + *		handler in driver
> + *
> + * The return values from this function are intended to be directly
> returned
> + * from vidioc_expbuf handler in driver.
> + */
> +int vb2_expbuf(struct vb2_queue *q, struct v4l2_exportbuffer *eb)
> +{
> +	struct vb2_buffer *vb = NULL;
> +	struct vb2_plane *vb_plane;
> +	unsigned int buffer, plane;
> +	int ret;
> +	struct dma_buf *dbuf;
> +
> +	if (q->memory != V4L2_MEMORY_MMAP) {
> +		dprintk(1, "Queue is not currently set up for mmap\n");
> +		return -EINVAL;
> +	}
> +
> +	if (!q->mem_ops->get_dmabuf) {
> +		dprintk(1, "Queue does not support DMA buffer exporting\n");
> +		return -EINVAL;
> +	}
> +
> +	/*
> +	 * Find the plane corresponding to the offset passed by userspace.
> +	 */
> +	ret = __find_plane_by_offset(q, eb->mem_offset, &buffer, &plane);
> +	if (ret) {
> +		dprintk(1, "invalid offset %u\n", eb->mem_offset);
> +		return ret;
> +	}
> +
> +	vb = q->bufs[buffer];
> +	vb_plane = &vb->planes[plane];
> +
> +	dbuf = call_memop(q, get_dmabuf, vb_plane->mem_priv);
> +	if (IS_ERR_OR_NULL(dbuf)) {
> +		dprintk(1, "Failed to export buffer %d, plane %d\n",
> +			buffer, plane);
> +		return -EINVAL;
> +	}
> +
> +	ret = dma_buf_fd(dbuf);
> +	if (ret < 0) {
> +		dprintk(3, "buffer %d, plane %d failed to export (%d)\n",
> +			buffer, plane, ret);
> +		return ret;
> +	}
> +
> +	dprintk(3, "buffer %d, plane %d exported as %d descriptor\n",
> +		buffer, plane, ret);
> +	eb->fd = ret;
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(vb2_expbuf);
> +
>  /**
>   * __vb2_queue_cancel() - cancel and stop (pause) streaming
>   *
> diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h
> index 412c6a4..548252b 100644
> --- a/include/media/videobuf2-core.h
> +++ b/include/media/videobuf2-core.h
> @@ -79,6 +79,7 @@ struct vb2_mem_ops {
>  	void		(*prepare)(void *buf_priv);
>  	void		(*finish)(void *buf_priv);
>  	void		(*put)(void *buf_priv);
> +	struct dma_buf *(*get_dmabuf)(void *buf_priv);
> 
>  	void		*(*get_userptr)(void *alloc_ctx, unsigned long vaddr,
>  					unsigned long size, int write);
> @@ -348,6 +349,7 @@ int vb2_queue_init(struct vb2_queue *q);
>  void vb2_queue_release(struct vb2_queue *q);
> 
>  int vb2_qbuf(struct vb2_queue *q, struct v4l2_buffer *b);
> +int vb2_expbuf(struct vb2_queue *q, struct v4l2_exportbuffer *eb);
>  int vb2_dqbuf(struct vb2_queue *q, struct v4l2_buffer *b, bool
> nonblocking);
> 
>  int vb2_streamon(struct vb2_queue *q, enum v4l2_buf_type type);

-- 
Regards,

Laurent Pinchart