Re: [RFC PATCH v3 2/5] media: videobuf2: Restructure vb2_buffer

[Junghak Sung <jh1009.sung@samsung.com>]

Hello Mauro,

First of all, thank you for your detailed review.


On 08/27/2015 07:28 PM, Mauro Carvalho Chehab wrote:
> Em Wed, 26 Aug 2015 20:59:29 +0900
> Junghak Sung <jh1009.sung@samsung.com> escreveu:
>
>> Remove v4l2-specific stuff from struct vb2_buffer and add member variables
>> related with buffer management.
>>
>> struct vb2_plane {
>>          <snip>
>>          /* plane information */
>>          unsigned int            bytesused;
>>          unsigned int            length;
>>          union {
>>                  unsigned int    offset;
>>                  unsigned long   userptr;
>>                  int             fd;
>>          } m;
>>          unsigned int            data_offset;
>> }
>>
>> struct vb2_buffer {
>>          <snip>
>>          /* buffer information */
>>          unsigned int            num_planes;
>>          unsigned int            index;
>>          unsigned int            type;
>>          unsigned int            memory;
>>
>>          struct vb2_plane        planes[VIDEO_MAX_PLANES];
>>          <snip>
>> };
>>
>> And create struct vb2_v4l2_buffer as container buffer for v4l2 use.
>>
>> struct vb2_v4l2_buffer {
>>          struct vb2_buffer       vb2_buf;
>>
>>          __u32                   flags;
>>          __u32                   field;
>>          struct timeval          timestamp;
>>          struct v4l2_timecode    timecode;
>>          __u32                   sequence;
>> };
>
> The comments seemed a little hard for me to read, but the changes
> look ok.
>

Ok, I will modify these comments more clearly at next round.

> I made some comments mostly regarding to documentation. See below.
>
>> This patch includes only changes inside of videobuf2. So, it is required to
>> modify all device drivers which use videobuf2.
>
> So, in practice, we need to fold both patches 2 and 3 when merging upstream,
> to avoid breaking git bisectability.
>

I'm sorry, but I can not understand the meaning of "fold both patches".
Would you please explain more detailed what should I do at next round.
If these two patches are get together to one patch, the size will
be over 300KB, which causes a size problem to send it to ML.

