| Message ID | Pine.LNX.4.64.1107201025120.12084@axis700.grange (mailing list archive) |
|---|---|
| State | RFC |
| Headers | show |
Hi, Guennadi! Thanks for the patch! On Wed, Jul 20, 2011 at 10:43:08AM +0200, 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 <g.liakhovetski@gmx.de> > --- > > It's been almost a month since v2, the only comments were a request to > increase the reserved space in the new ioctl() and to improve > documentation. The reserved field is increased in this version, > documentation also has been improved in multiple locations. I think, > documentation can be further improved at any time, but if there are no > objections against the actual contents of this patch, maybe we can commit > this version. I still don't see v3.0;-), so, maybe we even can push it for > 3.1. A trivial comparison with v2 shows the size of the reserved field as > the only change in the API, and the compatibility fix as the only two > functional changes. > > v3: addressed multiple comments by Sakari Ailus > > 1. increased reserved field in "struct v4l2_create_buffers" to 8 32-bit > ints > 2. multiple documentation fixes and improvements > 3. fixed misplaced "case VIDIOC_PREPARE_BUF" in ioctl 32-bit compatibility > processing > > v2: > > 1. add preliminary Documentation > 2. add flag V4L2_BUFFER_FLAG_NO_CACHE_CLEAN > 3. remove VIDIOC_DESTROY_BUFS > 4. rename SUBMIT to VIDIOC_PREPARE_BUF > 5. add reserved field to struct v4l2_create_buffers > 6. cache handling flags moved to struct v4l2_buffer for processing during > VIDIOC_PREPARE_BUF > 7. VIDIOC_PREPARE_BUF now uses struct v4l2_buffer as its argument > > > Documentation/DocBook/media/v4l/io.xml | 17 +++ > Documentation/DocBook/media/v4l/v4l2.xml | 2 + > .../DocBook/media/v4l/vidioc-create-bufs.xml | 152 ++++++++++++++++++++ > .../DocBook/media/v4l/vidioc-prepare-buf.xml | 96 ++++++++++++ > drivers/media/video/v4l2-compat-ioctl32.c | 68 ++++++++- > drivers/media/video/v4l2-ioctl.c | 32 ++++ > include/linux/videodev2.h | 16 ++ > include/media/v4l2-ioctl.h | 2 + > 8 files changed, 377 insertions(+), 8 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..6249d0e 100644 > --- a/Documentation/DocBook/media/v4l/io.xml > +++ b/Documentation/DocBook/media/v4l/io.xml > @@ -927,6 +927,23 @@ ioctl is called.</entry> > Applications set or clear this flag before calling the > <constant>VIDIOC_QBUF</constant> ioctl.</entry> > </row> > + <row> > + <entry><constant>V4L2_BUF_FLAG_NO_CACHE_INVALIDATE</constant></entry> > + <entry>0x0400</entry> > + <entry>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 br touched by the CPU, instead the buffer will, probably, be > +passed on to a DMA-capable hardware unit for further processing or output. > +</entry> > + </row> > + <row> > + <entry><constant>V4L2_BUF_FLAG_NO_CACHE_CLEAN</constant></entry> > + <entry>0x0800</entry> > + <entry>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.</entry> > + </row> > </tbody> > </tgroup> > </table> > 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.</revremark> > &sub-close; > &sub-ioctl; > <!-- All ioctls go here. --> > + &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.</revremark> > &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..5f0158c > --- /dev/null > +++ b/Documentation/DocBook/media/v4l/vidioc-create-bufs.xml > @@ -0,0 +1,152 @@ > +<refentry id="vidioc-create-bufs"> > + <refmeta> > + <refentrytitle>ioctl VIDIOC_CREATE_BUFS</refentrytitle> > + &manvol; > + </refmeta> > + > + <refnamediv> > + <refname>VIDIOC_CREATE_BUFS</refname> > + <refpurpose>Create buffers for Memory Mapped or User Pointer I/O</refpurpose> > + </refnamediv> > + > + <refsynopsisdiv> > + <funcsynopsis> > + <funcprototype> > + <funcdef>int <function>ioctl</function></funcdef> > + <paramdef>int <parameter>fd</parameter></paramdef> > + <paramdef>int <parameter>request</parameter></paramdef> > + <paramdef>struct v4l2_create_buffers *<parameter>argp</parameter></paramdef> > + </funcprototype> > + </funcsynopsis> > + </refsynopsisdiv> > + > + <refsect1> > + <title>Arguments</title> > + > + <variablelist> > + <varlistentry> > + <term><parameter>fd</parameter></term> > + <listitem> > + <para>&fd;</para> > + </listitem> > + </varlistentry> > + <varlistentry> > + <term><parameter>request</parameter></term> > + <listitem> > + <para>VIDIOC_CREATE_BUFS</para> > + </listitem> > + </varlistentry> > + <varlistentry> > + <term><parameter>argp</parameter></term> > + <listitem> > + <para></para> > + </listitem> > + </varlistentry> > + </variablelist> > + </refsect1> > + > + <refsect1> > + <title>Description</title> > + > + <para>This ioctl is used to create buffers for <link linkend="mmap">memory > +mapped</link> or <link linkend="userp">user pointer</link> > +I/O. It can be used as an alternative to the <constant>VIDIOC_REQBUFS</constant> > +ioctl, when a tighter control over buffers is required. This ioctl can be called > +multiple times to create buffers of different sizes. > + > + <para>To allocate device buffers applications initialize all > +fields of the <structname>v4l2_create_buffers</structname> structure. > +They set the <structfield>type</structfield> field in the > +<structname>v4l2_format</structname> structure, embedded in this > +structure, to the respective stream or buffer type. > +<structfield>count</structfield> must be set to the number of required > +buffers. <structfield>memory</structfield> specifies the required I/O > +method. Applications have two possibilities to specify the size of buffers > +to be prepared: they can either set the <structfield>size</structfield> > +field explicitly to a non-zero value, or fill in the frame format data in the > +<structfield>format</structfield> field. In the latter case buffer sizes > +will be calculated automatically by the driver. The > +<structfield>reserved</structfield> 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 <structfield>count</structfield> and > +the <structfield>index</structfield> fields respectively. > +<structfield>count</structfield> can be smaller than the number requested. > +-ENOMEM is returned, if the driver runs out of free memory.</para> No need to mention -ENOMEM here. It's already in the return values below; the same goes for the EINVAL just below. > + <para>When the I/O method is not supported the ioctl > +returns an &EINVAL;.</para> > + > + <table pgwide="1" frame="none" id="v4l2-create-buffers"> > + <title>struct <structname>v4l2_create_buffers</structname></title> > + <tgroup cols="3"> > + &cs-str; > + <tbody valign="top"> > + <row> > + <entry>__u32</entry> > + <entry><structfield>index</structfield></entry> > + <entry>The starting buffer index, returned by the driver.</entry> > + </row> > + <row> > + <entry>__u32</entry> > + <entry><structfield>count</structfield></entry> > + <entry>The number of buffers requested or granted.</entry> > + </row> > + <row> > + <entry>&v4l2-memory;</entry> > + <entry><structfield>memory</structfield></entry> > + <entry>Applications set this field to > +<constant>V4L2_MEMORY_MMAP</constant> or > +<constant>V4L2_MEMORY_USERPTR</constant>.</entry> > + </row> > + <row> > + <entry>__u32</entry> > + <entry><structfield>size</structfield></entry> > + <entry>Explicit size of buffers, being created.</entry> > + </row> > + <row> > + <entry>&v4l2-format;</entry> > + <entry><structfield>format</structfield></entry> > + <entry>Application has to set the <structfield>type</structfield> > +field, other fields should be used, if the application wants to allocate buffers > +for a specific frame format.</entry> > + </row> > + <row> > + <entry>__u32</entry> > + <entry><structfield>reserved</structfield>[8]</entry> > + <entry>A place holder for future extensions.</entry> > + </row> > + </tbody> > + </tgroup> > + </table> > + </refsect1> > + > + <refsect1> > + &return-value; > + > + <variablelist> > + <varlistentry> > + <term><errorcode>ENOMEM</errorcode></term> > + <listitem> > + <para>No memory to allocate buffers for <link linkend="mmap">memory > +mapped</link> I/O.</para> > + </listitem> > + </varlistentry> > + <varlistentry> > + <term><errorcode>EINVAL</errorcode></term> > + <listitem> > + <para>The buffer type (<structfield>type</structfield> field) or the > +requested I/O method (<structfield>memory</structfield>) is not > +supported.</para> > + </listitem> > + </varlistentry> > + </variablelist> > + </refsect1> > +</refentry> > + > +<!-- > +Local Variables: > +mode: sgml > +sgml-parent-document: "v4l2.sgml" > +indent-tabs-mode: nil > +End: > +--> > 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 @@ > +<refentry id="vidioc-prepare-buf"> > + <refmeta> > + <refentrytitle>ioctl VIDIOC_PREPARE_BUF</refentrytitle> > + &manvol; > + </refmeta> > + > + <refnamediv> > + <refname>VIDIOC_PREPARE_BUF</refname> > + <refpurpose>Prepare a buffer for I/O</refpurpose> > + </refnamediv> > + > + <refsynopsisdiv> > + <funcsynopsis> > + <funcprototype> > + <funcdef>int <function>ioctl</function></funcdef> > + <paramdef>int <parameter>fd</parameter></paramdef> > + <paramdef>int <parameter>request</parameter></paramdef> > + <paramdef>struct v4l2_buffer *<parameter>argp</parameter></paramdef> I'm not sure if we want to use struct v4l2_buffer here. It's an obvious choice, but there's an issue in using it. The struct is almost full i.e. there are very few reserved fields in it, namely two, counting the input field which was unused AFAIR. There are now three ioctls using it as their argument. Future functionality which would be nice: - Format counters. Every format set by S_FMT (or gotten by G_FMT) should come with a counter value so that the user would know the format of dequeued buffers when setting the format on-the-fly. Currently there are only bytesperline and length, but the format can't be explicitly determined from those. - Binding to generic DMA buffers. This can likely be achieved by using the m union in struct v4l2_buffer. (Speaking of which: these buffers likely have a cache behaviour defined for them, e.g. they might be non-cacheable. This probably has no effect on the cache flags in V4L2 but it might still be good to keep this in mind.) I don't have an obvious replacement for it either. There are two options I can see: Create a new struct v4l2_ext_buffer which contains the old v4l2_buffer plus a bunch of reserved fields, or use the two last fields to hold a pointer to an extension struct, v4l2_buffer_ext. The new structure would later be used to contain the reserved fields. I admit that this is not an issue right now, myt point is that I don't want this to be an issue in the future either. > + </funcprototype> > + </funcsynopsis> > + </refsynopsisdiv> > + > + <refsect1> > + <title>Arguments</title> > + > + <variablelist> > + <varlistentry> > + <term><parameter>fd</parameter></term> > + <listitem> > + <para>&fd;</para> > + </listitem> > + </varlistentry> > + <varlistentry> > + <term><parameter>request</parameter></term> > + <listitem> > + <para>VIDIOC_PREPARE_BUF</para> > + </listitem> > + </varlistentry> > + <varlistentry> > + <term><parameter>argp</parameter></term> > + <listitem> > + <para></para> > + </listitem> > + </varlistentry> > + </variablelist> > + </refsect1> > + > + <refsect1> > + <title>Description</title> > + > + <para>Applications can optionally call the > +<constant>VIDIOC_PREPARE_BUF</constant> ioctl to pass ownership of the buffer > +to the driver before actually enqueuing it, using the > +<constant>VIDIOC_QBUF</constant> 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 > +<constant>V4L2_BUF_FLAG_NO_CACHE_INVALIDATE</constant> and > +<constant>V4L2_BUF_FLAG_NO_CACHE_CLEAN</constant> flags to skip the respective > +step.</para> > + > + <para>The <structname>v4l2_buffer</structname> structure is > +specified in <xref linkend="buffer" />.</para> > + </refsect1> > + > + <refsect1> > + &return-value; > + > + <variablelist> > + <varlistentry> > + <term><errorcode>EBUSY</errorcode></term> > + <listitem> > + <para>File I/O is in progress.</para> > + </listitem> > + </varlistentry> > + <varlistentry> > + <term><errorcode>EINVAL</errorcode></term> > + <listitem> > + <para>The buffer <structfield>type</structfield> is not > +supported, or the <structfield>index</structfield> is out of bounds, > +or no buffers have been allocated yet, or the > +<structfield>userptr</structfield> or > +<structfield>length</structfield> are invalid.</para> > + </listitem> > + </varlistentry> > + </variablelist> > + </refsect1> > +</refentry> > + > +<!-- > +Local Variables: > +mode: sgml > +sgml-parent-document: "v4l2.sgml" > +indent-tabs-mode: nil > +End: > +--> > diff --git a/drivers/media/video/v4l2-compat-ioctl32.c b/drivers/media/video/v4l2-compat-ioctl32.c > index 61979b7..4105f69 100644 > --- a/drivers/media/video/v4l2-compat-ioctl32.c > +++ b/drivers/media/video/v4l2-compat-ioctl32.c > @@ -159,11 +159,16 @@ struct v4l2_format32 { > } fmt; > }; > > -static int get_v4l2_format32(struct v4l2_format *kp, struct v4l2_format32 __user *up) > +struct v4l2_create_buffers32 { > + __u32 index; /* output: buffers index...index + count - 1 have been created */ > + __u32 count; > + enum v4l2_memory memory; > + __u32 size; /* Explicit size, e.g., for compressed streams */ > + struct v4l2_format32 format; /* "type" is used always, the rest if size == 0 */ > +}; > + > +static int __get_v4l2_format32(struct v4l2_format *kp, struct v4l2_format32 __user *up) > { > - if (!access_ok(VERIFY_READ, up, sizeof(struct v4l2_format32)) || > - get_user(kp->type, &up->type)) > - return -EFAULT; > switch (kp->type) { > case V4L2_BUF_TYPE_VIDEO_CAPTURE: > case V4L2_BUF_TYPE_VIDEO_OUTPUT: > @@ -192,11 +197,24 @@ static int get_v4l2_format32(struct v4l2_format *kp, struct v4l2_format32 __user > } > } > > -static int put_v4l2_format32(struct v4l2_format *kp, struct v4l2_format32 __user *up) > +static int get_v4l2_format32(struct v4l2_format *kp, struct v4l2_format32 __user *up) > +{ > + if (!access_ok(VERIFY_READ, up, sizeof(struct v4l2_format32)) || > + get_user(kp->type, &up->type)) > + return -EFAULT; > + return __get_v4l2_format32(kp, up); > +} > + > +static int get_v4l2_create32(struct v4l2_create_buffers *kp, struct v4l2_create_buffers32 __user *up) > +{ > + if (!access_ok(VERIFY_READ, up, sizeof(struct v4l2_create_buffers32)) || > + copy_from_user(kp, up, offsetof(struct v4l2_create_buffers32, format.fmt))) > + return -EFAULT; > + return __get_v4l2_format32(&kp->format, &up->format); > +} > + > +static int __put_v4l2_format32(struct v4l2_format *kp, struct v4l2_format32 __user *up) > { > - if (!access_ok(VERIFY_WRITE, up, sizeof(struct v4l2_format32)) || > - put_user(kp->type, &up->type)) > - return -EFAULT; > switch (kp->type) { > case V4L2_BUF_TYPE_VIDEO_CAPTURE: > case V4L2_BUF_TYPE_VIDEO_OUTPUT: > @@ -225,6 +243,22 @@ static int put_v4l2_format32(struct v4l2_format *kp, struct v4l2_format32 __user > } > } > > +static int put_v4l2_format32(struct v4l2_format *kp, struct v4l2_format32 __user *up) > +{ > + if (!access_ok(VERIFY_WRITE, up, sizeof(struct v4l2_format32)) || > + put_user(kp->type, &up->type)) > + return -EFAULT; > + return __put_v4l2_format32(kp, up); > +} > + > +static int put_v4l2_create32(struct v4l2_create_buffers *kp, struct v4l2_create_buffers32 __user *up) > +{ > + if (!access_ok(VERIFY_WRITE, up, sizeof(struct v4l2_create_buffers32)) || > + copy_to_user(up, kp, offsetof(struct v4l2_create_buffers32, format.fmt))) > + return -EFAULT; > + return __put_v4l2_format32(&kp->format, &up->format); > +} > + > struct v4l2_standard32 { > __u32 index; > __u32 id[2]; /* __u64 would get the alignment wrong */ > @@ -702,6 +736,8 @@ 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_CREATE_BUFS32 _IOWR('V', 92, struct v4l2_create_buffers32) > +#define VIDIOC_PREPARE_BUF32 _IOWR('V', 93, struct v4l2_buffer32) > > #define VIDIOC_OVERLAY32 _IOW ('V', 14, s32) > #define VIDIOC_STREAMON32 _IOW ('V', 18, s32) > @@ -721,6 +757,7 @@ static long do_video_ioctl(struct file *file, unsigned int cmd, unsigned long ar > struct v4l2_standard v2s; > struct v4l2_ext_controls v2ecs; > struct v4l2_event v2ev; > + struct v4l2_create_buffers v2crt; > unsigned long vx; > int vi; > } karg; > @@ -751,6 +788,8 @@ 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_CREATE_BUFS32: cmd = VIDIOC_CREATE_BUFS; break; > + case VIDIOC_PREPARE_BUF32: cmd = VIDIOC_PREPARE_BUF; break; > } > > switch (cmd) { > @@ -775,6 +814,12 @@ static long do_video_ioctl(struct file *file, unsigned int cmd, unsigned long ar > compatible_arg = 0; > break; > > + case VIDIOC_CREATE_BUFS: > + err = get_v4l2_create32(&karg.v2crt, up); > + compatible_arg = 0; > + break; > + > + case VIDIOC_PREPARE_BUF: > case VIDIOC_QUERYBUF: > case VIDIOC_QBUF: > case VIDIOC_DQBUF: > @@ -860,6 +905,11 @@ static long do_video_ioctl(struct file *file, unsigned int cmd, unsigned long ar > err = put_v4l2_format32(&karg.v2f, up); > break; > > + case VIDIOC_CREATE_BUFS: > + err = put_v4l2_create32(&karg.v2crt, up); > + break; > + > + case VIDIOC_PREPARE_BUF: > case VIDIOC_QUERYBUF: > case VIDIOC_QBUF: > case VIDIOC_DQBUF: > @@ -959,6 +1009,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_BUFS32: > + 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..7824152 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,36 @@ 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 = check_fmt(ops, create->format.type); > + if (ret) > + break; > + > + if (create->size) > + CLEAR_AFTER_FIELD(create, count); > + ret = ops->vidioc_create_bufs(file, fh, create); > + > + dbgarg(cmd, "count=%d\n", create->count); > + 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..0b594c7 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,16 @@ 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; > + enum v4l2_memory memory; Aren't enums something to avoid in public APIs? -> __u32? > + __u32 size; /* Explicit size, e.g., for compressed streams */ > + struct v4l2_format format; /* "type" is used always, the rest if size == 0 */ > + __u32 reserved[8]; > +}; > + > /* > * 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 +2195,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 > Regards,
On Wed, 20 Jul 2011, Sakari Ailus wrote: > Hi, Guennadi! > > Thanks for the patch! > > On Wed, Jul 20, 2011 at 10:43:08AM +0200, 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 <g.liakhovetski@gmx.de> > > --- > > > > It's been almost a month since v2, the only comments were a request to > > increase the reserved space in the new ioctl() and to improve > > documentation. The reserved field is increased in this version, > > documentation also has been improved in multiple locations. I think, > > documentation can be further improved at any time, but if there are no > > objections against the actual contents of this patch, maybe we can commit > > this version. I still don't see v3.0;-), so, maybe we even can push it for > > 3.1. A trivial comparison with v2 shows the size of the reserved field as > > the only change in the API, and the compatibility fix as the only two > > functional changes. > > > > v3: addressed multiple comments by Sakari Ailus > > > > 1. increased reserved field in "struct v4l2_create_buffers" to 8 32-bit > > ints > > 2. multiple documentation fixes and improvements > > 3. fixed misplaced "case VIDIOC_PREPARE_BUF" in ioctl 32-bit compatibility > > processing > > > > v2: > > > > 1. add preliminary Documentation > > 2. add flag V4L2_BUFFER_FLAG_NO_CACHE_CLEAN > > 3. remove VIDIOC_DESTROY_BUFS > > 4. rename SUBMIT to VIDIOC_PREPARE_BUF > > 5. add reserved field to struct v4l2_create_buffers > > 6. cache handling flags moved to struct v4l2_buffer for processing during > > VIDIOC_PREPARE_BUF > > 7. VIDIOC_PREPARE_BUF now uses struct v4l2_buffer as its argument > > > > > > Documentation/DocBook/media/v4l/io.xml | 17 +++ > > Documentation/DocBook/media/v4l/v4l2.xml | 2 + > > .../DocBook/media/v4l/vidioc-create-bufs.xml | 152 ++++++++++++++++++++ > > .../DocBook/media/v4l/vidioc-prepare-buf.xml | 96 ++++++++++++ > > drivers/media/video/v4l2-compat-ioctl32.c | 68 ++++++++- > > drivers/media/video/v4l2-ioctl.c | 32 ++++ > > include/linux/videodev2.h | 16 ++ > > include/media/v4l2-ioctl.h | 2 + > > 8 files changed, 377 insertions(+), 8 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..6249d0e 100644 > > --- a/Documentation/DocBook/media/v4l/io.xml > > +++ b/Documentation/DocBook/media/v4l/io.xml > > @@ -927,6 +927,23 @@ ioctl is called.</entry> > > Applications set or clear this flag before calling the > > <constant>VIDIOC_QBUF</constant> ioctl.</entry> > > </row> > > + <row> > > + <entry><constant>V4L2_BUF_FLAG_NO_CACHE_INVALIDATE</constant></entry> > > + <entry>0x0400</entry> > > + <entry>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 br touched by the CPU, instead the buffer will, probably, be > > +passed on to a DMA-capable hardware unit for further processing or output. > > +</entry> > > + </row> > > + <row> > > + <entry><constant>V4L2_BUF_FLAG_NO_CACHE_CLEAN</constant></entry> > > + <entry>0x0800</entry> > > + <entry>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.</entry> > > + </row> > > </tbody> > > </tgroup> > > </table> > > 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.</revremark> > > &sub-close; > > &sub-ioctl; > > <!-- All ioctls go here. --> > > + &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.</revremark> > > &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..5f0158c > > --- /dev/null > > +++ b/Documentation/DocBook/media/v4l/vidioc-create-bufs.xml > > @@ -0,0 +1,152 @@ > > +<refentry id="vidioc-create-bufs"> > > + <refmeta> > > + <refentrytitle>ioctl VIDIOC_CREATE_BUFS</refentrytitle> > > + &manvol; > > + </refmeta> > > + > > + <refnamediv> > > + <refname>VIDIOC_CREATE_BUFS</refname> > > + <refpurpose>Create buffers for Memory Mapped or User Pointer I/O</refpurpose> > > + </refnamediv> > > + > > + <refsynopsisdiv> > > + <funcsynopsis> > > + <funcprototype> > > + <funcdef>int <function>ioctl</function></funcdef> > > + <paramdef>int <parameter>fd</parameter></paramdef> > > + <paramdef>int <parameter>request</parameter></paramdef> > > + <paramdef>struct v4l2_create_buffers *<parameter>argp</parameter></paramdef> > > + </funcprototype> > > + </funcsynopsis> > > + </refsynopsisdiv> > > + > > + <refsect1> > > + <title>Arguments</title> > > + > > + <variablelist> > > + <varlistentry> > > + <term><parameter>fd</parameter></term> > > + <listitem> > > + <para>&fd;</para> > > + </listitem> > > + </varlistentry> > > + <varlistentry> > > + <term><parameter>request</parameter></term> > > + <listitem> > > + <para>VIDIOC_CREATE_BUFS</para> > > + </listitem> > > + </varlistentry> > > + <varlistentry> > > + <term><parameter>argp</parameter></term> > > + <listitem> > > + <para></para> > > + </listitem> > > + </varlistentry> > > + </variablelist> > > + </refsect1> > > + > > + <refsect1> > > + <title>Description</title> > > + > > + <para>This ioctl is used to create buffers for <link linkend="mmap">memory > > +mapped</link> or <link linkend="userp">user pointer</link> > > +I/O. It can be used as an alternative to the <constant>VIDIOC_REQBUFS</constant> > > +ioctl, when a tighter control over buffers is required. This ioctl can be called > > +multiple times to create buffers of different sizes. > > + > > + <para>To allocate device buffers applications initialize all > > +fields of the <structname>v4l2_create_buffers</structname> structure. > > +They set the <structfield>type</structfield> field in the > > +<structname>v4l2_format</structname> structure, embedded in this > > +structure, to the respective stream or buffer type. > > +<structfield>count</structfield> must be set to the number of required > > +buffers. <structfield>memory</structfield> specifies the required I/O > > +method. Applications have two possibilities to specify the size of buffers > > +to be prepared: they can either set the <structfield>size</structfield> > > +field explicitly to a non-zero value, or fill in the frame format data in the > > +<structfield>format</structfield> field. In the latter case buffer sizes > > +will be calculated automatically by the driver. The > > +<structfield>reserved</structfield> 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 <structfield>count</structfield> and > > +the <structfield>index</structfield> fields respectively. > > +<structfield>count</structfield> can be smaller than the number requested. > > +-ENOMEM is returned, if the driver runs out of free memory.</para> > > No need to mention -ENOMEM here. It's already in the return values below; > the same goes for the EINVAL just below. Yes, incremental patches welcome. > > > + <para>When the I/O method is not supported the ioctl > > +returns an &EINVAL;.</para> > > + > > + <table pgwide="1" frame="none" id="v4l2-create-buffers"> > > + <title>struct <structname>v4l2_create_buffers</structname></title> > > + <tgroup cols="3"> > > + &cs-str; > > + <tbody valign="top"> > > + <row> > > + <entry>__u32</entry> > > + <entry><structfield>index</structfield></entry> > > + <entry>The starting buffer index, returned by the driver.</entry> > > + </row> > > + <row> > > + <entry>__u32</entry> > > + <entry><structfield>count</structfield></entry> > > + <entry>The number of buffers requested or granted.</entry> > > + </row> > > + <row> > > + <entry>&v4l2-memory;</entry> > > + <entry><structfield>memory</structfield></entry> > > + <entry>Applications set this field to > > +<constant>V4L2_MEMORY_MMAP</constant> or > > +<constant>V4L2_MEMORY_USERPTR</constant>.</entry> > > + </row> > > + <row> > > + <entry>__u32</entry> > > + <entry><structfield>size</structfield></entry> > > + <entry>Explicit size of buffers, being created.</entry> > > + </row> > > + <row> > > + <entry>&v4l2-format;</entry> > > + <entry><structfield>format</structfield></entry> > > + <entry>Application has to set the <structfield>type</structfield> > > +field, other fields should be used, if the application wants to allocate buffers > > +for a specific frame format.</entry> > > + </row> > > + <row> > > + <entry>__u32</entry> > > + <entry><structfield>reserved</structfield>[8]</entry> > > + <entry>A place holder for future extensions.</entry> > > + </row> > > + </tbody> > > + </tgroup> > > + </table> > > + </refsect1> > > + > > + <refsect1> > > + &return-value; > > + > > + <variablelist> > > + <varlistentry> > > + <term><errorcode>ENOMEM</errorcode></term> > > + <listitem> > > + <para>No memory to allocate buffers for <link linkend="mmap">memory > > +mapped</link> I/O.</para> > > + </listitem> > > + </varlistentry> > > + <varlistentry> > > + <term><errorcode>EINVAL</errorcode></term> > > + <listitem> > > + <para>The buffer type (<structfield>type</structfield> field) or the > > +requested I/O method (<structfield>memory</structfield>) is not > > +supported.</para> > > + </listitem> > > + </varlistentry> > > + </variablelist> > > + </refsect1> > > +</refentry> > > + > > +<!-- > > +Local Variables: > > +mode: sgml > > +sgml-parent-document: "v4l2.sgml" > > +indent-tabs-mode: nil > > +End: > > +--> > > 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 @@ > > +<refentry id="vidioc-prepare-buf"> > > + <refmeta> > > + <refentrytitle>ioctl VIDIOC_PREPARE_BUF</refentrytitle> > > + &manvol; > > + </refmeta> > > + > > + <refnamediv> > > + <refname>VIDIOC_PREPARE_BUF</refname> > > + <refpurpose>Prepare a buffer for I/O</refpurpose> > > + </refnamediv> > > + > > + <refsynopsisdiv> > > + <funcsynopsis> > > + <funcprototype> > > + <funcdef>int <function>ioctl</function></funcdef> > > + <paramdef>int <parameter>fd</parameter></paramdef> > > + <paramdef>int <parameter>request</parameter></paramdef> > > + <paramdef>struct v4l2_buffer *<parameter>argp</parameter></paramdef> > > I'm not sure if we want to use struct v4l2_buffer here. It's an obvious > choice, but there's an issue in using it. > > The struct is almost full i.e. there are very few reserved fields in it, > namely two, counting the input field which was unused AFAIR. There are now > three ioctls using it as their argument. > > Future functionality which would be nice: > > - Format counters. Every format set by S_FMT (or gotten by G_FMT) should > come with a counter value so that the user would know the format of > dequeued buffers when setting the format on-the-fly. Currently there are > only bytesperline and length, but the format can't be explicitly > determined from those. > > - Binding to generic DMA buffers. This can likely be achieved by using the m > union in struct v4l2_buffer. (Speaking of which: these buffers likely have > a cache behaviour defined for them, e.g. they might be non-cacheable. This > probably has no effect on the cache flags in V4L2 but it might still be > good to keep this in mind.) > > I don't have an obvious replacement for it either. There are two options I > can see: Create a new struct v4l2_ext_buffer which contains the old > v4l2_buffer plus a bunch of reserved fields, or use the two last fields to > hold a pointer to an extension struct, v4l2_buffer_ext. The new structure > would later be used to contain the reserved fields. > > I admit that this is not an issue right now, myt point is that I don't want > this to be an issue in the future either. Good, then we'll address it, when it becomes an issue. > > > + </funcprototype> > > + </funcsynopsis> > > + </refsynopsisdiv> > > + > > + <refsect1> > > + <title>Arguments</title> > > + > > + <variablelist> > > + <varlistentry> > > + <term><parameter>fd</parameter></term> > > + <listitem> > > + <para>&fd;</para> > > + </listitem> > > + </varlistentry> > > + <varlistentry> > > + <term><parameter>request</parameter></term> > > + <listitem> > > + <para>VIDIOC_PREPARE_BUF</para> > > + </listitem> > > + </varlistentry> > > + <varlistentry> > > + <term><parameter>argp</parameter></term> > > + <listitem> > > + <para></para> > > + </listitem> > > + </varlistentry> > > + </variablelist> > > + </refsect1> > > + > > + <refsect1> > > + <title>Description</title> > > + > > + <para>Applications can optionally call the > > +<constant>VIDIOC_PREPARE_BUF</constant> ioctl to pass ownership of the buffer > > +to the driver before actually enqueuing it, using the > > +<constant>VIDIOC_QBUF</constant> 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 > > +<constant>V4L2_BUF_FLAG_NO_CACHE_INVALIDATE</constant> and > > +<constant>V4L2_BUF_FLAG_NO_CACHE_CLEAN</constant> flags to skip the respective > > +step.</para> > > + > > + <para>The <structname>v4l2_buffer</structname> structure is > > +specified in <xref linkend="buffer" />.</para> > > + </refsect1> > > + > > + <refsect1> > > + &return-value; > > + > > + <variablelist> > > + <varlistentry> > > + <term><errorcode>EBUSY</errorcode></term> > > + <listitem> > > + <para>File I/O is in progress.</para> > > + </listitem> > > + </varlistentry> > > + <varlistentry> > > + <term><errorcode>EINVAL</errorcode></term> > > + <listitem> > > + <para>The buffer <structfield>type</structfield> is not > > +supported, or the <structfield>index</structfield> is out of bounds, > > +or no buffers have been allocated yet, or the > > +<structfield>userptr</structfield> or > > +<structfield>length</structfield> are invalid.</para> > > + </listitem> > > + </varlistentry> > > + </variablelist> > > + </refsect1> > > +</refentry> > > + > > +<!-- > > +Local Variables: > > +mode: sgml > > +sgml-parent-document: "v4l2.sgml" > > +indent-tabs-mode: nil > > +End: > > +--> > > diff --git a/drivers/media/video/v4l2-compat-ioctl32.c b/drivers/media/video/v4l2-compat-ioctl32.c > > index 61979b7..4105f69 100644 > > --- a/drivers/media/video/v4l2-compat-ioctl32.c > > +++ b/drivers/media/video/v4l2-compat-ioctl32.c > > @@ -159,11 +159,16 @@ struct v4l2_format32 { > > } fmt; > > }; > > > > -static int get_v4l2_format32(struct v4l2_format *kp, struct v4l2_format32 __user *up) > > +struct v4l2_create_buffers32 { > > + __u32 index; /* output: buffers index...index + count - 1 have been created */ > > + __u32 count; > > + enum v4l2_memory memory; > > + __u32 size; /* Explicit size, e.g., for compressed streams */ > > + struct v4l2_format32 format; /* "type" is used always, the rest if size == 0 */ > > +}; > > + > > +static int __get_v4l2_format32(struct v4l2_format *kp, struct v4l2_format32 __user *up) > > { > > - if (!access_ok(VERIFY_READ, up, sizeof(struct v4l2_format32)) || > > - get_user(kp->type, &up->type)) > > - return -EFAULT; > > switch (kp->type) { > > case V4L2_BUF_TYPE_VIDEO_CAPTURE: > > case V4L2_BUF_TYPE_VIDEO_OUTPUT: > > @@ -192,11 +197,24 @@ static int get_v4l2_format32(struct v4l2_format *kp, struct v4l2_format32 __user > > } > > } > > > > -static int put_v4l2_format32(struct v4l2_format *kp, struct v4l2_format32 __user *up) > > +static int get_v4l2_format32(struct v4l2_format *kp, struct v4l2_format32 __user *up) > > +{ > > + if (!access_ok(VERIFY_READ, up, sizeof(struct v4l2_format32)) || > > + get_user(kp->type, &up->type)) > > + return -EFAULT; > > + return __get_v4l2_format32(kp, up); > > +} > > + > > +static int get_v4l2_create32(struct v4l2_create_buffers *kp, struct v4l2_create_buffers32 __user *up) > > +{ > > + if (!access_ok(VERIFY_READ, up, sizeof(struct v4l2_create_buffers32)) || > > + copy_from_user(kp, up, offsetof(struct v4l2_create_buffers32, format.fmt))) > > + return -EFAULT; > > + return __get_v4l2_format32(&kp->format, &up->format); > > +} > > + > > +static int __put_v4l2_format32(struct v4l2_format *kp, struct v4l2_format32 __user *up) > > { > > - if (!access_ok(VERIFY_WRITE, up, sizeof(struct v4l2_format32)) || > > - put_user(kp->type, &up->type)) > > - return -EFAULT; > > switch (kp->type) { > > case V4L2_BUF_TYPE_VIDEO_CAPTURE: > > case V4L2_BUF_TYPE_VIDEO_OUTPUT: > > @@ -225,6 +243,22 @@ static int put_v4l2_format32(struct v4l2_format *kp, struct v4l2_format32 __user > > } > > } > > > > +static int put_v4l2_format32(struct v4l2_format *kp, struct v4l2_format32 __user *up) > > +{ > > + if (!access_ok(VERIFY_WRITE, up, sizeof(struct v4l2_format32)) || > > + put_user(kp->type, &up->type)) > > + return -EFAULT; > > + return __put_v4l2_format32(kp, up); > > +} > > + > > +static int put_v4l2_create32(struct v4l2_create_buffers *kp, struct v4l2_create_buffers32 __user *up) > > +{ > > + if (!access_ok(VERIFY_WRITE, up, sizeof(struct v4l2_create_buffers32)) || > > + copy_to_user(up, kp, offsetof(struct v4l2_create_buffers32, format.fmt))) > > + return -EFAULT; > > + return __put_v4l2_format32(&kp->format, &up->format); > > +} > > + > > struct v4l2_standard32 { > > __u32 index; > > __u32 id[2]; /* __u64 would get the alignment wrong */ > > @@ -702,6 +736,8 @@ 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_CREATE_BUFS32 _IOWR('V', 92, struct v4l2_create_buffers32) > > +#define VIDIOC_PREPARE_BUF32 _IOWR('V', 93, struct v4l2_buffer32) > > > > #define VIDIOC_OVERLAY32 _IOW ('V', 14, s32) > > #define VIDIOC_STREAMON32 _IOW ('V', 18, s32) > > @@ -721,6 +757,7 @@ static long do_video_ioctl(struct file *file, unsigned int cmd, unsigned long ar > > struct v4l2_standard v2s; > > struct v4l2_ext_controls v2ecs; > > struct v4l2_event v2ev; > > + struct v4l2_create_buffers v2crt; > > unsigned long vx; > > int vi; > > } karg; > > @@ -751,6 +788,8 @@ 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_CREATE_BUFS32: cmd = VIDIOC_CREATE_BUFS; break; > > + case VIDIOC_PREPARE_BUF32: cmd = VIDIOC_PREPARE_BUF; break; > > } > > > > switch (cmd) { > > @@ -775,6 +814,12 @@ static long do_video_ioctl(struct file *file, unsigned int cmd, unsigned long ar > > compatible_arg = 0; > > break; > > > > + case VIDIOC_CREATE_BUFS: > > + err = get_v4l2_create32(&karg.v2crt, up); > > + compatible_arg = 0; > > + break; > > + > > + case VIDIOC_PREPARE_BUF: > > case VIDIOC_QUERYBUF: > > case VIDIOC_QBUF: > > case VIDIOC_DQBUF: > > @@ -860,6 +905,11 @@ static long do_video_ioctl(struct file *file, unsigned int cmd, unsigned long ar > > err = put_v4l2_format32(&karg.v2f, up); > > break; > > > > + case VIDIOC_CREATE_BUFS: > > + err = put_v4l2_create32(&karg.v2crt, up); > > + break; > > + > > + case VIDIOC_PREPARE_BUF: > > case VIDIOC_QUERYBUF: > > case VIDIOC_QBUF: > > case VIDIOC_DQBUF: > > @@ -959,6 +1009,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_BUFS32: > > + 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..7824152 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,36 @@ 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 = check_fmt(ops, create->format.type); > > + if (ret) > > + break; > > + > > + if (create->size) > > + CLEAR_AFTER_FIELD(create, count); > > + ret = ops->vidioc_create_bufs(file, fh, create); > > + > > + dbgarg(cmd, "count=%d\n", create->count); > > + 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..0b594c7 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,16 @@ 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; > > + enum v4l2_memory memory; > > Aren't enums something to avoid in public APIs? -> __u32? They are, but this specific enum is already used in "struct v4l2_requestbuffers" and "struct v4l2_buffer" so, I decided, it would be consistent to use it here too, even if it involves extra compat craft, and the embedded struct v4l2_format also includes an enum in it anyway, so, basically, they are all over the place. > > > + __u32 size; /* Explicit size, e.g., for compressed streams */ > > + struct v4l2_format format; /* "type" is used always, the rest if size == 0 */ > > + __u32 reserved[8]; > > +}; > > + > > /* > > * 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 +2195,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 > > > > Regards, So, I take it as an ack;-) Thanks Guennadi --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/ -- 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
On Wed, Jul 20, 2011 at 04:47:46PM +0200, Guennadi Liakhovetski wrote: > On Wed, 20 Jul 2011, Sakari Ailus wrote: > > > Hi, Guennadi! > > > > Thanks for the patch! > > > > On Wed, Jul 20, 2011 at 10:43:08AM +0200, 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 <g.liakhovetski@gmx.de> > > > --- > > > > > > It's been almost a month since v2, the only comments were a request to > > > increase the reserved space in the new ioctl() and to improve > > > documentation. The reserved field is increased in this version, > > > documentation also has been improved in multiple locations. I think, > > > documentation can be further improved at any time, but if there are no > > > objections against the actual contents of this patch, maybe we can commit > > > this version. I still don't see v3.0;-), so, maybe we even can push it for > > > 3.1. A trivial comparison with v2 shows the size of the reserved field as > > > the only change in the API, and the compatibility fix as the only two > > > functional changes. > > > > > > v3: addressed multiple comments by Sakari Ailus > > > > > > 1. increased reserved field in "struct v4l2_create_buffers" to 8 32-bit > > > ints > > > 2. multiple documentation fixes and improvements > > > 3. fixed misplaced "case VIDIOC_PREPARE_BUF" in ioctl 32-bit compatibility > > > processing > > > > > > v2: > > > > > > 1. add preliminary Documentation > > > 2. add flag V4L2_BUFFER_FLAG_NO_CACHE_CLEAN > > > 3. remove VIDIOC_DESTROY_BUFS > > > 4. rename SUBMIT to VIDIOC_PREPARE_BUF > > > 5. add reserved field to struct v4l2_create_buffers > > > 6. cache handling flags moved to struct v4l2_buffer for processing during > > > VIDIOC_PREPARE_BUF > > > 7. VIDIOC_PREPARE_BUF now uses struct v4l2_buffer as its argument > > > > > > > > > Documentation/DocBook/media/v4l/io.xml | 17 +++ > > > Documentation/DocBook/media/v4l/v4l2.xml | 2 + > > > .../DocBook/media/v4l/vidioc-create-bufs.xml | 152 ++++++++++++++++++++ > > > .../DocBook/media/v4l/vidioc-prepare-buf.xml | 96 ++++++++++++ > > > drivers/media/video/v4l2-compat-ioctl32.c | 68 ++++++++- > > > drivers/media/video/v4l2-ioctl.c | 32 ++++ > > > include/linux/videodev2.h | 16 ++ > > > include/media/v4l2-ioctl.h | 2 + > > > 8 files changed, 377 insertions(+), 8 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..6249d0e 100644 > > > --- a/Documentation/DocBook/media/v4l/io.xml > > > +++ b/Documentation/DocBook/media/v4l/io.xml > > > @@ -927,6 +927,23 @@ ioctl is called.</entry> > > > Applications set or clear this flag before calling the > > > <constant>VIDIOC_QBUF</constant> ioctl.</entry> > > > </row> > > > + <row> > > > + <entry><constant>V4L2_BUF_FLAG_NO_CACHE_INVALIDATE</constant></entry> > > > + <entry>0x0400</entry> > > > + <entry>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 br touched by the CPU, instead the buffer will, probably, be > > > +passed on to a DMA-capable hardware unit for further processing or output. > > > +</entry> > > > + </row> > > > + <row> > > > + <entry><constant>V4L2_BUF_FLAG_NO_CACHE_CLEAN</constant></entry> > > > + <entry>0x0800</entry> > > > + <entry>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.</entry> > > > + </row> > > > </tbody> > > > </tgroup> > > > </table> > > > 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.</revremark> > > > &sub-close; > > > &sub-ioctl; > > > <!-- All ioctls go here. --> > > > + &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.</revremark> > > > &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..5f0158c > > > --- /dev/null > > > +++ b/Documentation/DocBook/media/v4l/vidioc-create-bufs.xml > > > @@ -0,0 +1,152 @@ > > > +<refentry id="vidioc-create-bufs"> > > > + <refmeta> > > > + <refentrytitle>ioctl VIDIOC_CREATE_BUFS</refentrytitle> > > > + &manvol; > > > + </refmeta> > > > + > > > + <refnamediv> > > > + <refname>VIDIOC_CREATE_BUFS</refname> > > > + <refpurpose>Create buffers for Memory Mapped or User Pointer I/O</refpurpose> > > > + </refnamediv> > > > + > > > + <refsynopsisdiv> > > > + <funcsynopsis> > > > + <funcprototype> > > > + <funcdef>int <function>ioctl</function></funcdef> > > > + <paramdef>int <parameter>fd</parameter></paramdef> > > > + <paramdef>int <parameter>request</parameter></paramdef> > > > + <paramdef>struct v4l2_create_buffers *<parameter>argp</parameter></paramdef> > > > + </funcprototype> > > > + </funcsynopsis> > > > + </refsynopsisdiv> > > > + > > > + <refsect1> > > > + <title>Arguments</title> > > > + > > > + <variablelist> > > > + <varlistentry> > > > + <term><parameter>fd</parameter></term> > > > + <listitem> > > > + <para>&fd;</para> > > > + </listitem> > > > + </varlistentry> > > > + <varlistentry> > > > + <term><parameter>request</parameter></term> > > > + <listitem> > > > + <para>VIDIOC_CREATE_BUFS</para> > > > + </listitem> > > > + </varlistentry> > > > + <varlistentry> > > > + <term><parameter>argp</parameter></term> > > > + <listitem> > > > + <para></para> > > > + </listitem> > > > + </varlistentry> > > > + </variablelist> > > > + </refsect1> > > > + > > > + <refsect1> > > > + <title>Description</title> > > > + > > > + <para>This ioctl is used to create buffers for <link linkend="mmap">memory > > > +mapped</link> or <link linkend="userp">user pointer</link> > > > +I/O. It can be used as an alternative to the <constant>VIDIOC_REQBUFS</constant> > > > +ioctl, when a tighter control over buffers is required. This ioctl can be called > > > +multiple times to create buffers of different sizes. > > > + > > > + <para>To allocate device buffers applications initialize all > > > +fields of the <structname>v4l2_create_buffers</structname> structure. > > > +They set the <structfield>type</structfield> field in the > > > +<structname>v4l2_format</structname> structure, embedded in this > > > +structure, to the respective stream or buffer type. > > > +<structfield>count</structfield> must be set to the number of required > > > +buffers. <structfield>memory</structfield> specifies the required I/O > > > +method. Applications have two possibilities to specify the size of buffers > > > +to be prepared: they can either set the <structfield>size</structfield> > > > +field explicitly to a non-zero value, or fill in the frame format data in the > > > +<structfield>format</structfield> field. In the latter case buffer sizes > > > +will be calculated automatically by the driver. The > > > +<structfield>reserved</structfield> 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 <structfield>count</structfield> and > > > +the <structfield>index</structfield> fields respectively. > > > +<structfield>count</structfield> can be smaller than the number requested. > > > +-ENOMEM is returned, if the driver runs out of free memory.</para> > > > > No need to mention -ENOMEM here. It's already in the return values below; > > the same goes for the EINVAL just below. > > Yes, incremental patches welcome. > > > > > > + <para>When the I/O method is not supported the ioctl > > > +returns an &EINVAL;.</para> > > > + > > > + <table pgwide="1" frame="none" id="v4l2-create-buffers"> > > > + <title>struct <structname>v4l2_create_buffers</structname></title> > > > + <tgroup cols="3"> > > > + &cs-str; > > > + <tbody valign="top"> > > > + <row> > > > + <entry>__u32</entry> > > > + <entry><structfield>index</structfield></entry> > > > + <entry>The starting buffer index, returned by the driver.</entry> > > > + </row> > > > + <row> > > > + <entry>__u32</entry> > > > + <entry><structfield>count</structfield></entry> > > > + <entry>The number of buffers requested or granted.</entry> > > > + </row> > > > + <row> > > > + <entry>&v4l2-memory;</entry> > > > + <entry><structfield>memory</structfield></entry> > > > + <entry>Applications set this field to > > > +<constant>V4L2_MEMORY_MMAP</constant> or > > > +<constant>V4L2_MEMORY_USERPTR</constant>.</entry> > > > + </row> > > > + <row> > > > + <entry>__u32</entry> > > > + <entry><structfield>size</structfield></entry> > > > + <entry>Explicit size of buffers, being created.</entry> > > > + </row> > > > + <row> > > > + <entry>&v4l2-format;</entry> > > > + <entry><structfield>format</structfield></entry> > > > + <entry>Application has to set the <structfield>type</structfield> > > > +field, other fields should be used, if the application wants to allocate buffers > > > +for a specific frame format.</entry> > > > + </row> > > > + <row> > > > + <entry>__u32</entry> > > > + <entry><structfield>reserved</structfield>[8]</entry> > > > + <entry>A place holder for future extensions.</entry> > > > + </row> > > > + </tbody> > > > + </tgroup> > > > + </table> > > > + </refsect1> > > > + > > > + <refsect1> > > > + &return-value; > > > + > > > + <variablelist> > > > + <varlistentry> > > > + <term><errorcode>ENOMEM</errorcode></term> > > > + <listitem> > > > + <para>No memory to allocate buffers for <link linkend="mmap">memory > > > +mapped</link> I/O.</para> > > > + </listitem> > > > + </varlistentry> > > > + <varlistentry> > > > + <term><errorcode>EINVAL</errorcode></term> > > > + <listitem> > > > + <para>The buffer type (<structfield>type</structfield> field) or the > > > +requested I/O method (<structfield>memory</structfield>) is not > > > +supported.</para> > > > + </listitem> > > > + </varlistentry> > > > + </variablelist> > > > + </refsect1> > > > +</refentry> > > > + > > > +<!-- > > > +Local Variables: > > > +mode: sgml > > > +sgml-parent-document: "v4l2.sgml" > > > +indent-tabs-mode: nil > > > +End: > > > +--> > > > 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 @@ > > > +<refentry id="vidioc-prepare-buf"> > > > + <refmeta> > > > + <refentrytitle>ioctl VIDIOC_PREPARE_BUF</refentrytitle> > > > + &manvol; > > > + </refmeta> > > > + > > > + <refnamediv> > > > + <refname>VIDIOC_PREPARE_BUF</refname> > > > + <refpurpose>Prepare a buffer for I/O</refpurpose> > > > + </refnamediv> > > > + > > > + <refsynopsisdiv> > > > + <funcsynopsis> > > > + <funcprototype> > > > + <funcdef>int <function>ioctl</function></funcdef> > > > + <paramdef>int <parameter>fd</parameter></paramdef> > > > + <paramdef>int <parameter>request</parameter></paramdef> > > > + <paramdef>struct v4l2_buffer *<parameter>argp</parameter></paramdef> > > > > I'm not sure if we want to use struct v4l2_buffer here. It's an obvious > > choice, but there's an issue in using it. > > > > The struct is almost full i.e. there are very few reserved fields in it, > > namely two, counting the input field which was unused AFAIR. There are now > > three ioctls using it as their argument. > > > > Future functionality which would be nice: > > > > - Format counters. Every format set by S_FMT (or gotten by G_FMT) should > > come with a counter value so that the user would know the format of > > dequeued buffers when setting the format on-the-fly. Currently there are > > only bytesperline and length, but the format can't be explicitly > > determined from those. > > > > - Binding to generic DMA buffers. This can likely be achieved by using the m > > union in struct v4l2_buffer. (Speaking of which: these buffers likely have > > a cache behaviour defined for them, e.g. they might be non-cacheable. This > > probably has no effect on the cache flags in V4L2 but it might still be > > good to keep this in mind.) > > > > I don't have an obvious replacement for it either. There are two options I > > can see: Create a new struct v4l2_ext_buffer which contains the old > > v4l2_buffer plus a bunch of reserved fields, or use the two last fields to > > hold a pointer to an extension struct, v4l2_buffer_ext. The new structure > > would later be used to contain the reserved fields. > > > > I admit that this is not an issue right now, myt point is that I don't want > > this to be an issue in the future either. > > Good, then we'll address it, when it becomes an issue. Defining v4l2_ext_buffer will mean there's a new ioctl. I think we should be a little more future proof if we can. Extending the current v4l2_buffer by another structure with a pointer from v4l2_buffer doesn't need this. I would like to have someone else's opinion on what to do with this. The resolution might as well be to accept the situation and resolve it when there's an absolute need to. > > > > > + </funcprototype> > > > + </funcsynopsis> > > > + </refsynopsisdiv> > > > + > > > + <refsect1> > > > + <title>Arguments</title> > > > + > > > + <variablelist> > > > + <varlistentry> > > > + <term><parameter>fd</parameter></term> > > > + <listitem> > > > + <para>&fd;</para> > > > + </listitem> > > > + </varlistentry> > > > + <varlistentry> > > > + <term><parameter>request</parameter></term> > > > + <listitem> > > > + <para>VIDIOC_PREPARE_BUF</para> > > > + </listitem> > > > + </varlistentry> > > > + <varlistentry> > > > + <term><parameter>argp</parameter></term> > > > + <listitem> > > > + <para></para> > > > + </listitem> > > > + </varlistentry> > > > + </variablelist> > > > + </refsect1> > > > + > > > + <refsect1> > > > + <title>Description</title> > > > + > > > + <para>Applications can optionally call the > > > +<constant>VIDIOC_PREPARE_BUF</constant> ioctl to pass ownership of the buffer > > > +to the driver before actually enqueuing it, using the > > > +<constant>VIDIOC_QBUF</constant> 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 > > > +<constant>V4L2_BUF_FLAG_NO_CACHE_INVALIDATE</constant> and > > > +<constant>V4L2_BUF_FLAG_NO_CACHE_CLEAN</constant> flags to skip the respective > > > +step.</para> > > > + > > > + <para>The <structname>v4l2_buffer</structname> structure is > > > +specified in <xref linkend="buffer" />.</para> > > > + </refsect1> > > > + > > > + <refsect1> > > > + &return-value; > > > + > > > + <variablelist> > > > + <varlistentry> > > > + <term><errorcode>EBUSY</errorcode></term> > > > + <listitem> > > > + <para>File I/O is in progress.</para> > > > + </listitem> > > > + </varlistentry> > > > + <varlistentry> > > > + <term><errorcode>EINVAL</errorcode></term> > > > + <listitem> > > > + <para>The buffer <structfield>type</structfield> is not > > > +supported, or the <structfield>index</structfield> is out of bounds, > > > +or no buffers have been allocated yet, or the > > > +<structfield>userptr</structfield> or > > > +<structfield>length</structfield> are invalid.</para> > > > + </listitem> > > > + </varlistentry> > > > + </variablelist> > > > + </refsect1> > > > +</refentry> > > > + > > > +<!-- > > > +Local Variables: > > > +mode: sgml > > > +sgml-parent-document: "v4l2.sgml" > > > +indent-tabs-mode: nil > > > +End: > > > +--> > > > diff --git a/drivers/media/video/v4l2-compat-ioctl32.c b/drivers/media/video/v4l2-compat-ioctl32.c > > > index 61979b7..4105f69 100644 > > > --- a/drivers/media/video/v4l2-compat-ioctl32.c > > > +++ b/drivers/media/video/v4l2-compat-ioctl32.c > > > @@ -159,11 +159,16 @@ struct v4l2_format32 { > > > } fmt; > > > }; > > > > > > -static int get_v4l2_format32(struct v4l2_format *kp, struct v4l2_format32 __user *up) > > > +struct v4l2_create_buffers32 { > > > + __u32 index; /* output: buffers index...index + count - 1 have been created */ > > > + __u32 count; > > > + enum v4l2_memory memory; > > > + __u32 size; /* Explicit size, e.g., for compressed streams */ > > > + struct v4l2_format32 format; /* "type" is used always, the rest if size == 0 */ > > > +}; > > > + > > > +static int __get_v4l2_format32(struct v4l2_format *kp, struct v4l2_format32 __user *up) > > > { > > > - if (!access_ok(VERIFY_READ, up, sizeof(struct v4l2_format32)) || > > > - get_user(kp->type, &up->type)) > > > - return -EFAULT; > > > switch (kp->type) { > > > case V4L2_BUF_TYPE_VIDEO_CAPTURE: > > > case V4L2_BUF_TYPE_VIDEO_OUTPUT: > > > @@ -192,11 +197,24 @@ static int get_v4l2_format32(struct v4l2_format *kp, struct v4l2_format32 __user > > > } > > > } > > > > > > -static int put_v4l2_format32(struct v4l2_format *kp, struct v4l2_format32 __user *up) > > > +static int get_v4l2_format32(struct v4l2_format *kp, struct v4l2_format32 __user *up) > > > +{ > > > + if (!access_ok(VERIFY_READ, up, sizeof(struct v4l2_format32)) || > > > + get_user(kp->type, &up->type)) > > > + return -EFAULT; > > > + return __get_v4l2_format32(kp, up); > > > +} > > > + > > > +static int get_v4l2_create32(struct v4l2_create_buffers *kp, struct v4l2_create_buffers32 __user *up) > > > +{ > > > + if (!access_ok(VERIFY_READ, up, sizeof(struct v4l2_create_buffers32)) || > > > + copy_from_user(kp, up, offsetof(struct v4l2_create_buffers32, format.fmt))) > > > + return -EFAULT; > > > + return __get_v4l2_format32(&kp->format, &up->format); > > > +} > > > + > > > +static int __put_v4l2_format32(struct v4l2_format *kp, struct v4l2_format32 __user *up) > > > { > > > - if (!access_ok(VERIFY_WRITE, up, sizeof(struct v4l2_format32)) || > > > - put_user(kp->type, &up->type)) > > > - return -EFAULT; > > > switch (kp->type) { > > > case V4L2_BUF_TYPE_VIDEO_CAPTURE: > > > case V4L2_BUF_TYPE_VIDEO_OUTPUT: > > > @@ -225,6 +243,22 @@ static int put_v4l2_format32(struct v4l2_format *kp, struct v4l2_format32 __user > > > } > > > } > > > > > > +static int put_v4l2_format32(struct v4l2_format *kp, struct v4l2_format32 __user *up) > > > +{ > > > + if (!access_ok(VERIFY_WRITE, up, sizeof(struct v4l2_format32)) || > > > + put_user(kp->type, &up->type)) > > > + return -EFAULT; > > > + return __put_v4l2_format32(kp, up); > > > +} > > > + > > > +static int put_v4l2_create32(struct v4l2_create_buffers *kp, struct v4l2_create_buffers32 __user *up) > > > +{ > > > + if (!access_ok(VERIFY_WRITE, up, sizeof(struct v4l2_create_buffers32)) || > > > + copy_to_user(up, kp, offsetof(struct v4l2_create_buffers32, format.fmt))) > > > + return -EFAULT; > > > + return __put_v4l2_format32(&kp->format, &up->format); > > > +} > > > + > > > struct v4l2_standard32 { > > > __u32 index; > > > __u32 id[2]; /* __u64 would get the alignment wrong */ > > > @@ -702,6 +736,8 @@ 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_CREATE_BUFS32 _IOWR('V', 92, struct v4l2_create_buffers32) > > > +#define VIDIOC_PREPARE_BUF32 _IOWR('V', 93, struct v4l2_buffer32) > > > > > > #define VIDIOC_OVERLAY32 _IOW ('V', 14, s32) > > > #define VIDIOC_STREAMON32 _IOW ('V', 18, s32) > > > @@ -721,6 +757,7 @@ static long do_video_ioctl(struct file *file, unsigned int cmd, unsigned long ar > > > struct v4l2_standard v2s; > > > struct v4l2_ext_controls v2ecs; > > > struct v4l2_event v2ev; > > > + struct v4l2_create_buffers v2crt; > > > unsigned long vx; > > > int vi; > > > } karg; > > > @@ -751,6 +788,8 @@ 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_CREATE_BUFS32: cmd = VIDIOC_CREATE_BUFS; break; > > > + case VIDIOC_PREPARE_BUF32: cmd = VIDIOC_PREPARE_BUF; break; > > > } > > > > > > switch (cmd) { > > > @@ -775,6 +814,12 @@ static long do_video_ioctl(struct file *file, unsigned int cmd, unsigned long ar > > > compatible_arg = 0; > > > break; > > > > > > + case VIDIOC_CREATE_BUFS: > > > + err = get_v4l2_create32(&karg.v2crt, up); > > > + compatible_arg = 0; > > > + break; > > > + > > > + case VIDIOC_PREPARE_BUF: > > > case VIDIOC_QUERYBUF: > > > case VIDIOC_QBUF: > > > case VIDIOC_DQBUF: > > > @@ -860,6 +905,11 @@ static long do_video_ioctl(struct file *file, unsigned int cmd, unsigned long ar > > > err = put_v4l2_format32(&karg.v2f, up); > > > break; > > > > > > + case VIDIOC_CREATE_BUFS: > > > + err = put_v4l2_create32(&karg.v2crt, up); > > > + break; > > > + > > > + case VIDIOC_PREPARE_BUF: > > > case VIDIOC_QUERYBUF: > > > case VIDIOC_QBUF: > > > case VIDIOC_DQBUF: > > > @@ -959,6 +1009,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_BUFS32: > > > + 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..7824152 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,36 @@ 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 = check_fmt(ops, create->format.type); > > > + if (ret) > > > + break; > > > + > > > + if (create->size) > > > + CLEAR_AFTER_FIELD(create, count); > > > + ret = ops->vidioc_create_bufs(file, fh, create); > > > + > > > + dbgarg(cmd, "count=%d\n", create->count); > > > + 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..0b594c7 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,16 @@ 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; > > > + enum v4l2_memory memory; > > > > Aren't enums something to avoid in public APIs? -> __u32? > > They are, but this specific enum is already used in "struct > v4l2_requestbuffers" and "struct v4l2_buffer" so, I decided, it would be > consistent to use it here too, even if it involves extra compat craft, and > the embedded struct v4l2_format also includes an enum in it anyway, so, > basically, they are all over the place. In my understanding that goes for all enums, not only those that would be newly introduced to the API. E.g. struct v4l2_event_ctrl.type is actually enum v4l2_ctrl_type rather than __u32. struct v4l2_queryctrl uses the enum, v4l2_event_ctrl does not. > > > > > + __u32 size; /* Explicit size, e.g., for compressed streams */ > > > + struct v4l2_format format; /* "type" is used always, the rest if size == 0 */ > > > + __u32 reserved[8]; > > > +}; Btw. what about __attribute__ ((packed)) for the structure? Not all interface structs use it, though. > > > /* > > > * 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 +2195,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 > > > > > > > Regards, > > So, I take it as an ack;-) It's not an ack yet but we're definitely close. :-)
Just some typos/documentation improvements: On Wednesday, July 20, 2011 10:43:08 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 <g.liakhovetski@gmx.de> > --- > > It's been almost a month since v2, the only comments were a request to > increase the reserved space in the new ioctl() and to improve > documentation. The reserved field is increased in this version, > documentation also has been improved in multiple locations. I think, > documentation can be further improved at any time, but if there are no > objections against the actual contents of this patch, maybe we can commit > this version. I still don't see v3.0;-), so, maybe we even can push it for > 3.1. A trivial comparison with v2 shows the size of the reserved field as > the only change in the API, and the compatibility fix as the only two > functional changes. > > v3: addressed multiple comments by Sakari Ailus > > 1. increased reserved field in "struct v4l2_create_buffers" to 8 32-bit > ints > 2. multiple documentation fixes and improvements > 3. fixed misplaced "case VIDIOC_PREPARE_BUF" in ioctl 32-bit compatibility > processing > > v2: > > 1. add preliminary Documentation > 2. add flag V4L2_BUFFER_FLAG_NO_CACHE_CLEAN > 3. remove VIDIOC_DESTROY_BUFS > 4. rename SUBMIT to VIDIOC_PREPARE_BUF > 5. add reserved field to struct v4l2_create_buffers > 6. cache handling flags moved to struct v4l2_buffer for processing during > VIDIOC_PREPARE_BUF > 7. VIDIOC_PREPARE_BUF now uses struct v4l2_buffer as its argument > > > Documentation/DocBook/media/v4l/io.xml | 17 +++ > Documentation/DocBook/media/v4l/v4l2.xml | 2 + > .../DocBook/media/v4l/vidioc-create-bufs.xml | 152 ++++++++++++++++++++ > .../DocBook/media/v4l/vidioc-prepare-buf.xml | 96 ++++++++++++ > drivers/media/video/v4l2-compat-ioctl32.c | 68 ++++++++- > drivers/media/video/v4l2-ioctl.c | 32 ++++ > include/linux/videodev2.h | 16 ++ > include/media/v4l2-ioctl.h | 2 + > 8 files changed, 377 insertions(+), 8 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..6249d0e 100644 > --- a/Documentation/DocBook/media/v4l/io.xml > +++ b/Documentation/DocBook/media/v4l/io.xml > @@ -927,6 +927,23 @@ ioctl is called.</entry> > Applications set or clear this flag before calling the > <constant>VIDIOC_QBUF</constant> ioctl.</entry> > </row> > + <row> > + <entry><constant>V4L2_BUF_FLAG_NO_CACHE_INVALIDATE</constant></entry> > + <entry>0x0400</entry> > + <entry>Caches do not have to be invalidated for this buffer. > +Typically applications shall use this flag, if the data, captured in the buffer The two commas can be removed in the line above. > +is not going to br touched by the CPU, instead the buffer will, probably, be typo: br -> be > +passed on to a DMA-capable hardware unit for further processing or output. > +</entry> > + </row> > + <row> > + <entry><constant>V4L2_BUF_FLAG_NO_CACHE_CLEAN</constant></entry> > + <entry>0x0800</entry> > + <entry>Caches do not have to be cleaned for this buffer. > +Typically applications shall use this flag for output buffers, if the data Comma can be removed. > +in this buffer has not been created by the CPU, but by some DMA-capable unit, Comma before 'but' can be removed. > +in which case caches have not been used.</entry> > + </row> > </tbody> > </tgroup> > </table> > 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.</revremark> > &sub-close; > &sub-ioctl; > <!-- All ioctls go here. --> > + &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.</revremark> > &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..5f0158c > --- /dev/null > +++ b/Documentation/DocBook/media/v4l/vidioc-create-bufs.xml > @@ -0,0 +1,152 @@ > +<refentry id="vidioc-create-bufs"> > + <refmeta> > + <refentrytitle>ioctl VIDIOC_CREATE_BUFS</refentrytitle> > + &manvol; > + </refmeta> > + > + <refnamediv> > + <refname>VIDIOC_CREATE_BUFS</refname> > + <refpurpose>Create buffers for Memory Mapped or User Pointer I/O</refpurpose> > + </refnamediv> > + > + <refsynopsisdiv> > + <funcsynopsis> > + <funcprototype> > + <funcdef>int <function>ioctl</function></funcdef> > + <paramdef>int <parameter>fd</parameter></paramdef> > + <paramdef>int <parameter>request</parameter></paramdef> > + <paramdef>struct v4l2_create_buffers *<parameter>argp</parameter></paramdef> > + </funcprototype> > + </funcsynopsis> > + </refsynopsisdiv> > + > + <refsect1> > + <title>Arguments</title> > + > + <variablelist> > + <varlistentry> > + <term><parameter>fd</parameter></term> > + <listitem> > + <para>&fd;</para> > + </listitem> > + </varlistentry> > + <varlistentry> > + <term><parameter>request</parameter></term> > + <listitem> > + <para>VIDIOC_CREATE_BUFS</para> > + </listitem> > + </varlistentry> > + <varlistentry> > + <term><parameter>argp</parameter></term> > + <listitem> > + <para></para> > + </listitem> > + </varlistentry> > + </variablelist> > + </refsect1> > + > + <refsect1> > + <title>Description</title> > + > + <para>This ioctl is used to create buffers for <link linkend="mmap">memory > +mapped</link> or <link linkend="userp">user pointer</link> > +I/O. It can be used as an alternative to the <constant>VIDIOC_REQBUFS</constant> > +ioctl, when a tighter control over buffers is required. This ioctl can be called > +multiple times to create buffers of different sizes. > + > + <para>To allocate device buffers applications initialize all > +fields of the <structname>v4l2_create_buffers</structname> structure. > +They set the <structfield>type</structfield> field in the > +<structname>v4l2_format</structname> structure, embedded in this > +structure, to the respective stream or buffer type. > +<structfield>count</structfield> must be set to the number of required > +buffers. <structfield>memory</structfield> specifies the required I/O > +method. Applications have two possibilities to specify the size of buffers > +to be prepared: they can either set the <structfield>size</structfield> > +field explicitly to a non-zero value, or fill in the frame format data in the > +<structfield>format</structfield> field. In the latter case buffer sizes > +will be calculated automatically by the driver. The > +<structfield>reserved</structfield> 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 <structfield>count</structfield> and > +the <structfield>index</structfield> fields respectively. > +<structfield>count</structfield> can be smaller than the number requested. > +-ENOMEM is returned, if the driver runs out of free memory.</para> > + <para>When the I/O method is not supported the ioctl > +returns an &EINVAL;.</para> > + > + <table pgwide="1" frame="none" id="v4l2-create-buffers"> > + <title>struct <structname>v4l2_create_buffers</structname></title> > + <tgroup cols="3"> > + &cs-str; > + <tbody valign="top"> > + <row> > + <entry>__u32</entry> > + <entry><structfield>index</structfield></entry> > + <entry>The starting buffer index, returned by the driver.</entry> > + </row> > + <row> > + <entry>__u32</entry> > + <entry><structfield>count</structfield></entry> > + <entry>The number of buffers requested or granted.</entry> > + </row> > + <row> > + <entry>&v4l2-memory;</entry> > + <entry><structfield>memory</structfield></entry> > + <entry>Applications set this field to > +<constant>V4L2_MEMORY_MMAP</constant> or > +<constant>V4L2_MEMORY_USERPTR</constant>.</entry> > + </row> > + <row> > + <entry>__u32</entry> > + <entry><structfield>size</structfield></entry> > + <entry>Explicit size of buffers, being created.</entry> > + </row> > + <row> > + <entry>&v4l2-format;</entry> > + <entry><structfield>format</structfield></entry> > + <entry>Application has to set the <structfield>type</structfield> > +field, other fields should be used, if the application wants to allocate buffers > +for a specific frame format.</entry> This sentence doesn't make sense to me. I suggest: <entry>The application must always set the <structfield>type</structfield> field of &v4l2-format. The other fields only need to be set if the application set the <structfield>size</structfield> field to 0. In that case the driver will allocated buffers for that specific frame format.</entry> > + </row> > + <row> > + <entry>__u32</entry> > + <entry><structfield>reserved</structfield>[8]</entry> > + <entry>A place holder for future extensions.</entry> > + </row> > + </tbody> > + </tgroup> > + </table> > + </refsect1> > + > + <refsect1> > + &return-value; > + > + <variablelist> > + <varlistentry> > + <term><errorcode>ENOMEM</errorcode></term> > + <listitem> > + <para>No memory to allocate buffers for <link linkend="mmap">memory > +mapped</link> I/O.</para> > + </listitem> > + </varlistentry> > + <varlistentry> > + <term><errorcode>EINVAL</errorcode></term> > + <listitem> > + <para>The buffer type (<structfield>type</structfield> field) or the > +requested I/O method (<structfield>memory</structfield>) is not > +supported.</para> > + </listitem> > + </varlistentry> > + </variablelist> > + </refsect1> > +</refentry> > + > +<!-- > +Local Variables: > +mode: sgml > +sgml-parent-document: "v4l2.sgml" > +indent-tabs-mode: nil > +End: > +--> > 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 @@ > +<refentry id="vidioc-prepare-buf"> > + <refmeta> > + <refentrytitle>ioctl VIDIOC_PREPARE_BUF</refentrytitle> > + &manvol; > + </refmeta> > + > + <refnamediv> > + <refname>VIDIOC_PREPARE_BUF</refname> > + <refpurpose>Prepare a buffer for I/O</refpurpose> > + </refnamediv> > + > + <refsynopsisdiv> > + <funcsynopsis> > + <funcprototype> > + <funcdef>int <function>ioctl</function></funcdef> > + <paramdef>int <parameter>fd</parameter></paramdef> > + <paramdef>int <parameter>request</parameter></paramdef> > + <paramdef>struct v4l2_buffer *<parameter>argp</parameter></paramdef> > + </funcprototype> > + </funcsynopsis> > + </refsynopsisdiv> > + > + <refsect1> > + <title>Arguments</title> > + > + <variablelist> > + <varlistentry> > + <term><parameter>fd</parameter></term> > + <listitem> > + <para>&fd;</para> > + </listitem> > + </varlistentry> > + <varlistentry> > + <term><parameter>request</parameter></term> > + <listitem> > + <para>VIDIOC_PREPARE_BUF</para> > + </listitem> > + </varlistentry> > + <varlistentry> > + <term><parameter>argp</parameter></term> > + <listitem> > + <para></para> > + </listitem> > + </varlistentry> > + </variablelist> > + </refsect1> > + > + <refsect1> > + <title>Description</title> > + > + <para>Applications can optionally call the > +<constant>VIDIOC_PREPARE_BUF</constant> ioctl to pass ownership of the buffer > +to the driver before actually enqueuing it, using the > +<constant>VIDIOC_QBUF</constant> 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 > +<constant>V4L2_BUF_FLAG_NO_CACHE_INVALIDATE</constant> and > +<constant>V4L2_BUF_FLAG_NO_CACHE_CLEAN</constant> flags to skip the respective > +step.</para> > + > + <para>The <structname>v4l2_buffer</structname> structure is > +specified in <xref linkend="buffer" />.</para> > + </refsect1> > + > + <refsect1> > + &return-value; > + > + <variablelist> > + <varlistentry> > + <term><errorcode>EBUSY</errorcode></term> > + <listitem> > + <para>File I/O is in progress.</para> > + </listitem> > + </varlistentry> > + <varlistentry> > + <term><errorcode>EINVAL</errorcode></term> > + <listitem> > + <para>The buffer <structfield>type</structfield> is not > +supported, or the <structfield>index</structfield> is out of bounds, > +or no buffers have been allocated yet, or the > +<structfield>userptr</structfield> or > +<structfield>length</structfield> are invalid.</para> > + </listitem> > + </varlistentry> > + </variablelist> > + </refsect1> > +</refentry> > + > +<!-- > +Local Variables: > +mode: sgml > +sgml-parent-document: "v4l2.sgml" > +indent-tabs-mode: nil > +End: > +--> > diff --git a/drivers/media/video/v4l2-compat-ioctl32.c b/drivers/media/video/v4l2-compat-ioctl32.c > index 61979b7..4105f69 100644 > --- a/drivers/media/video/v4l2-compat-ioctl32.c > +++ b/drivers/media/video/v4l2-compat-ioctl32.c > @@ -159,11 +159,16 @@ struct v4l2_format32 { > } fmt; > }; > > -static int get_v4l2_format32(struct v4l2_format *kp, struct v4l2_format32 __user *up) > +struct v4l2_create_buffers32 { > + __u32 index; /* output: buffers index...index + count - 1 have been created */ > + __u32 count; > + enum v4l2_memory memory; > + __u32 size; /* Explicit size, e.g., for compressed streams */ > + struct v4l2_format32 format; /* "type" is used always, the rest if size == 0 */ > +}; > + > +static int __get_v4l2_format32(struct v4l2_format *kp, struct v4l2_format32 __user *up) > { > - if (!access_ok(VERIFY_READ, up, sizeof(struct v4l2_format32)) || > - get_user(kp->type, &up->type)) > - return -EFAULT; > switch (kp->type) { > case V4L2_BUF_TYPE_VIDEO_CAPTURE: > case V4L2_BUF_TYPE_VIDEO_OUTPUT: > @@ -192,11 +197,24 @@ static int get_v4l2_format32(struct v4l2_format *kp, struct v4l2_format32 __user > } > } > > -static int put_v4l2_format32(struct v4l2_format *kp, struct v4l2_format32 __user *up) > +static int get_v4l2_format32(struct v4l2_format *kp, struct v4l2_format32 __user *up) > +{ > + if (!access_ok(VERIFY_READ, up, sizeof(struct v4l2_format32)) || > + get_user(kp->type, &up->type)) > + return -EFAULT; > + return __get_v4l2_format32(kp, up); > +} > + > +static int get_v4l2_create32(struct v4l2_create_buffers *kp, struct v4l2_create_buffers32 __user *up) > +{ > + if (!access_ok(VERIFY_READ, up, sizeof(struct v4l2_create_buffers32)) || > + copy_from_user(kp, up, offsetof(struct v4l2_create_buffers32, format.fmt))) > + return -EFAULT; > + return __get_v4l2_format32(&kp->format, &up->format); > +} > + > +static int __put_v4l2_format32(struct v4l2_format *kp, struct v4l2_format32 __user *up) > { > - if (!access_ok(VERIFY_WRITE, up, sizeof(struct v4l2_format32)) || > - put_user(kp->type, &up->type)) > - return -EFAULT; > switch (kp->type) { > case V4L2_BUF_TYPE_VIDEO_CAPTURE: > case V4L2_BUF_TYPE_VIDEO_OUTPUT: > @@ -225,6 +243,22 @@ static int put_v4l2_format32(struct v4l2_format *kp, struct v4l2_format32 __user > } > } > > +static int put_v4l2_format32(struct v4l2_format *kp, struct v4l2_format32 __user *up) > +{ > + if (!access_ok(VERIFY_WRITE, up, sizeof(struct v4l2_format32)) || > + put_user(kp->type, &up->type)) > + return -EFAULT; > + return __put_v4l2_format32(kp, up); > +} > + > +static int put_v4l2_create32(struct v4l2_create_buffers *kp, struct v4l2_create_buffers32 __user *up) > +{ > + if (!access_ok(VERIFY_WRITE, up, sizeof(struct v4l2_create_buffers32)) || > + copy_to_user(up, kp, offsetof(struct v4l2_create_buffers32, format.fmt))) > + return -EFAULT; > + return __put_v4l2_format32(&kp->format, &up->format); > +} > + > struct v4l2_standard32 { > __u32 index; > __u32 id[2]; /* __u64 would get the alignment wrong */ > @@ -702,6 +736,8 @@ 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_CREATE_BUFS32 _IOWR('V', 92, struct v4l2_create_buffers32) > +#define VIDIOC_PREPARE_BUF32 _IOWR('V', 93, struct v4l2_buffer32) > > #define VIDIOC_OVERLAY32 _IOW ('V', 14, s32) > #define VIDIOC_STREAMON32 _IOW ('V', 18, s32) > @@ -721,6 +757,7 @@ static long do_video_ioctl(struct file *file, unsigned int cmd, unsigned long ar > struct v4l2_standard v2s; > struct v4l2_ext_controls v2ecs; > struct v4l2_event v2ev; > + struct v4l2_create_buffers v2crt; > unsigned long vx; > int vi; > } karg; > @@ -751,6 +788,8 @@ 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_CREATE_BUFS32: cmd = VIDIOC_CREATE_BUFS; break; > + case VIDIOC_PREPARE_BUF32: cmd = VIDIOC_PREPARE_BUF; break; > } > > switch (cmd) { > @@ -775,6 +814,12 @@ static long do_video_ioctl(struct file *file, unsigned int cmd, unsigned long ar > compatible_arg = 0; > break; > > + case VIDIOC_CREATE_BUFS: > + err = get_v4l2_create32(&karg.v2crt, up); > + compatible_arg = 0; > + break; > + > + case VIDIOC_PREPARE_BUF: > case VIDIOC_QUERYBUF: > case VIDIOC_QBUF: > case VIDIOC_DQBUF: > @@ -860,6 +905,11 @@ static long do_video_ioctl(struct file *file, unsigned int cmd, unsigned long ar > err = put_v4l2_format32(&karg.v2f, up); > break; > > + case VIDIOC_CREATE_BUFS: > + err = put_v4l2_create32(&karg.v2crt, up); > + break; > + > + case VIDIOC_PREPARE_BUF: > case VIDIOC_QUERYBUF: > case VIDIOC_QBUF: > case VIDIOC_DQBUF: > @@ -959,6 +1009,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_BUFS32: > + 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..7824152 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,36 @@ 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 = check_fmt(ops, create->format.type); > + if (ret) > + break; > + > + if (create->size) > + CLEAR_AFTER_FIELD(create, count); > + > + ret = ops->vidioc_create_bufs(file, fh, create); > + > + dbgarg(cmd, "count=%d\n", create->count); > + 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..0b594c7 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,16 @@ 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; > + enum v4l2_memory memory; > + __u32 size; /* Explicit size, e.g., for compressed streams */ > + struct v4l2_format format; /* "type" is used always, the rest if size == 0 */ > + __u32 reserved[8]; > +}; > + > /* > * 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 +2195,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, > Regards, Hans -- 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
On Wednesday, July 20, 2011 17:19:46 Sakari Ailus wrote: > On Wed, Jul 20, 2011 at 04:47:46PM +0200, Guennadi Liakhovetski wrote: > > On Wed, 20 Jul 2011, Sakari Ailus wrote: > > > > > Hi, Guennadi! > > > > > > Thanks for the patch! > > > > > > On Wed, Jul 20, 2011 at 10:43:08AM +0200, 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 <g.liakhovetski@gmx.de> > > > > --- > > > > > > > > It's been almost a month since v2, the only comments were a request to > > > > increase the reserved space in the new ioctl() and to improve > > > > documentation. The reserved field is increased in this version, > > > > documentation also has been improved in multiple locations. I think, > > > > documentation can be further improved at any time, but if there are no > > > > objections against the actual contents of this patch, maybe we can commit > > > > this version. I still don't see v3.0;-), so, maybe we even can push it for > > > > 3.1. A trivial comparison with v2 shows the size of the reserved field as > > > > the only change in the API, and the compatibility fix as the only two > > > > functional changes. > > > > > > > > v3: addressed multiple comments by Sakari Ailus > > > > > > > > 1. increased reserved field in "struct v4l2_create_buffers" to 8 32-bit > > > > ints > > > > 2. multiple documentation fixes and improvements > > > > 3. fixed misplaced "case VIDIOC_PREPARE_BUF" in ioctl 32-bit compatibility > > > > processing > > > > > > > > v2: > > > > > > > > 1. add preliminary Documentation > > > > 2. add flag V4L2_BUFFER_FLAG_NO_CACHE_CLEAN > > > > 3. remove VIDIOC_DESTROY_BUFS > > > > 4. rename SUBMIT to VIDIOC_PREPARE_BUF > > > > 5. add reserved field to struct v4l2_create_buffers > > > > 6. cache handling flags moved to struct v4l2_buffer for processing during > > > > VIDIOC_PREPARE_BUF > > > > 7. VIDIOC_PREPARE_BUF now uses struct v4l2_buffer as its argument > > > > > > > > > > > > Documentation/DocBook/media/v4l/io.xml | 17 +++ > > > > Documentation/DocBook/media/v4l/v4l2.xml | 2 + > > > > .../DocBook/media/v4l/vidioc-create-bufs.xml | 152 ++++++++++++++++++++ > > > > .../DocBook/media/v4l/vidioc-prepare-buf.xml | 96 ++++++++++++ > > > > drivers/media/video/v4l2-compat-ioctl32.c | 68 ++++++++- > > > > drivers/media/video/v4l2-ioctl.c | 32 ++++ > > > > include/linux/videodev2.h | 16 ++ > > > > include/media/v4l2-ioctl.h | 2 + > > > > 8 files changed, 377 insertions(+), 8 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..6249d0e 100644 > > > > --- a/Documentation/DocBook/media/v4l/io.xml > > > > +++ b/Documentation/DocBook/media/v4l/io.xml > > > > @@ -927,6 +927,23 @@ ioctl is called.</entry> > > > > Applications set or clear this flag before calling the > > > > <constant>VIDIOC_QBUF</constant> ioctl.</entry> > > > > </row> > > > > + <row> > > > > + <entry><constant>V4L2_BUF_FLAG_NO_CACHE_INVALIDATE</constant></entry> > > > > + <entry>0x0400</entry> > > > > + <entry>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 br touched by the CPU, instead the buffer will, probably, be > > > > +passed on to a DMA-capable hardware unit for further processing or output. > > > > +</entry> > > > > + </row> > > > > + <row> > > > > + <entry><constant>V4L2_BUF_FLAG_NO_CACHE_CLEAN</constant></entry> > > > > + <entry>0x0800</entry> > > > > + <entry>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.</entry> > > > > + </row> > > > > </tbody> > > > > </tgroup> > > > > </table> > > > > 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.</revremark> > > > > &sub-close; > > > > &sub-ioctl; > > > > <!-- All ioctls go here. --> > > > > + &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.</revremark> > > > > &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..5f0158c > > > > --- /dev/null > > > > +++ b/Documentation/DocBook/media/v4l/vidioc-create-bufs.xml > > > > @@ -0,0 +1,152 @@ > > > > +<refentry id="vidioc-create-bufs"> > > > > + <refmeta> > > > > + <refentrytitle>ioctl VIDIOC_CREATE_BUFS</refentrytitle> > > > > + &manvol; > > > > + </refmeta> > > > > + > > > > + <refnamediv> > > > > + <refname>VIDIOC_CREATE_BUFS</refname> > > > > + <refpurpose>Create buffers for Memory Mapped or User Pointer I/O</refpurpose> > > > > + </refnamediv> > > > > + > > > > + <refsynopsisdiv> > > > > + <funcsynopsis> > > > > + <funcprototype> > > > > + <funcdef>int <function>ioctl</function></funcdef> > > > > + <paramdef>int <parameter>fd</parameter></paramdef> > > > > + <paramdef>int <parameter>request</parameter></paramdef> > > > > + <paramdef>struct v4l2_create_buffers *<parameter>argp</parameter></paramdef> > > > > + </funcprototype> > > > > + </funcsynopsis> > > > > + </refsynopsisdiv> > > > > + > > > > + <refsect1> > > > > + <title>Arguments</title> > > > > + > > > > + <variablelist> > > > > + <varlistentry> > > > > + <term><parameter>fd</parameter></term> > > > > + <listitem> > > > > + <para>&fd;</para> > > > > + </listitem> > > > > + </varlistentry> > > > > + <varlistentry> > > > > + <term><parameter>request</parameter></term> > > > > + <listitem> > > > > + <para>VIDIOC_CREATE_BUFS</para> > > > > + </listitem> > > > > + </varlistentry> > > > > + <varlistentry> > > > > + <term><parameter>argp</parameter></term> > > > > + <listitem> > > > > + <para></para> > > > > + </listitem> > > > > + </varlistentry> > > > > + </variablelist> > > > > + </refsect1> > > > > + > > > > + <refsect1> > > > > + <title>Description</title> > > > > + > > > > + <para>This ioctl is used to create buffers for <link linkend="mmap">memory > > > > +mapped</link> or <link linkend="userp">user pointer</link> > > > > +I/O. It can be used as an alternative to the <constant>VIDIOC_REQBUFS</constant> > > > > +ioctl, when a tighter control over buffers is required. This ioctl can be called > > > > +multiple times to create buffers of different sizes. I realized that it is not clear from the documentation whether it is possible to call VIDIOC_REQBUFS and make additional calls to VIDIOC_CREATE_BUFS afterwards. I can't remember whether the code allows it or not, but it should be clearly documented. > > > > + > > > > + <para>To allocate device buffers applications initialize all > > > > +fields of the <structname>v4l2_create_buffers</structname> structure. > > > > +They set the <structfield>type</structfield> field in the > > > > +<structname>v4l2_format</structname> structure, embedded in this > > > > +structure, to the respective stream or buffer type. > > > > +<structfield>count</structfield> must be set to the number of required > > > > +buffers. <structfield>memory</structfield> specifies the required I/O > > > > +method. Applications have two possibilities to specify the size of buffers > > > > +to be prepared: they can either set the <structfield>size</structfield> > > > > +field explicitly to a non-zero value, or fill in the frame format data in the > > > > +<structfield>format</structfield> field. In the latter case buffer sizes > > > > +will be calculated automatically by the driver. The > > > > +<structfield>reserved</structfield> 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 <structfield>count</structfield> and > > > > +the <structfield>index</structfield> fields respectively. > > > > +<structfield>count</structfield> can be smaller than the number requested. > > > > +-ENOMEM is returned, if the driver runs out of free memory.</para> > > > > > > No need to mention -ENOMEM here. It's already in the return values below; > > > the same goes for the EINVAL just below. > > > > Yes, incremental patches welcome. > > > > > > > > > + <para>When the I/O method is not supported the ioctl > > > > +returns an &EINVAL;.</para> > > > > + > > > > + <table pgwide="1" frame="none" id="v4l2-create-buffers"> > > > > + <title>struct <structname>v4l2_create_buffers</structname></title> > > > > + <tgroup cols="3"> > > > > + &cs-str; > > > > + <tbody valign="top"> > > > > + <row> > > > > + <entry>__u32</entry> > > > > + <entry><structfield>index</structfield></entry> > > > > + <entry>The starting buffer index, returned by the driver.</entry> > > > > + </row> > > > > + <row> > > > > + <entry>__u32</entry> > > > > + <entry><structfield>count</structfield></entry> > > > > + <entry>The number of buffers requested or granted.</entry> > > > > + </row> > > > > + <row> > > > > + <entry>&v4l2-memory;</entry> > > > > + <entry><structfield>memory</structfield></entry> > > > > + <entry>Applications set this field to > > > > +<constant>V4L2_MEMORY_MMAP</constant> or > > > > +<constant>V4L2_MEMORY_USERPTR</constant>.</entry> > > > > + </row> > > > > + <row> > > > > + <entry>__u32</entry> > > > > + <entry><structfield>size</structfield></entry> > > > > + <entry>Explicit size of buffers, being created.</entry> > > > > + </row> > > > > + <row> > > > > + <entry>&v4l2-format;</entry> > > > > + <entry><structfield>format</structfield></entry> > > > > + <entry>Application has to set the <structfield>type</structfield> > > > > +field, other fields should be used, if the application wants to allocate buffers > > > > +for a specific frame format.</entry> > > > > + </row> > > > > + <row> > > > > + <entry>__u32</entry> > > > > + <entry><structfield>reserved</structfield>[8]</entry> > > > > + <entry>A place holder for future extensions.</entry> > > > > + </row> > > > > + </tbody> > > > > + </tgroup> > > > > + </table> > > > > + </refsect1> > > > > + > > > > + <refsect1> > > > > + &return-value; > > > > + > > > > + <variablelist> > > > > + <varlistentry> > > > > + <term><errorcode>ENOMEM</errorcode></term> > > > > + <listitem> > > > > + <para>No memory to allocate buffers for <link linkend="mmap">memory > > > > +mapped</link> I/O.</para> > > > > + </listitem> > > > > + </varlistentry> > > > > + <varlistentry> > > > > + <term><errorcode>EINVAL</errorcode></term> > > > > + <listitem> > > > > + <para>The buffer type (<structfield>type</structfield> field) or the > > > > +requested I/O method (<structfield>memory</structfield>) is not > > > > +supported.</para> > > > > + </listitem> > > > > + </varlistentry> > > > > + </variablelist> > > > > + </refsect1> > > > > +</refentry> > > > > + > > > > +<!-- > > > > +Local Variables: > > > > +mode: sgml > > > > +sgml-parent-document: "v4l2.sgml" > > > > +indent-tabs-mode: nil > > > > +End: > > > > +--> > > > > 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 @@ > > > > +<refentry id="vidioc-prepare-buf"> > > > > + <refmeta> > > > > + <refentrytitle>ioctl VIDIOC_PREPARE_BUF</refentrytitle> > > > > + &manvol; > > > > + </refmeta> > > > > + > > > > + <refnamediv> > > > > + <refname>VIDIOC_PREPARE_BUF</refname> > > > > + <refpurpose>Prepare a buffer for I/O</refpurpose> > > > > + </refnamediv> > > > > + > > > > + <refsynopsisdiv> > > > > + <funcsynopsis> > > > > + <funcprototype> > > > > + <funcdef>int <function>ioctl</function></funcdef> > > > > + <paramdef>int <parameter>fd</parameter></paramdef> > > > > + <paramdef>int <parameter>request</parameter></paramdef> > > > > + <paramdef>struct v4l2_buffer *<parameter>argp</parameter></paramdef> > > > > > > I'm not sure if we want to use struct v4l2_buffer here. It's an obvious > > > choice, but there's an issue in using it. > > > > > > The struct is almost full i.e. there are very few reserved fields in it, > > > namely two, counting the input field which was unused AFAIR. There are now > > > three ioctls using it as their argument. > > > > > > Future functionality which would be nice: > > > > > > - Format counters. Every format set by S_FMT (or gotten by G_FMT) should > > > come with a counter value so that the user would know the format of > > > dequeued buffers when setting the format on-the-fly. Currently there are > > > only bytesperline and length, but the format can't be explicitly > > > determined from those. Actually, the index field will give you that information. When you create the buffers you know that range [index, index + count - 1] is associated with that specific format. > > > - Binding to generic DMA buffers. This can likely be achieved by using the m > > > union in struct v4l2_buffer. (Speaking of which: these buffers likely have > > > a cache behaviour defined for them, e.g. they might be non-cacheable. This > > > probably has no effect on the cache flags in V4L2 but it might still be > > > good to keep this in mind.) That will definitely be done through the 'm' union. > > > I don't have an obvious replacement for it either. There are two options I > > > can see: Create a new struct v4l2_ext_buffer which contains the old > > > v4l2_buffer plus a bunch of reserved fields, or use the two last fields to > > > hold a pointer to an extension struct, v4l2_buffer_ext. The new structure > > > would later be used to contain the reserved fields. > > > > > > I admit that this is not an issue right now, myt point is that I don't want > > > this to be an issue in the future either. > > > > Good, then we'll address it, when it becomes an issue. > > Defining v4l2_ext_buffer will mean there's a new ioctl. I think we should be > a little more future proof if we can. Extending the current v4l2_buffer by > another structure with a pointer from v4l2_buffer doesn't need this. > > I would like to have someone else's opinion on what to do with this. The > resolution might as well be to accept the situation and resolve it when > there's an absolute need to. Trying to define a new structure when we don't know if we need one, let alone what should be in there, doesn't seem useful to me. Especially since it would require yet another struct. I'm with Guennadi on this one. > > > > > > > > + </funcprototype> > > > > + </funcsynopsis> > > > > + </refsynopsisdiv> > > > > + > > > > + <refsect1> > > > > + <title>Arguments</title> > > > > + > > > > + <variablelist> > > > > + <varlistentry> > > > > + <term><parameter>fd</parameter></term> > > > > + <listitem> > > > > + <para>&fd;</para> > > > > + </listitem> > > > > + </varlistentry> > > > > + <varlistentry> > > > > + <term><parameter>request</parameter></term> > > > > + <listitem> > > > > + <para>VIDIOC_PREPARE_BUF</para> > > > > + </listitem> > > > > + </varlistentry> > > > > + <varlistentry> > > > > + <term><parameter>argp</parameter></term> > > > > + <listitem> > > > > + <para></para> > > > > + </listitem> > > > > + </varlistentry> > > > > + </variablelist> > > > > + </refsect1> > > > > + > > > > + <refsect1> > > > > + <title>Description</title> > > > > + > > > > + <para>Applications can optionally call the > > > > +<constant>VIDIOC_PREPARE_BUF</constant> ioctl to pass ownership of the buffer > > > > +to the driver before actually enqueuing it, using the > > > > +<constant>VIDIOC_QBUF</constant> 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 > > > > +<constant>V4L2_BUF_FLAG_NO_CACHE_INVALIDATE</constant> and > > > > +<constant>V4L2_BUF_FLAG_NO_CACHE_CLEAN</constant> flags to skip the respective > > > > +step.</para> > > > > + > > > > + <para>The <structname>v4l2_buffer</structname> structure is > > > > +specified in <xref linkend="buffer" />.</para> > > > > + </refsect1> > > > > + > > > > + <refsect1> > > > > + &return-value; > > > > + > > > > + <variablelist> > > > > + <varlistentry> > > > > + <term><errorcode>EBUSY</errorcode></term> > > > > + <listitem> > > > > + <para>File I/O is in progress.</para> > > > > + </listitem> > > > > + </varlistentry> > > > > + <varlistentry> > > > > + <term><errorcode>EINVAL</errorcode></term> > > > > + <listitem> > > > > + <para>The buffer <structfield>type</structfield> is not > > > > +supported, or the <structfield>index</structfield> is out of bounds, > > > > +or no buffers have been allocated yet, or the > > > > +<structfield>userptr</structfield> or > > > > +<structfield>length</structfield> are invalid.</para> > > > > + </listitem> > > > > + </varlistentry> > > > > + </variablelist> > > > > + </refsect1> > > > > +</refentry> > > > > + > > > > +<!-- > > > > +Local Variables: > > > > +mode: sgml > > > > +sgml-parent-document: "v4l2.sgml" > > > > +indent-tabs-mode: nil > > > > +End: > > > > +--> > > > > diff --git a/drivers/media/video/v4l2-compat-ioctl32.c b/drivers/media/video/v4l2-compat-ioctl32.c > > > > index 61979b7..4105f69 100644 > > > > --- a/drivers/media/video/v4l2-compat-ioctl32.c > > > > +++ b/drivers/media/video/v4l2-compat-ioctl32.c > > > > @@ -159,11 +159,16 @@ struct v4l2_format32 { > > > > } fmt; > > > > }; > > > > > > > > -static int get_v4l2_format32(struct v4l2_format *kp, struct v4l2_format32 __user *up) > > > > +struct v4l2_create_buffers32 { > > > > + __u32 index; /* output: buffers index...index + count - 1 have been created */ > > > > + __u32 count; > > > > + enum v4l2_memory memory; > > > > + __u32 size; /* Explicit size, e.g., for compressed streams */ > > > > + struct v4l2_format32 format; /* "type" is used always, the rest if size == 0 */ > > > > +}; > > > > + > > > > +static int __get_v4l2_format32(struct v4l2_format *kp, struct v4l2_format32 __user *up) > > > > { > > > > - if (!access_ok(VERIFY_READ, up, sizeof(struct v4l2_format32)) || > > > > - get_user(kp->type, &up->type)) > > > > - return -EFAULT; > > > > switch (kp->type) { > > > > case V4L2_BUF_TYPE_VIDEO_CAPTURE: > > > > case V4L2_BUF_TYPE_VIDEO_OUTPUT: > > > > @@ -192,11 +197,24 @@ static int get_v4l2_format32(struct v4l2_format *kp, struct v4l2_format32 __user > > > > } > > > > } > > > > > > > > -static int put_v4l2_format32(struct v4l2_format *kp, struct v4l2_format32 __user *up) > > > > +static int get_v4l2_format32(struct v4l2_format *kp, struct v4l2_format32 __user *up) > > > > +{ > > > > + if (!access_ok(VERIFY_READ, up, sizeof(struct v4l2_format32)) || > > > > + get_user(kp->type, &up->type)) > > > > + return -EFAULT; > > > > + return __get_v4l2_format32(kp, up); > > > > +} > > > > + > > > > +static int get_v4l2_create32(struct v4l2_create_buffers *kp, struct v4l2_create_buffers32 __user *up) > > > > +{ > > > > + if (!access_ok(VERIFY_READ, up, sizeof(struct v4l2_create_buffers32)) || > > > > + copy_from_user(kp, up, offsetof(struct v4l2_create_buffers32, format.fmt))) > > > > + return -EFAULT; > > > > + return __get_v4l2_format32(&kp->format, &up->format); > > > > +} > > > > + > > > > +static int __put_v4l2_format32(struct v4l2_format *kp, struct v4l2_format32 __user *up) > > > > { > > > > - if (!access_ok(VERIFY_WRITE, up, sizeof(struct v4l2_format32)) || > > > > - put_user(kp->type, &up->type)) > > > > - return -EFAULT; > > > > switch (kp->type) { > > > > case V4L2_BUF_TYPE_VIDEO_CAPTURE: > > > > case V4L2_BUF_TYPE_VIDEO_OUTPUT: > > > > @@ -225,6 +243,22 @@ static int put_v4l2_format32(struct v4l2_format *kp, struct v4l2_format32 __user > > > > } > > > > } > > > > > > > > +static int put_v4l2_format32(struct v4l2_format *kp, struct v4l2_format32 __user *up) > > > > +{ > > > > + if (!access_ok(VERIFY_WRITE, up, sizeof(struct v4l2_format32)) || > > > > + put_user(kp->type, &up->type)) > > > > + return -EFAULT; > > > > + return __put_v4l2_format32(kp, up); > > > > +} > > > > + > > > > +static int put_v4l2_create32(struct v4l2_create_buffers *kp, struct v4l2_create_buffers32 __user *up) > > > > +{ > > > > + if (!access_ok(VERIFY_WRITE, up, sizeof(struct v4l2_create_buffers32)) || > > > > + copy_to_user(up, kp, offsetof(struct v4l2_create_buffers32, format.fmt))) > > > > + return -EFAULT; > > > > + return __put_v4l2_format32(&kp->format, &up->format); > > > > +} > > > > + > > > > struct v4l2_standard32 { > > > > __u32 index; > > > > __u32 id[2]; /* __u64 would get the alignment wrong */ > > > > @@ -702,6 +736,8 @@ 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_CREATE_BUFS32 _IOWR('V', 92, struct v4l2_create_buffers32) > > > > +#define VIDIOC_PREPARE_BUF32 _IOWR('V', 93, struct v4l2_buffer32) > > > > > > > > #define VIDIOC_OVERLAY32 _IOW ('V', 14, s32) > > > > #define VIDIOC_STREAMON32 _IOW ('V', 18, s32) > > > > @@ -721,6 +757,7 @@ static long do_video_ioctl(struct file *file, unsigned int cmd, unsigned long ar > > > > struct v4l2_standard v2s; > > > > struct v4l2_ext_controls v2ecs; > > > > struct v4l2_event v2ev; > > > > + struct v4l2_create_buffers v2crt; > > > > unsigned long vx; > > > > int vi; > > > > } karg; > > > > @@ -751,6 +788,8 @@ 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_CREATE_BUFS32: cmd = VIDIOC_CREATE_BUFS; break; > > > > + case VIDIOC_PREPARE_BUF32: cmd = VIDIOC_PREPARE_BUF; break; > > > > } > > > > > > > > switch (cmd) { > > > > @@ -775,6 +814,12 @@ static long do_video_ioctl(struct file *file, unsigned int cmd, unsigned long ar > > > > compatible_arg = 0; > > > > break; > > > > > > > > + case VIDIOC_CREATE_BUFS: > > > > + err = get_v4l2_create32(&karg.v2crt, up); > > > > + compatible_arg = 0; > > > > + break; > > > > + > > > > + case VIDIOC_PREPARE_BUF: > > > > case VIDIOC_QUERYBUF: > > > > case VIDIOC_QBUF: > > > > case VIDIOC_DQBUF: > > > > @@ -860,6 +905,11 @@ static long do_video_ioctl(struct file *file, unsigned int cmd, unsigned long ar > > > > err = put_v4l2_format32(&karg.v2f, up); > > > > break; > > > > > > > > + case VIDIOC_CREATE_BUFS: > > > > + err = put_v4l2_create32(&karg.v2crt, up); > > > > + break; > > > > + > > > > + case VIDIOC_PREPARE_BUF: > > > > case VIDIOC_QUERYBUF: > > > > case VIDIOC_QBUF: > > > > case VIDIOC_DQBUF: > > > > @@ -959,6 +1009,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_BUFS32: > > > > + 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..7824152 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,36 @@ 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 = check_fmt(ops, create->format.type); > > > > + if (ret) > > > > + break; > > > > + > > > > + if (create->size) > > > > + CLEAR_AFTER_FIELD(create, count); > > > > + ret = ops->vidioc_create_bufs(file, fh, create); > > > > + > > > > + dbgarg(cmd, "count=%d\n", create->count); > > > > + 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..0b594c7 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,16 @@ 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; > > > > + enum v4l2_memory memory; > > > > > > Aren't enums something to avoid in public APIs? -> __u32? > > > > They are, but this specific enum is already used in "struct > > v4l2_requestbuffers" and "struct v4l2_buffer" so, I decided, it would be > > consistent to use it here too, even if it involves extra compat craft, and > > the embedded struct v4l2_format also includes an enum in it anyway, so, > > basically, they are all over the place. > > In my understanding that goes for all enums, not only those that would be > newly introduced to the API. E.g. struct v4l2_event_ctrl.type is actually > enum v4l2_ctrl_type rather than __u32. struct v4l2_queryctrl uses the enum, > v4l2_event_ctrl does not. True, but since v4l2_create_buffers uses struct v4l2_format already which has enums I see no advantage to using __u32 over enum. In struct v4l2_event_ctrl the use of __u32 prevented needing more complex compat code. > > > > > > > > + __u32 size; /* Explicit size, e.g., for compressed streams */ > > > > + struct v4l2_format format; /* "type" is used always, the rest if size == 0 */ > > > > + __u32 reserved[8]; > > > > +}; > > Btw. what about __attribute__ ((packed)) for the structure? Not all > interface structs use it, though. It doesn't really help you here. In some cases it prevents having to make compat32 code, but since we need it anyway... > > > > > /* > > > > * 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 +2195,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, > > > > > > Regards, > > > > So, I take it as an ack;-) > > It's not an ack yet but we're definitely close. :-) > > I am happy with the API. The only thing that's unclear to me is whether you can call CREATE_BUFS after REQBUFS. And if not, then why not? It would also be helpful to see the full patch series as the last one was from April. It is interesting to see how this will interface with vb2. Regards, Hans -- 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
Hi Hans and Guennadi, On Tue, Jul 26, 2011 at 01:05:29PM +0200, Hans Verkuil wrote: > On Wednesday, July 20, 2011 17:19:46 Sakari Ailus wrote: > > On Wed, Jul 20, 2011 at 04:47:46PM +0200, Guennadi Liakhovetski wrote: > > > On Wed, 20 Jul 2011, Sakari Ailus wrote: > > > > > > > Hi, Guennadi! > > > > > > > > Thanks for the patch! > > > > > > > > On Wed, Jul 20, 2011 at 10:43:08AM +0200, 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 <g.liakhovetski@gmx.de> > > > > > --- > > > > > > > > > > It's been almost a month since v2, the only comments were a request to > > > > > increase the reserved space in the new ioctl() and to improve > > > > > documentation. The reserved field is increased in this version, > > > > > documentation also has been improved in multiple locations. I think, > > > > > documentation can be further improved at any time, but if there are no > > > > > objections against the actual contents of this patch, maybe we can commit > > > > > this version. I still don't see v3.0;-), so, maybe we even can push it for > > > > > 3.1. A trivial comparison with v2 shows the size of the reserved field as > > > > > the only change in the API, and the compatibility fix as the only two > > > > > functional changes. > > > > > > > > > > v3: addressed multiple comments by Sakari Ailus > > > > > > > > > > 1. increased reserved field in "struct v4l2_create_buffers" to 8 32-bit > > > > > ints > > > > > 2. multiple documentation fixes and improvements > > > > > 3. fixed misplaced "case VIDIOC_PREPARE_BUF" in ioctl 32-bit compatibility > > > > > processing > > > > > > > > > > v2: > > > > > > > > > > 1. add preliminary Documentation > > > > > 2. add flag V4L2_BUFFER_FLAG_NO_CACHE_CLEAN > > > > > 3. remove VIDIOC_DESTROY_BUFS > > > > > 4. rename SUBMIT to VIDIOC_PREPARE_BUF > > > > > 5. add reserved field to struct v4l2_create_buffers > > > > > 6. cache handling flags moved to struct v4l2_buffer for processing during > > > > > VIDIOC_PREPARE_BUF > > > > > 7. VIDIOC_PREPARE_BUF now uses struct v4l2_buffer as its argument > > > > > > > > > > > > > > > Documentation/DocBook/media/v4l/io.xml | 17 +++ > > > > > Documentation/DocBook/media/v4l/v4l2.xml | 2 + > > > > > .../DocBook/media/v4l/vidioc-create-bufs.xml | 152 ++++++++++++++++++++ > > > > > .../DocBook/media/v4l/vidioc-prepare-buf.xml | 96 ++++++++++++ > > > > > drivers/media/video/v4l2-compat-ioctl32.c | 68 ++++++++- > > > > > drivers/media/video/v4l2-ioctl.c | 32 ++++ > > > > > include/linux/videodev2.h | 16 ++ > > > > > include/media/v4l2-ioctl.h | 2 + > > > > > 8 files changed, 377 insertions(+), 8 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..6249d0e 100644 > > > > > --- a/Documentation/DocBook/media/v4l/io.xml > > > > > +++ b/Documentation/DocBook/media/v4l/io.xml > > > > > @@ -927,6 +927,23 @@ ioctl is called.</entry> > > > > > Applications set or clear this flag before calling the > > > > > <constant>VIDIOC_QBUF</constant> ioctl.</entry> > > > > > </row> > > > > > + <row> > > > > > + <entry><constant>V4L2_BUF_FLAG_NO_CACHE_INVALIDATE</constant></entry> > > > > > + <entry>0x0400</entry> > > > > > + <entry>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 br touched by the CPU, instead the buffer will, probably, be > > > > > +passed on to a DMA-capable hardware unit for further processing or output. > > > > > +</entry> > > > > > + </row> > > > > > + <row> > > > > > + <entry><constant>V4L2_BUF_FLAG_NO_CACHE_CLEAN</constant></entry> > > > > > + <entry>0x0800</entry> > > > > > + <entry>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.</entry> > > > > > + </row> > > > > > </tbody> > > > > > </tgroup> > > > > > </table> > > > > > 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.</revremark> > > > > > &sub-close; > > > > > &sub-ioctl; > > > > > <!-- All ioctls go here. --> > > > > > + &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.</revremark> > > > > > &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..5f0158c > > > > > --- /dev/null > > > > > +++ b/Documentation/DocBook/media/v4l/vidioc-create-bufs.xml > > > > > @@ -0,0 +1,152 @@ > > > > > +<refentry id="vidioc-create-bufs"> > > > > > + <refmeta> > > > > > + <refentrytitle>ioctl VIDIOC_CREATE_BUFS</refentrytitle> > > > > > + &manvol; > > > > > + </refmeta> > > > > > + > > > > > + <refnamediv> > > > > > + <refname>VIDIOC_CREATE_BUFS</refname> > > > > > + <refpurpose>Create buffers for Memory Mapped or User Pointer I/O</refpurpose> > > > > > + </refnamediv> > > > > > + > > > > > + <refsynopsisdiv> > > > > > + <funcsynopsis> > > > > > + <funcprototype> > > > > > + <funcdef>int <function>ioctl</function></funcdef> > > > > > + <paramdef>int <parameter>fd</parameter></paramdef> > > > > > + <paramdef>int <parameter>request</parameter></paramdef> > > > > > + <paramdef>struct v4l2_create_buffers *<parameter>argp</parameter></paramdef> > > > > > + </funcprototype> > > > > > + </funcsynopsis> > > > > > + </refsynopsisdiv> > > > > > + > > > > > + <refsect1> > > > > > + <title>Arguments</title> > > > > > + > > > > > + <variablelist> > > > > > + <varlistentry> > > > > > + <term><parameter>fd</parameter></term> > > > > > + <listitem> > > > > > + <para>&fd;</para> > > > > > + </listitem> > > > > > + </varlistentry> > > > > > + <varlistentry> > > > > > + <term><parameter>request</parameter></term> > > > > > + <listitem> > > > > > + <para>VIDIOC_CREATE_BUFS</para> > > > > > + </listitem> > > > > > + </varlistentry> > > > > > + <varlistentry> > > > > > + <term><parameter>argp</parameter></term> > > > > > + <listitem> > > > > > + <para></para> > > > > > + </listitem> > > > > > + </varlistentry> > > > > > + </variablelist> > > > > > + </refsect1> > > > > > + > > > > > + <refsect1> > > > > > + <title>Description</title> > > > > > + > > > > > + <para>This ioctl is used to create buffers for <link linkend="mmap">memory > > > > > +mapped</link> or <link linkend="userp">user pointer</link> > > > > > +I/O. It can be used as an alternative to the <constant>VIDIOC_REQBUFS</constant> > > > > > +ioctl, when a tighter control over buffers is required. This ioctl can be called > > > > > +multiple times to create buffers of different sizes. > > I realized that it is not clear from the documentation whether it is possible to call > VIDIOC_REQBUFS and make additional calls to VIDIOC_CREATE_BUFS afterwards. That's actually a must if one wants to release buffers. Currently no other method than requesting 0 buffers using REQBUFS is provided (apart from closing the file handle). > I can't remember whether the code allows it or not, but it should be clearly documented. I would guess no user application would have to call REQBUFS with other than zero buffers when using CREATE_BUFS. This must be an exception if mixing REQBUFS and CREATE_BUFS is not allowed in general. That said, I don't see a reason to prohibit either, but perhaps Guennadi has more informed opinion on this. > > > > > + > > > > > + <para>To allocate device buffers applications initialize all > > > > > +fields of the <structname>v4l2_create_buffers</structname> structure. > > > > > +They set the <structfield>type</structfield> field in the > > > > > +<structname>v4l2_format</structname> structure, embedded in this > > > > > +structure, to the respective stream or buffer type. > > > > > +<structfield>count</structfield> must be set to the number of required > > > > > +buffers. <structfield>memory</structfield> specifies the required I/O > > > > > +method. Applications have two possibilities to specify the size of buffers > > > > > +to be prepared: they can either set the <structfield>size</structfield> > > > > > +field explicitly to a non-zero value, or fill in the frame format data in the > > > > > +<structfield>format</structfield> field. In the latter case buffer sizes > > > > > +will be calculated automatically by the driver. The > > > > > +<structfield>reserved</structfield> 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 <structfield>count</structfield> and > > > > > +the <structfield>index</structfield> fields respectively. > > > > > +<structfield>count</structfield> can be smaller than the number requested. > > > > > +-ENOMEM is returned, if the driver runs out of free memory.</para> > > > > > > > > No need to mention -ENOMEM here. It's already in the return values below; > > > > the same goes for the EINVAL just below. > > > > > > Yes, incremental patches welcome. > > > > > > > > > > > > + <para>When the I/O method is not supported the ioctl > > > > > +returns an &EINVAL;.</para> > > > > > + > > > > > + <table pgwide="1" frame="none" id="v4l2-create-buffers"> > > > > > + <title>struct <structname>v4l2_create_buffers</structname></title> > > > > > + <tgroup cols="3"> > > > > > + &cs-str; > > > > > + <tbody valign="top"> > > > > > + <row> > > > > > + <entry>__u32</entry> > > > > > + <entry><structfield>index</structfield></entry> > > > > > + <entry>The starting buffer index, returned by the driver.</entry> > > > > > + </row> > > > > > + <row> > > > > > + <entry>__u32</entry> > > > > > + <entry><structfield>count</structfield></entry> > > > > > + <entry>The number of buffers requested or granted.</entry> > > > > > + </row> > > > > > + <row> > > > > > + <entry>&v4l2-memory;</entry> > > > > > + <entry><structfield>memory</structfield></entry> > > > > > + <entry>Applications set this field to > > > > > +<constant>V4L2_MEMORY_MMAP</constant> or > > > > > +<constant>V4L2_MEMORY_USERPTR</constant>.</entry> > > > > > + </row> > > > > > + <row> > > > > > + <entry>__u32</entry> > > > > > + <entry><structfield>size</structfield></entry> > > > > > + <entry>Explicit size of buffers, being created.</entry> > > > > > + </row> > > > > > + <row> > > > > > + <entry>&v4l2-format;</entry> > > > > > + <entry><structfield>format</structfield></entry> > > > > > + <entry>Application has to set the <structfield>type</structfield> > > > > > +field, other fields should be used, if the application wants to allocate buffers > > > > > +for a specific frame format.</entry> > > > > > + </row> > > > > > + <row> > > > > > + <entry>__u32</entry> > > > > > + <entry><structfield>reserved</structfield>[8]</entry> > > > > > + <entry>A place holder for future extensions.</entry> > > > > > + </row> > > > > > + </tbody> > > > > > + </tgroup> > > > > > + </table> > > > > > + </refsect1> > > > > > + > > > > > + <refsect1> > > > > > + &return-value; > > > > > + > > > > > + <variablelist> > > > > > + <varlistentry> > > > > > + <term><errorcode>ENOMEM</errorcode></term> > > > > > + <listitem> > > > > > + <para>No memory to allocate buffers for <link linkend="mmap">memory > > > > > +mapped</link> I/O.</para> > > > > > + </listitem> > > > > > + </varlistentry> > > > > > + <varlistentry> > > > > > + <term><errorcode>EINVAL</errorcode></term> > > > > > + <listitem> > > > > > + <para>The buffer type (<structfield>type</structfield> field) or the > > > > > +requested I/O method (<structfield>memory</structfield>) is not > > > > > +supported.</para> > > > > > + </listitem> > > > > > + </varlistentry> > > > > > + </variablelist> > > > > > + </refsect1> > > > > > +</refentry> > > > > > + > > > > > +<!-- > > > > > +Local Variables: > > > > > +mode: sgml > > > > > +sgml-parent-document: "v4l2.sgml" > > > > > +indent-tabs-mode: nil > > > > > +End: > > > > > +--> > > > > > 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 @@ > > > > > +<refentry id="vidioc-prepare-buf"> > > > > > + <refmeta> > > > > > + <refentrytitle>ioctl VIDIOC_PREPARE_BUF</refentrytitle> > > > > > + &manvol; > > > > > + </refmeta> > > > > > + > > > > > + <refnamediv> > > > > > + <refname>VIDIOC_PREPARE_BUF</refname> > > > > > + <refpurpose>Prepare a buffer for I/O</refpurpose> > > > > > + </refnamediv> > > > > > + > > > > > + <refsynopsisdiv> > > > > > + <funcsynopsis> > > > > > + <funcprototype> > > > > > + <funcdef>int <function>ioctl</function></funcdef> > > > > > + <paramdef>int <parameter>fd</parameter></paramdef> > > > > > + <paramdef>int <parameter>request</parameter></paramdef> > > > > > + <paramdef>struct v4l2_buffer *<parameter>argp</parameter></paramdef> > > > > > > > > I'm not sure if we want to use struct v4l2_buffer here. It's an obvious > > > > choice, but there's an issue in using it. > > > > > > > > The struct is almost full i.e. there are very few reserved fields in it, > > > > namely two, counting the input field which was unused AFAIR. There are now > > > > three ioctls using it as their argument. > > > > > > > > Future functionality which would be nice: > > > > > > > > - Format counters. Every format set by S_FMT (or gotten by G_FMT) should > > > > come with a counter value so that the user would know the format of > > > > dequeued buffers when setting the format on-the-fly. Currently there are > > > > only bytesperline and length, but the format can't be explicitly > > > > determined from those. > > Actually, the index field will give you that information. When you create the > buffers you know that range [index, index + count - 1] is associated with that > specific format. Some hardware is able to change the format while streaming is ongoing (for example: OMAP 3). The problem is that the user should be able to know which frame has the new format. Of course one could stop streaming but this would mean lost frames. A flag has been proposed to this previously. That's one option but forces the user to keep track of the changes since only one change is allowed until it has taken effect. > > > > - Binding to generic DMA buffers. This can likely be achieved by using the m > > > > union in struct v4l2_buffer. (Speaking of which: these buffers likely have > > > > a cache behaviour defined for them, e.g. they might be non-cacheable. This > > > > probably has no effect on the cache flags in V4L2 but it might still be > > > > good to keep this in mind.) > > That will definitely be done through the 'm' union. > > > > > I don't have an obvious replacement for it either. There are two options I > > > > can see: Create a new struct v4l2_ext_buffer which contains the old > > > > v4l2_buffer plus a bunch of reserved fields, or use the two last fields to > > > > hold a pointer to an extension struct, v4l2_buffer_ext. The new structure > > > > would later be used to contain the reserved fields. > > > > > > > > I admit that this is not an issue right now, myt point is that I don't want > > > > this to be an issue in the future either. > > > > > > Good, then we'll address it, when it becomes an issue. > > > > Defining v4l2_ext_buffer will mean there's a new ioctl. I think we should be > > a little more future proof if we can. Extending the current v4l2_buffer by > > another structure with a pointer from v4l2_buffer doesn't need this. > > > > I would like to have someone else's opinion on what to do with this. The > > resolution might as well be to accept the situation and resolve it when > > there's an absolute need to. > > Trying to define a new structure when we don't know if we need one, let alone > what should be in there, doesn't seem useful to me. Especially since it would > require yet another struct. I'm with Guennadi on this one. Then this is settled on my behalf. [clip] Regards,
On Tuesday, July 26, 2011 13:44:28 Sakari Ailus wrote: > Hi Hans and Guennadi, <snip> > > I realized that it is not clear from the documentation whether it is possible to call > > VIDIOC_REQBUFS and make additional calls to VIDIOC_CREATE_BUFS afterwards. > > That's actually a must if one wants to release buffers. Currently no other > method than requesting 0 buffers using REQBUFS is provided (apart from > closing the file handle). I was referring to the non-0 use-case :-) > > I can't remember whether the code allows it or not, but it should be clearly documented. > > I would guess no user application would have to call REQBUFS with other than > zero buffers when using CREATE_BUFS. This must be an exception if mixing > REQBUFS and CREATE_BUFS is not allowed in general. That said, I don't see a > reason to prohibit either, but perhaps Guennadi has more informed opinion > on this. <snip> > > > > > Future functionality which would be nice: > > > > > > > > > > - Format counters. Every format set by S_FMT (or gotten by G_FMT) should > > > > > come with a counter value so that the user would know the format of > > > > > dequeued buffers when setting the format on-the-fly. Currently there are > > > > > only bytesperline and length, but the format can't be explicitly > > > > > determined from those. > > > > Actually, the index field will give you that information. When you create the > > buffers you know that range [index, index + count - 1] is associated with that > > specific format. > > Some hardware is able to change the format while streaming is ongoing (for > example: OMAP 3). The problem is that the user should be able to know which > frame has the new format. Ah, of course. > Of course one could stop streaming but this would mean lost frames. > > A flag has been proposed to this previously. That's one option but forces > the user to keep track of the changes since only one change is allowed until > it has taken effect. Something to discuss next week, I think. Regards, Hans -- 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
Hi Gueannadi, On 07/20/2011 10:43 AM, 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 <g.liakhovetski@gmx.de> > --- > > It's been almost a month since v2, the only comments were a request to > increase the reserved space in the new ioctl() and to improve > documentation. The reserved field is increased in this version, > documentation also has been improved in multiple locations. I think, > documentation can be further improved at any time, but if there are no > objections against the actual contents of this patch, maybe we can commit > this version. I still don't see v3.0;-), so, maybe we even can push it for > 3.1. A trivial comparison with v2 shows the size of the reserved field as > the only change in the API, and the compatibility fix as the only two > functional changes. > > v3: addressed multiple comments by Sakari Ailus > > 1. increased reserved field in "struct v4l2_create_buffers" to 8 32-bit > ints > 2. multiple documentation fixes and improvements > 3. fixed misplaced "case VIDIOC_PREPARE_BUF" in ioctl 32-bit compatibility > processing > > v2: > > 1. add preliminary Documentation > 2. add flag V4L2_BUFFER_FLAG_NO_CACHE_CLEAN > 3. remove VIDIOC_DESTROY_BUFS > 4. rename SUBMIT to VIDIOC_PREPARE_BUF > 5. add reserved field to struct v4l2_create_buffers > 6. cache handling flags moved to struct v4l2_buffer for processing during > VIDIOC_PREPARE_BUF > 7. VIDIOC_PREPARE_BUF now uses struct v4l2_buffer as its argument > > > Documentation/DocBook/media/v4l/io.xml | 17 +++ > Documentation/DocBook/media/v4l/v4l2.xml | 2 + > .../DocBook/media/v4l/vidioc-create-bufs.xml | 152 ++++++++++++++++++++ > .../DocBook/media/v4l/vidioc-prepare-buf.xml | 96 ++++++++++++ > drivers/media/video/v4l2-compat-ioctl32.c | 68 ++++++++- > drivers/media/video/v4l2-ioctl.c | 32 ++++ > include/linux/videodev2.h | 16 ++ > include/media/v4l2-ioctl.h | 2 + > 8 files changed, 377 insertions(+), 8 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..6249d0e 100644 > --- a/Documentation/DocBook/media/v4l/io.xml > +++ b/Documentation/DocBook/media/v4l/io.xml > @@ -927,6 +927,23 @@ ioctl is called.</entry> > Applications set or clear this flag before calling the > <constant>VIDIOC_QBUF</constant> ioctl.</entry> > </row> > + <row> > + <entry><constant>V4L2_BUF_FLAG_NO_CACHE_INVALIDATE</constant></entry> > + <entry>0x0400</entry> > + <entry>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 br touched by the CPU, instead the buffer will, probably, be > +passed on to a DMA-capable hardware unit for further processing or output. > +</entry> > + </row> > + <row> > + <entry><constant>V4L2_BUF_FLAG_NO_CACHE_CLEAN</constant></entry> > + <entry>0x0800</entry> > + <entry>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.</entry> > + </row> > </tbody> > </tgroup> > </table> > 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.</revremark> > &sub-close; > &sub-ioctl; > <!-- All ioctls go here. --> > + &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.</revremark> > &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..5f0158c > --- /dev/null > +++ b/Documentation/DocBook/media/v4l/vidioc-create-bufs.xml > @@ -0,0 +1,152 @@ > +<refentry id="vidioc-create-bufs"> > + <refmeta> > + <refentrytitle>ioctl VIDIOC_CREATE_BUFS</refentrytitle> > + &manvol; > + </refmeta> > + > + <refnamediv> > + <refname>VIDIOC_CREATE_BUFS</refname> > + <refpurpose>Create buffers for Memory Mapped or User Pointer I/O</refpurpose> > + </refnamediv> > + > + <refsynopsisdiv> > + <funcsynopsis> > + <funcprototype> > + <funcdef>int <function>ioctl</function></funcdef> > + <paramdef>int <parameter>fd</parameter></paramdef> > + <paramdef>int <parameter>request</parameter></paramdef> > + <paramdef>struct v4l2_create_buffers *<parameter>argp</parameter></paramdef> > + </funcprototype> > + </funcsynopsis> > + </refsynopsisdiv> > + > + <refsect1> > + <title>Arguments</title> > + > + <variablelist> > + <varlistentry> > + <term><parameter>fd</parameter></term> > + <listitem> > + <para>&fd;</para> > + </listitem> > + </varlistentry> > + <varlistentry> > + <term><parameter>request</parameter></term> > + <listitem> > + <para>VIDIOC_CREATE_BUFS</para> > + </listitem> > + </varlistentry> > + <varlistentry> > + <term><parameter>argp</parameter></term> > + <listitem> > + <para></para> > + </listitem> > + </varlistentry> > + </variablelist> > + </refsect1> > + > + <refsect1> > + <title>Description</title> > + > + <para>This ioctl is used to create buffers for <link linkend="mmap">memory > +mapped</link> or <link linkend="userp">user pointer</link> > +I/O. It can be used as an alternative to the <constant>VIDIOC_REQBUFS</constant> > +ioctl, when a tighter control over buffers is required. This ioctl can be called > +multiple times to create buffers of different sizes. It looks like there is a </para> tag missing, that line should be: +multiple times to create buffers of different sizes. </para> Otherwise the compilation fails. -- Regards, Sylwester Nawrocki
Hi Guennadi, On Wed, Jul 20, 2011 at 01:43, Guennadi Liakhovetski <g.liakhovetski@gmx.de> 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 <g.liakhovetski@gmx.de> > --- > <snip> This looks nicer, I like how we got rid of destroy and gave up on making holes, it would've given us a lot of headaches. I'm thinking about some issues though and also have some comments/questions further below. Already mentioned by others mixing of REQBUFS and CREATE_BUFS. Personally I'd like to allow mixing, including REQBUFS for non-zero, because I think it would be easy to do. I think it could work in the same way as REQBUFS for !=0 works currently (at least in vb2), if we already have some buffers allocated and they are not in use, we free them and a new set is allocated. So I guess it could just stay this way. REQBUFS(0) would of course free everything. Passing format to CREATE_BUFS will make vb2 a bit format-aware, as it would have to pass it forward to the driver somehow. The obvious way would be just vb2 calling the driver's s_fmt handler, but that won't work, as you can't pass indexes to s_fmt. So we'd have to implement a new driver callback for setting formats per index. I guess there is no way around it, unless we actually take the format struct out of CREATE_BUFS and somehow do it via S_FMT. The single-planar structure is full already though, the only way would be to use v4l2_pix_format_mplane instead with plane count = 1 (or more if needed). Another thing is the case of passing size to CREATE_BUFS. vb2, when allocating buffers, gets their sizes from the driver (via queue_setup), it never "suggest" any particular size. So that flow would have to be changed as well. I guess vb2 could pass the size to queue_setup in a similar way as it does with buffer count. This would mean though that the ioctl would fail if the driver didn't agree to the given size. Right now I don't see an option to "negotiate" the size with the driver via this option. The more I think of it though, why do we need the size argument? It's not needed in the existing flows (that use S_FMT) and, more importantly, I don't think the application can know more than the driver can, so why giving that option? The driver should know the size for a format at least as well as the application... Also, is there a real use case of providing just size, will the driver know which format to use given a size? > + <para>To allocate device buffers applications initialize all > +fields of the <structname>v4l2_create_buffers</structname> structure. > +They set the <structfield>type</structfield> field in the > +<structname>v4l2_format</structname> structure, embedded in this > +structure, to the respective stream or buffer type. > +<structfield>count</structfield> must be set to the number of required > +buffers. <structfield>memory</structfield> specifies the required I/O > +method. Applications have two possibilities to specify the size of buffers > +to be prepared: they can either set the <structfield>size</structfield> > +field explicitly to a non-zero value, or fill in the frame format data in the > +<structfield>format</structfield> field. In the latter case buffer sizes > +will be calculated automatically by the driver. The Technically, we shouldn't say "applications initialize all fields" in the first sentence if they do not have to initialize both size and format fields at the same time. > +<structfield>reserved</structfield> 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 <structfield>count</structfield> and > +the <structfield>index</structfield> fields respectively. > +<structfield>count</structfield> can be smaller than the number requested. > +-ENOMEM is returned, if the driver runs out of free memory.</para> > + <para>When the I/O method is not supported the ioctl > +returns an &EINVAL;.</para> > + > + <table pgwide="1" frame="none" id="v4l2-create-buffers"> > + <title>struct <structname>v4l2_create_buffers</structname></title> > + <tgroup cols="3"> > + &cs-str; > + <tbody valign="top"> > + <row> > + <entry>__u32</entry> > + <entry><structfield>index</structfield></entry> > + <entry>The starting buffer index, returned by the driver.</entry> > + </row> > + <row> > + <entry>__u32</entry> > + <entry><structfield>count</structfield></entry> > + <entry>The number of buffers requested or granted.</entry> > + </row> > + <row> > + <entry>&v4l2-memory;</entry> > + <entry><structfield>memory</structfield></entry> > + <entry>Applications set this field to > +<constant>V4L2_MEMORY_MMAP</constant> or > +<constant>V4L2_MEMORY_USERPTR</constant>.</entry> > + </row> > + <row> > + <entry>__u32</entry> > + <entry><structfield>size</structfield></entry> > + <entry>Explicit size of buffers, being created.</entry> > + </row> > + <row> > + <entry>&v4l2-format;</entry> > + <entry><structfield>format</structfield></entry> > + <entry>Application has to set the <structfield>type</structfield> > +field, other fields should be used, if the application wants to allocate buffers > +for a specific frame format.</entry> > + </row> > + <row> > + <entry>__u32</entry> > + <entry><structfield>reserved</structfield>[8]</entry> > + <entry>A place holder for future extensions.</entry> > + </row> > + </tbody> > + </tgroup> > + </table> > + </refsect1> > + > + <refsect1> > + &return-value; > + > + <variablelist> > + <varlistentry> > + <term><errorcode>ENOMEM</errorcode></term> > + <listitem> > + <para>No memory to allocate buffers for <link linkend="mmap">memory > +mapped</link> I/O.</para> > + </listitem> > + </varlistentry> > + <varlistentry> > + <term><errorcode>EINVAL</errorcode></term> > + <listitem> > + <para>The buffer type (<structfield>type</structfield> field) or the > +requested I/O method (<structfield>memory</structfield>) is not > +supported.</para> > + </listitem> > + </varlistentry> > + </variablelist> > + </refsect1> > +</refentry> > + What happens if the driver does not agree to the provided size? EINVAL? > +<!-- > +Local Variables: > +mode: sgml > +sgml-parent-document: "v4l2.sgml" > +indent-tabs-mode: nil > +End: > +--> > 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 @@ > +<refentry id="vidioc-prepare-buf"> > + <refmeta> > + <refentrytitle>ioctl VIDIOC_PREPARE_BUF</refentrytitle> > + &manvol; > + </refmeta> > + > + <refnamediv> > + <refname>VIDIOC_PREPARE_BUF</refname> > + <refpurpose>Prepare a buffer for I/O</refpurpose> > + </refnamediv> > + > + <refsynopsisdiv> > + <funcsynopsis> > + <funcprototype> > + <funcdef>int <function>ioctl</function></funcdef> > + <paramdef>int <parameter>fd</parameter></paramdef> > + <paramdef>int <parameter>request</parameter></paramdef> > + <paramdef>struct v4l2_buffer *<parameter>argp</parameter></paramdef> > + </funcprototype> > + </funcsynopsis> > + </refsynopsisdiv> > + > + <refsect1> > + <title>Arguments</title> > + > + <variablelist> > + <varlistentry> > + <term><parameter>fd</parameter></term> > + <listitem> > + <para>&fd;</para> > + </listitem> > + </varlistentry> > + <varlistentry> > + <term><parameter>request</parameter></term> > + <listitem> > + <para>VIDIOC_PREPARE_BUF</para> > + </listitem> > + </varlistentry> > + <varlistentry> > + <term><parameter>argp</parameter></term> > + <listitem> > + <para></para> > + </listitem> > + </varlistentry> > + </variablelist> > + </refsect1> > + > + <refsect1> > + <title>Description</title> > + > + <para>Applications can optionally call the > +<constant>VIDIOC_PREPARE_BUF</constant> ioctl to pass ownership of the buffer > +to the driver before actually enqueuing it, using the > +<constant>VIDIOC_QBUF</constant> 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 > +<constant>V4L2_BUF_FLAG_NO_CACHE_INVALIDATE</constant> and > +<constant>V4L2_BUF_FLAG_NO_CACHE_CLEAN</constant> flags to skip the respective > +step.</para> > + I'm probably forgetting something, but why would we want to do both PREPARE_BUF and QBUF? Why not queue in advance? Could you give a full sequence of ioctls how it will be used (including streamons, etc.)? I'm trying to picture how passing of both types of buffers will have to look like between vb2 and a driver. <snip> -- Best regards, Pawel Osciak -- 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
On Thursday, July 28, 2011 06:11:38 Pawel Osciak wrote: > Hi Guennadi, > > On Wed, Jul 20, 2011 at 01:43, Guennadi Liakhovetski > <g.liakhovetski@gmx.de> 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 <g.liakhovetski@gmx.de> > > --- > > > <snip> > > This looks nicer, I like how we got rid of destroy and gave up on > making holes, it would've given us a lot of headaches. I'm thinking > about some issues though and also have some comments/questions further > below. > > Already mentioned by others mixing of REQBUFS and CREATE_BUFS. > Personally I'd like to allow mixing, including REQBUFS for non-zero, > because I think it would be easy to do. I think it could work in the > same way as REQBUFS for !=0 works currently (at least in vb2), if we > already have some buffers allocated and they are not in use, we free > them and a new set is allocated. So I guess it could just stay this > way. REQBUFS(0) would of course free everything. > > Passing format to CREATE_BUFS will make vb2 a bit format-aware, as it > would have to pass it forward to the driver somehow. The obvious way > would be just vb2 calling the driver's s_fmt handler, but that won't > work, as you can't pass indexes to s_fmt. So we'd have to implement a > new driver callback for setting formats per index. I guess there is no > way around it, unless we actually take the format struct out of > CREATE_BUFS and somehow do it via S_FMT. The single-planar structure > is full already though, the only way would be to use > v4l2_pix_format_mplane instead with plane count = 1 (or more if > needed). I just got an idea for this: use TRY_FMT. That will do exactly what you want. In fact, perhaps we should remove the format struct from CREATE_BUFS and use __u32 sizes[VIDEO_MAX_PLANES] instead. Let the application call TRY_FMT and initialize the sizes array instead of putting that into vb2. We may need a num_planes field as well. If the sizes are all 0 (or num_planes is 0), then the driver can use the current format, just as it does with REQBUFS. Or am I missing something? > Another thing is the case of passing size to CREATE_BUFS. vb2, when > allocating buffers, gets their sizes from the driver (via > queue_setup), it never "suggest" any particular size. So that flow > would have to be changed as well. I guess vb2 could pass the size to > queue_setup in a similar way as it does with buffer count. This would > mean though that the ioctl would fail if the driver didn't agree to > the given size. Right now I don't see an option to "negotiate" the > size with the driver via this option. I think the driver can always increase the size if needed and return the new size. > > The more I think of it though, why do we need the size argument? It's > not needed in the existing flows (that use S_FMT) and, more > importantly, I don't think the application can know more than the > driver can, so why giving that option? The driver should know the size > for a format at least as well as the application... Also, is there a > real use case of providing just size, will the driver know which > format to use given a size? There are cases where you want a larger buffer size than the format really required. The most common case being 1920x1080 formats where you want to capture in a 1920x1088 buffer (since 1088 is a multiple of 16 and is more suitable for MPEG et al encoding). So, yes, you do want the option to set the size explicitly. > > > > + <para>To allocate device buffers applications initialize all > > +fields of the <structname>v4l2_create_buffers</structname> structure. > > +They set the <structfield>type</structfield> field in the > > +<structname>v4l2_format</structname> structure, embedded in this > > +structure, to the respective stream or buffer type. > > +<structfield>count</structfield> must be set to the number of required > > +buffers. <structfield>memory</structfield> specifies the required I/O > > +method. Applications have two possibilities to specify the size of buffers > > +to be prepared: they can either set the <structfield>size</structfield> > > +field explicitly to a non-zero value, or fill in the frame format data in the > > +<structfield>format</structfield> field. In the latter case buffer sizes > > +will be calculated automatically by the driver. The > > Technically, we shouldn't say "applications initialize all fields" in > the first sentence if they do not have to initialize both size and > format fields at the same time. > > > +<structfield>reserved</structfield> 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 <structfield>count</structfield> and > > +the <structfield>index</structfield> fields respectively. > > +<structfield>count</structfield> can be smaller than the number requested. > > +-ENOMEM is returned, if the driver runs out of free memory.</para> > > + <para>When the I/O method is not supported the ioctl > > +returns an &EINVAL;.</para> > > + > > + <table pgwide="1" frame="none" id="v4l2-create-buffers"> > > + <title>struct <structname>v4l2_create_buffers</structname></title> > > + <tgroup cols="3"> > > + &cs-str; > > + <tbody valign="top"> > > + <row> > > + <entry>__u32</entry> > > + <entry><structfield>index</structfield></entry> > > + <entry>The starting buffer index, returned by the driver.</entry> > > + </row> > > + <row> > > + <entry>__u32</entry> > > + <entry><structfield>count</structfield></entry> > > + <entry>The number of buffers requested or granted.</entry> > > + </row> > > + <row> > > + <entry>&v4l2-memory;</entry> > > + <entry><structfield>memory</structfield></entry> > > + <entry>Applications set this field to > > +<constant>V4L2_MEMORY_MMAP</constant> or > > +<constant>V4L2_MEMORY_USERPTR</constant>.</entry> > > + </row> > > + <row> > > + <entry>__u32</entry> > > + <entry><structfield>size</structfield></entry> > > + <entry>Explicit size of buffers, being created.</entry> > > + </row> > > + <row> > > + <entry>&v4l2-format;</entry> > > + <entry><structfield>format</structfield></entry> > > + <entry>Application has to set the <structfield>type</structfield> > > +field, other fields should be used, if the application wants to allocate buffers > > +for a specific frame format.</entry> > > + </row> > > + <row> > > + <entry>__u32</entry> > > + <entry><structfield>reserved</structfield>[8]</entry> > > + <entry>A place holder for future extensions.</entry> > > + </row> > > + </tbody> > > + </tgroup> > > + </table> > > + </refsect1> > > + > > + <refsect1> > > + &return-value; > > + > > + <variablelist> > > + <varlistentry> > > + <term><errorcode>ENOMEM</errorcode></term> > > + <listitem> > > + <para>No memory to allocate buffers for <link linkend="mmap">memory > > +mapped</link> I/O.</para> > > + </listitem> > > + </varlistentry> > > + <varlistentry> > > + <term><errorcode>EINVAL</errorcode></term> > > + <listitem> > > + <para>The buffer type (<structfield>type</structfield> field) or the > > +requested I/O method (<structfield>memory</structfield>) is not > > +supported.</para> > > + </listitem> > > + </varlistentry> > > + </variablelist> > > + </refsect1> > > +</refentry> > > + > > What happens if the driver does not agree to the provided size? EINVAL? > > > +<!-- > > +Local Variables: > > +mode: sgml > > +sgml-parent-document: "v4l2.sgml" > > +indent-tabs-mode: nil > > +End: > > +--> > > 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 @@ > > +<refentry id="vidioc-prepare-buf"> > > + <refmeta> > > + <refentrytitle>ioctl VIDIOC_PREPARE_BUF</refentrytitle> > > + &manvol; > > + </refmeta> > > + > > + <refnamediv> > > + <refname>VIDIOC_PREPARE_BUF</refname> > > + <refpurpose>Prepare a buffer for I/O</refpurpose> > > + </refnamediv> > > + > > + <refsynopsisdiv> > > + <funcsynopsis> > > + <funcprototype> > > + <funcdef>int <function>ioctl</function></funcdef> > > + <paramdef>int <parameter>fd</parameter></paramdef> > > + <paramdef>int <parameter>request</parameter></paramdef> > > + <paramdef>struct v4l2_buffer *<parameter>argp</parameter></paramdef> > > + </funcprototype> > > + </funcsynopsis> > > + </refsynopsisdiv> > > + > > + <refsect1> > > + <title>Arguments</title> > > + > > + <variablelist> > > + <varlistentry> > > + <term><parameter>fd</parameter></term> > > + <listitem> > > + <para>&fd;</para> > > + </listitem> > > + </varlistentry> > > + <varlistentry> > > + <term><parameter>request</parameter></term> > > + <listitem> > > + <para>VIDIOC_PREPARE_BUF</para> > > + </listitem> > > + </varlistentry> > > + <varlistentry> > > + <term><parameter>argp</parameter></term> > > + <listitem> > > + <para></para> > > + </listitem> > > + </varlistentry> > > + </variablelist> > > + </refsect1> > > + > > + <refsect1> > > + <title>Description</title> > > + > > + <para>Applications can optionally call the > > +<constant>VIDIOC_PREPARE_BUF</constant> ioctl to pass ownership of the buffer > > +to the driver before actually enqueuing it, using the > > +<constant>VIDIOC_QBUF</constant> 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 > > +<constant>V4L2_BUF_FLAG_NO_CACHE_INVALIDATE</constant> and > > +<constant>V4L2_BUF_FLAG_NO_CACHE_CLEAN</constant> flags to skip the respective > > +step.</para> > > + > > I'm probably forgetting something, but why would we want to do both > PREPARE_BUF and QBUF? Why not queue in advance? QBUF both prepares the buffer and makes it available for use with DMA. You don't want that. You just want to prepare it (that's the part that is expensive) and postpone the actual queuing until the buffer is needed to e.g. take a snapshot. Regards, Hans > Could you give a full sequence of ioctls how it will be used > (including streamons, etc.)? I'm trying to picture how passing of both > types of buffers will have to look like between vb2 and a driver. > > <snip> > > -- > Best regards, > Pawel Osciak > > -- 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
On Tue, 26 Jul 2011, Hans Verkuil wrote: [snip] > I am happy with the API. The only thing that's unclear to me is whether you can call > CREATE_BUFS after REQBUFS. And if not, then why not? It would also be helpful to see > the full patch series as the last one was from April. It is interesting to see how > this will interface with vb2. I think it would be logical to allow both, since we anyway need REQBUFS(count=0) for freeing of buffers. This would confirm, that buffers vreated with either of these methods are indistinguishable. Then we shall define: CREATE_BUFS after REQBUFS or after earlier CREATE_BUFS adds more buffers REQBUFS(count=0) frees all buffers REQBUFS(count > 0) frees all buffers and allocates new ones Yes, I'll extend the documentation with this. Thanks Guennadi --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/ -- 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
Hi Sylwester On Wed, 27 Jul 2011, Sylwester Nawrocki wrote: > Hi Gueannadi, > > On 07/20/2011 10:43 AM, 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 <g.liakhovetski@gmx.de> > > --- > > > > It's been almost a month since v2, the only comments were a request to > > increase the reserved space in the new ioctl() and to improve > > documentation. The reserved field is increased in this version, > > documentation also has been improved in multiple locations. I think, > > documentation can be further improved at any time, but if there are no > > objections against the actual contents of this patch, maybe we can commit > > this version. I still don't see v3.0;-), so, maybe we even can push it for > > 3.1. A trivial comparison with v2 shows the size of the reserved field as > > the only change in the API, and the compatibility fix as the only two > > functional changes. > > > > v3: addressed multiple comments by Sakari Ailus > > > > 1. increased reserved field in "struct v4l2_create_buffers" to 8 32-bit > > ints > > 2. multiple documentation fixes and improvements > > 3. fixed misplaced "case VIDIOC_PREPARE_BUF" in ioctl 32-bit compatibility > > processing > > > > v2: > > > > 1. add preliminary Documentation > > 2. add flag V4L2_BUFFER_FLAG_NO_CACHE_CLEAN > > 3. remove VIDIOC_DESTROY_BUFS > > 4. rename SUBMIT to VIDIOC_PREPARE_BUF > > 5. add reserved field to struct v4l2_create_buffers > > 6. cache handling flags moved to struct v4l2_buffer for processing during > > VIDIOC_PREPARE_BUF > > 7. VIDIOC_PREPARE_BUF now uses struct v4l2_buffer as its argument > > > > > > Documentation/DocBook/media/v4l/io.xml | 17 +++ > > Documentation/DocBook/media/v4l/v4l2.xml | 2 + > > .../DocBook/media/v4l/vidioc-create-bufs.xml | 152 ++++++++++++++++++++ > > .../DocBook/media/v4l/vidioc-prepare-buf.xml | 96 ++++++++++++ > > drivers/media/video/v4l2-compat-ioctl32.c | 68 ++++++++- > > drivers/media/video/v4l2-ioctl.c | 32 ++++ > > include/linux/videodev2.h | 16 ++ > > include/media/v4l2-ioctl.h | 2 + > > 8 files changed, 377 insertions(+), 8 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..6249d0e 100644 > > --- a/Documentation/DocBook/media/v4l/io.xml > > +++ b/Documentation/DocBook/media/v4l/io.xml > > @@ -927,6 +927,23 @@ ioctl is called.</entry> > > Applications set or clear this flag before calling the > > <constant>VIDIOC_QBUF</constant> ioctl.</entry> > > </row> > > + <row> > > + <entry><constant>V4L2_BUF_FLAG_NO_CACHE_INVALIDATE</constant></entry> > > + <entry>0x0400</entry> > > + <entry>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 br touched by the CPU, instead the buffer will, probably, be > > +passed on to a DMA-capable hardware unit for further processing or output. > > +</entry> > > + </row> > > + <row> > > + <entry><constant>V4L2_BUF_FLAG_NO_CACHE_CLEAN</constant></entry> > > + <entry>0x0800</entry> > > + <entry>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.</entry> > > + </row> > > </tbody> > > </tgroup> > > </table> > > 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.</revremark> > > &sub-close; > > &sub-ioctl; > > <!-- All ioctls go here. --> > > + &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.</revremark> > > &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..5f0158c > > --- /dev/null > > +++ b/Documentation/DocBook/media/v4l/vidioc-create-bufs.xml > > @@ -0,0 +1,152 @@ > > +<refentry id="vidioc-create-bufs"> > > + <refmeta> > > + <refentrytitle>ioctl VIDIOC_CREATE_BUFS</refentrytitle> > > + &manvol; > > + </refmeta> > > + > > + <refnamediv> > > + <refname>VIDIOC_CREATE_BUFS</refname> > > + <refpurpose>Create buffers for Memory Mapped or User Pointer I/O</refpurpose> > > + </refnamediv> > > + > > + <refsynopsisdiv> > > + <funcsynopsis> > > + <funcprototype> > > + <funcdef>int <function>ioctl</function></funcdef> > > + <paramdef>int <parameter>fd</parameter></paramdef> > > + <paramdef>int <parameter>request</parameter></paramdef> > > + <paramdef>struct v4l2_create_buffers *<parameter>argp</parameter></paramdef> > > + </funcprototype> > > + </funcsynopsis> > > + </refsynopsisdiv> > > + > > + <refsect1> > > + <title>Arguments</title> > > + > > + <variablelist> > > + <varlistentry> > > + <term><parameter>fd</parameter></term> > > + <listitem> > > + <para>&fd;</para> > > + </listitem> > > + </varlistentry> > > + <varlistentry> > > + <term><parameter>request</parameter></term> > > + <listitem> > > + <para>VIDIOC_CREATE_BUFS</para> > > + </listitem> > > + </varlistentry> > > + <varlistentry> > > + <term><parameter>argp</parameter></term> > > + <listitem> > > + <para></para> > > + </listitem> > > + </varlistentry> > > + </variablelist> > > + </refsect1> > > + > > + <refsect1> > > + <title>Description</title> > > + > > + <para>This ioctl is used to create buffers for <link linkend="mmap">memory > > +mapped</link> or <link linkend="userp">user pointer</link> > > +I/O. It can be used as an alternative to the <constant>VIDIOC_REQBUFS</constant> > > +ioctl, when a tighter control over buffers is required. This ioctl can be called > > +multiple times to create buffers of different sizes. > > It looks like there is a </para> tag missing, that line should be: > > +multiple times to create buffers of different sizes. </para> > > Otherwise the compilation fails. Right, will be fixed. Thanks Guennadi --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/ -- 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
On Thu, 28 Jul 2011, Hans Verkuil wrote: > On Thursday, July 28, 2011 06:11:38 Pawel Osciak wrote: > > Hi Guennadi, > > > > On Wed, Jul 20, 2011 at 01:43, Guennadi Liakhovetski > > <g.liakhovetski@gmx.de> 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 <g.liakhovetski@gmx.de> > > > --- > > > > > <snip> > > > > This looks nicer, I like how we got rid of destroy and gave up on > > making holes, it would've given us a lot of headaches. I'm thinking > > about some issues though and also have some comments/questions further > > below. > > > > Already mentioned by others mixing of REQBUFS and CREATE_BUFS. > > Personally I'd like to allow mixing, including REQBUFS for non-zero, > > because I think it would be easy to do. I think it could work in the > > same way as REQBUFS for !=0 works currently (at least in vb2), if we > > already have some buffers allocated and they are not in use, we free > > them and a new set is allocated. So I guess it could just stay this > > way. REQBUFS(0) would of course free everything. > > > > Passing format to CREATE_BUFS will make vb2 a bit format-aware, as it > > would have to pass it forward to the driver somehow. The obvious way > > would be just vb2 calling the driver's s_fmt handler, but that won't > > work, as you can't pass indexes to s_fmt. So we'd have to implement a > > new driver callback for setting formats per index. I guess there is no > > way around it, unless we actually take the format struct out of > > CREATE_BUFS and somehow do it via S_FMT. The single-planar structure > > is full already though, the only way would be to use > > v4l2_pix_format_mplane instead with plane count = 1 (or more if > > needed). > > I just got an idea for this: use TRY_FMT. That will do exactly what > you want. In fact, perhaps we should remove the format struct from > CREATE_BUFS and use __u32 sizes[VIDEO_MAX_PLANES] instead. Let the > application call TRY_FMT and initialize the sizes array instead of > putting that into vb2. We may need a num_planes field as well. If the > sizes are all 0 (or num_planes is 0), then the driver can use the current > format, just as it does with REQBUFS. Hm, I think, I like this idea. It gives applications more flexibility and removes the size == 0 vs. size != 0 dilemma. So, we get /* VIDIOC_CREATE_BUFS */ struct v4l2_create_buffers { __u32 index; /* output: buffers index...index + count - 1 have been created */ __u32 count; __u32 num_planes; __u32 sizes[VIDEO_MAX_PLANES]; enum v4l2_memory memory; enum v4l2_buf_type type; __u32 reserved[8]; }; ? Thanks Guennadi --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/ -- 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
On Thursday, July 28, 2011 14:29:38 Guennadi Liakhovetski wrote: > On Thu, 28 Jul 2011, Hans Verkuil wrote: > > > On Thursday, July 28, 2011 06:11:38 Pawel Osciak wrote: > > > Hi Guennadi, > > > > > > On Wed, Jul 20, 2011 at 01:43, Guennadi Liakhovetski > > > <g.liakhovetski@gmx.de> 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 <g.liakhovetski@gmx.de> > > > > --- > > > > > > > <snip> > > > > > > This looks nicer, I like how we got rid of destroy and gave up on > > > making holes, it would've given us a lot of headaches. I'm thinking > > > about some issues though and also have some comments/questions further > > > below. > > > > > > Already mentioned by others mixing of REQBUFS and CREATE_BUFS. > > > Personally I'd like to allow mixing, including REQBUFS for non-zero, > > > because I think it would be easy to do. I think it could work in the > > > same way as REQBUFS for !=0 works currently (at least in vb2), if we > > > already have some buffers allocated and they are not in use, we free > > > them and a new set is allocated. So I guess it could just stay this > > > way. REQBUFS(0) would of course free everything. > > > > > > Passing format to CREATE_BUFS will make vb2 a bit format-aware, as it > > > would have to pass it forward to the driver somehow. The obvious way > > > would be just vb2 calling the driver's s_fmt handler, but that won't > > > work, as you can't pass indexes to s_fmt. So we'd have to implement a > > > new driver callback for setting formats per index. I guess there is no > > > way around it, unless we actually take the format struct out of > > > CREATE_BUFS and somehow do it via S_FMT. The single-planar structure > > > is full already though, the only way would be to use > > > v4l2_pix_format_mplane instead with plane count = 1 (or more if > > > needed). > > > > I just got an idea for this: use TRY_FMT. That will do exactly what > > you want. In fact, perhaps we should remove the format struct from > > CREATE_BUFS and use __u32 sizes[VIDEO_MAX_PLANES] instead. Let the > > application call TRY_FMT and initialize the sizes array instead of > > putting that into vb2. We may need a num_planes field as well. If the > > sizes are all 0 (or num_planes is 0), then the driver can use the current > > format, just as it does with REQBUFS. > > Hm, I think, I like this idea. It gives applications more flexibility and > removes the size == 0 vs. size != 0 dilemma. So, we get > > /* VIDIOC_CREATE_BUFS */ > struct v4l2_create_buffers { > __u32 index; /* output: buffers index...index + count - 1 have been created */ > __u32 count; > __u32 num_planes; > __u32 sizes[VIDEO_MAX_PLANES]; > enum v4l2_memory memory; > enum v4l2_buf_type type; > __u32 reserved[8]; > }; > > ? Yes. I'd probably rearrange the fields a bit, though: /* 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 num_planes; __u32 sizes[VIDEO_MAX_PLANES]; __u32 reserved[8]; }; The order of the count, type and memory fields is now identical to that of v4l2_requestbuffers. I also changed the enums to u32 since without the v4l2_format struct we shouldn't use enums. As an additional bonus this also simplifies the compat32 code. Regards, Hans -- 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
On Thu, Jul 28, 2011 at 02:42:52PM +0200, Hans Verkuil wrote: > On Thursday, July 28, 2011 14:29:38 Guennadi Liakhovetski wrote: > > On Thu, 28 Jul 2011, Hans Verkuil wrote: > > > > > On Thursday, July 28, 2011 06:11:38 Pawel Osciak wrote: > > > > Hi Guennadi, > > > > > > > > On Wed, Jul 20, 2011 at 01:43, Guennadi Liakhovetski > > > > <g.liakhovetski@gmx.de> 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 <g.liakhovetski@gmx.de> > > > > > --- > > > > > > > > > <snip> > > > > > > > > This looks nicer, I like how we got rid of destroy and gave up on > > > > making holes, it would've given us a lot of headaches. I'm thinking > > > > about some issues though and also have some comments/questions further > > > > below. > > > > > > > > Already mentioned by others mixing of REQBUFS and CREATE_BUFS. > > > > Personally I'd like to allow mixing, including REQBUFS for non-zero, > > > > because I think it would be easy to do. I think it could work in the > > > > same way as REQBUFS for !=0 works currently (at least in vb2), if we > > > > already have some buffers allocated and they are not in use, we free > > > > them and a new set is allocated. So I guess it could just stay this > > > > way. REQBUFS(0) would of course free everything. > > > > > > > > Passing format to CREATE_BUFS will make vb2 a bit format-aware, as it > > > > would have to pass it forward to the driver somehow. The obvious way > > > > would be just vb2 calling the driver's s_fmt handler, but that won't > > > > work, as you can't pass indexes to s_fmt. So we'd have to implement a > > > > new driver callback for setting formats per index. I guess there is no > > > > way around it, unless we actually take the format struct out of > > > > CREATE_BUFS and somehow do it via S_FMT. The single-planar structure > > > > is full already though, the only way would be to use > > > > v4l2_pix_format_mplane instead with plane count = 1 (or more if > > > > needed). > > > > > > I just got an idea for this: use TRY_FMT. That will do exactly what > > > you want. In fact, perhaps we should remove the format struct from > > > CREATE_BUFS and use __u32 sizes[VIDEO_MAX_PLANES] instead. Let the > > > application call TRY_FMT and initialize the sizes array instead of > > > putting that into vb2. We may need a num_planes field as well. If the > > > sizes are all 0 (or num_planes is 0), then the driver can use the current > > > format, just as it does with REQBUFS. > > > > Hm, I think, I like this idea. It gives applications more flexibility and > > removes the size == 0 vs. size != 0 dilemma. So, we get > > > > /* VIDIOC_CREATE_BUFS */ > > struct v4l2_create_buffers { > > __u32 index; /* output: buffers index...index + count - 1 have been created */ > > __u32 count; > > __u32 num_planes; > > __u32 sizes[VIDEO_MAX_PLANES]; > > enum v4l2_memory memory; > > enum v4l2_buf_type type; > > __u32 reserved[8]; > > }; > > > > ? > > Yes. I'd probably rearrange the fields a bit, though: > > /* 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 num_planes; > __u32 sizes[VIDEO_MAX_PLANES]; > __u32 reserved[8]; > }; > > The order of the count, type and memory fields is now identical to that of > v4l2_requestbuffers. > > I also changed the enums to u32 since without the v4l2_format struct we shouldn't > use enums. As an additional bonus this also simplifies the compat32 code. I have to say I like what I see above. :-) I have one comment at this point, which is what I always have: reserved fields. If we want to later add per-plane fields, 8 u32s isn't much for that. CREATE_BUFS isn't time critical either at all, so I'd go with 19 (to align the buffer size to a power of 2) or even more.
Hi Hans, On Wed, Jul 27, 2011 at 23:56, Hans Verkuil <hverkuil@xs4all.nl> wrote: > On Thursday, July 28, 2011 06:11:38 Pawel Osciak wrote: >> Hi Guennadi, >> >> On Wed, Jul 20, 2011 at 01:43, Guennadi Liakhovetski >> <g.liakhovetski@gmx.de> 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 <g.liakhovetski@gmx.de> >> > --- >> > >> <snip> >> >> This looks nicer, I like how we got rid of destroy and gave up on >> making holes, it would've given us a lot of headaches. I'm thinking >> about some issues though and also have some comments/questions further >> below. >> >> Already mentioned by others mixing of REQBUFS and CREATE_BUFS. >> Personally I'd like to allow mixing, including REQBUFS for non-zero, >> because I think it would be easy to do. I think it could work in the >> same way as REQBUFS for !=0 works currently (at least in vb2), if we >> already have some buffers allocated and they are not in use, we free >> them and a new set is allocated. So I guess it could just stay this >> way. REQBUFS(0) would of course free everything. >> >> Passing format to CREATE_BUFS will make vb2 a bit format-aware, as it >> would have to pass it forward to the driver somehow. The obvious way >> would be just vb2 calling the driver's s_fmt handler, but that won't >> work, as you can't pass indexes to s_fmt. So we'd have to implement a >> new driver callback for setting formats per index. I guess there is no >> way around it, unless we actually take the format struct out of >> CREATE_BUFS and somehow do it via S_FMT. The single-planar structure >> is full already though, the only way would be to use >> v4l2_pix_format_mplane instead with plane count = 1 (or more if >> needed). > > I just got an idea for this: use TRY_FMT. That will do exactly what > you want. In fact, perhaps we should remove the format struct from > CREATE_BUFS and use __u32 sizes[VIDEO_MAX_PLANES] instead. Let the > application call TRY_FMT and initialize the sizes array instead of > putting that into vb2. We may need a num_planes field as well. If the > sizes are all 0 (or num_planes is 0), then the driver can use the current > format, just as it does with REQBUFS. > > Or am I missing something? > I'm not sure I'm following. Could you give a usage example? Do you mean we want the application to: - SET_FMT and REQBUFS for normal buffers - TRY_FMT and CREATE_BUFS for additional buffers? I'm still not clear how would driver know for which set of buffers the *_FMT is being called... I'm pretty sure I'm misunderstanding your idea though... >> Another thing is the case of passing size to CREATE_BUFS. vb2, when >> allocating buffers, gets their sizes from the driver (via >> queue_setup), it never "suggest" any particular size. So that flow >> would have to be changed as well. I guess vb2 could pass the size to >> queue_setup in a similar way as it does with buffer count. This would >> mean though that the ioctl would fail if the driver didn't agree to >> the given size. Right now I don't see an option to "negotiate" the >> size with the driver via this option. > > I think the driver can always increase the size if needed and return the > new size. > Right, this can easily be done in vb2. But if we do that, it will introduce asymmetry with REQBUFS. For SET_FMT/TRY_FMT+REQBUFS the driver would only be able to adjust buffer sizes on *_FMT, while with *_FMT+CREATE_BUFS it could do that for both calls. Why do we need an additional chance to adjust? Or am I misunderstanding your suggestion? >> >> The more I think of it though, why do we need the size argument? It's >> not needed in the existing flows (that use S_FMT) and, more >> importantly, I don't think the application can know more than the >> driver can, so why giving that option? The driver should know the size >> for a format at least as well as the application... Also, is there a >> real use case of providing just size, will the driver know which >> format to use given a size? > > There are cases where you want a larger buffer size than the format > really required. The most common case being 1920x1080 formats where you > want to capture in a 1920x1088 buffer (since 1088 is a multiple of 16 > and is more suitable for MPEG et al encoding). > > So, yes, you do want the option to set the size explicitly. Yes, that's true, but my point is in the current API it works too, because the driver can adjust the size on TRY/S_FMT. Even though the application knows that 1088 is needed instead of 1080, shouldn't the driver know it as well (or actually, know even better than the app) and be able to adjust? So when we CREATE_BUFS, the driver can adjust the format struct (or if we somehow use *_FMT for CREATE_BUFS buffers, it could adjust the size there). Sorry, I still can't see why we need an explicit size argument... <snip> >> >> I'm probably forgetting something, but why would we want to do both >> PREPARE_BUF and QBUF? Why not queue in advance? > > QBUF both prepares the buffer and makes it available for use with DMA. You don't want > that. You just want to prepare it (that's the part that is expensive) and postpone > the actual queuing until the buffer is needed to e.g. take a snapshot. Just to confirm I understood correctly: we don't want to QBUF, because then it'd would immediately be used and we want to time when it is to be used? So when the driver sees a larger buffer being queued, it is to change the current streaming format for the duration of filling that buffer and switch back afterwards?
On Saturday, July 30, 2011 06:21:37 Pawel Osciak wrote: > Hi Hans, > > On Wed, Jul 27, 2011 at 23:56, Hans Verkuil <hverkuil@xs4all.nl> wrote: > > On Thursday, July 28, 2011 06:11:38 Pawel Osciak wrote: > >> Hi Guennadi, > >> > >> On Wed, Jul 20, 2011 at 01:43, Guennadi Liakhovetski > >> <g.liakhovetski@gmx.de> 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 <g.liakhovetski@gmx.de> > >> > --- > >> > > >> <snip> > >> > >> This looks nicer, I like how we got rid of destroy and gave up on > >> making holes, it would've given us a lot of headaches. I'm thinking > >> about some issues though and also have some comments/questions further > >> below. > >> > >> Already mentioned by others mixing of REQBUFS and CREATE_BUFS. > >> Personally I'd like to allow mixing, including REQBUFS for non-zero, > >> because I think it would be easy to do. I think it could work in the > >> same way as REQBUFS for !=0 works currently (at least in vb2), if we > >> already have some buffers allocated and they are not in use, we free > >> them and a new set is allocated. So I guess it could just stay this > >> way. REQBUFS(0) would of course free everything. > >> > >> Passing format to CREATE_BUFS will make vb2 a bit format-aware, as it > >> would have to pass it forward to the driver somehow. The obvious way > >> would be just vb2 calling the driver's s_fmt handler, but that won't > >> work, as you can't pass indexes to s_fmt. So we'd have to implement a > >> new driver callback for setting formats per index. I guess there is no > >> way around it, unless we actually take the format struct out of > >> CREATE_BUFS and somehow do it via S_FMT. The single-planar structure > >> is full already though, the only way would be to use > >> v4l2_pix_format_mplane instead with plane count = 1 (or more if > >> needed). > > > > I just got an idea for this: use TRY_FMT. That will do exactly what > > you want. In fact, perhaps we should remove the format struct from > > CREATE_BUFS and use __u32 sizes[VIDEO_MAX_PLANES] instead. Let the > > application call TRY_FMT and initialize the sizes array instead of > > putting that into vb2. We may need a num_planes field as well. If the > > sizes are all 0 (or num_planes is 0), then the driver can use the current > > format, just as it does with REQBUFS. > > > > Or am I missing something? > > > > I'm not sure I'm following. Could you give a usage example? Do you > mean we want the application to: > - SET_FMT and REQBUFS for normal buffers > - TRY_FMT and CREATE_BUFS for additional buffers? Right. > > I'm still not clear how would driver know for which set of buffers the > *_FMT is being called... I'm pretty sure I'm misunderstanding your > idea though... S_FMT programs the pipeline. It is up to the application to ensure that the buffers queued to the DMA engine are big enough to handle it (and up to the driver to reject buffers if they are too small). REQBUFS just calculates the size of the mmap buffers based on the current format. This is very limiting in certain situations. With CREATE_BUFS I would prefer being more flexible and let the application determine the buffer sizes, possibly based on what TRY_FMT returns. > >> Another thing is the case of passing size to CREATE_BUFS. vb2, when > >> allocating buffers, gets their sizes from the driver (via > >> queue_setup), it never "suggest" any particular size. So that flow > >> would have to be changed as well. I guess vb2 could pass the size to > >> queue_setup in a similar way as it does with buffer count. This would > >> mean though that the ioctl would fail if the driver didn't agree to > >> the given size. Right now I don't see an option to "negotiate" the > >> size with the driver via this option. > > > > I think the driver can always increase the size if needed and return the > > new size. > > > > Right, this can easily be done in vb2. But if we do that, it will > introduce asymmetry with REQBUFS. For SET_FMT/TRY_FMT+REQBUFS the > driver would only be able to adjust buffer sizes on *_FMT, while with > *_FMT+CREATE_BUFS it could do that for both calls. Why do we need an > additional chance to adjust? Or am I misunderstanding your suggestion? I don't really understand your question. With REQBUFS the size used to allocate the mmap buffers is derived from the current format by the driver. You have no chance to change that for the V4L2_MEMORY_MMAP buffers. It would be much more flexible if you let the application set up the buffer size(s). The application can always call TRY_FMT (or G_FMT) to learn the buffer size and fill it in for CREATE_BUFS. > >> > >> The more I think of it though, why do we need the size argument? It's > >> not needed in the existing flows (that use S_FMT) and, more > >> importantly, I don't think the application can know more than the > >> driver can, so why giving that option? The driver should know the size > >> for a format at least as well as the application... Also, is there a > >> real use case of providing just size, will the driver know which > >> format to use given a size? > > > > There are cases where you want a larger buffer size than the format > > really required. The most common case being 1920x1080 formats where you > > want to capture in a 1920x1088 buffer (since 1088 is a multiple of 16 > > and is more suitable for MPEG et al encoding). > > > > So, yes, you do want the option to set the size explicitly. > > Yes, that's true, but my point is in the current API it works too, > because the driver can adjust the size on TRY/S_FMT. Even though the > application knows that 1088 is needed instead of 1080, shouldn't the > driver know it as well (or actually, know even better than the app) > and be able to adjust? The driver will of course adjust it if hardware limitations require it. But it shouldn't make assumptions on how the buffer is going to be used. To take this example: an application that will just capture 1920x1080 and output it on another HDMI output without any encoding doesn't need 1088. But if it is fed to an encoder, then selecting 1088 as size is very useful. Perhaps the application would also like to record meta data after the picture, thus requiring an even larger size. > So when we CREATE_BUFS, the driver can adjust > the format struct (or if we somehow use *_FMT for CREATE_BUFS buffers, > it could adjust the size there). Sorry, I still can't see why we need > an explicit size argument... The size is what vb2 needs. It shouldn't care about the format. The format is only used one time to calculate the sizes. By moving that to the application you allow the application to modify sizes as needed. Suppose we pass in the format to CREATE_BUFS as originally proposed. What would happen is that vb2 would probably call TRY_FMT and use the returned imagesize(s) to fill in the vb2 internals. And the application would have no way of changing those. > > <snip> > > >> > >> I'm probably forgetting something, but why would we want to do both > >> PREPARE_BUF and QBUF? Why not queue in advance? > > > > QBUF both prepares the buffer and makes it available for use with DMA. You don't want > > that. You just want to prepare it (that's the part that is expensive) and postpone > > the actual queuing until the buffer is needed to e.g. take a snapshot. > > Just to confirm I understood correctly: we don't want to QBUF, because > then it'd would immediately be used and we want to time when it is to > be used? Right. > So when the driver sees a larger buffer being queued, it is > to change the current streaming format for the duration of filling > that buffer and switch back afterwards? No. We do not have a mechanism (yet) to tie a pipeline configuration to a queued buffer (to be discussed in next week in Cambourne). It is my understanding that you change the current streaming format, and then queue the large (prepared) buffer and switch back afterwards. Somebody please correct me if I am wrong. Regards, Hans -- 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
On Sat, 30 Jul 2011, Hans Verkuil wrote: > On Saturday, July 30, 2011 06:21:37 Pawel Osciak wrote: [snip] > > So when the driver sees a larger buffer being queued, it is > > to change the current streaming format for the duration of filling > > that buffer and switch back afterwards? > > No. We do not have a mechanism (yet) to tie a pipeline configuration to > a queued buffer (to be discussed in next week in Cambourne). > > It is my understanding that you change the current streaming format, and > then queue the large (prepared) buffer and switch back afterwards. Yes, this is also how I see it - the driver just has a chance to verify, whether the queued buffer is not large enough for the current format and fail QBUF, although this behaviour is currently not documented in the QBUF specification... Thanks Guennadi --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/ -- 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
On Tue, 26 Jul 2011, Hans Verkuil wrote: > On Tuesday, July 26, 2011 13:44:28 Sakari Ailus wrote: > > Hi Hans and Guennadi, > > <snip> > > > > I realized that it is not clear from the documentation whether it is possible to call > > > VIDIOC_REQBUFS and make additional calls to VIDIOC_CREATE_BUFS afterwards. > > > > That's actually a must if one wants to release buffers. Currently no other > > method than requesting 0 buffers using REQBUFS is provided (apart from > > closing the file handle). > > I was referring to the non-0 use-case :-) > > > > I can't remember whether the code allows it or not, but it should be clearly documented. > > > > I would guess no user application would have to call REQBUFS with other than > > zero buffers when using CREATE_BUFS. This must be an exception if mixing > > REQBUFS and CREATE_BUFS is not allowed in general. That said, I don't see a > > reason to prohibit either, but perhaps Guennadi has more informed opinion > > on this. > > <snip> > > > > > > > Future functionality which would be nice: > > > > > > > > > > > > - Format counters. Every format set by S_FMT (or gotten by G_FMT) should > > > > > > come with a counter value so that the user would know the format of > > > > > > dequeued buffers when setting the format on-the-fly. Currently there are > > > > > > only bytesperline and length, but the format can't be explicitly > > > > > > determined from those. > > > > > > Actually, the index field will give you that information. When you create the > > > buffers you know that range [index, index + count - 1] is associated with that > > > specific format. > > > > Some hardware is able to change the format while streaming is ongoing (for > > example: OMAP 3). The problem is that the user should be able to know which > > frame has the new format. How exactly does this work or should it work? You mean, you just configure your hardware with new frame size parameters without stopping the current streaming, and the ISP will change frame sizes, beginning with some future frame? How does the driver then get to know, which frame already has the new sizes? You actually want to know this in advance to already queue a suitably sized buffer to the hardware? Thanks Guennadi > Ah, of course. > > > Of course one could stop streaming but this would mean lost frames. > > > > A flag has been proposed to this previously. That's one option but forces > > the user to keep track of the changes since only one change is allowed until > > it has taken effect. > > Something to discuss next week, I think. > > Regards, > > Hans > --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/ -- 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
On Sat, 30 Jul 2011, Hans Verkuil wrote: [snip] > No. We do not have a mechanism (yet) to tie a pipeline configuration to > a queued buffer (to be discussed in next week in Cambourne). > > It is my understanding that you change the current streaming format, and > then queue the large (prepared) buffer and switch back afterwards. As far as I understand, currently we have 2 unclarified points with this patch: 1. size of the reserved field. Sakari proposed 19 32-bit words for possible per-plane extensions with a max of 8 planes. That would make up to 2 32-bit words per plane plus 3 words for the common part. So, although reserving 76 bytes "just in case" seems like a huge overkill to me, I kinda can see the reasoning... We could also do with 10 reserved words - 1 word per plane + 2 common. What do others think? 2. as in the above quote - we do not know yet, how to tell the user, when the format changes. AFAIU, the problem is the following: (a) we prepare buffers of two sizes, say - small for preview and large for snapshot (b) S_FMT(preview) (c) QBUF(preview)... (d) STREAMON (e) capture for some time... (f) S_FMT(snapshot) (g) now, we do not yet necessarily want to queue our big buffers, because the hardware will not switch immediately (h) when the hardware has switch we QBUF(snapshor) ... Currently, there's no way to find out when the hardware switched to the new frame format. Maybe an event is needed? Besides, the TRY_FMT idea is nice, but it doesn't give us an ultimate solution either. What if a TRY_FMT now returns different plane sizes, than when we actually perform S_FMT? E.g., if the user also issues an S_CROP between those and that also changes the output format because of driver limitations? So, my question is: shall I prepare a new version of this RFC now, also providing examples for vb2 and a hardware driver, or we're waiting for the brainstorming and following ML discussion results on the above? Thanks Guennadi --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/ -- 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
Guennadi Liakhovetski wrote: > On Tue, 26 Jul 2011, Hans Verkuil wrote: > >> On Tuesday, July 26, 2011 13:44:28 Sakari Ailus wrote: >>> Hi Hans and Guennadi, >> >> <snip> >> >>>> I realized that it is not clear from the documentation whether it is possible to call >>>> VIDIOC_REQBUFS and make additional calls to VIDIOC_CREATE_BUFS afterwards. >>> >>> That's actually a must if one wants to release buffers. Currently no other >>> method than requesting 0 buffers using REQBUFS is provided (apart from >>> closing the file handle). >> >> I was referring to the non-0 use-case :-) >> >>>> I can't remember whether the code allows it or not, but it should be clearly documented. >>> >>> I would guess no user application would have to call REQBUFS with other than >>> zero buffers when using CREATE_BUFS. This must be an exception if mixing >>> REQBUFS and CREATE_BUFS is not allowed in general. That said, I don't see a >>> reason to prohibit either, but perhaps Guennadi has more informed opinion >>> on this. >> >> <snip> >> >>>>>>> Future functionality which would be nice: >>>>>>> >>>>>>> - Format counters. Every format set by S_FMT (or gotten by G_FMT) should >>>>>>> come with a counter value so that the user would know the format of >>>>>>> dequeued buffers when setting the format on-the-fly. Currently there are >>>>>>> only bytesperline and length, but the format can't be explicitly >>>>>>> determined from those. >>>> >>>> Actually, the index field will give you that information. When you create the >>>> buffers you know that range [index, index + count - 1] is associated with that >>>> specific format. >>> >>> Some hardware is able to change the format while streaming is ongoing (for >>> example: OMAP 3). The problem is that the user should be able to know which >>> frame has the new format. > > How exactly does this work or should it work? You mean, you just configure > your hardware with new frame size parameters without stopping the current > streaming, and the ISP will change frame sizes, beginning with some future > frame? How does the driver then get to know, which frame already has the That's correct. > new sizes? You actually want to know this in advance to already queue a > suitably sized buffer to the hardware? The driver knows that since it has configured the hardware to produce that frame size. The assumption is that all the buffers have suitable size for all the formats. This must be checked by the driver, something which also must be taken into account.
On Mon, 1 Aug 2011, Sakari Ailus wrote: > Guennadi Liakhovetski wrote: > > On Tue, 26 Jul 2011, Hans Verkuil wrote: > > > >> On Tuesday, July 26, 2011 13:44:28 Sakari Ailus wrote: > >>> Hi Hans and Guennadi, > >> > >> <snip> > >> > >>>> I realized that it is not clear from the documentation whether it is possible to call > >>>> VIDIOC_REQBUFS and make additional calls to VIDIOC_CREATE_BUFS afterwards. > >>> > >>> That's actually a must if one wants to release buffers. Currently no other > >>> method than requesting 0 buffers using REQBUFS is provided (apart from > >>> closing the file handle). > >> > >> I was referring to the non-0 use-case :-) > >> > >>>> I can't remember whether the code allows it or not, but it should be clearly documented. > >>> > >>> I would guess no user application would have to call REQBUFS with other than > >>> zero buffers when using CREATE_BUFS. This must be an exception if mixing > >>> REQBUFS and CREATE_BUFS is not allowed in general. That said, I don't see a > >>> reason to prohibit either, but perhaps Guennadi has more informed opinion > >>> on this. > >> > >> <snip> > >> > >>>>>>> Future functionality which would be nice: > >>>>>>> > >>>>>>> - Format counters. Every format set by S_FMT (or gotten by G_FMT) should > >>>>>>> come with a counter value so that the user would know the format of > >>>>>>> dequeued buffers when setting the format on-the-fly. Currently there are > >>>>>>> only bytesperline and length, but the format can't be explicitly > >>>>>>> determined from those. > >>>> > >>>> Actually, the index field will give you that information. When you create the > >>>> buffers you know that range [index, index + count - 1] is associated with that > >>>> specific format. > >>> > >>> Some hardware is able to change the format while streaming is ongoing (for > >>> example: OMAP 3). The problem is that the user should be able to know which > >>> frame has the new format. > > > > How exactly does this work or should it work? You mean, you just configure > > your hardware with new frame size parameters without stopping the current > > streaming, and the ISP will change frame sizes, beginning with some future > > frame? How does the driver then get to know, which frame already has the > > That's correct. > > > new sizes? You actually want to know this in advance to already queue a > > suitably sized buffer to the hardware? > > The driver knows that since it has configured the hardware to produce > that frame size. > > The assumption is that all the buffers have suitable size for all the > formats. This must be checked by the driver, something which also must > be taken into account. Hm, but do you then at all need different buffers? Thanks Guennadi --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/ -- 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
Guennadi Liakhovetski wrote: > On Mon, 1 Aug 2011, Sakari Ailus wrote: > >> Guennadi Liakhovetski wrote: >>> On Tue, 26 Jul 2011, Hans Verkuil wrote: >>> >>>> On Tuesday, July 26, 2011 13:44:28 Sakari Ailus wrote: >>>>> Hi Hans and Guennadi, >>>> >>>> <snip> >>>> >>>>>> I realized that it is not clear from the documentation whether it is possible to call >>>>>> VIDIOC_REQBUFS and make additional calls to VIDIOC_CREATE_BUFS afterwards. >>>>> >>>>> That's actually a must if one wants to release buffers. Currently no other >>>>> method than requesting 0 buffers using REQBUFS is provided (apart from >>>>> closing the file handle). >>>> >>>> I was referring to the non-0 use-case :-) >>>> >>>>>> I can't remember whether the code allows it or not, but it should be clearly documented. >>>>> >>>>> I would guess no user application would have to call REQBUFS with other than >>>>> zero buffers when using CREATE_BUFS. This must be an exception if mixing >>>>> REQBUFS and CREATE_BUFS is not allowed in general. That said, I don't see a >>>>> reason to prohibit either, but perhaps Guennadi has more informed opinion >>>>> on this. >>>> >>>> <snip> >>>> >>>>>>>>> Future functionality which would be nice: >>>>>>>>> >>>>>>>>> - Format counters. Every format set by S_FMT (or gotten by G_FMT) should >>>>>>>>> come with a counter value so that the user would know the format of >>>>>>>>> dequeued buffers when setting the format on-the-fly. Currently there are >>>>>>>>> only bytesperline and length, but the format can't be explicitly >>>>>>>>> determined from those. >>>>>> >>>>>> Actually, the index field will give you that information. When you create the >>>>>> buffers you know that range [index, index + count - 1] is associated with that >>>>>> specific format. >>>>> >>>>> Some hardware is able to change the format while streaming is ongoing (for >>>>> example: OMAP 3). The problem is that the user should be able to know which >>>>> frame has the new format. >>> >>> How exactly does this work or should it work? You mean, you just configure >>> your hardware with new frame size parameters without stopping the current >>> streaming, and the ISP will change frame sizes, beginning with some future >>> frame? How does the driver then get to know, which frame already has the >> >> That's correct. >> >>> new sizes? You actually want to know this in advance to already queue a >>> suitably sized buffer to the hardware? >> >> The driver knows that since it has configured the hardware to produce >> that frame size. >> >> The assumption is that all the buffers have suitable size for all the >> formats. This must be checked by the driver, something which also must >> be taken into account. > > Hm, but do you then at all need different buffers? Not in this case, but this is a different case after all: streaming with buffers of different size, not still capture.
On Tue, 2 Aug 2011, Sakari Ailus wrote: > Guennadi Liakhovetski wrote: > > On Mon, 1 Aug 2011, Sakari Ailus wrote: > > > >> Guennadi Liakhovetski wrote: > >>> On Tue, 26 Jul 2011, Hans Verkuil wrote: > >>> > >>>> On Tuesday, July 26, 2011 13:44:28 Sakari Ailus wrote: > >>>>> Hi Hans and Guennadi, > >>>> > >>>> <snip> > >>>> > >>>>>> I realized that it is not clear from the documentation whether it is possible to call > >>>>>> VIDIOC_REQBUFS and make additional calls to VIDIOC_CREATE_BUFS afterwards. > >>>>> > >>>>> That's actually a must if one wants to release buffers. Currently no other > >>>>> method than requesting 0 buffers using REQBUFS is provided (apart from > >>>>> closing the file handle). > >>>> > >>>> I was referring to the non-0 use-case :-) > >>>> > >>>>>> I can't remember whether the code allows it or not, but it should be clearly documented. > >>>>> > >>>>> I would guess no user application would have to call REQBUFS with other than > >>>>> zero buffers when using CREATE_BUFS. This must be an exception if mixing > >>>>> REQBUFS and CREATE_BUFS is not allowed in general. That said, I don't see a > >>>>> reason to prohibit either, but perhaps Guennadi has more informed opinion > >>>>> on this. > >>>> > >>>> <snip> > >>>> > >>>>>>>>> Future functionality which would be nice: > >>>>>>>>> > >>>>>>>>> - Format counters. Every format set by S_FMT (or gotten by G_FMT) should > >>>>>>>>> come with a counter value so that the user would know the format of > >>>>>>>>> dequeued buffers when setting the format on-the-fly. Currently there are > >>>>>>>>> only bytesperline and length, but the format can't be explicitly > >>>>>>>>> determined from those. > >>>>>> > >>>>>> Actually, the index field will give you that information. When you create the > >>>>>> buffers you know that range [index, index + count - 1] is associated with that > >>>>>> specific format. > >>>>> > >>>>> Some hardware is able to change the format while streaming is ongoing (for > >>>>> example: OMAP 3). The problem is that the user should be able to know which > >>>>> frame has the new format. > >>> > >>> How exactly does this work or should it work? You mean, you just configure > >>> your hardware with new frame size parameters without stopping the current > >>> streaming, and the ISP will change frame sizes, beginning with some future > >>> frame? How does the driver then get to know, which frame already has the > >> > >> That's correct. > >> > >>> new sizes? You actually want to know this in advance to already queue a > >>> suitably sized buffer to the hardware? > >> > >> The driver knows that since it has configured the hardware to produce > >> that frame size. > >> > >> The assumption is that all the buffers have suitable size for all the > >> formats. This must be checked by the driver, something which also must > >> be taken into account. > > > > Hm, but do you then at all need different buffers? > > Not in this case, but this is a different case after all: streaming with > buffers of different size, not still capture. Sorry, I've lost you completely. Do you have a real-life example, where your software does not know which buffer type must be used with a specific frame or you don't have such examples? Thanks Guennadi --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/ -- 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
Guennadi Liakhovetski wrote: > On Tue, 2 Aug 2011, Sakari Ailus wrote: > >> Guennadi Liakhovetski wrote: >>> On Mon, 1 Aug 2011, Sakari Ailus wrote: >>> >>>> Guennadi Liakhovetski wrote: >>>>> On Tue, 26 Jul 2011, Hans Verkuil wrote: >>>>> >>>>>> On Tuesday, July 26, 2011 13:44:28 Sakari Ailus wrote: >>>>>>> Hi Hans and Guennadi, >>>>>> >>>>>> <snip> >>>>>> >>>>>>>> I realized that it is not clear from the documentation whether it is possible to call >>>>>>>> VIDIOC_REQBUFS and make additional calls to VIDIOC_CREATE_BUFS afterwards. >>>>>>> >>>>>>> That's actually a must if one wants to release buffers. Currently no other >>>>>>> method than requesting 0 buffers using REQBUFS is provided (apart from >>>>>>> closing the file handle). >>>>>> >>>>>> I was referring to the non-0 use-case :-) >>>>>> >>>>>>>> I can't remember whether the code allows it or not, but it should be clearly documented. >>>>>>> >>>>>>> I would guess no user application would have to call REQBUFS with other than >>>>>>> zero buffers when using CREATE_BUFS. This must be an exception if mixing >>>>>>> REQBUFS and CREATE_BUFS is not allowed in general. That said, I don't see a >>>>>>> reason to prohibit either, but perhaps Guennadi has more informed opinion >>>>>>> on this. >>>>>> >>>>>> <snip> >>>>>> >>>>>>>>>>> Future functionality which would be nice: >>>>>>>>>>> >>>>>>>>>>> - Format counters. Every format set by S_FMT (or gotten by G_FMT) should >>>>>>>>>>> come with a counter value so that the user would know the format of >>>>>>>>>>> dequeued buffers when setting the format on-the-fly. Currently there are >>>>>>>>>>> only bytesperline and length, but the format can't be explicitly >>>>>>>>>>> determined from those. >>>>>>>> >>>>>>>> Actually, the index field will give you that information. When you create the >>>>>>>> buffers you know that range [index, index + count - 1] is associated with that >>>>>>>> specific format. >>>>>>> >>>>>>> Some hardware is able to change the format while streaming is ongoing (for >>>>>>> example: OMAP 3). The problem is that the user should be able to know which >>>>>>> frame has the new format. >>>>> >>>>> How exactly does this work or should it work? You mean, you just configure >>>>> your hardware with new frame size parameters without stopping the current >>>>> streaming, and the ISP will change frame sizes, beginning with some future >>>>> frame? How does the driver then get to know, which frame already has the >>>> >>>> That's correct. >>>> >>>>> new sizes? You actually want to know this in advance to already queue a >>>>> suitably sized buffer to the hardware? >>>> >>>> The driver knows that since it has configured the hardware to produce >>>> that frame size. >>>> >>>> The assumption is that all the buffers have suitable size for all the >>>> formats. This must be checked by the driver, something which also must >>>> be taken into account. >>> >>> Hm, but do you then at all need different buffers? >> >> Not in this case, but this is a different case after all: streaming with >> buffers of different size, not still capture. > > Sorry, I've lost you completely. Do you have a real-life example, where > your software does not know which buffer type must be used with a specific > frame or you don't have such examples? There is one, now that you asked. It's not a buffer type but format. In video calls the resolution of the video frames need to be changed, along with the frame rate, depending on the quality of the link, for example. In this case it's important to know which format a dequeued frame has. The alternative is to stop streaming, reconfigure and start it again, but this leads to lost frames. The hardware can do this so I don't think the interface should prevent it.
On Tue, 2 Aug 2011, Sakari Ailus wrote: > Guennadi Liakhovetski wrote: > > On Tue, 2 Aug 2011, Sakari Ailus wrote: > > > >> Guennadi Liakhovetski wrote: > >>> On Mon, 1 Aug 2011, Sakari Ailus wrote: > >>> > >>>> Guennadi Liakhovetski wrote: > >>>>> On Tue, 26 Jul 2011, Hans Verkuil wrote: > >>>>> > >>>>>> On Tuesday, July 26, 2011 13:44:28 Sakari Ailus wrote: > >>>>>>> Hi Hans and Guennadi, > >>>>>> > >>>>>> <snip> > >>>>>> > >>>>>>>> I realized that it is not clear from the documentation whether it is possible to call > >>>>>>>> VIDIOC_REQBUFS and make additional calls to VIDIOC_CREATE_BUFS afterwards. > >>>>>>> > >>>>>>> That's actually a must if one wants to release buffers. Currently no other > >>>>>>> method than requesting 0 buffers using REQBUFS is provided (apart from > >>>>>>> closing the file handle). > >>>>>> > >>>>>> I was referring to the non-0 use-case :-) > >>>>>> > >>>>>>>> I can't remember whether the code allows it or not, but it should be clearly documented. > >>>>>>> > >>>>>>> I would guess no user application would have to call REQBUFS with other than > >>>>>>> zero buffers when using CREATE_BUFS. This must be an exception if mixing > >>>>>>> REQBUFS and CREATE_BUFS is not allowed in general. That said, I don't see a > >>>>>>> reason to prohibit either, but perhaps Guennadi has more informed opinion > >>>>>>> on this. > >>>>>> > >>>>>> <snip> > >>>>>> > >>>>>>>>>>> Future functionality which would be nice: > >>>>>>>>>>> > >>>>>>>>>>> - Format counters. Every format set by S_FMT (or gotten by G_FMT) should > >>>>>>>>>>> come with a counter value so that the user would know the format of > >>>>>>>>>>> dequeued buffers when setting the format on-the-fly. Currently there are > >>>>>>>>>>> only bytesperline and length, but the format can't be explicitly > >>>>>>>>>>> determined from those. > >>>>>>>> > >>>>>>>> Actually, the index field will give you that information. When you create the > >>>>>>>> buffers you know that range [index, index + count - 1] is associated with that > >>>>>>>> specific format. > >>>>>>> > >>>>>>> Some hardware is able to change the format while streaming is ongoing (for > >>>>>>> example: OMAP 3). The problem is that the user should be able to know which > >>>>>>> frame has the new format. > >>>>> > >>>>> How exactly does this work or should it work? You mean, you just configure > >>>>> your hardware with new frame size parameters without stopping the current > >>>>> streaming, and the ISP will change frame sizes, beginning with some future > >>>>> frame? How does the driver then get to know, which frame already has the > >>>> > >>>> That's correct. > >>>> > >>>>> new sizes? You actually want to know this in advance to already queue a > >>>>> suitably sized buffer to the hardware? > >>>> > >>>> The driver knows that since it has configured the hardware to produce > >>>> that frame size. > >>>> > >>>> The assumption is that all the buffers have suitable size for all the > >>>> formats. This must be checked by the driver, something which also must > >>>> be taken into account. > >>> > >>> Hm, but do you then at all need different buffers? > >> > >> Not in this case, but this is a different case after all: streaming with > >> buffers of different size, not still capture. > > > > Sorry, I've lost you completely. Do you have a real-life example, where > > your software does not know which buffer type must be used with a specific > > frame or you don't have such examples? > > There is one, now that you asked. It's not a buffer type but format. > > In video calls the resolution of the video frames need to be changed, > along with the frame rate, depending on the quality of the link, for > example. > > In this case it's important to know which format a dequeued frame has. > The alternative is to stop streaming, reconfigure and start it again, > but this leads to lost frames. The hardware can do this so I don't think > the interface should prevent it. Sorry, let me try again. Yes, we already know, that OMAP3 can switch from one frame format to another one on the fly. And you told us, that for this to work you need buffers, that can hold both frame formats, because otherwise you have no chance to identify when the hardware is going to switch and you have no time to queue another buffer type. But this doesn't concern us atm. We're not discussing how to inform the user-space about the new frame format, we're discussing switching between different buffer types. AFAICS, you don't have such an example. So, unless I'm missing something, I'd say: case dismissed. Thanks Guennadi --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/ -- 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
Guennadi Liakhovetski wrote: > On Tue, 2 Aug 2011, Sakari Ailus wrote: > >> Guennadi Liakhovetski wrote: >>> On Tue, 2 Aug 2011, Sakari Ailus wrote: >>> >>>> Guennadi Liakhovetski wrote: >>>>> On Mon, 1 Aug 2011, Sakari Ailus wrote: >>>>> >>>>>> Guennadi Liakhovetski wrote: >>>>>>> On Tue, 26 Jul 2011, Hans Verkuil wrote: >>>>>>> >>>>>>>> On Tuesday, July 26, 2011 13:44:28 Sakari Ailus wrote: >>>>>>>>> Hi Hans and Guennadi, >>>>>>>> >>>>>>>> <snip> >>>>>>>> >>>>>>>>>> I realized that it is not clear from the documentation whether it is possible to call >>>>>>>>>> VIDIOC_REQBUFS and make additional calls to VIDIOC_CREATE_BUFS afterwards. >>>>>>>>> >>>>>>>>> That's actually a must if one wants to release buffers. Currently no other >>>>>>>>> method than requesting 0 buffers using REQBUFS is provided (apart from >>>>>>>>> closing the file handle). >>>>>>>> >>>>>>>> I was referring to the non-0 use-case :-) >>>>>>>> >>>>>>>>>> I can't remember whether the code allows it or not, but it should be clearly documented. >>>>>>>>> >>>>>>>>> I would guess no user application would have to call REQBUFS with other than >>>>>>>>> zero buffers when using CREATE_BUFS. This must be an exception if mixing >>>>>>>>> REQBUFS and CREATE_BUFS is not allowed in general. That said, I don't see a >>>>>>>>> reason to prohibit either, but perhaps Guennadi has more informed opinion >>>>>>>>> on this. >>>>>>>> >>>>>>>> <snip> >>>>>>>> >>>>>>>>>>>>> Future functionality which would be nice: >>>>>>>>>>>>> >>>>>>>>>>>>> - Format counters. Every format set by S_FMT (or gotten by G_FMT) should >>>>>>>>>>>>> come with a counter value so that the user would know the format of >>>>>>>>>>>>> dequeued buffers when setting the format on-the-fly. Currently there are >>>>>>>>>>>>> only bytesperline and length, but the format can't be explicitly >>>>>>>>>>>>> determined from those. >>>>>>>>>> >>>>>>>>>> Actually, the index field will give you that information. When you create the >>>>>>>>>> buffers you know that range [index, index + count - 1] is associated with that >>>>>>>>>> specific format. >>>>>>>>> >>>>>>>>> Some hardware is able to change the format while streaming is ongoing (for >>>>>>>>> example: OMAP 3). The problem is that the user should be able to know which >>>>>>>>> frame has the new format. >>>>>>> >>>>>>> How exactly does this work or should it work? You mean, you just configure >>>>>>> your hardware with new frame size parameters without stopping the current >>>>>>> streaming, and the ISP will change frame sizes, beginning with some future >>>>>>> frame? How does the driver then get to know, which frame already has the >>>>>> >>>>>> That's correct. >>>>>> >>>>>>> new sizes? You actually want to know this in advance to already queue a >>>>>>> suitably sized buffer to the hardware? >>>>>> >>>>>> The driver knows that since it has configured the hardware to produce >>>>>> that frame size. >>>>>> >>>>>> The assumption is that all the buffers have suitable size for all the >>>>>> formats. This must be checked by the driver, something which also must >>>>>> be taken into account. >>>>> >>>>> Hm, but do you then at all need different buffers? >>>> >>>> Not in this case, but this is a different case after all: streaming with >>>> buffers of different size, not still capture. >>> >>> Sorry, I've lost you completely. Do you have a real-life example, where >>> your software does not know which buffer type must be used with a specific >>> frame or you don't have such examples? >> >> There is one, now that you asked. It's not a buffer type but format. >> >> In video calls the resolution of the video frames need to be changed, >> along with the frame rate, depending on the quality of the link, for >> example. >> >> In this case it's important to know which format a dequeued frame has. >> The alternative is to stop streaming, reconfigure and start it again, >> but this leads to lost frames. The hardware can do this so I don't think >> the interface should prevent it. > > Sorry, let me try again. Yes, we already know, that OMAP3 can switch from > one frame format to another one on the fly. And you told us, that for this > to work you need buffers, that can hold both frame formats, because > otherwise you have no chance to identify when the hardware is going to > switch and you have no time to queue another buffer type. > > But this doesn't concern us atm. We're not discussing how to inform the > user-space about the new frame format, we're discussing switching between > different buffer types. That is a different topic then. I gave an example of the "future functionality which would be nice" above and that had no direct dependency to this patchset --- it was related to whether a new structure to replace v4l2_buffer is needed, but the conclusion to that discussion has been already reached: we're fine with the current v4l2_buffer. The format counters can be implemented (if that's the solution) later when the driver will be able to do this. Also pipeline configuration needs to change to implement this. > AFAICS, you don't have such an example. So, unless I'm missing something, > I'd say: case dismissed. Uh, I'm a bit lost now. Do different buffer types (e.g. capture, overlay and input) need to be taken into account here, and if so, how? My understanding was this is not related to preparing buffers. Regards,
On Tue, 2 Aug 2011, Sakari Ailus wrote: > Uh, I'm a bit lost now. Do different buffer types (e.g. capture, overlay > and input) need to be taken into account here, and if so, how? My > understanding was this is not related to preparing buffers. They all should be dealt with correctly. If you find any problems, please, report. Thanks Guennadi --- Guennadi Liakhovetski, Ph.D. Freelance Open-Source Software Developer http://www.open-technology.de/ -- 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
On Wed, Jul 27, 2011 at 09:11:38PM -0700, Pawel Osciak wrote: > Hi Guennadi, Hi Pawel, > On Wed, Jul 20, 2011 at 01:43, Guennadi Liakhovetski > <g.liakhovetski@gmx.de> 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 <g.liakhovetski@gmx.de> > > --- > > > <snip> > > This looks nicer, I like how we got rid of destroy and gave up on > making holes, it would've given us a lot of headaches. I'm thinking > about some issues though and also have some comments/questions further > below. Holes could be re-introduced by defining DESTROY_BUFS. But it would be up to the application to use it or not. > Already mentioned by others mixing of REQBUFS and CREATE_BUFS. > Personally I'd like to allow mixing, including REQBUFS for non-zero, > because I think it would be easy to do. I think it could work in the > same way as REQBUFS for !=0 works currently (at least in vb2), if we > already have some buffers allocated and they are not in use, we free > them and a new set is allocated. So I guess it could just stay this > way. REQBUFS(0) would of course free everything. > > Passing format to CREATE_BUFS will make vb2 a bit format-aware, as it > would have to pass it forward to the driver somehow. The obvious way > would be just vb2 calling the driver's s_fmt handler, but that won't > work, as you can't pass indexes to s_fmt. So we'd have to implement a > new driver callback for setting formats per index. I guess there is no > way around it, unless we actually take the format struct out of > CREATE_BUFS and somehow do it via S_FMT. The single-planar structure > is full already though, the only way would be to use > v4l2_pix_format_mplane instead with plane count = 1 (or more if > needed). I've been pondering a while an idea to change v4l2_format. None of the structures in the union use the full size of it. This would allow adding fields to the end of the union and reduce its size accordingly. No-one would be harmed in the process, struct v4l2_format would just change slightly. Guennadi might have thought this in the past but at least I misunderstood it at the time. :-) > Another thing is the case of passing size to CREATE_BUFS. vb2, when > allocating buffers, gets their sizes from the driver (via > queue_setup), it never "suggest" any particular size. So that flow > would have to be changed as well. I guess vb2 could pass the size to > queue_setup in a similar way as it does with buffer count. This would > mean though that the ioctl would fail if the driver didn't agree to > the given size. Right now I don't see an option to "negotiate" the > size with the driver via this option. > > The more I think of it though, why do we need the size argument? It's > not needed in the existing flows (that use S_FMT) and, more > importantly, I don't think the application can know more than the > driver can, so why giving that option? The driver should know the size > for a format at least as well as the application... Also, is there a > real use case of providing just size, will the driver know which > format to use given a size? > > > > + <para>To allocate device buffers applications initialize all > > +fields of the <structname>v4l2_create_buffers</structname> structure. > > +They set the <structfield>type</structfield> field in the > > +<structname>v4l2_format</structname> structure, embedded in this > > +structure, to the respective stream or buffer type. > > +<structfield>count</structfield> must be set to the number of required > > +buffers. <structfield>memory</structfield> specifies the required I/O > > +method. Applications have two possibilities to specify the size of buffers > > +to be prepared: they can either set the <structfield>size</structfield> > > +field explicitly to a non-zero value, or fill in the frame format data in the > > +<structfield>format</structfield> field. In the latter case buffer sizes > > +will be calculated automatically by the driver. The > > Technically, we shouldn't say "applications initialize all fields" in > the first sentence if they do not have to initialize both size and > format fields at the same time. > > > +<structfield>reserved</structfield> 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 <structfield>count</structfield> and > > +the <structfield>index</structfield> fields respectively. > > +<structfield>count</structfield> can be smaller than the number requested. > > +-ENOMEM is returned, if the driver runs out of free memory.</para> > > + <para>When the I/O method is not supported the ioctl > > +returns an &EINVAL;.</para> > > + > > + <table pgwide="1" frame="none" id="v4l2-create-buffers"> > > + <title>struct <structname>v4l2_create_buffers</structname></title> > > + <tgroup cols="3"> > > + &cs-str; > > + <tbody valign="top"> > > + <row> > > + <entry>__u32</entry> > > + <entry><structfield>index</structfield></entry> > > + <entry>The starting buffer index, returned by the driver.</entry> > > + </row> > > + <row> > > + <entry>__u32</entry> > > + <entry><structfield>count</structfield></entry> > > + <entry>The number of buffers requested or granted.</entry> > > + </row> > > + <row> > > + <entry>&v4l2-memory;</entry> > > + <entry><structfield>memory</structfield></entry> > > + <entry>Applications set this field to > > +<constant>V4L2_MEMORY_MMAP</constant> or > > +<constant>V4L2_MEMORY_USERPTR</constant>.</entry> > > + </row> > > + <row> > > + <entry>__u32</entry> > > + <entry><structfield>size</structfield></entry> > > + <entry>Explicit size of buffers, being created.</entry> > > + </row> > > + <row> > > + <entry>&v4l2-format;</entry> > > + <entry><structfield>format</structfield></entry> > > + <entry>Application has to set the <structfield>type</structfield> > > +field, other fields should be used, if the application wants to allocate buffers > > +for a specific frame format.</entry> > > + </row> > > + <row> > > + <entry>__u32</entry> > > + <entry><structfield>reserved</structfield>[8]</entry> > > + <entry>A place holder for future extensions.</entry> > > + </row> > > + </tbody> > > + </tgroup> > > + </table> > > + </refsect1> > > + > > + <refsect1> > > + &return-value; > > + > > + <variablelist> > > + <varlistentry> > > + <term><errorcode>ENOMEM</errorcode></term> > > + <listitem> > > + <para>No memory to allocate buffers for <link linkend="mmap">memory > > +mapped</link> I/O.</para> > > + </listitem> > > + </varlistentry> > > + <varlistentry> > > + <term><errorcode>EINVAL</errorcode></term> > > + <listitem> > > + <para>The buffer type (<structfield>type</structfield> field) or the > > +requested I/O method (<structfield>memory</structfield>) is not > > +supported.</para> > > + </listitem> > > + </varlistentry> > > + </variablelist> > > + </refsect1> > > +</refentry> > > + > > What happens if the driver does not agree to the provided size? EINVAL? > > > +<!-- > > +Local Variables: > > +mode: sgml > > +sgml-parent-document: "v4l2.sgml" > > +indent-tabs-mode: nil > > +End: > > +--> > > 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 @@ > > +<refentry id="vidioc-prepare-buf"> > > + <refmeta> > > + <refentrytitle>ioctl VIDIOC_PREPARE_BUF</refentrytitle> > > + &manvol; > > + </refmeta> > > + > > + <refnamediv> > > + <refname>VIDIOC_PREPARE_BUF</refname> > > + <refpurpose>Prepare a buffer for I/O</refpurpose> > > + </refnamediv> > > + > > + <refsynopsisdiv> > > + <funcsynopsis> > > + <funcprototype> > > + <funcdef>int <function>ioctl</function></funcdef> > > + <paramdef>int <parameter>fd</parameter></paramdef> > > + <paramdef>int <parameter>request</parameter></paramdef> > > + <paramdef>struct v4l2_buffer *<parameter>argp</parameter></paramdef> > > + </funcprototype> > > + </funcsynopsis> > > + </refsynopsisdiv> > > + > > + <refsect1> > > + <title>Arguments</title> > > + > > + <variablelist> > > + <varlistentry> > > + <term><parameter>fd</parameter></term> > > + <listitem> > > + <para>&fd;</para> > > + </listitem> > > + </varlistentry> > > + <varlistentry> > > + <term><parameter>request</parameter></term> > > + <listitem> > > + <para>VIDIOC_PREPARE_BUF</para> > > + </listitem> > > + </varlistentry> > > + <varlistentry> > > + <term><parameter>argp</parameter></term> > > + <listitem> > > + <para></para> > > + </listitem> > > + </varlistentry> > > + </variablelist> > > + </refsect1> > > + > > + <refsect1> > > + <title>Description</title> > > + > > + <para>Applications can optionally call the > > +<constant>VIDIOC_PREPARE_BUF</constant> ioctl to pass ownership of the buffer > > +to the driver before actually enqueuing it, using the > > +<constant>VIDIOC_QBUF</constant> 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 > > +<constant>V4L2_BUF_FLAG_NO_CACHE_INVALIDATE</constant> and > > +<constant>V4L2_BUF_FLAG_NO_CACHE_CLEAN</constant> flags to skip the respective > > +step.</para> > > + > > I'm probably forgetting something, but why would we want to do both > PREPARE_BUF and QBUF? Why not queue in advance? > Could you give a full sequence of ioctls how it will be used > (including streamons, etc.)? I'm trying to picture how passing of both > types of buffers will have to look like between vb2 and a driver. > > <snip> > > -- > Best regards, > Pawel Osciak
diff --git a/Documentation/DocBook/media/v4l/io.xml b/Documentation/DocBook/media/v4l/io.xml index 227e7ac..6249d0e 100644 --- a/Documentation/DocBook/media/v4l/io.xml +++ b/Documentation/DocBook/media/v4l/io.xml @@ -927,6 +927,23 @@ ioctl is called.</entry> Applications set or clear this flag before calling the <constant>VIDIOC_QBUF</constant> ioctl.</entry> </row> + <row> + <entry><constant>V4L2_BUF_FLAG_NO_CACHE_INVALIDATE</constant></entry> + <entry>0x0400</entry> + <entry>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 br touched by the CPU, instead the buffer will, probably, be +passed on to a DMA-capable hardware unit for further processing or output. +</entry> + </row> + <row> + <entry><constant>V4L2_BUF_FLAG_NO_CACHE_CLEAN</constant></entry> + <entry>0x0800</entry> + <entry>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.</entry> + </row> </tbody> </tgroup> </table> 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.</revremark> &sub-close; &sub-ioctl; <!-- All ioctls go here. --> + &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.</revremark> &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..5f0158c --- /dev/null +++ b/Documentation/DocBook/media/v4l/vidioc-create-bufs.xml @@ -0,0 +1,152 @@ +<refentry id="vidioc-create-bufs"> + <refmeta> + <refentrytitle>ioctl VIDIOC_CREATE_BUFS</refentrytitle> + &manvol; + </refmeta> + + <refnamediv> + <refname>VIDIOC_CREATE_BUFS</refname> + <refpurpose>Create buffers for Memory Mapped or User Pointer I/O</refpurpose> + </refnamediv> + + <refsynopsisdiv> + <funcsynopsis> + <funcprototype> + <funcdef>int <function>ioctl</function></funcdef> + <paramdef>int <parameter>fd</parameter></paramdef> + <paramdef>int <parameter>request</parameter></paramdef> + <paramdef>struct v4l2_create_buffers *<parameter>argp</parameter></paramdef> + </funcprototype> + </funcsynopsis> + </refsynopsisdiv> + + <refsect1> + <title>Arguments</title> + + <variablelist> + <varlistentry> + <term><parameter>fd</parameter></term> + <listitem> + <para>&fd;</para> + </listitem> + </varlistentry> + <varlistentry> + <term><parameter>request</parameter></term> + <listitem> + <para>VIDIOC_CREATE_BUFS</para> + </listitem> + </varlistentry> + <varlistentry> + <term><parameter>argp</parameter></term> + <listitem> + <para></para> + </listitem> + </varlistentry> + </variablelist> + </refsect1> + + <refsect1> + <title>Description</title> + + <para>This ioctl is used to create buffers for <link linkend="mmap">memory +mapped</link> or <link linkend="userp">user pointer</link> +I/O. It can be used as an alternative to the <constant>VIDIOC_REQBUFS</constant> +ioctl, when a tighter control over buffers is required. This ioctl can be called +multiple times to create buffers of different sizes. + + <para>To allocate device buffers applications initialize all +fields of the <structname>v4l2_create_buffers</structname> structure. +They set the <structfield>type</structfield> field in the +<structname>v4l2_format</structname> structure, embedded in this +structure, to the respective stream or buffer type. +<structfield>count</structfield> must be set to the number of required +buffers. <structfield>memory</structfield> specifies the required I/O +method. Applications have two possibilities to specify the size of buffers +to be prepared: they can either set the <structfield>size</structfield> +field explicitly to a non-zero value, or fill in the frame format data in the +<structfield>format</structfield> field. In the latter case buffer sizes +will be calculated automatically by the driver. The +<structfield>reserved</structfield> 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 <structfield>count</structfield> and +the <structfield>index</structfield> fields respectively. +<structfield>count</structfield> can be smaller than the number requested. +-ENOMEM is returned, if the driver runs out of free memory.</para> + <para>When the I/O method is not supported the ioctl +returns an &EINVAL;.</para> + + <table pgwide="1" frame="none" id="v4l2-create-buffers"> + <title>struct <structname>v4l2_create_buffers</structname></title> + <tgroup cols="3"> + &cs-str; + <tbody valign="top"> + <row> + <entry>__u32</entry> + <entry><structfield>index</structfield></entry> + <entry>The starting buffer index, returned by the driver.</entry> + </row> + <row> + <entry>__u32</entry> + <entry><structfield>count</structfield></entry> + <entry>The number of buffers requested or granted.</entry> + </row> + <row> + <entry>&v4l2-memory;</entry> + <entry><structfield>memory</structfield></entry> + <entry>Applications set this field to +<constant>V4L2_MEMORY_MMAP</constant> or +<constant>V4L2_MEMORY_USERPTR</constant>.</entry> + </row> + <row> + <entry>__u32</entry> + <entry><structfield>size</structfield></entry> + <entry>Explicit size of buffers, being created.</entry> + </row> + <row> + <entry>&v4l2-format;</entry> + <entry><structfield>format</structfield></entry> + <entry>Application has to set the <structfield>type</structfield> +field, other fields should be used, if the application wants to allocate buffers +for a specific frame format.</entry> + </row> + <row> + <entry>__u32</entry> + <entry><structfield>reserved</structfield>[8]</entry> + <entry>A place holder for future extensions.</entry> + </row> + </tbody> + </tgroup> + </table> + </refsect1> + + <refsect1> + &return-value; + + <variablelist> + <varlistentry> + <term><errorcode>ENOMEM</errorcode></term> + <listitem> + <para>No memory to allocate buffers for <link linkend="mmap">memory +mapped</link> I/O.</para> + </listitem> + </varlistentry> + <varlistentry> + <term><errorcode>EINVAL</errorcode></term> + <listitem> + <para>The buffer type (<structfield>type</structfield> field) or the +requested I/O method (<structfield>memory</structfield>) is not +supported.</para> + </listitem> + </varlistentry> + </variablelist> + </refsect1> +</refentry> + +<!-- +Local Variables: +mode: sgml +sgml-parent-document: "v4l2.sgml" +indent-tabs-mode: nil +End: +--> 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 @@ +<refentry id="vidioc-prepare-buf"> + <refmeta> + <refentrytitle>ioctl VIDIOC_PREPARE_BUF</refentrytitle> + &manvol; + </refmeta> + + <refnamediv> + <refname>VIDIOC_PREPARE_BUF</refname> + <refpurpose>Prepare a buffer for I/O</refpurpose> + </refnamediv> + + <refsynopsisdiv> + <funcsynopsis> + <funcprototype> + <funcdef>int <function>ioctl</function></funcdef> + <paramdef>int <parameter>fd</parameter></paramdef> + <paramdef>int <parameter>request</parameter></paramdef> + <paramdef>struct v4l2_buffer *<parameter>argp</parameter></paramdef> + </funcprototype> + </funcsynopsis> + </refsynopsisdiv> + + <refsect1> + <title>Arguments</title> + + <variablelist> + <varlistentry> + <term><parameter>fd</parameter></term> + <listitem> + <para>&fd;</para> + </listitem> + </varlistentry> + <varlistentry> + <term><parameter>request</parameter></term> + <listitem> + <para>VIDIOC_PREPARE_BUF</para> + </listitem> + </varlistentry> + <varlistentry> + <term><parameter>argp</parameter></term> + <listitem> + <para></para> + </listitem> + </varlistentry> + </variablelist> + </refsect1> + + <refsect1> + <title>Description</title> + + <para>Applications can optionally call the +<constant>VIDIOC_PREPARE_BUF</constant> ioctl to pass ownership of the buffer +to the driver before actually enqueuing it, using the +<constant>VIDIOC_QBUF</constant> 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 +<constant>V4L2_BUF_FLAG_NO_CACHE_INVALIDATE</constant> and +<constant>V4L2_BUF_FLAG_NO_CACHE_CLEAN</constant> flags to skip the respective +step.</para> + + <para>The <structname>v4l2_buffer</structname> structure is +specified in <xref linkend="buffer" />.</para> + </refsect1> + + <refsect1> + &return-value; + + <variablelist> + <varlistentry> + <term><errorcode>EBUSY</errorcode></term> + <listitem> + <para>File I/O is in progress.</para> + </listitem> + </varlistentry> + <varlistentry> + <term><errorcode>EINVAL</errorcode></term> + <listitem> + <para>The buffer <structfield>type</structfield> is not +supported, or the <structfield>index</structfield> is out of bounds, +or no buffers have been allocated yet, or the +<structfield>userptr</structfield> or +<structfield>length</structfield> are invalid.</para> + </listitem> + </varlistentry> + </variablelist> + </refsect1> +</refentry> + +<!-- +Local Variables: +mode: sgml +sgml-parent-document: "v4l2.sgml" +indent-tabs-mode: nil +End: +--> diff --git a/drivers/media/video/v4l2-compat-ioctl32.c b/drivers/media/video/v4l2-compat-ioctl32.c index 61979b7..4105f69 100644 --- a/drivers/media/video/v4l2-compat-ioctl32.c +++ b/drivers/media/video/v4l2-compat-ioctl32.c @@ -159,11 +159,16 @@ struct v4l2_format32 { } fmt; }; -static int get_v4l2_format32(struct v4l2_format *kp, struct v4l2_format32 __user *up) +struct v4l2_create_buffers32 { + __u32 index; /* output: buffers index...index + count - 1 have been created */ + __u32 count; + enum v4l2_memory memory; + __u32 size; /* Explicit size, e.g., for compressed streams */ + struct v4l2_format32 format; /* "type" is used always, the rest if size == 0 */ +}; + +static int __get_v4l2_format32(struct v4l2_format *kp, struct v4l2_format32 __user *up) { - if (!access_ok(VERIFY_READ, up, sizeof(struct v4l2_format32)) || - get_user(kp->type, &up->type)) - return -EFAULT; switch (kp->type) { case V4L2_BUF_TYPE_VIDEO_CAPTURE: case V4L2_BUF_TYPE_VIDEO_OUTPUT: @@ -192,11 +197,24 @@ static int get_v4l2_format32(struct v4l2_format *kp, struct v4l2_format32 __user } } -static int put_v4l2_format32(struct v4l2_format *kp, struct v4l2_format32 __user *up) +static int get_v4l2_format32(struct v4l2_format *kp, struct v4l2_format32 __user *up) +{ + if (!access_ok(VERIFY_READ, up, sizeof(struct v4l2_format32)) || + get_user(kp->type, &up->type)) + return -EFAULT; + return __get_v4l2_format32(kp, up); +} + +static int get_v4l2_create32(struct v4l2_create_buffers *kp, struct v4l2_create_buffers32 __user *up) +{ + if (!access_ok(VERIFY_READ, up, sizeof(struct v4l2_create_buffers32)) || + copy_from_user(kp, up, offsetof(struct v4l2_create_buffers32, format.fmt))) + return -EFAULT; + return __get_v4l2_format32(&kp->format, &up->format); +} + +static int __put_v4l2_format32(struct v4l2_format *kp, struct v4l2_format32 __user *up) { - if (!access_ok(VERIFY_WRITE, up, sizeof(struct v4l2_format32)) || - put_user(kp->type, &up->type)) - return -EFAULT; switch (kp->type) { case V4L2_BUF_TYPE_VIDEO_CAPTURE: case V4L2_BUF_TYPE_VIDEO_OUTPUT: @@ -225,6 +243,22 @@ static int put_v4l2_format32(struct v4l2_format *kp, struct v4l2_format32 __user } } +static int put_v4l2_format32(struct v4l2_format *kp, struct v4l2_format32 __user *up) +{ + if (!access_ok(VERIFY_WRITE, up, sizeof(struct v4l2_format32)) || + put_user(kp->type, &up->type)) + return -EFAULT; + return __put_v4l2_format32(kp, up); +} + +static int put_v4l2_create32(struct v4l2_create_buffers *kp, struct v4l2_create_buffers32 __user *up) +{ + if (!access_ok(VERIFY_WRITE, up, sizeof(struct v4l2_create_buffers32)) || + copy_to_user(up, kp, offsetof(struct v4l2_create_buffers32, format.fmt))) + return -EFAULT; + return __put_v4l2_format32(&kp->format, &up->format); +} + struct v4l2_standard32 { __u32 index; __u32 id[2]; /* __u64 would get the alignment wrong */ @@ -702,6 +736,8 @@ 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_CREATE_BUFS32 _IOWR('V', 92, struct v4l2_create_buffers32) +#define VIDIOC_PREPARE_BUF32 _IOWR('V', 93, struct v4l2_buffer32) #define VIDIOC_OVERLAY32 _IOW ('V', 14, s32) #define VIDIOC_STREAMON32 _IOW ('V', 18, s32) @@ -721,6 +757,7 @@ static long do_video_ioctl(struct file *file, unsigned int cmd, unsigned long ar struct v4l2_standard v2s; struct v4l2_ext_controls v2ecs; struct v4l2_event v2ev; + struct v4l2_create_buffers v2crt; unsigned long vx; int vi; } karg; @@ -751,6 +788,8 @@ 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_CREATE_BUFS32: cmd = VIDIOC_CREATE_BUFS; break; + case VIDIOC_PREPARE_BUF32: cmd = VIDIOC_PREPARE_BUF; break; } switch (cmd) { @@ -775,6 +814,12 @@ static long do_video_ioctl(struct file *file, unsigned int cmd, unsigned long ar compatible_arg = 0; break; + case VIDIOC_CREATE_BUFS: + err = get_v4l2_create32(&karg.v2crt, up); + compatible_arg = 0; + break; + + case VIDIOC_PREPARE_BUF: case VIDIOC_QUERYBUF: case VIDIOC_QBUF: case VIDIOC_DQBUF: @@ -860,6 +905,11 @@ static long do_video_ioctl(struct file *file, unsigned int cmd, unsigned long ar err = put_v4l2_format32(&karg.v2f, up); break; + case VIDIOC_CREATE_BUFS: + err = put_v4l2_create32(&karg.v2crt, up); + break; + + case VIDIOC_PREPARE_BUF: case VIDIOC_QUERYBUF: case VIDIOC_QBUF: case VIDIOC_DQBUF: @@ -959,6 +1009,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_BUFS32: + 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..7824152 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,36 @@ 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 = check_fmt(ops, create->format.type); + if (ret) + break; + + if (create->size) + CLEAR_AFTER_FIELD(create, count); + + ret = ops->vidioc_create_bufs(file, fh, create); + + dbgarg(cmd, "count=%d\n", create->count); + 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..0b594c7 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,16 @@ 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; + enum v4l2_memory memory; + __u32 size; /* Explicit size, e.g., for compressed streams */ + struct v4l2_format format; /* "type" is used always, the rest if size == 0 */ + __u32 reserved[8]; +}; + /* * 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 +2195,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,
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 <g.liakhovetski@gmx.de> --- It's been almost a month since v2, the only comments were a request to increase the reserved space in the new ioctl() and to improve documentation. The reserved field is increased in this version, documentation also has been improved in multiple locations. I think, documentation can be further improved at any time, but if there are no objections against the actual contents of this patch, maybe we can commit this version. I still don't see v3.0;-), so, maybe we even can push it for 3.1. A trivial comparison with v2 shows the size of the reserved field as the only change in the API, and the compatibility fix as the only two functional changes. v3: addressed multiple comments by Sakari Ailus 1. increased reserved field in "struct v4l2_create_buffers" to 8 32-bit ints 2. multiple documentation fixes and improvements 3. fixed misplaced "case VIDIOC_PREPARE_BUF" in ioctl 32-bit compatibility processing v2: 1. add preliminary Documentation 2. add flag V4L2_BUFFER_FLAG_NO_CACHE_CLEAN 3. remove VIDIOC_DESTROY_BUFS 4. rename SUBMIT to VIDIOC_PREPARE_BUF 5. add reserved field to struct v4l2_create_buffers 6. cache handling flags moved to struct v4l2_buffer for processing during VIDIOC_PREPARE_BUF 7. VIDIOC_PREPARE_BUF now uses struct v4l2_buffer as its argument Documentation/DocBook/media/v4l/io.xml | 17 +++ Documentation/DocBook/media/v4l/v4l2.xml | 2 + .../DocBook/media/v4l/vidioc-create-bufs.xml | 152 ++++++++++++++++++++ .../DocBook/media/v4l/vidioc-prepare-buf.xml | 96 ++++++++++++ drivers/media/video/v4l2-compat-ioctl32.c | 68 ++++++++- drivers/media/video/v4l2-ioctl.c | 32 ++++ include/linux/videodev2.h | 16 ++ include/media/v4l2-ioctl.h | 2 + 8 files changed, 377 insertions(+), 8 deletions(-) create mode 100644 Documentation/DocBook/media/v4l/vidioc-create-bufs.xml create mode 100644 Documentation/DocBook/media/v4l/vidioc-prepare-buf.xml