On Mon 24 September 2012 14:17:00 Tomasz Stanislawski wrote:
> Hi Hans,
> Thank you for review.
>
> On 08/22/2012 12:47 PM, Hans Verkuil wrote:
> > On Tue August 14 2012 17:34:32 Tomasz Stanislawski wrote:
> >> This patch adds description and usage examples for importing
> >> DMABUF file descriptor in V4L2.
> >>
> >> Signed-off-by: Tomasz Stanislawski <t.stanislaws@xxxxxxxxxxx>
> >> Signed-off-by: Kyungmin Park <kyungmin.park@xxxxxxxxxxx>
> >> CC: linux-doc@xxxxxxxxxxxxxxx
> >> ---
> >> Documentation/DocBook/media/v4l/compat.xml | 4 +
> >> Documentation/DocBook/media/v4l/io.xml | 180 ++++++++++++++++++++
> >> .../DocBook/media/v4l/vidioc-create-bufs.xml | 3 +-
> >> Documentation/DocBook/media/v4l/vidioc-qbuf.xml | 15 ++
> >> Documentation/DocBook/media/v4l/vidioc-reqbufs.xml | 47 ++---
> >> 5 files changed, 226 insertions(+), 23 deletions(-)
> >>
>
> [snip]
>
> >> +&v4l2-plane; in the multi-planar API case). The driver must be switched into
> >> +DMABUF I/O mode by calling the &VIDIOC-REQBUFS; with the desired buffer type.
> >> +No buffers (planes) are allocated beforehand, consequently they are not indexed
> >> +and cannot be queried like mapped buffers with the
> >> +<constant>VIDIOC_QUERYBUF</constant> ioctl.</para>
> >
> > I disagree with that. Userptr buffers can use QUERYBUF just fine. Even for the
> > userptr you still have to fill in the buffer index when calling QBUF.
> >
> > So I see no reason why you couldn't use QUERYBUF in the DMABUF case. The only
> > difference is that the fd field is undefined (set to -1 perhaps?) if the bufffer
> > isn't queued.
> >
> > QUERYBUF can be very useful for debugging, for example to see what the status
> > is of each buffer and how many are queued.
> >
>
> Ok. I agree that QUERYBUF can be useful for debugging. The value of fd field
> should be the last value passed using QBUF. It would simplify streaming
> because an application would not have to keep the file descriptor around.
If a buffer isn't queued, then fd really is undefined. I don't see any advantage
of remembering the last value used there (and after a REQBUFS there isn't a 'last
value' anyway).
> >> +
> >> + <example>
> >> + <title>Initiating streaming I/O with DMABUF file descriptors</title>
> >> +
> >> + <programlisting>
> >> +&v4l2-requestbuffers; reqbuf;
> >> +
> >> +memset (&reqbuf, 0, sizeof (reqbuf));
> >> +reqbuf.type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
> >> +reqbuf.memory = V4L2_MEMORY_DMABUF;
> >> +reqbuf.count = 1;
> >> +
> >> +if (ioctl (fd, &VIDIOC-REQBUFS;, &reqbuf) == -1) {
> >> + if (errno == EINVAL)
> >> + printf ("Video capturing or DMABUF streaming is not supported\n");
> >> + else
> >> + perror ("VIDIOC_REQBUFS");
> >> +
> >> + exit (EXIT_FAILURE);
> >
> > Let's stick to the kernel coding style, so no ' ' before '(' in function calls.
> > Same for the other program examples below.
> >
>
> The ' ' before function was used for userptr and mmap usage examples.
> These examples should be fixed too.
>
> >> +}
> >> + </programlisting>
> >> + </example>
> >> +
> >> + <para>Buffer (plane) file descriptor is passed on the fly with the
> >
> > s/Buffer/The buffer/
> >
> >> +&VIDIOC-QBUF; ioctl. In case of multiplanar buffers, every plane can be
> >
> > 'Can be', 'should be' or 'must be'? Does it ever make sense to have the same
> > fd for different planes? Do we have restrictions on this in the userptr case?
> >
>
> I think that we should keep to 'can be'. I see no good reason to
> prevent the same dmabuf to be used for different planes.
> Allowing reusing of dmabufs with assistance of data_offset field
> would allow to pass a 2-planar YUV420 from V4L2-single-plane API
> to a driver with V4L2-multi-plane API.
That's true.
>
> [snip]
>
> >> diff --git a/Documentation/DocBook/media/v4l/vidioc-qbuf.xml b/Documentation/DocBook/media/v4l/vidioc-qbuf.xml
> >> index 77ff5be..436d21c 100644
> >> --- a/Documentation/DocBook/media/v4l/vidioc-qbuf.xml
> >> +++ b/Documentation/DocBook/media/v4l/vidioc-qbuf.xml
> >> @@ -109,6 +109,21 @@ they cannot be swapped out to disk. Buffers remain locked until
> >> dequeued, until the &VIDIOC-STREAMOFF; or &VIDIOC-REQBUFS; ioctl is
> >> called, or until the device is closed.</para>
> >>
> >> + <para>To enqueue a <link linkend="dmabuf">DMABUF</link> buffer applications
> >> +set the <structfield>memory</structfield> field to
> >> +<constant>V4L2_MEMORY_DMABUF</constant> and the <structfield>m.fd</structfield>
> >> +field to a file descriptor associated with a DMABUF buffer. When the
> >> +multi-planar API is used <structfield>m.fd</structfield> of the passed array of
> >
> > multi-planar API is used the <structfield>m.fd</structfield> fields of the passed array of
> >
> >> +&v4l2-plane; have to be used instead. When <constant>VIDIOC_QBUF</constant> is
> >> +called with a pointer to this structure the driver sets the
> >> +<constant>V4L2_BUF_FLAG_QUEUED</constant> flag and clears the
> >> +<constant>V4L2_BUF_FLAG_MAPPED</constant> and
> >> +<constant>V4L2_BUF_FLAG_DONE</constant> flags in the
What does it do to the PREPARED flag I wonder?
> >> +<structfield>flags</structfield> field, or it returns an error code. This
> >> +ioctl locks the buffer. Buffers remain locked until dequeued, until the
> >> +&VIDIOC-STREAMOFF; or &VIDIOC-REQBUFS; ioctl is called, or until the device is
> >> +closed.</para>
> >
> > You need to explain what a 'locked buffer' means.
>
> I propose following definition:
> "Locking a buffer means passing it to a driver for an access by hardware.
> "If an application accesses (reads/writes) a locked buffer then the result
> is undefined."
>
> What is your opinion about proposed definition?
Sounds good.
>
> >
> >> +
> >> <para>Applications call the <constant>VIDIOC_DQBUF</constant>
> >> ioctl to dequeue a filled (capturing) or displayed (output) buffer
> >> from the driver's outgoing queue. They just set the
> >> diff --git a/Documentation/DocBook/media/v4l/vidioc-reqbufs.xml b/Documentation/DocBook/media/v4l/vidioc-reqbufs.xml
> >> index d7c9505..20f4323 100644
> >> --- a/Documentation/DocBook/media/v4l/vidioc-reqbufs.xml
> >> +++ b/Documentation/DocBook/media/v4l/vidioc-reqbufs.xml
> >> @@ -48,28 +48,30 @@
> >> <refsect1>
> >> <title>Description</title>
> >>
> >> - <para>This ioctl is used to initiate <link linkend="mmap">memory
> >> -mapped</link> or <link linkend="userp">user pointer</link>
> >> -I/O. Memory mapped buffers are located in device memory and must be
> >> -allocated with this ioctl before they can be mapped into the
> >> -application's address space. User buffers are allocated by
> >> -applications themselves, and this ioctl is merely used to switch the
> >> -driver into user pointer I/O mode and to setup some internal structures.</para>
> >> +<para>This ioctl is used to initiate <link linkend="mmap">memory mapped</link>,
> >> +<link linkend="userp">user pointer</link> or <link
> >> +linkend="dmabuf">DMABUF</link> based I/O. Memory mapped buffers are located in
> >> +device memory and must be allocated with this ioctl before they can be mapped
> >> +into the application's address space. User buffers are allocated by
> >> +applications themselves, and this ioctl is merely used to switch the driver
> >> +into user pointer I/O mode and to setup some internal structures.
> >> +Similarly, DMABUF buffers are allocated by applications through a device
> >> +driver, and this ioctl only configures the driver into DMABUF I/O mode without
> >> +performing any direct allocation.</para>
> >>
> >> - <para>To allocate device buffers applications initialize all
> >> -fields of the <structname>v4l2_requestbuffers</structname> structure.
> >> -They set the <structfield>type</structfield> field to the respective
> >> -stream or buffer type, the <structfield>count</structfield> field to
> >> -the desired number of buffers, <structfield>memory</structfield>
> >> -must be set to the requested I/O method and 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
> >> -the requested number of buffers and it stores the actual number
> >> -allocated in the <structfield>count</structfield> field. It can be
> >> -smaller than the number requested, even zero, when the driver runs out
> >> -of free memory. A larger number is also possible when the driver requires
> >> -more buffers to function correctly. For example video output requires at least two buffers,
> >> -one displayed and one filled by the application.</para>
> >> + <para>To allocate device buffers applications initialize all fields of the
> >> +<structname>v4l2_requestbuffers</structname> structure. They set the
> >> +<structfield>type</structfield> field to the respective stream or buffer type,
> >> +the <structfield>count</structfield> field to the desired number of buffers,
> >> +<structfield>memory</structfield> must be set to the requested I/O method and
> >> +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 the
> >> +requested number of buffers and it stores the actual number allocated in the
> >> +<structfield>count</structfield> field. It can be smaller than the number
> >> +requested, even zero, when the driver runs out of free memory. A larger number
> >> +is also possible when the driver requires more buffers to function correctly.
> >> +For example video output requires at least two buffers, one displayed and one
> >> +filled by the application.</para>
> >> <para>When the I/O method is not supported the ioctl
> >> returns an &EINVAL;.</para>
> >>
> >> @@ -102,7 +104,8 @@ as the &v4l2-format; <structfield>type</structfield> field. See <xref
> >> <entry>__u32</entry>
> >> <entry><structfield>memory</structfield></entry>
> >> <entry>Applications set this field to
> >> -<constant>V4L2_MEMORY_MMAP</constant> or
> >> +<constant>V4L2_MEMORY_MMAP</constant>,
> >> +<constant>V4L2_MEMORY_DMABUF</constant> or
> >> <constant>V4L2_MEMORY_USERPTR</constant>. See <xref linkend="v4l2-memory"
> >> />.</entry>
> >> </row>
> >>
> >
> > You also have to update the VIDIOC_CREATE_BUFS ioctl documentation!
> >
> > I think the VIDIOC_PREPARE_BUF ioctl documentation is OK, but you might want to
> > check that yourself, just in case.
> >
>
> Ok.
> Regards,
> Tomasz Stanislawski
>
> > Regards,
> >
> > Hans
> >
>
> --
> 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
>
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
