From patchwork Fri Sep 28 09:30:09 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Hans Verkuil X-Patchwork-Id: 1517931 Return-Path: X-Original-To: patchwork-linux-media@patchwork.kernel.org Delivered-To: patchwork-process-083081@patchwork2.kernel.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by patchwork2.kernel.org (Postfix) with ESMTP id 75B48DF283 for ; Fri, 28 Sep 2012 09:31:12 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754891Ab2I1JbI (ORCPT ); Fri, 28 Sep 2012 05:31:08 -0400 Received: from smtp-vbr7.xs4all.nl ([194.109.24.27]:4057 "EHLO smtp-vbr7.xs4all.nl" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756967Ab2I1JbH (ORCPT ); Fri, 28 Sep 2012 05:31:07 -0400 Received: from alastor.dyndns.org (166.80-203-20.nextgentel.com [80.203.20.166] (may be forged)) (authenticated bits=0) by smtp-vbr7.xs4all.nl (8.13.8/8.13.8) with ESMTP id q8S9UMQm067661 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=FAIL); Fri, 28 Sep 2012 11:30:24 +0200 (CEST) (envelope-from hverkuil@xs4all.nl) Received: from tschai.cisco.com (64-103-25-233.cisco.com [64.103.25.233]) (Authenticated sender: hans) by alastor.dyndns.org (Postfix) with ESMTPSA id 1EAFA35C00B7; Fri, 28 Sep 2012 11:30:21 +0200 (CEST) From: Hans Verkuil To: linux-media@vger.kernel.org Cc: Sylwester Nawrocki , Pawel Osciak , Marek Szyprowski , Hans Verkuil Subject: [RFCv2 PATCH 1/4] videobuf2-core: move plane verification out of __fill_v4l2/vb_buffer. Date: Fri, 28 Sep 2012 11:30:09 +0200 Message-Id: <2ffce29d048fbe637273d08abd506d128194d7d9.1348824367.git.hans.verkuil@cisco.com> X-Mailer: git-send-email 1.7.10.4 In-Reply-To: <1348824612-2857-1-git-send-email-hverkuil@xs4all.nl> References: <1348824612-2857-1-git-send-email-hverkuil@xs4all.nl> X-Virus-Scanned: by XS4ALL Virus Scanner Sender: linux-media-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-media@vger.kernel.org From: Hans Verkuil The plane verification should be done before actually queuing or dequeuing buffers, so move it out of __fill_v4l2_buffer and __fill_vb_buffer and call it as a separate step. This also makes it possible to change the return type of __fill_v4l2/vb_buffer to void. The dqbuf case took some special care: before removing a buffer from the done_list you have to verify that the receiving v4l2_buffer has enough room for all the planes. The number of planes can differ between buffers due to the fact that buffers for other formats can be prepared using VIDIOC_PREPARE_BUF. Signed-off-by: Hans Verkuil --- drivers/media/v4l2-core/videobuf2-core.c | 78 +++++++++++++++--------------- 1 file changed, 38 insertions(+), 40 deletions(-) diff --git a/drivers/media/v4l2-core/videobuf2-core.c b/drivers/media/v4l2-core/videobuf2-core.c index e6a26b4..5a132b2 100644 --- a/drivers/media/v4l2-core/videobuf2-core.c +++ b/drivers/media/v4l2-core/videobuf2-core.c @@ -276,6 +276,9 @@ static void __vb2_queue_free(struct vb2_queue *q, unsigned int buffers) */ static int __verify_planes_array(struct vb2_buffer *vb, const struct v4l2_buffer *b) { + if (!V4L2_TYPE_IS_MULTIPLANAR(b->type)) + return 0; + /* Is memory for copying plane information present? */ if (NULL == b->m.planes) { dprintk(1, "Multi-planar buffer passed but " @@ -331,10 +334,9 @@ static bool __buffers_in_use(struct vb2_queue *q) * __fill_v4l2_buffer() - fill in a struct v4l2_buffer with information to be * returned to userspace */ -static int __fill_v4l2_buffer(struct vb2_buffer *vb, struct v4l2_buffer *b) +static void __fill_v4l2_buffer(struct vb2_buffer *vb, struct v4l2_buffer *b) { struct vb2_queue *q = vb->vb2_queue; - int ret; /* Copy back data such as timestamp, flags, etc. */ memcpy(b, &vb->v4l2_buf, offsetof(struct v4l2_buffer, m)); @@ -342,13 +344,9 @@ static int __fill_v4l2_buffer(struct vb2_buffer *vb, struct v4l2_buffer *b) b->reserved = vb->v4l2_buf.reserved; if (V4L2_TYPE_IS_MULTIPLANAR(q->type)) { - ret = __verify_planes_array(vb, b); - if (ret) - return ret; - /* * Fill in plane-related data if userspace provided an array - * for it. The memory and size is verified above. + * for it. The caller has already verified memory and size. */ memcpy(b->m.planes, vb->v4l2_planes, b->length * sizeof(struct v4l2_plane)); @@ -391,8 +389,6 @@ static int __fill_v4l2_buffer(struct vb2_buffer *vb, struct v4l2_buffer *b) if (__buffer_in_use(q, vb)) b->flags |= V4L2_BUF_FLAG_MAPPED; - - return 0; } /** @@ -411,6 +407,7 @@ static int __fill_v4l2_buffer(struct vb2_buffer *vb, struct v4l2_buffer *b) int vb2_querybuf(struct vb2_queue *q, struct v4l2_buffer *b) { struct vb2_buffer *vb; + int ret; if (b->type != q->type) { dprintk(1, "querybuf: wrong buffer type\n"); @@ -422,8 +419,10 @@ int vb2_querybuf(struct vb2_queue *q, struct v4l2_buffer *b) return -EINVAL; } vb = q->bufs[b->index]; - - return __fill_v4l2_buffer(vb, b); + ret = __verify_planes_array(vb, b); + if (!ret) + __fill_v4l2_buffer(vb, b); + return ret; } EXPORT_SYMBOL(vb2_querybuf); @@ -813,24 +812,16 @@ void vb2_buffer_done(struct vb2_buffer *vb, enum vb2_buffer_state state) EXPORT_SYMBOL_GPL(vb2_buffer_done); /** - * __fill_vb2_buffer() - fill a vb2_buffer with information provided in - * a v4l2_buffer by the userspace + * __fill_vb2_buffer() - fill a vb2_buffer with information provided in a + * v4l2_buffer by the userspace. The caller has already verified that struct + * v4l2_buffer has a valid number of planes. */ -static int __fill_vb2_buffer(struct vb2_buffer *vb, const struct v4l2_buffer *b, +static void __fill_vb2_buffer(struct vb2_buffer *vb, const struct v4l2_buffer *b, struct v4l2_plane *v4l2_planes) { unsigned int plane; - int ret; if (V4L2_TYPE_IS_MULTIPLANAR(b->type)) { - /* - * Verify that the userspace gave us a valid array for - * plane information. - */ - ret = __verify_planes_array(vb, b); - if (ret) - return ret; - /* Fill in driver-provided information for OUTPUT types */ if (V4L2_TYPE_IS_OUTPUT(b->type)) { /* @@ -872,8 +863,6 @@ static int __fill_vb2_buffer(struct vb2_buffer *vb, const struct v4l2_buffer *b, vb->v4l2_buf.field = b->field; vb->v4l2_buf.timestamp = b->timestamp; vb->v4l2_buf.flags = b->flags & ~V4L2_BUFFER_STATE_FLAGS; - - return 0; } /** @@ -888,10 +877,8 @@ static int __qbuf_userptr(struct vb2_buffer *vb, const struct v4l2_buffer *b) int ret; int write = !V4L2_TYPE_IS_OUTPUT(q->type); - /* Verify and copy relevant information provided by the userspace */ - ret = __fill_vb2_buffer(vb, b, planes); - if (ret) - return ret; + /* Copy relevant information provided by the userspace */ + __fill_vb2_buffer(vb, b, planes); for (plane = 0; plane < vb->num_planes; ++plane) { /* Skip the plane if already verified */ @@ -966,7 +953,8 @@ err: */ static int __qbuf_mmap(struct vb2_buffer *vb, const struct v4l2_buffer *b) { - return __fill_vb2_buffer(vb, b, vb->v4l2_planes); + __fill_vb2_buffer(vb, b, vb->v4l2_planes); + return 0; } /** @@ -1059,7 +1047,9 @@ int vb2_prepare_buf(struct vb2_queue *q, struct v4l2_buffer *b) dprintk(1, "%s(): invalid buffer state %d\n", __func__, vb->state); return -EINVAL; } - + ret = __verify_planes_array(vb, b); + if (ret < 0) + return ret; ret = __buf_prepare(vb, b); if (ret < 0) return ret; @@ -1147,6 +1137,9 @@ int vb2_qbuf(struct vb2_queue *q, struct v4l2_buffer *b) ret = -EINVAL; goto unlock; } + ret = __verify_planes_array(vb, b); + if (ret) + goto unlock; switch (vb->state) { case VB2_BUF_STATE_DEQUEUED: @@ -1243,8 +1236,10 @@ static int __vb2_wait_for_done_vb(struct vb2_queue *q, int nonblocking) * the locks or return an error if one occurred. */ call_qop(q, wait_finish, q); - if (ret) + if (ret) { + dprintk(1, "Sleep was interrupted\n"); return ret; + } } return 0; } @@ -1255,7 +1250,7 @@ static int __vb2_wait_for_done_vb(struct vb2_queue *q, int nonblocking) * Will sleep if required for nonblocking == false. */ static int __vb2_get_done_vb(struct vb2_queue *q, struct vb2_buffer **vb, - int nonblocking) + struct v4l2_buffer *b, int nonblocking) { unsigned long flags; int ret; @@ -1273,10 +1268,16 @@ static int __vb2_get_done_vb(struct vb2_queue *q, struct vb2_buffer **vb, */ spin_lock_irqsave(&q->done_lock, flags); *vb = list_first_entry(&q->done_list, struct vb2_buffer, done_entry); - list_del(&(*vb)->done_entry); + /* + * Only remove the buffer from done_list if v4l2_buffer can handle all + * the planes. + */ + ret = __verify_planes_array(*vb, b); + if (!ret) + list_del(&(*vb)->done_entry); spin_unlock_irqrestore(&q->done_lock, flags); - return 0; + return ret; } /** @@ -1335,12 +1336,9 @@ int vb2_dqbuf(struct vb2_queue *q, struct v4l2_buffer *b, bool nonblocking) dprintk(1, "dqbuf: invalid buffer type\n"); return -EINVAL; } - - ret = __vb2_get_done_vb(q, &vb, nonblocking); - if (ret < 0) { - dprintk(1, "dqbuf: error getting next done buffer\n"); + ret = __vb2_get_done_vb(q, &vb, b, nonblocking); + if (ret < 0) return ret; - } ret = call_qop(q, buf_finish, vb); if (ret) {