Arguments

From mboxrd@z Thu Jan 1 00:00:00 1970 Return-path: Received: from mail-qy0-f174.google.com ([209.85.216.174]:40042 "EHLO mail-qy0-f174.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752129Ab1HFVwM convert rfc822-to-8bit (ORCPT ); Sat, 6 Aug 2011 17:52:12 -0400 Received: by qyk38 with SMTP id 38so953561qyk.19 for ; Sat, 06 Aug 2011 14:52:11 -0700 (PDT) MIME-Version: 1.0 In-Reply-To: References: From: Pawel Osciak Date: Sat, 6 Aug 2011 14:51:51 -0700 Message-ID: Subject: Re: [PATCH 1/6 v4] V4L: add two new ioctl()s for multi-size videobuffer management To: Guennadi Liakhovetski Cc: Linux Media Mailing List , Sakari Ailus , Hans Verkuil , Sakari Ailus , Laurent Pinchart , Mauro Carvalho Chehab Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 8BIT Sender: linux-media-owner@vger.kernel.org List-ID: Hi Guennadi, On Fri, Aug 5, 2011 at 00:47, Guennadi Liakhovetski wrote: > A possibility to preallocate and initialise buffers of different sizes > in V4L2 is required for an efficient implementation of asnapshot mode. > This patch adds two new ioctl()s: VIDIOC_CREATE_BUFS and > VIDIOC_PREPARE_BUF and defines respective data structures. > > Signed-off-by: Guennadi Liakhovetski > --- > > v4: > > 1. CREATE_BUFS now takes an array of plane sizes and a fourcc code in its > argument, instead of a frame format specification, including > documentation update > 2. documentation improvements, as suggested by Hans > 3. increased reserved fields to 18, as suggested by Sakari > > Documentation/DocBook/media/v4l/io.xml | 17 ++ > Documentation/DocBook/media/v4l/v4l2.xml | 2 + > .../DocBook/media/v4l/vidioc-create-bufs.xml | 161 ++++++++++++++++++++ > .../DocBook/media/v4l/vidioc-prepare-buf.xml | 96 ++++++++++++ > drivers/media/video/v4l2-compat-ioctl32.c | 6 + > drivers/media/video/v4l2-ioctl.c | 26 +++ > include/linux/videodev2.h | 18 +++ > include/media/v4l2-ioctl.h | 2 + > 8 files changed, 328 insertions(+), 0 deletions(-) > create mode 100644 Documentation/DocBook/media/v4l/vidioc-create-bufs.xml > create mode 100644 Documentation/DocBook/media/v4l/vidioc-prepare-buf.xml > > diff --git a/Documentation/DocBook/media/v4l/io.xml b/Documentation/DocBook/media/v4l/io.xml > index 227e7ac..ff03dd2 100644 > --- a/Documentation/DocBook/media/v4l/io.xml > +++ b/Documentation/DocBook/media/v4l/io.xml > @@ -927,6 +927,23 @@ ioctl is called. > Applications set or clear this flag before calling the > VIDIOC_QBUF ioctl. > > + > + V4L2_BUF_FLAG_NO_CACHE_INVALIDATE > + 0x0400 > + Caches do not have to be invalidated for this buffer. > +Typically applications shall use this flag if the data captured in the buffer > +is not going to be touched by the CPU, instead the buffer will, probably, be > +passed on to a DMA-capable hardware unit for further processing or output. > + > + > + > + V4L2_BUF_FLAG_NO_CACHE_CLEAN > + 0x0800 > + Caches do not have to be cleaned for this buffer. > +Typically applications shall use this flag for output buffers if the data > +in this buffer has not been created by the CPU but by some DMA-capable unit, > +in which case caches have not been used. > + > > > > diff --git a/Documentation/DocBook/media/v4l/v4l2.xml b/Documentation/DocBook/media/v4l/v4l2.xml > index 0d05e87..06bb179 100644 > --- a/Documentation/DocBook/media/v4l/v4l2.xml > +++ b/Documentation/DocBook/media/v4l/v4l2.xml > @@ -462,6 +462,7 @@ and discussions on the V4L mailing list. > &sub-close; > &sub-ioctl; > > + &sub-create-bufs; > &sub-cropcap; > &sub-dbg-g-chip-ident; > &sub-dbg-g-register; > @@ -504,6 +505,7 @@ and discussions on the V4L mailing list. > &sub-queryctrl; > &sub-query-dv-preset; > &sub-querystd; > + &sub-prepare-buf; > &sub-reqbufs; > &sub-s-hw-freq-seek; > &sub-streamon; > diff --git a/Documentation/DocBook/media/v4l/vidioc-create-bufs.xml b/Documentation/DocBook/media/v4l/vidioc-create-bufs.xml > new file mode 100644 > index 0000000..b37b9a4 > --- /dev/null > +++ b/Documentation/DocBook/media/v4l/vidioc-create-bufs.xml > @@ -0,0 +1,161 @@ > + > + > + ioctl VIDIOC_CREATE_BUFS > + &manvol; > + > + > + > + VIDIOC_CREATE_BUFS > + Create buffers for Memory Mapped or User Pointer I/O > + > + > + > + > + > + int ioctl > + int fd > + int request > + struct v4l2_create_buffers *argp > + > + > + > + > + > + Arguments > + > + > + > + fd > + > + &fd; > + > + > + > + request > + > + VIDIOC_CREATE_BUFS > + > + > + > + argp > + > + > + > + > + > + > + > + > + Description > + > + This ioctl is used to create buffers for memory > +mapped or user pointer > +I/O. It can be used as an alternative or in addition to the > +VIDIOC_REQBUFS ioctl, when a tighter control over buffers > +is required. This ioctl can be called multiple times to create buffers of > +different sizes. > + > + To allocate device buffers applications initialize relevant > +fields of the v4l2_create_buffers structure. > +They set the type field to the respective stream > +or buffer type. count must be set to the number of > +required buffers. memory specifies the required I/O > +method. If num_planes == 0 or all elements of the > +sizes array are 0, then buffers, suitable for the > +currently configured video format, are allocated, exactly like for the > +VIDIOC_REQBUFS ioctl. If > +num_planes > 0 and at least some of the respective > +sizes elements are non-zero, this information will be Instead of "at least some must be filled", I would say it more strongly: "If num_planes > 0, the elements in range 0 to num_planes-1 have to be filled with desired plane sizes for each buffer. This information will then be used instead of current format for buffer allocation." or something similar. Unless we want to allow sizes for some planes to be taken from format and some from sizes[], which I doubt... > +used for buffer-allocation. The reserved array must > +be zeroed. When the ioctl is called with a pointer to this structure the driver > +will attempt to allocate up to the requested number of buffers and store the > +actual number allocated and the starting index in the > +count and the index > +fields respectively. On return count can be smaller Needs a comma after "return". > +than the number requested. > + When the I/O method is not supported the ioctl > +returns an &EINVAL;. > + > + > + struct <structname>v4l2_create_buffers</structname> > + > + &cs-str; > + > + > + __u32 > + index > + The starting buffer index, returned by the driver. > + > + > + __u32 > + count > + The number of buffers requested or granted. > + > + > + __u32 > + type > + V4L2 buffer type: one of V4L2_BUF_TYPE_* > +values. > + > + > + __u32 > + memory > + Applications set this field to > +V4L2_MEMORY_MMAP or > +V4L2_MEMORY_USERPTR. > + > + > + __u32 > + fourcc > + One of V4L2_PIX_FMT_* FOURCCs of the video data. > + If sizes is zero, we use the currently configured format, but if this is filled, this overrides the currently selected format, right? But in that case, sizes should be specified as well. I think it should be described explicitly above. So maybe "if fourcc is specified, sizes has to be filled and those parameters override the currently configured format; if either fourcc or sizes is not specified, the currently configured format is used". > + > + __u32 > + num_planes > + Number of planes or 0 to use the current format. > + > + > + __u32 > + size[VIDEO_MAX_PLANES] > + Explicit sizes of buffers, being created. > + > + > + __u32 > + reserved[18] > + A place holder for future extensions. > + > + > + > +

> + > + > + > + &return-value; > + > + > + > + ENOMEM > + > + No memory to allocate buffers for memory > +mapped I/O. > + > + > + > + EINVAL > + > + The buffer type (type field) or the > +requested I/O method (memory) is not > +supported. Do we also return this error if the provided fourcc format is not supported? > + > + > + > + > + > + > + > diff --git a/Documentation/DocBook/media/v4l/vidioc-prepare-buf.xml b/Documentation/DocBook/media/v4l/vidioc-prepare-buf.xml > new file mode 100644 > index 0000000..509e752 > --- /dev/null > +++ b/Documentation/DocBook/media/v4l/vidioc-prepare-buf.xml > @@ -0,0 +1,96 @@ > + > + > + ioctl VIDIOC_PREPARE_BUF > + &manvol; > + > + > + > + VIDIOC_PREPARE_BUF > + Prepare a buffer for I/O > + > + > + > + > + > + int ioctl > + int fd > + int request > + struct v4l2_buffer *argp > + > + > + > + > + > + Arguments > + > + > + > + fd > + > + &fd; > + > + > + > + request > + > + VIDIOC_PREPARE_BUF > + > + > + > + argp > + > + > + > + > + > + > + > + > + Description > + > + Applications can optionally call the > +VIDIOC_PREPARE_BUF ioctl to pass ownership of the buffer > +to the driver before actually enqueuing it, using the > +VIDIOC_QBUF ioctl, and to prepare it for future I/O. > +Such preparations may include cache invalidation or cleaning. Performing them > +in advance saves time during the actual I/O. In case such cache operations are > +not required, the application can use one of > +V4L2_BUF_FLAG_NO_CACHE_INVALIDATE and > +V4L2_BUF_FLAG_NO_CACHE_CLEAN flags to skip the respective > +step. > + > + The v4l2_buffer structure is > +specified in . > + > + > + > + &return-value; > + > + > + > + EBUSY > + > + File I/O is in progress. > + > + > + > + EINVAL > + > + The buffer type is not > +supported, or the index is out of bounds, > +or no buffers have been allocated yet, or the > +userptr or > +length are invalid. > + > + > + > + > + > + > + > diff --git a/drivers/media/video/v4l2-compat-ioctl32.c b/drivers/media/video/v4l2-compat-ioctl32.c > index 61979b7..ee5eec8 100644 > --- a/drivers/media/video/v4l2-compat-ioctl32.c > +++ b/drivers/media/video/v4l2-compat-ioctl32.c > @@ -702,6 +702,7 @@ static int put_v4l2_event32(struct v4l2_event *kp, struct v4l2_event32 __user *u > #define VIDIOC_S_EXT_CTRLS32 _IOWR('V', 72, struct v4l2_ext_controls32) > #define VIDIOC_TRY_EXT_CTRLS32 _IOWR('V', 73, struct v4l2_ext_controls32) > #define VIDIOC_DQEVENT32 _IOR ('V', 89, struct v4l2_event32) > +#define VIDIOC_PREPARE_BUF32 _IOWR('V', 93, struct v4l2_buffer32) > > #define VIDIOC_OVERLAY32 _IOW ('V', 14, s32) > #define VIDIOC_STREAMON32 _IOW ('V', 18, s32) > @@ -751,6 +752,7 @@ static long do_video_ioctl(struct file *file, unsigned int cmd, unsigned long ar > case VIDIOC_S_INPUT32: cmd = VIDIOC_S_INPUT; break; > case VIDIOC_G_OUTPUT32: cmd = VIDIOC_G_OUTPUT; break; > case VIDIOC_S_OUTPUT32: cmd = VIDIOC_S_OUTPUT; break; > + case VIDIOC_PREPARE_BUF32: cmd = VIDIOC_PREPARE_BUF; break; > } > > switch (cmd) { > @@ -775,6 +777,7 @@ static long do_video_ioctl(struct file *file, unsigned int cmd, unsigned long ar > compatible_arg = 0; > break; > > + case VIDIOC_PREPARE_BUF: > case VIDIOC_QUERYBUF: > case VIDIOC_QBUF: > case VIDIOC_DQBUF: > @@ -860,6 +863,7 @@ static long do_video_ioctl(struct file *file, unsigned int cmd, unsigned long ar > err = put_v4l2_format32(&karg.v2f, up); > break; > > + case VIDIOC_PREPARE_BUF: > case VIDIOC_QUERYBUF: > case VIDIOC_QBUF: > case VIDIOC_DQBUF: > @@ -959,6 +963,8 @@ long v4l2_compat_ioctl32(struct file *file, unsigned int cmd, unsigned long arg) > case VIDIOC_DQEVENT32: > case VIDIOC_SUBSCRIBE_EVENT: > case VIDIOC_UNSUBSCRIBE_EVENT: > + case VIDIOC_CREATE_BUFS: > + case VIDIOC_PREPARE_BUF32: > ret = do_video_ioctl(file, cmd, arg); > break; > > diff --git a/drivers/media/video/v4l2-ioctl.c b/drivers/media/video/v4l2-ioctl.c > index 002ce13..3da87c0 100644 > --- a/drivers/media/video/v4l2-ioctl.c > +++ b/drivers/media/video/v4l2-ioctl.c > @@ -260,6 +260,8 @@ static const char *v4l2_ioctls[] = { > [_IOC_NR(VIDIOC_DQEVENT)] = "VIDIOC_DQEVENT", > [_IOC_NR(VIDIOC_SUBSCRIBE_EVENT)] = "VIDIOC_SUBSCRIBE_EVENT", > [_IOC_NR(VIDIOC_UNSUBSCRIBE_EVENT)] = "VIDIOC_UNSUBSCRIBE_EVENT", > + [_IOC_NR(VIDIOC_CREATE_BUFS)] = "VIDIOC_CREATE_BUFS", > + [_IOC_NR(VIDIOC_PREPARE_BUF)] = "VIDIOC_PREPARE_BUF", > }; > #define V4L2_IOCTLS ARRAY_SIZE(v4l2_ioctls) > > @@ -2216,6 +2218,30 @@ static long __video_do_ioctl(struct file *file, > dbgarg(cmd, "type=0x%8.8x", sub->type); > break; > } > + case VIDIOC_CREATE_BUFS: > + { > + struct v4l2_create_buffers *create = arg; > + > + if (!ops->vidioc_create_bufs) > + break; > + > + ret = ops->vidioc_create_bufs(file, fh, create); > + > + dbgarg(cmd, "count=%u @ %u\n", create->count, create->index); > + break; > + } > + case VIDIOC_PREPARE_BUF: > + { > + struct v4l2_buffer *b = arg; > + > + if (!ops->vidioc_prepare_buf) > + break; > + > + ret = ops->vidioc_prepare_buf(file, fh, b); > + > + dbgarg(cmd, "index=%d", b->index); > + break; > + } > default: > { > bool valid_prio = true; > diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h > index fca24cc..3cd0cb3 100644 > --- a/include/linux/videodev2.h > +++ b/include/linux/videodev2.h > @@ -653,6 +653,9 @@ struct v4l2_buffer { > #define V4L2_BUF_FLAG_ERROR 0x0040 > #define V4L2_BUF_FLAG_TIMECODE 0x0100 /* timecode field is valid */ > #define V4L2_BUF_FLAG_INPUT 0x0200 /* input field is valid */ > +/* Cache handling flags */ > +#define V4L2_BUF_FLAG_NO_CACHE_INVALIDATE 0x0400 > +#define V4L2_BUF_FLAG_NO_CACHE_CLEAN 0x0800 > > /* > * O V E R L A Y P R E V I E W > @@ -2092,6 +2095,18 @@ struct v4l2_dbg_chip_ident { > __u32 revision; /* chip revision, chip specific */ > } __attribute__ ((packed)); > > +/* VIDIOC_CREATE_BUFS */ > +struct v4l2_create_buffers { > + __u32 index; /* output: buffers index...index + count - 1 have been created */ > + __u32 count; > + __u32 type; > + __u32 memory; > + __u32 fourcc; > + __u32 num_planes; > + __u32 sizes[VIDEO_MAX_PLANES]; > + __u32 reserved[18]; > +}; > + > /* > * I O C T L C O D E S F O R V I D E O D E V I C E S > * > @@ -2182,6 +2197,9 @@ struct v4l2_dbg_chip_ident { > #define VIDIOC_SUBSCRIBE_EVENT _IOW('V', 90, struct v4l2_event_subscription) > #define VIDIOC_UNSUBSCRIBE_EVENT _IOW('V', 91, struct v4l2_event_subscription) > > +#define VIDIOC_CREATE_BUFS _IOWR('V', 92, struct v4l2_create_buffers) > +#define VIDIOC_PREPARE_BUF _IOW('V', 93, struct v4l2_buffer) > + > /* Reminder: when adding new ioctls please add support for them to > drivers/media/video/v4l2-compat-ioctl32.c as well! */ > > diff --git a/include/media/v4l2-ioctl.h b/include/media/v4l2-ioctl.h > index dd9f1e7..4d1c74a 100644 > --- a/include/media/v4l2-ioctl.h > +++ b/include/media/v4l2-ioctl.h > @@ -122,6 +122,8 @@ struct v4l2_ioctl_ops { > int (*vidioc_qbuf) (struct file *file, void *fh, struct v4l2_buffer *b); > int (*vidioc_dqbuf) (struct file *file, void *fh, struct v4l2_buffer *b); > > + int (*vidioc_create_bufs)(struct file *file, void *fh, struct v4l2_create_buffers *b); > + int (*vidioc_prepare_buf)(struct file *file, void *fh, struct v4l2_buffer *b); > > int (*vidioc_overlay) (struct file *file, void *fh, unsigned int i); > int (*vidioc_g_fbuf) (struct file *file, void *fh, > -- > 1.7.2.5 > > -- Best regards, Pawel Osciak