Re: [RFC PATCH v3 2/5] media: videobuf2: Restructure vb2_buffer
On 08/31/2015 09:56 AM, Junghak Sung wrote: > > > On 08/31/2015 11:01 AM, Junghak Sung wrote: >> Hello Hans, >> >> Thank you for your review. >> I leave some comments in the body for reply. >> >> Regards, >> Junghak >> >> >> >> On 08/28/2015 10:31 PM, Hans Verkuil wrote: >>> Hi Junghak, >>> >>> Thanks for this patch, it looks much better. I do have a number of >>> comments, though... >>> >>> On 08/26/2015 01:59 PM, Junghak Sung wrote: Remove v4l2-specific stuff from struct vb2_buffer and add member variables related with buffer management. struct vb2_plane { /* plane information */ unsigned intbytesused; unsigned intlength; union { unsigned intoffset; unsigned long userptr; int fd; } m; unsigned intdata_offset; } struct vb2_buffer { /* buffer information */ unsigned intnum_planes; unsigned intindex; unsigned inttype; unsigned intmemory; struct vb2_planeplanes[VIDEO_MAX_PLANES]; }; 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_timecodetimecode; __u32 sequence; }; This patch includes only changes inside of videobuf2. So, it is required to modify all device drivers which use videobuf2. Signed-off-by: Junghak SungSigned-off-by: Geunyoung Kim Acked-by: Seung-Woo Kim Acked-by: Inki Dae --- 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);\ >>> >>> These are just whitespace changes, and that is something it see *a >>> lot* in this >>> patch. And usually for no clear reason. >>> >>> Please remove those whitespace changes, it makes a difficult patch >>> even harder >>> to read than it already is. >>> >> >> I just wanted to remove unnecessary white spaces or adjust alignment. >> OK, I will revert those whitespace changes for better review. >> } while (0) #ifdef CONFIG_VIDEO_ADV_DEBUG >>> >>> >>> @@ -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, >v4l2_buf, offsetof(struct v4l2_buffer, m)); -b->reserved2 = vb->v4l2_buf.reserved2; -b->reserved = vb->v4l2_buf.reserved; >>> >>> Hmm, I'm not sure why these reserved fields were copied here. I think >>> it was >>> for compatibility reasons for some old drivers that abused the >>> reserved field. >>> However, in the new code these reserved fields should probably be >>> explicitly >>> initialized to 0. >>> +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
Re: [RFC PATCH v3 2/5] media: videobuf2: Restructure vb2_buffer
On 08/31/2015 11:01 AM, Junghak Sung wrote: Hello Hans, Thank you for your review. I leave some comments in the body for reply. Regards, Junghak On 08/28/2015 10:31 PM, Hans Verkuil wrote: Hi Junghak, Thanks for this patch, it looks much better. I do have a number of comments, though... On 08/26/2015 01:59 PM, Junghak Sung wrote: Remove v4l2-specific stuff from struct vb2_buffer and add member variables related with buffer management. struct vb2_plane { /* plane information */ unsigned intbytesused; unsigned intlength; union { unsigned intoffset; unsigned long userptr; int fd; } m; unsigned intdata_offset; } struct vb2_buffer { /* buffer information */ unsigned intnum_planes; unsigned intindex; unsigned inttype; unsigned intmemory; struct vb2_planeplanes[VIDEO_MAX_PLANES]; }; 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_timecodetimecode; __u32 sequence; }; This patch includes only changes inside of videobuf2. So, it is required to modify all device drivers which use videobuf2. Signed-off-by: Junghak SungSigned-off-by: Geunyoung Kim Acked-by: Seung-Woo Kim Acked-by: Inki Dae --- 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);\ These are just whitespace changes, and that is something it see *a lot* in this patch. And usually for no clear reason. Please remove those whitespace changes, it makes a difficult patch even harder to read than it already is. I just wanted to remove unnecessary white spaces or adjust alignment. OK, I will revert those whitespace changes for better review. } while (0) #ifdef CONFIG_VIDEO_ADV_DEBUG @@ -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, >v4l2_buf, offsetof(struct v4l2_buffer, m)); -b->reserved2 = vb->v4l2_buf.reserved2; -b->reserved = vb->v4l2_buf.reserved; Hmm, I'm not sure why these reserved fields were copied here. I think it was for compatibility reasons for some old drivers that abused the reserved field. However, in the new code these reserved fields should probably be explicitly initialized to 0. +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)); A similar problem occurs here: the memcpy would have copied the reserved field in v4l2_plane as well, but that no longer happens, so you need to do an explicit memset for the reserved field in the new code. It means that I'd better add reserved fields to struct vb2_buffer and struct vb2_plane in order to keep the information in struct v4l2_buffer and struct v4l2_plane??? Oh, I've mistaken your
Re: [RFC PATCH v3 2/5] media: videobuf2: Restructure vb2_buffer
Hello Hans, Thank you for your review. I leave some comments in the body for reply. Regards, Junghak On 08/28/2015 10:31 PM, Hans Verkuil wrote: Hi Junghak, Thanks for this patch, it looks much better. I do have a number of comments, though... On 08/26/2015 01:59 PM, Junghak Sung wrote: Remove v4l2-specific stuff from struct vb2_buffer and add member variables related with buffer management. struct vb2_plane { snip /* plane information */ unsigned intbytesused; unsigned intlength; union { unsigned intoffset; unsigned long userptr; int fd; } m; unsigned intdata_offset; } struct vb2_buffer { snip /* buffer information */ unsigned intnum_planes; unsigned intindex; unsigned inttype; unsigned intmemory; struct vb2_planeplanes[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_timecodetimecode; __u32 sequence; }; This patch includes only changes inside of videobuf2. So, it is required to modify all device drivers which use videobuf2. Signed-off-by: Junghak Sung jh1009.s...@samsung.com Signed-off-by: Geunyoung Kim nenggun@samsung.com Acked-by: Seung-Woo Kim sw0312@samsung.com Acked-by: Inki Dae inki@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); \ These are just whitespace changes, and that is something it see *a lot* in this patch. And usually for no clear reason. Please remove those whitespace changes, it makes a difficult patch even harder to read than it already is. I just wanted to remove unnecessary white spaces or adjust alignment. OK, I will revert those whitespace changes for better review. } while (0) #ifdef CONFIG_VIDEO_ADV_DEBUG snip @@ -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; Hmm, I'm not sure why these reserved fields were copied here. I think it was for compatibility reasons for some old drivers that abused the reserved field. However, in the new code these reserved fields should probably be explicitly initialized to 0. + 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)); A similar problem occurs here: the memcpy would have copied the reserved field in v4l2_plane as well, but that no longer happens, so you need to do an explicit memset for the reserved field in the new code. It means that
Re: [RFC PATCH v3 2/5] media: videobuf2: Restructure vb2_buffer
Hi Junghak, Thanks for this patch, it looks much better. I do have a number of comments, though... On 08/26/2015 01:59 PM, Junghak Sung wrote: Remove v4l2-specific stuff from struct vb2_buffer and add member variables related with buffer management. struct vb2_plane { snip /* plane information */ unsigned intbytesused; unsigned intlength; union { unsigned intoffset; unsigned long userptr; int fd; } m; unsigned intdata_offset; } struct vb2_buffer { snip /* buffer information */ unsigned intnum_planes; unsigned intindex; unsigned inttype; unsigned intmemory; struct vb2_planeplanes[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_timecodetimecode; __u32 sequence; }; This patch includes only changes inside of videobuf2. So, it is required to modify all device drivers which use videobuf2. Signed-off-by: Junghak Sung jh1009.s...@samsung.com Signed-off-by: Geunyoung Kim nenggun@samsung.com Acked-by: Seung-Woo Kim sw0312@samsung.com Acked-by: Inki Dae inki@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); \ These are just whitespace changes, and that is something it see *a lot* in this patch. And usually for no clear reason. Please remove those whitespace changes, it makes a difficult patch even harder to read than it already is. } while (0) #ifdef CONFIG_VIDEO_ADV_DEBUG snip @@ -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; Hmm, I'm not sure why these reserved fields were copied here. I think it was for compatibility reasons for some old drivers that abused the reserved field. However, in the new code these reserved fields should probably be explicitly initialized to 0. + 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)); A similar problem occurs here: the memcpy would have copied the reserved field in v4l2_plane as well, but that no longer happens, so you need to do an explicit memset for the reserved field in the new code. + 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
Re: [RFC PATCH v3 2/5] media: videobuf2: Restructure vb2_buffer
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.s...@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 intbytesused; unsigned intlength; union { unsigned intoffset; unsigned long userptr; int fd; } m; unsigned intdata_offset; } struct vb2_buffer { snip /* buffer information */ unsigned intnum_planes; unsigned intindex; unsigned inttype; unsigned intmemory; struct vb2_planeplanes[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_timecodetimecode; __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.s...@samsung.com Signed-off-by: Geunyoung Kim nenggun@samsung.com Acked-by: Seung-Woo Kim sw0312@samsung.com Acked-by: Inki Dae inki@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 =
Re: [RFC PATCH v3 2/5] media: videobuf2: Restructure vb2_buffer
Em Wed, 26 Aug 2015 20:59:29 +0900 Junghak Sung jh1009.s...@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 intbytesused; unsigned intlength; union { unsigned intoffset; unsigned long userptr; int fd; } m; unsigned intdata_offset; } struct vb2_buffer { snip /* buffer information */ unsigned intnum_planes; unsigned intindex; unsigned inttype; unsigned intmemory; struct vb2_planeplanes[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_timecodetimecode; __u32 sequence; }; The comments seemed a little hard for me to read, but the changes look ok. 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. Signed-off-by: Junghak Sung jh1009.s...@samsung.com Signed-off-by: Geunyoung Kim nenggun@samsung.com Acked-by: Seung-Woo Kim sw0312@samsung.com Acked-by: Inki Dae inki@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, -
[RFC PATCH v3 2/5] media: videobuf2: Restructure vb2_buffer
Remove v4l2-specific stuff from struct vb2_buffer and add member variables related with buffer management. struct vb2_plane { snip /* plane information */ unsigned intbytesused; unsigned intlength; union { unsigned intoffset; unsigned long userptr; int fd; } m; unsigned intdata_offset; } struct vb2_buffer { snip /* buffer information */ unsigned intnum_planes; unsigned intindex; unsigned inttype; unsigned intmemory; struct vb2_planeplanes[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_timecodetimecode; __u32 sequence; }; This patch includes only changes inside of videobuf2. So, it is required to modify all device drivers which use videobuf2. Signed-off-by: Junghak Sung jh1009.s...@samsung.com Signed-off-by: Geunyoung Kim nenggun@samsung.com Acked-by: Seung-Woo Kim sw0312@samsung.com Acked-by: Inki Dae inki@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 =