>>
>> Signed-off-by: Junghak Sung <jh1009.sung@samsung.com>
>> Signed-off-by: Geunyoung Kim <nenggun.kim@samsung.com>
>> Acked-by: Seung-Woo Kim <sw0312.kim@samsung.com>
>> Acked-by: Inki Dae <inki.dae@samsung.com>
>> ---
>>   drivers/media/v4l2-core/videobuf2-core.c |  324 +++++++++++++++++-------------
>>   include/media/videobuf2-core.h           |   50 ++---
>>   include/media/videobuf2-v4l2.h           |   26 +++
>>   3 files changed, 236 insertions(+), 164 deletions(-)
>>
>> diff --git a/drivers/media/v4l2-core/videobuf2-core.c b/drivers/media/v4l2-core/videobuf2-core.c
>> index ab00ea0..9266d50 100644
>> --- a/drivers/media/v4l2-core/videobuf2-core.c
>> +++ b/drivers/media/v4l2-core/videobuf2-core.c
>> @@ -35,10 +35,10 @@
>>   static int debug;
>>   module_param(debug, int, 0644);
>>
>> -#define dprintk(level, fmt, arg...)					      \
>> -	do {								      \
>> -		if (debug >= level)					      \
>> -			pr_info("vb2: %s: " fmt, __func__, ## arg); \
>> +#define dprintk(level, fmt, arg...)					\
>> +	do {								\
>> +		if (debug >= level)					\
>> +			pr_info("vb2: %s: " fmt, __func__, ## arg);	\
>>   	} while (0)
>>
>>   #ifdef CONFIG_VIDEO_ADV_DEBUG
>> @@ -53,7 +53,7 @@ module_param(debug, int, 0644);
>>
>>   #define log_memop(vb, op)						\
>>   	dprintk(2, "call_memop(%p, %d, %s)%s\n",			\
>> -		(vb)->vb2_queue, (vb)->v4l2_buf.index, #op,		\
>> +		(vb)->vb2_queue, (vb)->index, #op,			\
>>   		(vb)->vb2_queue->mem_ops->op ? "" : " (nop)")
>>
>>   #define call_memop(vb, op, args...)					\
>> @@ -115,7 +115,7 @@ module_param(debug, int, 0644);
>>
>>   #define log_vb_qop(vb, op, args...)					\
>>   	dprintk(2, "call_vb_qop(%p, %d, %s)%s\n",			\
>> -		(vb)->vb2_queue, (vb)->v4l2_buf.index, #op,		\
>> +		(vb)->vb2_queue, (vb)->index, #op,			\
>>   		(vb)->vb2_queue->ops->op ? "" : " (nop)")
>>
>>   #define call_vb_qop(vb, op, args...)					\
>> @@ -205,13 +205,13 @@ static int __vb2_buf_mem_alloc(struct vb2_buffer *vb)
>>   		unsigned long size = PAGE_ALIGN(q->plane_sizes[plane]);
>>
>>   		mem_priv = call_ptr_memop(vb, alloc, q->alloc_ctx[plane],
>> -				      size, dma_dir, q->gfp_flags);
>> +					size, dma_dir, q->gfp_flags);
>>   		if (IS_ERR_OR_NULL(mem_priv))
>>   			goto free;
>>
>>   		/* Associate allocator private data with this plane */
>>   		vb->planes[plane].mem_priv = mem_priv;
>> -		vb->v4l2_planes[plane].length = q->plane_sizes[plane];
>> +		vb->planes[plane].length = q->plane_sizes[plane];
>>   	}
>>
>>   	return 0;
>> @@ -235,8 +235,7 @@ static void __vb2_buf_mem_free(struct vb2_buffer *vb)
>>   	for (plane = 0; plane < vb->num_planes; ++plane) {
>>   		call_void_memop(vb, put, vb->planes[plane].mem_priv);
>>   		vb->planes[plane].mem_priv = NULL;
>> -		dprintk(3, "freed plane %d of buffer %d\n", plane,
>> -			vb->v4l2_buf.index);
>> +		dprintk(3, "freed plane %d of buffer %d\n", plane, vb->index);
>>   	}
>>   }
>>
>> @@ -269,7 +268,9 @@ static void __vb2_plane_dmabuf_put(struct vb2_buffer *vb, struct vb2_plane *p)
>>
>>   	call_void_memop(vb, detach_dmabuf, p->mem_priv);
>>   	dma_buf_put(p->dbuf);
>> -	memset(p, 0, sizeof(*p));
>> +	p->mem_priv = NULL;
>> +	p->dbuf = NULL;
>> +	p->dbuf_mapped = 0;
>>   }
>>
>>   /**
>> @@ -299,7 +300,7 @@ static void __setup_lengths(struct vb2_queue *q, unsigned int n)
>>   			continue;
>>
>>   		for (plane = 0; plane < vb->num_planes; ++plane)
>> -			vb->v4l2_planes[plane].length = q->plane_sizes[plane];
>> +			vb->planes[plane].length = q->plane_sizes[plane];
>>   	}
>>   }
>>
>> @@ -314,10 +315,10 @@ static void __setup_offsets(struct vb2_queue *q, unsigned int n)
>>   	unsigned long off;
>>
>>   	if (q->num_buffers) {
>> -		struct v4l2_plane *p;
>> +		struct vb2_plane *p;
>>   		vb = q->bufs[q->num_buffers - 1];
>> -		p = &vb->v4l2_planes[vb->num_planes - 1];
>> -		off = PAGE_ALIGN(p->m.mem_offset + p->length);
>> +		p = &vb->planes[vb->num_planes - 1];
>> +		off = PAGE_ALIGN(p->m.offset + p->length);
>>   	} else {
>>   		off = 0;
>>   	}
>> @@ -328,12 +329,12 @@ static void __setup_offsets(struct vb2_queue *q, unsigned int n)
>>   			continue;
>>
>>   		for (plane = 0; plane < vb->num_planes; ++plane) {
>> -			vb->v4l2_planes[plane].m.mem_offset = off;
>> +			vb->planes[plane].m.offset = off;
>>
>>   			dprintk(3, "buffer %d, plane %d offset 0x%08lx\n",
>>   					buffer, plane, off);
>>
>> -			off += vb->v4l2_planes[plane].length;
>> +			off += vb->planes[plane].length;
>>   			off = PAGE_ALIGN(off);
>>   		}
>>   	}
>> @@ -347,7 +348,7 @@ static void __setup_offsets(struct vb2_queue *q, unsigned int n)
>>    * Returns the number of buffers successfully allocated.
>>    */
>>   static int __vb2_queue_alloc(struct vb2_queue *q, enum v4l2_memory memory,
>> -			     unsigned int num_buffers, unsigned int num_planes)
>> +			unsigned int num_buffers, unsigned int num_planes)
>>   {
>>   	unsigned int buffer;
>>   	struct vb2_buffer *vb;
>> @@ -361,16 +362,12 @@ static int __vb2_queue_alloc(struct vb2_queue *q, enum v4l2_memory memory,
>>   			break;
>>   		}
>>
>> -		/* Length stores number of planes for multiplanar buffers */
>> -		if (V4L2_TYPE_IS_MULTIPLANAR(q->type))
>> -			vb->v4l2_buf.length = num_planes;
>> -
>>   		vb->state = VB2_BUF_STATE_DEQUEUED;
>>   		vb->vb2_queue = q;
>>   		vb->num_planes = num_planes;
>> -		vb->v4l2_buf.index = q->num_buffers + buffer;
>> -		vb->v4l2_buf.type = q->type;
>> -		vb->v4l2_buf.memory = memory;
>> +		vb->index = q->num_buffers + buffer;
>> +		vb->type = q->type;
>> +		vb->memory = memory;
>>
>>   		/* Allocate video buffer memory for the MMAP type */
>>   		if (memory == V4L2_MEMORY_MMAP) {
>> @@ -418,7 +415,7 @@ static void __vb2_free_mem(struct vb2_queue *q, unsigned int buffers)
>>   	struct vb2_buffer *vb;
>>
>>   	for (buffer = q->num_buffers - buffers; buffer < q->num_buffers;
>> -	     ++buffer) {
>> +			++buffer) {
>>   		vb = q->bufs[buffer];
>>   		if (!vb)
>>   			continue;
>> @@ -451,7 +448,7 @@ static int __vb2_queue_free(struct vb2_queue *q, unsigned int buffers)
>>   	 * just return -EAGAIN.
>>   	 */
>>   	for (buffer = q->num_buffers - buffers; buffer < q->num_buffers;
>> -	     ++buffer) {
>> +			++buffer) {
>>   		if (q->bufs[buffer] == NULL)
>>   			continue;
>>   		if (q->bufs[buffer]->state == VB2_BUF_STATE_PREPARING) {
>> @@ -462,7 +459,7 @@ static int __vb2_queue_free(struct vb2_queue *q, unsigned int buffers)
>>
>>   	/* Call driver-provided cleanup function for each buffer, if provided */
>>   	for (buffer = q->num_buffers - buffers; buffer < q->num_buffers;
>> -	     ++buffer) {
>> +			++buffer) {
>>   		struct vb2_buffer *vb = q->bufs[buffer];
>>
>>   		if (vb && vb->planes[0].mem_priv)
>> @@ -536,7 +533,7 @@ static int __vb2_queue_free(struct vb2_queue *q, unsigned int buffers)
>>
>>   	/* Free videobuf buffers */
>>   	for (buffer = q->num_buffers - buffers; buffer < q->num_buffers;
>> -	     ++buffer) {
>> +			++buffer) {
>>   		kfree(q->bufs[buffer]);
>>   		q->bufs[buffer] = NULL;
>>   	}
>> @@ -590,23 +587,22 @@ static int __verify_length(struct vb2_buffer *vb, const struct v4l2_buffer *b)
>>   	if (V4L2_TYPE_IS_MULTIPLANAR(b->type)) {
>>   		for (plane = 0; plane < vb->num_planes; ++plane) {
>>   			length = (b->memory == V4L2_MEMORY_USERPTR ||
>> -				  b->memory == V4L2_MEMORY_DMABUF)
>> -			       ? b->m.planes[plane].length
>> -			       : vb->v4l2_planes[plane].length;
>> +				b->memory == V4L2_MEMORY_DMABUF)
>> +				? b->m.planes[plane].length
>> +				: vb->planes[plane].length;
>>   			bytesused = b->m.planes[plane].bytesused
>> -				  ? b->m.planes[plane].bytesused : length;
>> +				? b->m.planes[plane].bytesused : length;
>>
>>   			if (b->m.planes[plane].bytesused > length)
>>   				return -EINVAL;
>>
>>   			if (b->m.planes[plane].data_offset > 0 &&
>> -			    b->m.planes[plane].data_offset >= bytesused)
>> +				b->m.planes[plane].data_offset >= bytesused)
>>   				return -EINVAL;
>>   		}
>>   	} else {
>>   		length = (b->memory == V4L2_MEMORY_USERPTR)
>> -		       ? b->length : vb->v4l2_planes[0].length;
>> -		bytesused = b->bytesused ? b->bytesused : length;
>> +			? b->length : vb->planes[0].length;
>>
>>   		if (b->bytesused > length)
>>   			return -EINVAL;
>> @@ -656,12 +652,21 @@ static bool __buffers_in_use(struct vb2_queue *q)
>>    */
>>   static void __fill_v4l2_buffer(struct vb2_buffer *vb, struct v4l2_buffer *b)
>>   {
>> +	struct vb2_v4l2_buffer *vbuf = to_vb2_v4l2_buffer(vb);
>>   	struct vb2_queue *q = vb->vb2_queue;
>> +	unsigned int plane;
>>
>>   	/* Copy back data such as timestamp, flags, etc. */
>> -	memcpy(b, &vb->v4l2_buf, offsetof(struct v4l2_buffer, m));
>> -	b->reserved2 = vb->v4l2_buf.reserved2;
>> -	b->reserved = vb->v4l2_buf.reserved;
>> +	b->index = vb->index;
>> +	b->type = vb->type;
>> +	b->memory = vb->memory;
>> +	b->bytesused = 0;
>> +
>> +	b->flags = vbuf->flags;
>> +	b->field = vbuf->field;
>> +	b->timestamp = vbuf->timestamp;
>> +	b->timecode = vbuf->timecode;
>> +	b->sequence = vbuf->sequence;
>>
>>   	if (V4L2_TYPE_IS_MULTIPLANAR(q->type)) {
>>   		/*
>> @@ -669,21 +674,33 @@ static void __fill_v4l2_buffer(struct vb2_buffer *vb, struct v4l2_buffer *b)
>>   		 * for it. The caller has already verified memory and size.
>>   		 */
>>   		b->length = vb->num_planes;
>> -		memcpy(b->m.planes, vb->v4l2_planes,
>> -			b->length * sizeof(struct v4l2_plane));
>> +		for (plane = 0; plane < vb->num_planes; ++plane) {
>> +			struct v4l2_plane *pdst = &b->m.planes[plane];
>> +			struct vb2_plane *psrc = &vb->planes[plane];
>> +
>> +			pdst->bytesused = psrc->bytesused;
>> +			pdst->length = psrc->length;
>> +			if (q->memory == V4L2_MEMORY_MMAP)
>> +				pdst->m.mem_offset = psrc->m.offset;
>> +			else if (q->memory == V4L2_MEMORY_USERPTR)
>> +				pdst->m.userptr = psrc->m.userptr;
>> +			else if (q->memory == V4L2_MEMORY_DMABUF)
>> +				pdst->m.fd = psrc->m.fd;
>> +			pdst->data_offset = psrc->data_offset;
>> +		}
>>   	} else {
>>   		/*
>>   		 * We use length and offset in v4l2_planes array even for
>>   		 * single-planar buffers, but userspace does not.
>>   		 */
>> -		b->length = vb->v4l2_planes[0].length;
>> -		b->bytesused = vb->v4l2_planes[0].bytesused;
>> +		b->length = vb->planes[0].length;
>> +		b->bytesused = vb->planes[0].bytesused;
>>   		if (q->memory == V4L2_MEMORY_MMAP)
>> -			b->m.offset = vb->v4l2_planes[0].m.mem_offset;
>> +			b->m.offset = vb->planes[0].m.offset;
>>   		else if (q->memory == V4L2_MEMORY_USERPTR)
>> -			b->m.userptr = vb->v4l2_planes[0].m.userptr;
>> +			b->m.userptr = vb->planes[0].m.userptr;
>>   		else if (q->memory == V4L2_MEMORY_DMABUF)
>> -			b->m.fd = vb->v4l2_planes[0].m.fd;
>> +			b->m.fd = vb->planes[0].m.fd;
>>   	}
>>
>>   	/*
>> @@ -692,7 +709,7 @@ static void __fill_v4l2_buffer(struct vb2_buffer *vb, struct v4l2_buffer *b)
>>   	b->flags &= ~V4L2_BUFFER_MASK_FLAGS;
>>   	b->flags |= q->timestamp_flags & V4L2_BUF_FLAG_TIMESTAMP_MASK;
>>   	if ((q->timestamp_flags & V4L2_BUF_FLAG_TIMESTAMP_MASK) !=
>> -	    V4L2_BUF_FLAG_TIMESTAMP_COPY) {
>> +			V4L2_BUF_FLAG_TIMESTAMP_COPY) {
>>   		/*
>>   		 * For non-COPY timestamps, drop timestamp source bits
>>   		 * and obtain the timestamp source from the queue.
>> @@ -767,7 +784,7 @@ EXPORT_SYMBOL(vb2_querybuf);
>>   static int __verify_userptr_ops(struct vb2_queue *q)
>>   {
>>   	if (!(q->io_modes & VB2_USERPTR) || !q->mem_ops->get_userptr ||
>> -	    !q->mem_ops->put_userptr)
>> +			!q->mem_ops->put_userptr)
>>   		return -EINVAL;
>>
>>   	return 0;
>> @@ -780,7 +797,7 @@ static int __verify_userptr_ops(struct vb2_queue *q)
>>   static int __verify_mmap_ops(struct vb2_queue *q)
>>   {
>>   	if (!(q->io_modes & VB2_MMAP) || !q->mem_ops->alloc ||
>> -	    !q->mem_ops->put || !q->mem_ops->mmap)
>> +			!q->mem_ops->put || !q->mem_ops->mmap)
>>   		return -EINVAL;
>>
>>   	return 0;
>> @@ -793,8 +810,8 @@ static int __verify_mmap_ops(struct vb2_queue *q)
>>   static int __verify_dmabuf_ops(struct vb2_queue *q)
>>   {
>>   	if (!(q->io_modes & VB2_DMABUF) || !q->mem_ops->attach_dmabuf ||
>> -	    !q->mem_ops->detach_dmabuf  || !q->mem_ops->map_dmabuf ||
>> -	    !q->mem_ops->unmap_dmabuf)
>> +		!q->mem_ops->detach_dmabuf  || !q->mem_ops->map_dmabuf ||
>> +		!q->mem_ops->unmap_dmabuf)
>>   		return -EINVAL;
>>
>>   	return 0;
>> @@ -808,7 +825,7 @@ static int __verify_memory_type(struct vb2_queue *q,
>>   		enum v4l2_memory memory, enum v4l2_buf_type type)
>>   {
>>   	if (memory != V4L2_MEMORY_MMAP && memory != V4L2_MEMORY_USERPTR &&
>> -	    memory != V4L2_MEMORY_DMABUF) {
>> +			memory != V4L2_MEMORY_DMABUF) {
>>   		dprintk(1, "unsupported memory type\n");
>>   		return -EINVAL;
>>   	}
>> @@ -927,7 +944,7 @@ static int __reqbufs(struct vb2_queue *q, struct v4l2_requestbuffers *req)
>>   	 * Driver also sets the size and allocator context for each plane.
>>   	 */
>>   	ret = call_qop(q, queue_setup, q, NULL, &num_buffers, &num_planes,
>> -		       q->plane_sizes, q->alloc_ctx);
>> +			q->plane_sizes, q->alloc_ctx);
>>   	if (ret)
>>   		return ret;
>>
>> @@ -952,7 +969,7 @@ static int __reqbufs(struct vb2_queue *q, struct v4l2_requestbuffers *req)
>>   		num_buffers = allocated_buffers;
>>
>>   		ret = call_qop(q, queue_setup, q, NULL, &num_buffers,
>> -			       &num_planes, q->plane_sizes, q->alloc_ctx);
>> +				&num_planes, q->plane_sizes, q->alloc_ctx);
>>
>>   		if (!ret && allocated_buffers < num_buffers)
>>   			ret = -ENOMEM;
>> @@ -1040,7 +1057,7 @@ static int __create_bufs(struct vb2_queue *q, struct v4l2_create_buffers *create
>>   	 * buffer and their sizes are acceptable
>>   	 */
>>   	ret = call_qop(q, queue_setup, q, &create->format, &num_buffers,
>> -		       &num_planes, q->plane_sizes, q->alloc_ctx);
>> +			&num_planes, q->plane_sizes, q->alloc_ctx);
>>   	if (ret)
>>   		return ret;
>>
>> @@ -1063,7 +1080,7 @@ static int __create_bufs(struct vb2_queue *q, struct v4l2_create_buffers *create
>>   		 * queue driver has set up
>>   		 */
>>   		ret = call_qop(q, queue_setup, q, &create->format, &num_buffers,
>> -			       &num_planes, q->plane_sizes, q->alloc_ctx);
>> +				&num_planes, q->plane_sizes, q->alloc_ctx);
>>
>>   		if (!ret && allocated_buffers < num_buffers)
>>   			ret = -ENOMEM;
>> @@ -1183,8 +1200,8 @@ void vb2_buffer_done(struct vb2_buffer *vb, enum vb2_buffer_state state)
>>   		return;
>>
>>   	if (WARN_ON(state != VB2_BUF_STATE_DONE &&
>> -		    state != VB2_BUF_STATE_ERROR &&
>> -		    state != VB2_BUF_STATE_QUEUED))
>> +		state != VB2_BUF_STATE_ERROR &&
>> +		state != VB2_BUF_STATE_QUEUED))
>>   		state = VB2_BUF_STATE_ERROR;
>>
>>   #ifdef CONFIG_VIDEO_ADV_DEBUG
>> @@ -1195,7 +1212,7 @@ void vb2_buffer_done(struct vb2_buffer *vb, enum vb2_buffer_state state)
>>   	vb->cnt_buf_done++;
>>   #endif
>>   	dprintk(4, "done processing on buffer %d, state: %d\n",
>> -			vb->v4l2_buf.index, state);
>> +			vb->index, state);
>>
>>   	/* sync buffers */
>>   	for (plane = 0; plane < vb->num_planes; ++plane)
>> @@ -1268,25 +1285,26 @@ static void vb2_warn_zero_bytesused(struct vb2_buffer *vb)
>>    * v4l2_buffer by the userspace. The caller has already verified that struct
>>    * v4l2_buffer has a valid number of planes.
>>    */
>> -static void __fill_vb2_buffer(struct vb2_buffer *vb, const struct v4l2_buffer *b,
>> -				struct v4l2_plane *v4l2_planes)
>> +static void __fill_vb2_buffer(struct vb2_buffer *vb,
>> +		const struct v4l2_buffer *b, struct vb2_plane *planes)
>>   {
>> +	struct vb2_v4l2_buffer *vbuf = to_vb2_v4l2_buffer(vb);
>>   	unsigned int plane;
>>
>>   	if (V4L2_TYPE_IS_MULTIPLANAR(b->type)) {
>>   		if (b->memory == V4L2_MEMORY_USERPTR) {
>>   			for (plane = 0; plane < vb->num_planes; ++plane) {
>> -				v4l2_planes[plane].m.userptr =
>> +				planes[plane].m.userptr =
>>   					b->m.planes[plane].m.userptr;
>> -				v4l2_planes[plane].length =
>> +				planes[plane].length =
>>   					b->m.planes[plane].length;
>>   			}
>>   		}
>>   		if (b->memory == V4L2_MEMORY_DMABUF) {
>>   			for (plane = 0; plane < vb->num_planes; ++plane) {
>> -				v4l2_planes[plane].m.fd =
>> +				planes[plane].m.fd =
>>   					b->m.planes[plane].m.fd;
>> -				v4l2_planes[plane].length =
>> +				planes[plane].length =
>>   					b->m.planes[plane].length;
>>   			}
>>   		}
>> @@ -1310,7 +1328,7 @@ static void __fill_vb2_buffer(struct vb2_buffer *vb, const struct v4l2_buffer *b
>>   			 * applications working.
>>   			 */
>>   			for (plane = 0; plane < vb->num_planes; ++plane) {
>> -				struct v4l2_plane *pdst = &v4l2_planes[plane];
>> +				struct vb2_plane *pdst = &planes[plane];
>>   				struct v4l2_plane *psrc = &b->m.planes[plane];
>>
>>   				if (psrc->bytesused == 0)
>> @@ -1340,13 +1358,13 @@ static void __fill_vb2_buffer(struct vb2_buffer *vb, const struct v4l2_buffer *b
>>   		 * old userspace applications working.
>>   		 */
>>   		if (b->memory == V4L2_MEMORY_USERPTR) {
>> -			v4l2_planes[0].m.userptr = b->m.userptr;
>> -			v4l2_planes[0].length = b->length;
>> +			planes[0].m.userptr = b->m.userptr;
>> +			planes[0].length = b->length;
>>   		}
>>
>>   		if (b->memory == V4L2_MEMORY_DMABUF) {
>> -			v4l2_planes[0].m.fd = b->m.fd;
>> -			v4l2_planes[0].length = b->length;
>> +			planes[0].m.fd = b->m.fd;
>> +			planes[0].length = b->length;
>>   		}
>>
>>   		if (V4L2_TYPE_IS_OUTPUT(b->type)) {
>> @@ -1354,25 +1372,26 @@ static void __fill_vb2_buffer(struct vb2_buffer *vb, const struct v4l2_buffer *b
>>   				vb2_warn_zero_bytesused(vb);
>>
>>   			if (vb->vb2_queue->allow_zero_bytesused)
>> -				v4l2_planes[0].bytesused = b->bytesused;
>> +				planes[0].bytesused = b->bytesused;
>>   			else
>> -				v4l2_planes[0].bytesused = b->bytesused ?
>> -					b->bytesused : v4l2_planes[0].length;
>> +				planes[0].bytesused = b->bytesused ?
>> +					b->bytesused : planes[0].length;
>>   		} else
>> -			v4l2_planes[0].bytesused = 0;
>> +			planes[0].bytesused = 0;
>>
>>   	}
>>
>>   	/* Zero flags that the vb2 core handles */
>> -	vb->v4l2_buf.flags = b->flags & ~V4L2_BUFFER_MASK_FLAGS;
>> +	vbuf->flags = b->flags & ~V4L2_BUFFER_MASK_FLAGS;
>>   	if ((vb->vb2_queue->timestamp_flags & V4L2_BUF_FLAG_TIMESTAMP_MASK) !=
>> -	    V4L2_BUF_FLAG_TIMESTAMP_COPY || !V4L2_TYPE_IS_OUTPUT(b->type)) {
>> +			V4L2_BUF_FLAG_TIMESTAMP_COPY ||
>> +			!V4L2_TYPE_IS_OUTPUT(b->type)) {
>>   		/*
>>   		 * Non-COPY timestamps and non-OUTPUT queues will get
>>   		 * their timestamp and timestamp source flags from the
>>   		 * queue.
>>   		 */
>> -		vb->v4l2_buf.flags &= ~V4L2_BUF_FLAG_TSTAMP_SRC_MASK;
>> +		vbuf->flags &= ~V4L2_BUF_FLAG_TSTAMP_SRC_MASK;
>>   	}
>>
>>   	if (V4L2_TYPE_IS_OUTPUT(b->type)) {
>> @@ -1382,11 +1401,11 @@ static void __fill_vb2_buffer(struct vb2_buffer *vb, const struct v4l2_buffer *b
>>   		 * The 'field' is valid metadata for this output buffer
>>   		 * and so that needs to be copied here.
>>   		 */
>> -		vb->v4l2_buf.flags &= ~V4L2_BUF_FLAG_TIMECODE;
>> -		vb->v4l2_buf.field = b->field;
>> +		vbuf->flags &= ~V4L2_BUF_FLAG_TIMECODE;
>> +		vbuf->field = b->field;
>>   	} else {
>>   		/* Zero any output buffer flags as this is a capture buffer */
>> -		vb->v4l2_buf.flags &= ~V4L2_BUFFER_OUT_FLAGS;
>> +		vbuf->flags &= ~V4L2_BUFFER_OUT_FLAGS;
>>   	}
>>   }
>>
>> @@ -1395,7 +1414,7 @@ static void __fill_vb2_buffer(struct vb2_buffer *vb, const struct v4l2_buffer *b
>>    */
>>   static int __qbuf_mmap(struct vb2_buffer *vb, const struct v4l2_buffer *b)
>>   {
>> -	__fill_vb2_buffer(vb, b, vb->v4l2_planes);
>> +	__fill_vb2_buffer(vb, b, vb->planes);
>>   	return call_vb_qop(vb, buf_prepare, vb);
>>   }
>>
>> @@ -1404,7 +1423,7 @@ static int __qbuf_mmap(struct vb2_buffer *vb, const struct v4l2_buffer *b)
>>    */
>>   static int __qbuf_userptr(struct vb2_buffer *vb, const struct v4l2_buffer *b)
>>   {
>> -	struct v4l2_plane planes[VIDEO_MAX_PLANES];
>> +	struct vb2_plane planes[VIDEO_MAX_PLANES];
>>   	struct vb2_queue *q = vb->vb2_queue;
>>   	void *mem_priv;
>>   	unsigned int plane;
>> @@ -1419,9 +1438,9 @@ static int __qbuf_userptr(struct vb2_buffer *vb, const struct v4l2_buffer *b)
>>
>>   	for (plane = 0; plane < vb->num_planes; ++plane) {
>>   		/* Skip the plane if already verified */
>> -		if (vb->v4l2_planes[plane].m.userptr &&
>> -		    vb->v4l2_planes[plane].m.userptr == planes[plane].m.userptr
>> -		    && vb->v4l2_planes[plane].length == planes[plane].length)
>> +		if (vb->planes[plane].m.userptr &&
>> +			vb->planes[plane].m.userptr == planes[plane].m.userptr
>> +			&& vb->planes[plane].length == planes[plane].length)
>>   			continue;
>>
>>   		dprintk(3, "userspace address for plane %d changed, "
>> @@ -1447,12 +1466,15 @@ static int __qbuf_userptr(struct vb2_buffer *vb, const struct v4l2_buffer *b)
>>   		}
>>
>>   		vb->planes[plane].mem_priv = NULL;
>> -		memset(&vb->v4l2_planes[plane], 0, sizeof(struct v4l2_plane));
>> +		vb->planes[plane].bytesused = 0;
>> +		vb->planes[plane].length = 0;
>> +		vb->planes[plane].m.userptr = 0;
>> +		vb->planes[plane].data_offset = 0;
>>
>>   		/* Acquire each plane's memory */
>>   		mem_priv = call_ptr_memop(vb, get_userptr, q->alloc_ctx[plane],
>> -				      planes[plane].m.userptr,
>> -				      planes[plane].length, dma_dir);
>> +					planes[plane].m.userptr,
>> +					planes[plane].length, dma_dir);
>>   		if (IS_ERR_OR_NULL(mem_priv)) {
>>   			dprintk(1, "failed acquiring userspace "
>>   						"memory for plane %d\n", plane);
>> @@ -1466,8 +1488,12 @@ static int __qbuf_userptr(struct vb2_buffer *vb, const struct v4l2_buffer *b)
>>   	 * Now that everything is in order, copy relevant information
>>   	 * provided by userspace.
>>   	 */
>> -	for (plane = 0; plane < vb->num_planes; ++plane)
>> -		vb->v4l2_planes[plane] = planes[plane];
>> +	for (plane = 0; plane < vb->num_planes; ++plane) {
>> +		vb->planes[plane].bytesused = planes[plane].bytesused;
>> +		vb->planes[plane].length = planes[plane].length;
>> +		vb->planes[plane].m.userptr = planes[plane].m.userptr;
>> +		vb->planes[plane].data_offset = planes[plane].data_offset;
>> +	}
>>
>>   	if (reacquired) {
>>   		/*
>> @@ -1494,10 +1520,11 @@ err:
>>   	/* In case of errors, release planes that were already acquired */
>>   	for (plane = 0; plane < vb->num_planes; ++plane) {
>>   		if (vb->planes[plane].mem_priv)
>> -			call_void_memop(vb, put_userptr, vb->planes[plane].mem_priv);
>> +			call_void_memop(vb, put_userptr,
>> +				vb->planes[plane].mem_priv);
>>   		vb->planes[plane].mem_priv = NULL;
>> -		vb->v4l2_planes[plane].m.userptr = 0;
>> -		vb->v4l2_planes[plane].length = 0;
>> +		vb->planes[plane].m.userptr = 0;
>> +		vb->planes[plane].length = 0;
>>   	}
>>
>>   	return ret;
>> @@ -1508,7 +1535,7 @@ err:
>>    */
>>   static int __qbuf_dmabuf(struct vb2_buffer *vb, const struct v4l2_buffer *b)
>>   {
>> -	struct v4l2_plane planes[VIDEO_MAX_PLANES];
>> +	struct vb2_plane planes[VIDEO_MAX_PLANES];
>>   	struct vb2_queue *q = vb->vb2_queue;
>>   	void *mem_priv;
>>   	unsigned int plane;
>> @@ -1544,7 +1571,7 @@ static int __qbuf_dmabuf(struct vb2_buffer *vb, const struct v4l2_buffer *b)
>>
>>   		/* Skip the plane if already verified */
>>   		if (dbuf == vb->planes[plane].dbuf &&
>> -		    vb->v4l2_planes[plane].length == planes[plane].length) {
>> +			vb->planes[plane].length == planes[plane].length) {
>>   			dma_buf_put(dbuf);
>>   			continue;
>>   		}
>> @@ -1558,11 +1585,15 @@ static int __qbuf_dmabuf(struct vb2_buffer *vb, const struct v4l2_buffer *b)
>>
>>   		/* Release previously acquired memory if present */
>>   		__vb2_plane_dmabuf_put(vb, &vb->planes[plane]);
>> -		memset(&vb->v4l2_planes[plane], 0, sizeof(struct v4l2_plane));
>> +		vb->planes[plane].bytesused = 0;
>> +		vb->planes[plane].length = 0;
>> +		vb->planes[plane].m.fd = 0;
>> +		vb->planes[plane].data_offset = 0;
>>
>>   		/* Acquire each plane's memory */
>> -		mem_priv = call_ptr_memop(vb, attach_dmabuf, q->alloc_ctx[plane],
>> -			dbuf, planes[plane].length, dma_dir);
>> +		mem_priv = call_ptr_memop(vb, attach_dmabuf,
>> +			q->alloc_ctx[plane], dbuf, planes[plane].length,
>> +			dma_dir);
>>   		if (IS_ERR(mem_priv)) {
>>   			dprintk(1, "failed to attach dmabuf\n");
>>   			ret = PTR_ERR(mem_priv);
>> @@ -1592,8 +1623,12 @@ static int __qbuf_dmabuf(struct vb2_buffer *vb, const struct v4l2_buffer *b)
>>   	 * Now that everything is in order, copy relevant information
>>   	 * provided by userspace.
>>   	 */
>> -	for (plane = 0; plane < vb->num_planes; ++plane)
>> -		vb->v4l2_planes[plane] = planes[plane];
>> +	for (plane = 0; plane < vb->num_planes; ++plane) {
>> +		vb->planes[plane].bytesused = planes[plane].bytesused;
>> +		vb->planes[plane].length = planes[plane].length;
>> +		vb->planes[plane].m.fd = planes[plane].m.userptr;
>> +		vb->planes[plane].data_offset = planes[plane].data_offset;
>> +	}
>>
>>   	if (reacquired) {
>>   		/*
>> @@ -1644,6 +1679,7 @@ static void __enqueue_in_driver(struct vb2_buffer *vb)
>>
>>   static int __buf_prepare(struct vb2_buffer *vb, const struct v4l2_buffer *b)
>>   {
>> +	struct vb2_v4l2_buffer *vbuf = to_vb2_v4l2_buffer(vb);
>>   	struct vb2_queue *q = vb->vb2_queue;
>>   	int ret;
>>
>> @@ -1672,9 +1708,9 @@ static int __buf_prepare(struct vb2_buffer *vb, const struct v4l2_buffer *b)
>>   	}
>>
>>   	vb->state = VB2_BUF_STATE_PREPARING;
>> -	vb->v4l2_buf.timestamp.tv_sec = 0;
>> -	vb->v4l2_buf.timestamp.tv_usec = 0;
>> -	vb->v4l2_buf.sequence = 0;
>> +	vbuf->timestamp.tv_sec = 0;
>> +	vbuf->timestamp.tv_usec = 0;
>> +	vbuf->sequence = 0;
>>
>>   	switch (q->memory) {
>>   	case V4L2_MEMORY_MMAP:
>> @@ -1701,7 +1737,7 @@ static int __buf_prepare(struct vb2_buffer *vb, const struct v4l2_buffer *b)
>>   }
>>
>>   static int vb2_queue_or_prepare_buf(struct vb2_queue *q, struct v4l2_buffer *b,
>> -				    const char *opname)
>> +					const char *opname)
>>   {
>>   	if (b->type != q->type) {
>>   		dprintk(1, "%s: invalid buffer type\n", opname);
>> @@ -1768,7 +1804,7 @@ int vb2_prepare_buf(struct vb2_queue *q, struct v4l2_buffer *b)
>>   		/* Fill buffer information for the userspace */
>>   		__fill_v4l2_buffer(vb, b);
>>
>> -		dprintk(1, "prepare of buffer %d succeeded\n", vb->v4l2_buf.index);
>> +		dprintk(1, "prepare of buffer %d succeeded\n", vb->index);
>>   	}
>>   	return ret;
>>   }
>> @@ -1800,7 +1836,7 @@ static int vb2_start_streaming(struct vb2_queue *q)
>>   	/* Tell the driver to start streaming */
>>   	q->start_streaming_called = 1;
>>   	ret = call_qop(q, start_streaming, q,
>> -		       atomic_read(&q->owned_by_drv_count));
>> +			atomic_read(&q->owned_by_drv_count));
>>   	if (!ret)
>>   		return 0;
>>
>> @@ -1810,7 +1846,7 @@ static int vb2_start_streaming(struct vb2_queue *q)
>>   	/*
>>   	 * If you see this warning, then the driver isn't cleaning up properly
>>   	 * after a failed start_streaming(). See the start_streaming()
>> -	 * documentation in videobuf2-v4l2.h for more information how buffers
>> +	 * documentation in videobuf2-core.h for more information how buffers
>>   	 * should be returned to vb2 in start_streaming().
>>   	 */
>>   	if (WARN_ON(atomic_read(&q->owned_by_drv_count))) {
>> @@ -1841,11 +1877,13 @@ static int vb2_internal_qbuf(struct vb2_queue *q, struct v4l2_buffer *b)
>>   {
>>   	int ret = vb2_queue_or_prepare_buf(q, b, "qbuf");
>>   	struct vb2_buffer *vb;
>> +	struct vb2_v4l2_buffer *vbuf;
>>
>>   	if (ret)
>>   		return ret;
>>
>>   	vb = q->bufs[b->index];
>> +	vbuf = to_vb2_v4l2_buffer(vb);
>>
>>   	switch (vb->state) {
>>   	case VB2_BUF_STATE_DEQUEUED:
>> @@ -1877,11 +1915,11 @@ static int vb2_internal_qbuf(struct vb2_queue *q, struct v4l2_buffer *b)
>>   		 * and the timecode field and flag if needed.
>>   		 */
>>   		if ((q->timestamp_flags & V4L2_BUF_FLAG_TIMESTAMP_MASK) ==
>> -		    V4L2_BUF_FLAG_TIMESTAMP_COPY)
>> -			vb->v4l2_buf.timestamp = b->timestamp;
>> -		vb->v4l2_buf.flags |= b->flags & V4L2_BUF_FLAG_TIMECODE;
>> +				V4L2_BUF_FLAG_TIMESTAMP_COPY)
>> +			vbuf->timestamp = b->timestamp;
>> +		vbuf->flags |= b->flags & V4L2_BUF_FLAG_TIMECODE;
>>   		if (b->flags & V4L2_BUF_FLAG_TIMECODE)
>> -			vb->v4l2_buf.timecode = b->timecode;
>> +			vbuf->timecode = b->timecode;
>>   	}
>>
>>   	trace_vb2_qbuf(q, vb);
>> @@ -1903,13 +1941,13 @@ static int vb2_internal_qbuf(struct vb2_queue *q, struct v4l2_buffer *b)
>>   	 * then we can finally call start_streaming().
>>   	 */
>>   	if (q->streaming && !q->start_streaming_called &&
>> -	    q->queued_count >= q->min_buffers_needed) {
>> +			q->queued_count >= q->min_buffers_needed) {
>>   		ret = vb2_start_streaming(q);
>>   		if (ret)
>>   			return ret;
>>   	}
>>
>> -	dprintk(1, "qbuf of buffer %d succeeded\n", vb->v4l2_buf.index);
>> +	dprintk(1, "qbuf of buffer %d succeeded\n", vb->index);
>>   	return 0;
>>   }
>>
>> @@ -2099,9 +2137,11 @@ static void __vb2_dqbuf(struct vb2_buffer *vb)
>>   		}
>>   }
>>
>> -static int vb2_internal_dqbuf(struct vb2_queue *q, struct v4l2_buffer *b, bool nonblocking)
>> +static int vb2_internal_dqbuf(struct vb2_queue *q, struct v4l2_buffer *b,
>> +		bool nonblocking)
>>   {
>>   	struct vb2_buffer *vb = NULL;
>> +	struct vb2_v4l2_buffer *vbuf = NULL;
>>   	int ret;
>>
>>   	if (b->type != q->type) {
>> @@ -2134,14 +2174,15 @@ static int vb2_internal_dqbuf(struct vb2_queue *q, struct v4l2_buffer *b, bool n
>>
>>   	trace_vb2_dqbuf(q, vb);
>>
>> +	vbuf = to_vb2_v4l2_buffer(vb);
>>   	if (!V4L2_TYPE_IS_OUTPUT(q->type) &&
>> -	    vb->v4l2_buf.flags & V4L2_BUF_FLAG_LAST)
>> +			vbuf->flags & V4L2_BUF_FLAG_LAST)
>>   		q->last_buffer_dequeued = true;
>>   	/* go back to dequeued state */
>>   	__vb2_dqbuf(vb);
>>
>>   	dprintk(1, "dqbuf of buffer %d, with state %d\n",
>> -			vb->v4l2_buf.index, vb->state);
>> +			vb->index, vb->state);
>>
>>   	return 0;
>>   }
>> @@ -2197,7 +2238,7 @@ static void __vb2_queue_cancel(struct vb2_queue *q)
>>   	/*
>>   	 * If you see this warning, then the driver isn't cleaning up properly
>>   	 * in stop_streaming(). See the stop_streaming() documentation in
>> -	 * videobuf2-v4l2.h for more information how buffers should be returned
>> +	 * videobuf2-core.h for more information how buffers should be returned
>>   	 * to vb2 in stop_streaming().
>>   	 */
>>   	if (WARN_ON(atomic_read(&q->owned_by_drv_count))) {
>> @@ -2399,7 +2440,7 @@ static int __find_plane_by_offset(struct vb2_queue *q, unsigned long off,
>>   		vb = q->bufs[buffer];
>>
>>   		for (plane = 0; plane < vb->num_planes; ++plane) {
>> -			if (vb->v4l2_planes[plane].m.mem_offset == off) {
>> +			if (vb->planes[plane].m.offset == off) {
>>   				*_buffer = buffer;
>>   				*_plane = plane;
>>   				return 0;
>> @@ -2557,7 +2598,7 @@ int vb2_mmap(struct vb2_queue *q, struct vm_area_struct *vma)
>>   	 * The buffer length was page_aligned at __vb2_buf_mem_alloc(),
>>   	 * so, we need to do the same here.
>>   	 */
>> -	length = PAGE_ALIGN(vb->v4l2_planes[plane].length);
>> +	length = PAGE_ALIGN(vb->planes[plane].length);
>>   	if (length < (vma->vm_end - vma->vm_start)) {
>>   		dprintk(1,
>>   			"MMAP invalid, as it would overflow buffer length\n");
>> @@ -2577,10 +2618,10 @@ EXPORT_SYMBOL_GPL(vb2_mmap);
>>
>>   #ifndef CONFIG_MMU
>>   unsigned long vb2_get_unmapped_area(struct vb2_queue *q,
>> -				    unsigned long addr,
>> -				    unsigned long len,
>> -				    unsigned long pgoff,
>> -				    unsigned long flags)
>> +					unsigned long addr,
>> +					unsigned long len,
>> +					unsigned long pgoff,
>> +					unsigned long flags)
>>   {
>>   	unsigned long off = pgoff << PAGE_SHIFT;
>>   	struct vb2_buffer *vb;
>> @@ -2731,7 +2772,7 @@ EXPORT_SYMBOL_GPL(vb2_poll);
>>    * responsible of clearing it's content and setting initial values for some
>>    * required entries before calling this function.
>>    * q->ops, q->mem_ops, q->type and q->io_modes are mandatory. Please refer
>> - * to the struct vb2_queue description in include/media/videobuf2-v4l2.h
>> + * to the struct vb2_queue description in include/media/videobuf2-core.h
>>    * for more information.
>>    */
>>   int vb2_queue_init(struct vb2_queue *q)
>> @@ -2739,16 +2780,16 @@ int vb2_queue_init(struct vb2_queue *q)
>>   	/*
>>   	 * Sanity check
>>   	 */
>> -	if (WARN_ON(!q)			  ||
>> -	    WARN_ON(!q->ops)		  ||
>> -	    WARN_ON(!q->mem_ops)	  ||
>> -	    WARN_ON(!q->type)		  ||
>> -	    WARN_ON(!q->io_modes)	  ||
>> -	    WARN_ON(!q->ops->queue_setup) ||
>> -	    WARN_ON(!q->ops->buf_queue)   ||
>> -	    WARN_ON(q->timestamp_flags &
>> -		    ~(V4L2_BUF_FLAG_TIMESTAMP_MASK |
>> -		      V4L2_BUF_FLAG_TSTAMP_SRC_MASK)))
>> +	if (WARN_ON(!q)				||
>> +		WARN_ON(!q->ops)		||
>> +		WARN_ON(!q->mem_ops)		||
>> +		WARN_ON(!q->type)		||
>> +		WARN_ON(!q->io_modes)		||
>> +		WARN_ON(!q->ops->queue_setup)	||
>> +		WARN_ON(!q->ops->buf_queue)	||
>> +		WARN_ON(q->timestamp_flags &
>> +			~(V4L2_BUF_FLAG_TIMESTAMP_MASK |
>> +			V4L2_BUF_FLAG_TSTAMP_SRC_MASK)))
>>   		return -EINVAL;
>>
>>   	/* Warn that the driver should choose an appropriate timestamp type */
>> @@ -2852,7 +2893,7 @@ static int __vb2_init_fileio(struct vb2_queue *q, int read)
>>   	 * Sanity check
>>   	 */
>>   	if (WARN_ON((read && !(q->io_modes & VB2_READ)) ||
>> -		    (!read && !(q->io_modes & VB2_WRITE))))
>> +			(!read && !(q->io_modes & VB2_WRITE))))
>>   		return -EINVAL;
>>
>>   	/*
>> @@ -3063,9 +3104,12 @@ static size_t __vb2_perform_fileio(struct vb2_queue *q, char __user *data, size_
>>   		buf->queued = 0;
>>   		buf->size = read ? vb2_get_plane_payload(q->bufs[index], 0)
>>   				 : vb2_plane_size(q->bufs[index], 0);
>> -		/* Compensate for data_offset on read in the multiplanar case. */
>> +		/*
>> +		 * Compensate for data_offset on read
>> +		 * in the multiplanar case
>> +		 */
>>   		if (is_multiplanar && read &&
>> -		    fileio->b.m.planes[0].data_offset < buf->size) {
>> +			fileio->b.m.planes[0].data_offset < buf->size) {
>>   			buf->pos = fileio->b.m.planes[0].data_offset;
>>   			buf->size -= buf->pos;
>>   		}
>> @@ -3257,7 +3301,7 @@ static int vb2_thread(void *data)
>>    * contact the linux-media mailinglist first.
>>    */
>>   int vb2_thread_start(struct vb2_queue *q, vb2_thread_fnc fnc, void *priv,
>> -		     const char *thread_name)
>> +			const char *thread_name)
>>   {
>>   	struct vb2_threadio_data *threadio;
>>   	int ret = 0;
>> @@ -3491,7 +3535,7 @@ ssize_t vb2_fop_write(struct file *file, const char __user *buf,
>>   	if (vb2_queue_is_busy(vdev, file))
>>   		goto exit;
>>   	err = vb2_write(vdev->queue, buf, count, ppos,
>> -		       file->f_flags & O_NONBLOCK);
>> +			file->f_flags & O_NONBLOCK);
>>   	if (vdev->queue->fileio)
>>   		vdev->queue->owner = file->private_data;
>>   exit:
>> @@ -3515,7 +3559,7 @@ ssize_t vb2_fop_read(struct file *file, char __user *buf,
>>   	if (vb2_queue_is_busy(vdev, file))
>>   		goto exit;
>>   	err = vb2_read(vdev->queue, buf, count, ppos,
>> -		       file->f_flags & O_NONBLOCK);
>> +			file->f_flags & O_NONBLOCK);
>>   	if (vdev->queue->fileio)
>>   		vdev->queue->owner = file->private_data;
>>   exit:
>> diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h
>> index 155991e..8787a6c 100644
>> --- a/include/media/videobuf2-core.h
>> +++ b/include/media/videobuf2-core.h
>> @@ -115,6 +115,16 @@ struct vb2_plane {
>>   	void			*mem_priv;
>>   	struct dma_buf		*dbuf;
>>   	unsigned int		dbuf_mapped;
>> +
>> +	/* plane information */
>> +	unsigned int		bytesused;
>> +	unsigned int		length;
>> +	union {
>> +		unsigned int	offset;
>> +		unsigned long	userptr;
>> +		int		fd;
>> +	} m;
>> +	unsigned int		data_offset;
>>   };
>
> Nitpick: it would be good to add a documentation for struct vb2_plane,
> describing what each field means on this struct. Granted, this could
> be added after this patch series.
>
> Btw, I don't see much reason to have the:
> 	/* plane information */
> comment here, as this struct is all about plane information, right?
> Or, maybe you wanted, instead, to comment that those fields should
> have what's there at struct v4l2_plane? That would make sense ;)

Latter is right.

> So, I would change this comment to something like:
>
> 	/*
> 	 * Should contain enough plane information to contain the
> 	 * fields needed to fill struct v4l2_plane at videodev2.h
> 	 */
>
OK, I will change it.
>
>>
>>   /**
>> @@ -161,41 +171,33 @@ struct vb2_queue;
>>
>>   /**
>>    * struct vb2_buffer - represents a video buffer
>> - * @v4l2_buf:		struct v4l2_buffer associated with this buffer; can
>> - *			be read by the driver and relevant entries can be
>> - *			changed by the driver in case of CAPTURE types
>> - *			(such as timestamp)
>> - * @v4l2_planes:	struct v4l2_planes associated with this buffer; can
>> - *			be read by the driver and relevant entries can be
>> - *			changed by the driver in case of CAPTURE types
>> - *			(such as bytesused); NOTE that even for single-planar
>> - *			types, the v4l2_planes[0] struct should be used
>> - *			instead of v4l2_buf for filling bytesused - drivers
>> - *			should use the vb2_set_plane_payload() function for that
>>    * @vb2_queue:		the queue to which this driver belongs
>> - * @num_planes:		number of planes in the buffer
>> - *			on an internal driver queue
>>    * @state:		current buffer state; do not change
>>    * @queued_entry:	entry on the queued buffers list, which holds all
>>    *			buffers queued from userspace
>>    * @done_entry:		entry on the list that stores all buffers ready to
>>    *			be dequeued to userspace
>> + * @index:		id number of the buffer
>> + * @type:		buffer type
>> + * @memory:		the method, in which the actual data is passed
>> + * @num_planes:		number of planes in the buffer
>> + *			on an internal driver queue
>>    * @planes:		private per-plane information; do not change
>>    */
>>   struct vb2_buffer {
>> -	struct v4l2_buffer	v4l2_buf;
>> -	struct v4l2_plane	v4l2_planes[VIDEO_MAX_PLANES];
>> -
>>   	struct vb2_queue	*vb2_queue;
>>
>> -	unsigned int		num_planes;
>> -
>> -/* Private: internal use only */
>> +	/* Private: internal use only */
>>   	enum vb2_buffer_state	state;
>>
>>   	struct list_head	queued_entry;
>>   	struct list_head	done_entry;
>>
>> +	/* buffer information */
>> +	unsigned int		index;
>> +	unsigned int		type;
>> +	unsigned int		memory;
>> +	unsigned int		num_planes;
>
> Nitpick: Those comments that follow Documentation/kernel-doc-nano-HOWTO.txt
> are used to produce DocBook data, on both html and manpages formats.
> As documented there, DocBook discards documentation for all fields after a
> /*private: ...*/ comment.
>
> In other words, we need to take care of putting things after /*private:*/
> only when we're 100% sure that such fields are not meant to be filled/used
> by the callers, and will be used only internally, and don't deserve any
> documentation for the kABI.
>
> As you're adding documentation for those fields, and num_planes were
> documented before your series, I suspect that this is not what you want.
> So, please move them to be before the private: comment.

Oh, I didn't know that /*private: ...*/ comment have so much meaning.
I will change it at next round.

>
> Btw, if your patches are based on top of the current patchwork tree
> e. g. if it has this patch:
> 	http://git.linuxtv.org/cgit.cgi/media_tree.git/commit/?id=d071c833a0d30e7aae0ea565d92ef83c79106d6f
>
> Then you can make the Kernel to handle all those kernel-doc-nano
> comments with:
>
> 	make cleandocs && make DOCBOOKS=device-drivers.xml htmldocs
>
> It will produce some html pages like:
> 	http://linuxtv.org/downloads/v4l-dvb-internals/device-drivers/mediadev.html
>
> With can be useful to check if the documentation tags are properly placed.
>
>>   	struct vb2_plane	planes[VIDEO_MAX_PLANES];
>>
>>   #ifdef CONFIG_VIDEO_ADV_DEBUG
>> @@ -390,7 +392,7 @@ struct v4l2_fh;
>>    * @threadio:	thread io internal data, used only if thread is active
>>    */
>>   struct vb2_queue {
>> -	enum v4l2_buf_type		type;
>> +	unsigned int			type;
>>   	unsigned int			io_modes;
>>   	unsigned			fileio_read_once:1;
>>   	unsigned			fileio_write_immediately:1;
>> @@ -409,7 +411,7 @@ struct vb2_queue {
>>
>>   	/* private: internal use only */
>>   	struct mutex			mmap_lock;
>> -	enum v4l2_memory		memory;
>> +	unsigned int			memory;
>
> Will the vb2-core use type/memory fields or just vb2-v4l2? As you
> removed the enum, I suspect you won't be relying on having the videodev2.h
> header included here, right.

Exactly.

>
> If so, then the meaning of the type/memory fields are private to the
> caller of the VB2-core  (e. .g. the meaning are private to vb2-v4l2 and
> vb2-dvb). So, you should be changing the description of those fields
> at the doc-nano to:
> ...
>   * @type: private type whose content is defined by the vb2-core caller.
>   *        For example, for V4L2, it should match the V4L2_BUF_TYPE_*
>   *	  in include/uapi/linux/videodev2.h
> ...
>
> to let it clear.
>

0k, I will change it as your comment.

>>   	struct vb2_buffer		*bufs[VIDEO_MAX_FRAME];
>>   	unsigned int			num_buffers;
>>
>> @@ -571,7 +573,7 @@ static inline void vb2_set_plane_payload(struct vb2_buffer *vb,
>>   				 unsigned int plane_no, unsigned long size)
>>   {
>>   	if (plane_no < vb->num_planes)
>> -		vb->v4l2_planes[plane_no].bytesused = size;
>> +		vb->planes[plane_no].bytesused = size;
>>   }
>>
>>   /**
>> @@ -583,7 +585,7 @@ static inline unsigned long vb2_get_plane_payload(struct vb2_buffer *vb,
>>   				 unsigned int plane_no)
>>   {
>>   	if (plane_no < vb->num_planes)
>> -		return vb->v4l2_planes[plane_no].bytesused;
>> +		return vb->planes[plane_no].bytesused;
>>   	return 0;
>>   }
>>
>> @@ -596,7 +598,7 @@ static inline unsigned long
>>   vb2_plane_size(struct vb2_buffer *vb, unsigned int plane_no)
>>   {
>>   	if (plane_no < vb->num_planes)
>> -		return vb->v4l2_planes[plane_no].length;
>> +		return vb->planes[plane_no].length;
>>   	return 0;
>>   }
>>
>> diff --git a/include/media/videobuf2-v4l2.h b/include/media/videobuf2-v4l2.h
>> index d4a4d9a..fc2dbe9 100644
>> --- a/include/media/videobuf2-v4l2.h
>> +++ b/include/media/videobuf2-v4l2.h
>> @@ -12,6 +12,32 @@
>>   #ifndef _MEDIA_VIDEOBUF2_V4L2_H
>>   #define _MEDIA_VIDEOBUF2_V4L2_H
>>
>> +#include <linux/videodev2.h>
>>   #include <media/videobuf2-core.h>
>>
>> +/**
>> + * struct vb2_v4l2_buffer - video buffer information for v4l2
>> + * @vb2_buf:	video buffer 2
>> + * @flags:	buffer informational flags
>> + * @field:	enum v4l2_field; field order of the image in the buffer
>> + * @timestamp:	frame timestamp
>> + * @timecode:	frame timecode
>> + * @sequence:	sequence count of this frame
>> + */
>> +struct vb2_v4l2_buffer {
>> +	struct vb2_buffer	vb2_buf;
>> +
>> +	__u32			flags;
>> +	__u32			field;
>> +	struct timeval		timestamp;
>> +	struct v4l2_timecode	timecode;
>> +	__u32			sequence;
>> +};
>> +
>> +/**
>> + * to_vb2_v4l2_buffer() - cast struct vb2_buffer * to struct vb2_v4l2_buffer *
>> + */
>> +#define to_vb2_v4l2_buffer(vb) \
>> +	(container_of(vb, struct vb2_v4l2_buffer, vb2_buf))
>> +
>>   #endif /* _MEDIA_VIDEOBUF2_V4L2_H */
>
> Regards,
> Mauro
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-media" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>