Hi Guennadi, On Wed, Jul 20, 2011 at 01:43, Guennadi Liakhovetski <g.liakhovetski@xxxxxx> 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@xxxxxx> > --- > <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@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
