Hello everyone, This patchset adds support for DMABUF [2] importing to V4L2 stack. The support for DMABUF exporting was moved to separate patchset due to dependency on patches for DMA mapping redesign by Marek Szyprowski [4]. This patchset depends on new scatterlist constructor [5].
v7: - support for V4L2_MEMORY_DMABUF in v4l2-compact-ioctl32.c - cosmetic fixes to the documentation - added importing for vmalloc because vmap support in dmabuf for 3.5 was pull-requested - support for dmabuf importing for VIVI - resurrect allocation of dma-contig context - remove reference of alloc_ctx in dma-contig buffer - use sg_alloc_table_from_pages - fix DMA scatterlist calls to use orig_nents instead of nents - fix memleak in vb2_dc_sgt_foreach_page (use orig_nents instead of nents)
v6: - fixed missing entry in v4l2_memory_names - fixed a bug occuring after get_user_pages failure - fixed a bug caused by using invalid vma for get_user_pages - prepare/finish no longer call dma_sync for dmabuf buffers
v5: - removed change of importer/exporter behaviour - fixes vb2_dc_pages_to_sgt basing on Laurent's hints - changed pin/unpin words to lock/unlock in Doc
v4: - rebased on mainline 3.4-rc2 - included missing importing support for s5p-fimc and s5p-tv - added patch for changing map/unmap for importers - fixes to Documentation part - coding style fixes - pairing {map/unmap}_dmabuf in vb2-core - fixing variable types and semantic of arguments in videobufb2-dma-contig.c
v3: - rebased on mainline 3.4-rc1 - split 'code refactor' patch to multiple smaller patches - squashed fixes to Sumit's patches - patchset is no longer dependant on 'DMA mapping redesign' - separated path for handling IO and non-IO mappings - add documentation for DMABUF importing to V4L - removed all DMABUF exporter related code - removed usage of dma_get_pages extension
v2: - extended VIDIOC_EXPBUF argument from integer memoffset to struct v4l2_exportbuffer - added patch that breaks DMABUF spec on (un)map_atachment callcacks but allows to work with existing implementation of DMABUF prime in DRM - all dma-contig code refactoring patches were squashed - bugfixes
v1: List of changes since [1]. - support for DMA api extension dma_get_pages, the function is used to retrieve pages used to create DMA mapping. - small fixes/code cleanup to videobuf2 - added prepare and finish callbacks to vb2 allocators, it is used keep consistency between dma-cpu acess to the memory (by Marek Szyprowski) - support for exporting of DMABUF buffer in V4L2 and Videobuf2, originated from [3]. - support for dma-buf exporting in vb2-dma-contig allocator - support for DMABUF for s5p-tv and s5p-fimc (capture interface) drivers, originated from [3] - changed handling for userptr buffers (by Marek Szyprowski, Andrzej Pietrasiewicz) - let mmap method to use dma_mmap_writecombine call (by Marek Szyprowski)
[1] http://thread.gmane.org/gmane.linux.drivers.video-input-infrastructure/42966... [2] https://lkml.org/lkml/2011/12/26/29 [3] http://thread.gmane.org/gmane.linux.drivers.video-input-infrastructure/36354... [4] http://thread.gmane.org/gmane.linux.kernel.cross-arch/12819 [5] http://thread.gmane.org/gmane.linux.drivers.video-input-infrastructure/47983
Laurent Pinchart (2): v4l: vb2-dma-contig: Shorten vb2_dma_contig prefix to vb2_dc v4l: vb2-dma-contig: Reorder functions
Marek Szyprowski (2): v4l: vb2: add prepare/finish callbacks to allocators v4l: vb2-dma-contig: add prepare/finish to dma-contig allocator
Sumit Semwal (4): v4l: Add DMABUF as a memory type v4l: vb2: add support for shared buffer (dma_buf) v4l: vb: remove warnings about MEMORY_DMABUF v4l: vb2-dma-contig: add support for dma_buf importing
Tomasz Stanislawski (7): Documentation: media: description of DMABUF importing in V4L2 v4l: vb2-dma-contig: remove reference of alloc_ctx from a buffer v4l: vb2-dma-contig: add support for scatterlist in userptr mode v4l: vb2-vmalloc: add support for dmabuf importing v4l: vivi: support for dmabuf importing v4l: s5p-tv: mixer: support for dmabuf importing v4l: s5p-fimc: support for dmabuf importing
Documentation/DocBook/media/v4l/compat.xml | 4 + Documentation/DocBook/media/v4l/io.xml | 179 ++++++++ .../DocBook/media/v4l/vidioc-create-bufs.xml | 3 +- Documentation/DocBook/media/v4l/vidioc-qbuf.xml | 15 + Documentation/DocBook/media/v4l/vidioc-reqbufs.xml | 47 +- drivers/media/video/Kconfig | 1 + drivers/media/video/s5p-fimc/Kconfig | 1 + drivers/media/video/s5p-fimc/fimc-capture.c | 2 +- drivers/media/video/s5p-tv/Kconfig | 1 + drivers/media/video/s5p-tv/mixer_video.c | 2 +- drivers/media/video/v4l2-compat-ioctl32.c | 16 + drivers/media/video/v4l2-ioctl.c | 1 + drivers/media/video/videobuf-core.c | 4 + drivers/media/video/videobuf2-core.c | 207 ++++++++- drivers/media/video/videobuf2-dma-contig.c | 470 +++++++++++++++++--- drivers/media/video/videobuf2-vmalloc.c | 56 +++ drivers/media/video/vivi.c | 2 +- include/linux/videodev2.h | 7 + include/media/videobuf2-core.h | 34 ++ 19 files changed, 963 insertions(+), 89 deletions(-)
From: Sumit Semwal sumit.semwal@ti.com
Adds DMABUF memory type to v4l framework. Also adds the related file descriptor in v4l2_plane and v4l2_buffer.
Signed-off-by: Tomasz Stanislawski t.stanislaws@samsung.com [original work in the PoC for buffer sharing] Signed-off-by: Sumit Semwal sumit.semwal@ti.com Signed-off-by: Sumit Semwal sumit.semwal@linaro.org Acked-by: Laurent Pinchart laurent.pinchart@ideasonboard.com --- drivers/media/video/v4l2-compat-ioctl32.c | 16 ++++++++++++++++ drivers/media/video/v4l2-ioctl.c | 1 + include/linux/videodev2.h | 7 +++++++ 3 files changed, 24 insertions(+)
diff --git a/drivers/media/video/v4l2-compat-ioctl32.c b/drivers/media/video/v4l2-compat-ioctl32.c index 5327ad3..d33ab18 100644 --- a/drivers/media/video/v4l2-compat-ioctl32.c +++ b/drivers/media/video/v4l2-compat-ioctl32.c @@ -348,6 +348,9 @@ static int get_v4l2_plane32(struct v4l2_plane *up, struct v4l2_plane32 *up32, up_pln = compat_ptr(p); if (put_user((unsigned long)up_pln, &up->m.userptr)) return -EFAULT; + } else if (memory == V4L2_MEMORY_DMABUF) { + if (copy_in_user(&up->m.fd, &up32->m.fd, sizeof(int))) + return -EFAULT; } else { if (copy_in_user(&up->m.mem_offset, &up32->m.mem_offset, sizeof(__u32))) @@ -371,6 +374,11 @@ static int put_v4l2_plane32(struct v4l2_plane *up, struct v4l2_plane32 *up32, if (copy_in_user(&up32->m.mem_offset, &up->m.mem_offset, sizeof(__u32))) return -EFAULT; + /* For DMABUF, driver might've set up the fd, so copy it back. */ + if (memory == V4L2_MEMORY_DMABUF) + if (copy_in_user(&up32->m.fd, &up->m.fd, + sizeof(int))) + return -EFAULT;
return 0; } @@ -454,6 +462,10 @@ static int get_v4l2_buffer32(struct v4l2_buffer *kp, struct v4l2_buffer32 __user if (get_user(kp->m.offset, &up->m.offset)) return -EFAULT; break; + case V4L2_MEMORY_DMABUF: + if (get_user(kp->m.fd, &up->m.fd)) + return -EFAULT; + break; } }
@@ -518,6 +530,10 @@ static int put_v4l2_buffer32(struct v4l2_buffer *kp, struct v4l2_buffer32 __user if (put_user(kp->m.offset, &up->m.offset)) return -EFAULT; break; + case V4L2_MEMORY_DMABUF: + if (put_user(kp->m.fd, &up->m.fd)) + return -EFAULT; + break; } }
diff --git a/drivers/media/video/v4l2-ioctl.c b/drivers/media/video/v4l2-ioctl.c index 91be4e8..31fc2ad 100644 --- a/drivers/media/video/v4l2-ioctl.c +++ b/drivers/media/video/v4l2-ioctl.c @@ -175,6 +175,7 @@ static const char *v4l2_memory_names[] = { [V4L2_MEMORY_MMAP] = "mmap", [V4L2_MEMORY_USERPTR] = "userptr", [V4L2_MEMORY_OVERLAY] = "overlay", + [V4L2_MEMORY_DMABUF] = "dmabuf", };
#define prt_names(a, arr) ((((a) >= 0) && ((a) < ARRAY_SIZE(arr))) ? \ diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h index 370d111..51b20f4 100644 --- a/include/linux/videodev2.h +++ b/include/linux/videodev2.h @@ -185,6 +185,7 @@ enum v4l2_memory { V4L2_MEMORY_MMAP = 1, V4L2_MEMORY_USERPTR = 2, V4L2_MEMORY_OVERLAY = 3, + V4L2_MEMORY_DMABUF = 4, };
/* see also http://vektor.theorem.ca/graphics/ycbcr/ */ @@ -591,6 +592,8 @@ struct v4l2_requestbuffers { * should be passed to mmap() called on the video node) * @userptr: when memory is V4L2_MEMORY_USERPTR, a userspace pointer * pointing to this plane + * @fd: when memory is V4L2_MEMORY_DMABUF, a userspace file + * descriptor associated with this plane * @data_offset: offset in the plane to the start of data; usually 0, * unless there is a header in front of the data * @@ -605,6 +608,7 @@ struct v4l2_plane { union { __u32 mem_offset; unsigned long userptr; + int fd; } m; __u32 data_offset; __u32 reserved[11]; @@ -629,6 +633,8 @@ struct v4l2_plane { * (or a "cookie" that should be passed to mmap() as offset) * @userptr: for non-multiplanar buffers with memory == V4L2_MEMORY_USERPTR; * a userspace pointer pointing to this buffer + * @fd: for non-multiplanar buffers with memory == V4L2_MEMORY_DMABUF; + * a userspace file descriptor associated with this buffer * @planes: for multiplanar buffers; userspace pointer to the array of plane * info structs for this buffer * @length: size in bytes of the buffer (NOT its payload) for single-plane @@ -655,6 +661,7 @@ struct v4l2_buffer { __u32 offset; unsigned long userptr; struct v4l2_plane *planes; + int fd; } m; __u32 length; __u32 input;
Hi everyone, The automatic build system informed me that there is a shameful error in this patch.
The declarations of fd fields in v4l2_plane32 and v4l2_buffer32 are missing. It breaks build process for 64-bit architectures. I am deeply sorry for posting a patch without testing it enough.
The build-break will be fixed in the next version of the patchset.
Regards, Tomasz Stanislawski
On 06/14/2012 03:37 PM, Tomasz Stanislawski wrote:
From: Sumit Semwal sumit.semwal@ti.com
Adds DMABUF memory type to v4l framework. Also adds the related file descriptor in v4l2_plane and v4l2_buffer.
Signed-off-by: Tomasz Stanislawski t.stanislaws@samsung.com [original work in the PoC for buffer sharing] Signed-off-by: Sumit Semwal sumit.semwal@ti.com Signed-off-by: Sumit Semwal sumit.semwal@linaro.org Acked-by: Laurent Pinchart laurent.pinchart@ideasonboard.com
drivers/media/video/v4l2-compat-ioctl32.c | 16 ++++++++++++++++ drivers/media/video/v4l2-ioctl.c | 1 + include/linux/videodev2.h | 7 +++++++ 3 files changed, 24 insertions(+)
diff --git a/drivers/media/video/v4l2-compat-ioctl32.c b/drivers/media/video/v4l2-compat-ioctl32.c index 5327ad3..d33ab18 100644 --- a/drivers/media/video/v4l2-compat-ioctl32.c +++ b/drivers/media/video/v4l2-compat-ioctl32.c @@ -348,6 +348,9 @@ static int get_v4l2_plane32(struct v4l2_plane *up, struct v4l2_plane32 *up32, up_pln = compat_ptr(p); if (put_user((unsigned long)up_pln, &up->m.userptr)) return -EFAULT;
- } else if (memory == V4L2_MEMORY_DMABUF) {
if (copy_in_user(&up->m.fd, &up32->m.fd, sizeof(int)))
} else { if (copy_in_user(&up->m.mem_offset, &up32->m.mem_offset, sizeof(__u32)))return -EFAULT;
@@ -371,6 +374,11 @@ static int put_v4l2_plane32(struct v4l2_plane *up, struct v4l2_plane32 *up32, if (copy_in_user(&up32->m.mem_offset, &up->m.mem_offset, sizeof(__u32))) return -EFAULT;
- /* For DMABUF, driver might've set up the fd, so copy it back. */
- if (memory == V4L2_MEMORY_DMABUF)
if (copy_in_user(&up32->m.fd, &up->m.fd,
sizeof(int)))
return -EFAULT;
return 0; } @@ -454,6 +462,10 @@ static int get_v4l2_buffer32(struct v4l2_buffer *kp, struct v4l2_buffer32 __user if (get_user(kp->m.offset, &up->m.offset)) return -EFAULT; break;
case V4L2_MEMORY_DMABUF:
if (get_user(kp->m.fd, &up->m.fd))
return -EFAULT;
} }break;
@@ -518,6 +530,10 @@ static int put_v4l2_buffer32(struct v4l2_buffer *kp, struct v4l2_buffer32 __user if (put_user(kp->m.offset, &up->m.offset)) return -EFAULT; break;
case V4L2_MEMORY_DMABUF:
if (put_user(kp->m.fd, &up->m.fd))
return -EFAULT;
} }break;
diff --git a/drivers/media/video/v4l2-ioctl.c b/drivers/media/video/v4l2-ioctl.c index 91be4e8..31fc2ad 100644 --- a/drivers/media/video/v4l2-ioctl.c +++ b/drivers/media/video/v4l2-ioctl.c @@ -175,6 +175,7 @@ static const char *v4l2_memory_names[] = { [V4L2_MEMORY_MMAP] = "mmap", [V4L2_MEMORY_USERPTR] = "userptr", [V4L2_MEMORY_OVERLAY] = "overlay",
- [V4L2_MEMORY_DMABUF] = "dmabuf",
}; #define prt_names(a, arr) ((((a) >= 0) && ((a) < ARRAY_SIZE(arr))) ? \ diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h index 370d111..51b20f4 100644 --- a/include/linux/videodev2.h +++ b/include/linux/videodev2.h @@ -185,6 +185,7 @@ enum v4l2_memory { V4L2_MEMORY_MMAP = 1, V4L2_MEMORY_USERPTR = 2, V4L2_MEMORY_OVERLAY = 3,
- V4L2_MEMORY_DMABUF = 4,
}; /* see also http://vektor.theorem.ca/graphics/ycbcr/ */ @@ -591,6 +592,8 @@ struct v4l2_requestbuffers {
should be passed to mmap() called on the video node)
- @userptr: when memory is V4L2_MEMORY_USERPTR, a userspace pointer
pointing to this plane
- @fd: when memory is V4L2_MEMORY_DMABUF, a userspace file
descriptor associated with this plane
- @data_offset: offset in the plane to the start of data; usually 0,
unless there is a header in front of the data
@@ -605,6 +608,7 @@ struct v4l2_plane { union { __u32 mem_offset; unsigned long userptr;
} m; __u32 data_offset; __u32 reserved[11];int fd;
@@ -629,6 +633,8 @@ struct v4l2_plane {
(or a "cookie" that should be passed to mmap() as offset)
- @userptr: for non-multiplanar buffers with memory == V4L2_MEMORY_USERPTR;
a userspace pointer pointing to this buffer
- @fd: for non-multiplanar buffers with memory == V4L2_MEMORY_DMABUF;
a userspace file descriptor associated with this buffer
- @planes: for multiplanar buffers; userspace pointer to the array of plane
info structs for this buffer
- @length: size in bytes of the buffer (NOT its payload) for single-plane
@@ -655,6 +661,7 @@ struct v4l2_buffer { __u32 offset; unsigned long userptr; struct v4l2_plane *planes;
} m; __u32 length; __u32 input;int fd;
This patch adds description and usage examples for importing DMABUF file descriptor in V4L2.
Signed-off-by: Tomasz Stanislawski t.stanislaws@samsung.com Signed-off-by: Kyungmin Park kyungmin.park@samsung.com CC: linux-doc@vger.kernel.org --- Documentation/DocBook/media/v4l/compat.xml | 4 + Documentation/DocBook/media/v4l/io.xml | 179 ++++++++++++++++++++ .../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, 225 insertions(+), 23 deletions(-)
diff --git a/Documentation/DocBook/media/v4l/compat.xml b/Documentation/DocBook/media/v4l/compat.xml index ea42ef8..07a311f 100644 --- a/Documentation/DocBook/media/v4l/compat.xml +++ b/Documentation/DocBook/media/v4l/compat.xml @@ -2587,6 +2587,10 @@ ioctls.</para> <para><link linkend="v4l2-auto-focus-area"><constant> V4L2_CID_AUTO_FOCUS_AREA</constant></link> control.</para> </listitem> + <listitem> + <para>Importing DMABUF file descriptors as a new IO method described + in <xref linkend="dmabuf" />.</para> + </listitem> </itemizedlist> </section>
diff --git a/Documentation/DocBook/media/v4l/io.xml b/Documentation/DocBook/media/v4l/io.xml index fd6aca2..f55b0ab 100644 --- a/Documentation/DocBook/media/v4l/io.xml +++ b/Documentation/DocBook/media/v4l/io.xml @@ -472,6 +472,162 @@ rest should be evident.</para> </footnote></para> </section>
+ <section id="dmabuf"> + <title>Streaming I/O (DMA buffer importing)</title> + + <note> + <title>Experimental</title> + <para>This is an <link linkend="experimental"> experimental </link> + interface and may change in the future.</para> + </note> + +<para>The DMABUF framework provides a generic mean for sharing buffers between + multiple devices. Device drivers that support DMABUF can export a DMA buffer +to userspace as a file descriptor (known as the exporter role), import a DMA +buffer from userspace using a file descriptor previously exported for a +different or the same device (known as the importer role), or both. This +section describes the DMABUF importer role API in V4L2.</para> + +<para>Input and output devices support the streaming I/O method when the +<constant>V4L2_CAP_STREAMING</constant> flag in the +<structfield>capabilities</structfield> field of &v4l2-capability; returned by +the &VIDIOC-QUERYCAP; ioctl is set. Whether importing DMA buffers through +DMABUF file descriptors is supported is determined by calling the +&VIDIOC-REQBUFS; ioctl with the memory type set to +<constant>V4L2_MEMORY_DMABUF</constant>.</para> + + <para>This I/O method is dedicated for sharing DMA buffers between V4L and +other APIs. Buffers (planes) are allocated by a driver on behalf of the +application, and exported to the application as file descriptors using an API +specific to the allocator driver. Only those file descriptor are exchanged, +these files and meta-information are passed in &v4l2-buffer; (or in +&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> + + <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; + +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); +} + </programlisting> + </example> + + <para>Buffer (plane) file is passed on the fly with the &VIDIOC-QBUF; +ioctl. In case of multiplanar buffers, every plane can be associated with a +different DMABUF descriptor.Although buffers are commonly cycled, applications +can pass different DMABUF descriptor at each <constant>VIDIOC_QBUF</constant> +call.</para> + + <example> + <title>Queueing DMABUF using single plane API</title> + + <programlisting> +int buffer_queue(int v4lfd, int index, int dmafd) +{ + &v4l2-buffer; buf; + + memset(&buf, 0, sizeof buf); + buf.type = V4L2_BUF_TYPE_VIDEO_CAPTURE; + buf.memory = V4L2_MEMORY_DMABUF; + buf.index = index; + buf.m.fd = dmafd; + + if (ioctl (v4lfd, &VIDIOC-QBUF;, &buf) == -1) { + perror ("VIDIOC_QBUF"); + return -1; + } + + return 0; +} + </programlisting> + </example> + + <example> + <title>Queueing DMABUF using multi plane API</title> + + <programlisting> +int buffer_queue_mp(int v4lfd, int index, int dmafd[], int n_planes) +{ + &v4l2-buffer; buf; + &v4l2-plane; planes[VIDEO_MAX_PLANES]; + int i; + + memset(&buf, 0, sizeof buf); + buf.type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE; + buf.memory = V4L2_MEMORY_DMABUF; + buf.index = index; + buf.m.planes = planes; + buf.length = n_planes; + + memset(&planes, 0, sizeof planes); + + for (i = 0; i < n_planes; ++i) + buf.m.planes[i].m.fd = dmafd[i]; + + if (ioctl (v4lfd, &VIDIOC-QBUF;, &buf) == -1) { + perror ("VIDIOC_QBUF"); + return -1; + } + + return 0; +} + </programlisting> + </example> + + <para>Filled or displayed buffers are dequeued with the +&VIDIOC-DQBUF; ioctl. The driver can unlock the buffer at any +time between the completion of the DMA and this ioctl. The memory is +also unlocked when &VIDIOC-STREAMOFF; is called, &VIDIOC-REQBUFS;, or +when the device is closed.</para> + + <para>For capturing applications it is customary to enqueue a +number of empty buffers, to start capturing and enter the read loop. +Here the application waits until a filled buffer can be dequeued, and +re-enqueues the buffer when the data is no longer needed. Output +applications fill and enqueue buffers, when enough buffers are stacked +up output is started. In the write loop, when the application +runs out of free buffers it must wait until an empty buffer can be +dequeued and reused. Two methods exist to suspend execution of the +application until one or more buffers can be dequeued. By default +<constant>VIDIOC_DQBUF</constant> blocks when no buffer is in the +outgoing queue. When the <constant>O_NONBLOCK</constant> flag was +given to the &func-open; function, <constant>VIDIOC_DQBUF</constant> +returns immediately with an &EAGAIN; when no buffer is available. The +&func-select; or &func-poll; function are always available.</para> + + <para>To start and stop capturing or output applications call the +&VIDIOC-STREAMON; and &VIDIOC-STREAMOFF; ioctls. Note that +<constant>VIDIOC_STREAMOFF</constant> removes all buffers from both queues and +unlocks all buffers as a side effect. Since there is no notion of doing +anything "now" on a multitasking system, if an application needs to synchronize +with another event it should examine the &v4l2-buffer; +<structfield>timestamp</structfield> of captured buffers, or set the field +before enqueuing buffers for output.</para> + + <para>Drivers implementing DMABUF importing I/O must support the +<constant>VIDIOC_REQBUFS</constant>, <constant>VIDIOC_QBUF</constant>, +<constant>VIDIOC_DQBUF</constant>, <constant>VIDIOC_STREAMON</constant> and +<constant>VIDIOC_STREAMOFF</constant> ioctl, the <function>select()</function> +and <function>poll()</function> function.</para> + + </section> + <section id="async"> <title>Asynchronous I/O</title>
@@ -673,6 +829,14 @@ memory, set by the application. See <xref linkend="userp" /> for details. <structname>v4l2_buffer</structname> structure.</entry> </row> <row> + <entry></entry> + <entry>int</entry> + <entry><structfield>fd</structfield></entry> + <entry>For the single-plane API and when +<structfield>memory</structfield> is <constant>V4L2_MEMORY_DMABUF</constant> this +is the file descriptor associated with a DMABUF buffer.</entry> + </row> + <row> <entry>__u32</entry> <entry><structfield>length</structfield></entry> <entry></entry> @@ -748,6 +912,15 @@ should set this to 0.</entry> </entry> </row> <row> + <entry></entry> + <entry>int</entry> + <entry><structfield>fd</structfield></entry> + <entry>When the memory type in the containing &v4l2-buffer; is + <constant>V4L2_MEMORY_DMABUF</constant>, this is a file + descriptor associated with a DMABUF buffer, similar to the + <structfield>fd</structfield> field in &v4l2-buffer;.</entry> + </row> + <row> <entry>__u32</entry> <entry><structfield>data_offset</structfield></entry> <entry></entry> @@ -982,6 +1155,12 @@ pointer</link> I/O.</entry> <entry>3</entry> <entry>[to do]</entry> </row> + <row> + <entry><constant>V4L2_MEMORY_DMABUF</constant></entry> + <entry>4</entry> + <entry>The buffer is used for <link linkend="dmabuf">DMA shared +buffer</link> I/O.</entry> + </row> </tbody> </tgroup> </table> diff --git a/Documentation/DocBook/media/v4l/vidioc-create-bufs.xml b/Documentation/DocBook/media/v4l/vidioc-create-bufs.xml index 765549f..4444c66 100644 --- a/Documentation/DocBook/media/v4l/vidioc-create-bufs.xml +++ b/Documentation/DocBook/media/v4l/vidioc-create-bufs.xml @@ -103,7 +103,8 @@ information.</para> <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> diff --git a/Documentation/DocBook/media/v4l/vidioc-qbuf.xml b/Documentation/DocBook/media/v4l/vidioc-qbuf.xml index 9caa49a..cb5f5ff 100644 --- a/Documentation/DocBook/media/v4l/vidioc-qbuf.xml +++ b/Documentation/DocBook/media/v4l/vidioc-qbuf.xml @@ -112,6 +112,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> +to a file descriptor associated with a DMABUF buffer. When the multi-planar API is +used and <structfield>m.fd</structfield> 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 +<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> + <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>
Hi Thomas,
Thanks for the patch.
On Thursday 14 June 2012 15:37:36 Tomasz Stanislawski wrote:
This patch adds description and usage examples for importing DMABUF file descriptor in V4L2.
Signed-off-by: Tomasz Stanislawski t.stanislaws@samsung.com Signed-off-by: Kyungmin Park kyungmin.park@samsung.com CC: linux-doc@vger.kernel.org
I just have a couple of minor comments, after taking them into account you can add
Acked-by: Laurent Pinchart laurent.pinchart@ideasonboard.com
Documentation/DocBook/media/v4l/compat.xml | 4 + Documentation/DocBook/media/v4l/io.xml | 179 +++++++++++++++++ .../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, 225 insertions(+), 23 deletions(-)
[snip]
diff --git a/Documentation/DocBook/media/v4l/io.xml b/Documentation/DocBook/media/v4l/io.xml index fd6aca2..f55b0ab 100644 --- a/Documentation/DocBook/media/v4l/io.xml +++ b/Documentation/DocBook/media/v4l/io.xml
[snip]
<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;
You need to set reqbuf.count to a non-zero value.
+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);
+}
</programlisting>
</example>
- <para>Buffer (plane) file is passed on the fly with the &VIDIOC-QBUF;
s/file/file descriptor/
+ioctl. In case of multiplanar buffers, every plane can be associated with a +different DMABUF descriptor.Although buffers are commonly cycled,
s/descriptor./descriptor. /
applications +can pass different DMABUF descriptor at each
s/pass/pass a/
<constant>VIDIOC_QBUF</constant> +call.</para>
<example>
<title>Queueing DMABUF using single plane API</title>
<programlisting>
+int buffer_queue(int v4lfd, int index, int dmafd) +{
- &v4l2-buffer; buf;
- memset(&buf, 0, sizeof buf);
- buf.type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
- buf.memory = V4L2_MEMORY_DMABUF;
- buf.index = index;
- buf.m.fd = dmafd;
- if (ioctl (v4lfd, &VIDIOC-QBUF;, &buf) == -1) {
perror ("VIDIOC_QBUF");
return -1;
- }
- return 0;
+}
</programlisting>
</example>
<example>
<title>Queueing DMABUF using multi plane API</title>
<programlisting>
+int buffer_queue_mp(int v4lfd, int index, int dmafd[], int n_planes) +{
- &v4l2-buffer; buf;
- &v4l2-plane; planes[VIDEO_MAX_PLANES];
- int i;
- memset(&buf, 0, sizeof buf);
- buf.type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE;
- buf.memory = V4L2_MEMORY_DMABUF;
- buf.index = index;
- buf.m.planes = planes;
- buf.length = n_planes;
- memset(&planes, 0, sizeof planes);
- for (i = 0; i < n_planes; ++i)
buf.m.planes[i].m.fd = dmafd[i];
- if (ioctl (v4lfd, &VIDIOC-QBUF;, &buf) == -1) {
perror ("VIDIOC_QBUF");
return -1;
- }
- return 0;
+}
</programlisting>
</example>
- <para>Filled or displayed buffers are dequeued with the
+&VIDIOC-DQBUF; ioctl. The driver can unlock the buffer at any +time between the completion of the DMA and this ioctl. The memory is +also unlocked when &VIDIOC-STREAMOFF; is called, &VIDIOC-REQBUFS;, or +when the device is closed.</para>
- <para>For capturing applications it is customary to enqueue a
+number of empty buffers, to start capturing and enter the read loop. +Here the application waits until a filled buffer can be dequeued, and +re-enqueues the buffer when the data is no longer needed. Output +applications fill and enqueue buffers, when enough buffers are stacked +up output is started. In the write loop, when the application +runs out of free buffers it must wait until an empty buffer can be +dequeued and reused. Two methods exist to suspend execution of the +application until one or more buffers can be dequeued. By default +<constant>VIDIOC_DQBUF</constant> blocks when no buffer is in the +outgoing queue. When the <constant>O_NONBLOCK</constant> flag was +given to the &func-open; function, <constant>VIDIOC_DQBUF</constant> +returns immediately with an &EAGAIN; when no buffer is available. The +&func-select; or &func-poll; function are always available.</para>
- <para>To start and stop capturing or output applications call the
+&VIDIOC-STREAMON; and &VIDIOC-STREAMOFF; ioctls. Note that +<constant>VIDIOC_STREAMOFF</constant> removes all buffers from both queues and +unlocks all buffers as a side effect. Since there is no notion of doing +anything "now" on a multitasking system, if an application needs to synchronize +with another event it should examine the &v4l2-buffer; +<structfield>timestamp</structfield> of captured buffers, or set the field +before enqueuing buffers for output.</para>
- <para>Drivers implementing DMABUF importing I/O must support the
+<constant>VIDIOC_REQBUFS</constant>, <constant>VIDIOC_QBUF</constant>, +<constant>VIDIOC_DQBUF</constant>, <constant>VIDIOC_STREAMON</constant> and +<constant>VIDIOC_STREAMOFF</constant> ioctl, the
s/ioctl/ioctls/ s/, the/, and the/
<function>select()</function> +and <function>poll()</function> function.</para>
s/function/functions/
</section>
<section id="async"> <title>Asynchronous I/O</title>
@@ -673,6 +829,14 @@ memory, set by the application. See <xref linkend="userp" /> for details. <structname>v4l2_buffer</structname> structure.</entry> </row> <row>
<entry></entry>
<entry>int</entry>
<entry><structfield>fd</structfield></entry>
<entry>For the single-plane API and when
+<structfield>memory</structfield> is <constant>V4L2_MEMORY_DMABUF</constant> this +is the file descriptor associated with a DMABUF buffer.</entry>
</row>
<row> <entry>__u32</entry> <entry><structfield>length</structfield></entry> <entry></entry>
@@ -748,6 +912,15 @@ should set this to 0.</entry> </entry> </row> <row>
<entry></entry>
<entry>int</entry>
<entry><structfield>fd</structfield></entry>
<entry>When the memory type in the containing &v4l2-buffer; is
<constant>V4L2_MEMORY_DMABUF</constant>, this is a file
descriptor associated with a DMABUF buffer, similar to the
<structfield>fd</structfield> field in &v4l2-buffer;.</entry>
</row>
<row> <entry>__u32</entry> <entry><structfield>data_offset</structfield></entry> <entry></entry>
@@ -982,6 +1155,12 @@ pointer</link> I/O.</entry> <entry>3</entry> <entry>[to do]</entry> </row>
<row>
<entry><constant>V4L2_MEMORY_DMABUF</constant></entry>
<entry>4</entry>
<entry>The buffer is used for <link linkend="dmabuf">DMA shared
+buffer</link> I/O.</entry>
</row>
</tbody> </tgroup> </table>
diff --git a/Documentation/DocBook/media/v4l/vidioc-create-bufs.xml b/Documentation/DocBook/media/v4l/vidioc-create-bufs.xml index 765549f..4444c66 100644 --- a/Documentation/DocBook/media/v4l/vidioc-create-bufs.xml +++ b/Documentation/DocBook/media/v4l/vidioc-create-bufs.xml @@ -103,7 +103,8 @@ information.</para> <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> diff --git a/Documentation/DocBook/media/v4l/vidioc-qbuf.xml b/Documentation/DocBook/media/v4l/vidioc-qbuf.xml index 9caa49a..cb5f5ff 100644 --- a/Documentation/DocBook/media/v4l/vidioc-qbuf.xml +++ b/Documentation/DocBook/media/v4l/vidioc-qbuf.xml @@ -112,6 +112,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>
s/$/ field/
+to a file descriptor associated with a DMABUF buffer. When the multi-planar API is +used and
s/and//
<structfield>m.fd</structfield> 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 +<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>
- <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
From: Sumit Semwal sumit.semwal@ti.com
This patch adds support for DMABUF memory type in videobuf2. It calls relevant APIs of dma_buf for v4l reqbuf / qbuf / dqbuf operations.
For this version, the support is for videobuf2 as a user of the shared buffer; so the allocation of the buffer is done outside of V4L2. [A sample allocator of dma-buf shared buffer is given at [1]]
[1]: Rob Clark's DRM: https://github.com/robclark/kernel-omap4/commits/drmplane-dmabuf
Signed-off-by: Tomasz Stanislawski t.stanislaws@samsung.com [original work in the PoC for buffer sharing] Signed-off-by: Sumit Semwal sumit.semwal@ti.com Signed-off-by: Sumit Semwal sumit.semwal@linaro.org Acked-by: Laurent Pinchart laurent.pinchart@ideasonboard.com --- drivers/media/video/videobuf2-core.c | 196 +++++++++++++++++++++++++++++++++- include/media/videobuf2-core.h | 27 +++++ 2 files changed, 219 insertions(+), 4 deletions(-)
diff --git a/drivers/media/video/videobuf2-core.c b/drivers/media/video/videobuf2-core.c index 9d4e9ed..f43cfa4 100644 --- a/drivers/media/video/videobuf2-core.c +++ b/drivers/media/video/videobuf2-core.c @@ -109,6 +109,36 @@ static void __vb2_buf_userptr_put(struct vb2_buffer *vb) }
/** + * __vb2_plane_dmabuf_put() - release memory associated with + * a DMABUF shared plane + */ +static void __vb2_plane_dmabuf_put(struct vb2_queue *q, struct vb2_plane *p) +{ + if (!p->mem_priv) + return; + + if (p->dbuf_mapped) + call_memop(q, unmap_dmabuf, p->mem_priv); + + call_memop(q, detach_dmabuf, p->mem_priv); + dma_buf_put(p->dbuf); + memset(p, 0, sizeof *p); +} + +/** + * __vb2_buf_dmabuf_put() - release memory associated with + * a DMABUF shared buffer + */ +static void __vb2_buf_dmabuf_put(struct vb2_buffer *vb) +{ + struct vb2_queue *q = vb->vb2_queue; + unsigned int plane; + + for (plane = 0; plane < vb->num_planes; ++plane) + __vb2_plane_dmabuf_put(q, &vb->planes[plane]); +} + +/** * __setup_offsets() - setup unique offsets ("cookies") for every plane in * every buffer on the queue */ @@ -230,6 +260,8 @@ static void __vb2_free_mem(struct vb2_queue *q, unsigned int buffers) /* Free MMAP buffers or release USERPTR buffers */ if (q->memory == V4L2_MEMORY_MMAP) __vb2_buf_mem_free(vb); + else if (q->memory == V4L2_MEMORY_DMABUF) + __vb2_buf_dmabuf_put(vb); else __vb2_buf_userptr_put(vb); } @@ -352,6 +384,12 @@ static int __fill_v4l2_buffer(struct vb2_buffer *vb, struct v4l2_buffer *b) */ memcpy(b->m.planes, vb->v4l2_planes, b->length * sizeof(struct v4l2_plane)); + + if (q->memory == V4L2_MEMORY_DMABUF) { + unsigned int plane; + for (plane = 0; plane < vb->num_planes; ++plane) + b->m.planes[plane].m.fd = 0; + } } else { /* * We use length and offset in v4l2_planes array even for @@ -363,6 +401,8 @@ static int __fill_v4l2_buffer(struct vb2_buffer *vb, struct v4l2_buffer *b) b->m.offset = vb->v4l2_planes[0].m.mem_offset; else if (q->memory == V4L2_MEMORY_USERPTR) b->m.userptr = vb->v4l2_planes[0].m.userptr; + else if (q->memory == V4L2_MEMORY_DMABUF) + b->m.fd = 0; }
/* @@ -454,6 +494,20 @@ static int __verify_mmap_ops(struct vb2_queue *q) }
/** + * __verify_dmabuf_ops() - verify that all memory operations required for + * DMABUF queue type have been provided + */ +static int __verify_dmabuf_ops(struct vb2_queue *q) +{ + if (!(q->io_modes & VB2_DMABUF) || !q->mem_ops->attach_dmabuf || + !q->mem_ops->detach_dmabuf || !q->mem_ops->map_dmabuf || + !q->mem_ops->unmap_dmabuf) + return -EINVAL; + + return 0; +} + +/** * vb2_reqbufs() - Initiate streaming * @q: videobuf2 queue * @req: struct passed from userspace to vidioc_reqbufs handler in driver @@ -486,8 +540,9 @@ int vb2_reqbufs(struct vb2_queue *q, struct v4l2_requestbuffers *req) return -EBUSY; }
- if (req->memory != V4L2_MEMORY_MMAP - && req->memory != V4L2_MEMORY_USERPTR) { + if (req->memory != V4L2_MEMORY_MMAP && + req->memory != V4L2_MEMORY_DMABUF && + req->memory != V4L2_MEMORY_USERPTR) { dprintk(1, "reqbufs: unsupported memory type\n"); return -EINVAL; } @@ -516,6 +571,11 @@ int vb2_reqbufs(struct vb2_queue *q, struct v4l2_requestbuffers *req) return -EINVAL; }
+ if (req->memory == V4L2_MEMORY_DMABUF && __verify_dmabuf_ops(q)) { + dprintk(1, "reqbufs: DMABUF for current setup unsupported\n"); + return -EINVAL; + } + if (req->count == 0 || q->num_buffers != 0 || q->memory != req->memory) { /* * We already have buffers allocated, so first check if they @@ -622,8 +682,9 @@ int vb2_create_bufs(struct vb2_queue *q, struct v4l2_create_buffers *create) return -EBUSY; }
- if (create->memory != V4L2_MEMORY_MMAP - && create->memory != V4L2_MEMORY_USERPTR) { + if (create->memory != V4L2_MEMORY_MMAP && + create->memory != V4L2_MEMORY_USERPTR && + create->memory != V4L2_MEMORY_DMABUF) { dprintk(1, "%s(): unsupported memory type\n", __func__); return -EINVAL; } @@ -647,6 +708,11 @@ int vb2_create_bufs(struct vb2_queue *q, struct v4l2_create_buffers *create) return -EINVAL; }
+ if (create->memory == V4L2_MEMORY_DMABUF && __verify_dmabuf_ops(q)) { + dprintk(1, "%s(): DMABUF for current setup unsupported\n", __func__); + return -EINVAL; + } + if (q->num_buffers == VIDEO_MAX_FRAME) { dprintk(1, "%s(): maximum number of buffers already allocated\n", __func__); @@ -842,6 +908,14 @@ static int __fill_vb2_buffer(struct vb2_buffer *vb, const struct v4l2_buffer *b, b->m.planes[plane].length; } } + if (b->memory == V4L2_MEMORY_DMABUF) { + for (plane = 0; plane < vb->num_planes; ++plane) { + v4l2_planes[plane].bytesused = + b->m.planes[plane].bytesused; + v4l2_planes[plane].m.fd = + b->m.planes[plane].m.fd; + } + } } else { /* * Single-planar buffers do not use planes array, @@ -856,6 +930,10 @@ static int __fill_vb2_buffer(struct vb2_buffer *vb, const struct v4l2_buffer *b, v4l2_planes[0].m.userptr = b->m.userptr; v4l2_planes[0].length = b->length; } + + if (b->memory == V4L2_MEMORY_DMABUF) + v4l2_planes[0].m.fd = b->m.fd; + }
vb->v4l2_buf.field = b->field; @@ -960,6 +1038,100 @@ static int __qbuf_mmap(struct vb2_buffer *vb, const struct v4l2_buffer *b) }
/** + * __qbuf_dmabuf() - handle qbuf of a DMABUF buffer + */ +static int __qbuf_dmabuf(struct vb2_buffer *vb, const struct v4l2_buffer *b) +{ + struct v4l2_plane planes[VIDEO_MAX_PLANES]; + struct vb2_queue *q = vb->vb2_queue; + void *mem_priv; + unsigned int plane; + int ret; + int write = !V4L2_TYPE_IS_OUTPUT(q->type); + + /* Verify and copy relevant information provided by the userspace */ + ret = __fill_vb2_buffer(vb, b, planes); + if (ret) + return ret; + + for (plane = 0; plane < vb->num_planes; ++plane) { + struct dma_buf *dbuf = dma_buf_get(planes[plane].m.fd); + + if (IS_ERR_OR_NULL(dbuf)) { + dprintk(1, "qbuf: invalid dmabuf fd for " + "plane %d\n", plane); + ret = -EINVAL; + goto err; + } + + /* Skip the plane if already verified */ + if (dbuf == vb->planes[plane].dbuf) { + planes[plane].length = dbuf->size; + dma_buf_put(dbuf); + continue; + } + + dprintk(3, "qbuf: buffer description for plane %d changed, " + "reattaching dma buf\n", plane); + + /* Release previously acquired memory if present */ + __vb2_plane_dmabuf_put(q, &vb->planes[plane]); + + /* Acquire each plane's memory */ + mem_priv = call_memop(q, attach_dmabuf, q->alloc_ctx[plane], + dbuf, q->plane_sizes[plane], write); + if (IS_ERR(mem_priv)) { + dprintk(1, "qbuf: failed acquiring dmabuf " + "memory for plane %d\n", plane); + ret = PTR_ERR(mem_priv); + goto err; + } + + planes[plane].length = dbuf->size; + vb->planes[plane].dbuf = dbuf; + vb->planes[plane].mem_priv = mem_priv; + } + + /* TODO: This pins the buffer(s) with dma_buf_map_attachment()).. but + * really we want to do this just before the DMA, not while queueing + * the buffer(s).. + */ + for (plane = 0; plane < vb->num_planes; ++plane) { + ret = call_memop(q, map_dmabuf, vb->planes[plane].mem_priv); + if (ret) { + dprintk(1, "qbuf: failed mapping dmabuf " + "memory for plane %d\n", plane); + goto err; + } + vb->planes[plane].dbuf_mapped = 1; + } + + /* + * Call driver-specific initialization on the newly acquired buffer, + * if provided. + */ + ret = call_qop(q, buf_init, vb); + if (ret) { + dprintk(1, "qbuf: buffer initialization failed\n"); + goto err; + } + + /* + * Now that everything is in order, copy relevant information + * provided by userspace. + */ + for (plane = 0; plane < vb->num_planes; ++plane) + vb->v4l2_planes[plane] = planes[plane]; + + return 0; +err: + /* In case of errors, release planes that were already acquired */ + __vb2_buf_dmabuf_put(vb); + + return ret; +} + +/** * __enqueue_in_driver() - enqueue a vb2_buffer in driver for processing */ static void __enqueue_in_driver(struct vb2_buffer *vb) @@ -983,6 +1155,9 @@ static int __buf_prepare(struct vb2_buffer *vb, const struct v4l2_buffer *b) case V4L2_MEMORY_USERPTR: ret = __qbuf_userptr(vb, b); break; + case V4L2_MEMORY_DMABUF: + ret = __qbuf_dmabuf(vb, b); + break; default: WARN(1, "Invalid queue type\n"); ret = -EINVAL; @@ -1338,6 +1513,19 @@ int vb2_dqbuf(struct vb2_queue *q, struct v4l2_buffer *b, bool nonblocking) return ret; }
+ /* TODO: this unpins the buffer(dma_buf_unmap_attachment()).. but + * really we want to do this just after DMA, not when the + * buffer is dequeued.. + */ + if (q->memory == V4L2_MEMORY_DMABUF) { + unsigned int i; + + for (i = 0; i < vb->num_planes; ++i) { + call_memop(q, unmap_dmabuf, vb->planes[i].mem_priv); + vb->planes[i].dbuf_mapped = 0; + } + } + switch (vb->state) { case VB2_BUF_STATE_DONE: dprintk(3, "dqbuf: Returning done buffer\n"); diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h index a15d1f1..859bbaf 100644 --- a/include/media/videobuf2-core.h +++ b/include/media/videobuf2-core.h @@ -16,6 +16,7 @@ #include <linux/mutex.h> #include <linux/poll.h> #include <linux/videodev2.h> +#include <linux/dma-buf.h>
struct vb2_alloc_ctx; struct vb2_fileio_data; @@ -41,6 +42,20 @@ struct vb2_fileio_data; * argument to other ops in this structure * @put_userptr: inform the allocator that a USERPTR buffer will no longer * be used + * @attach_dmabuf: attach a shared struct dma_buf for a hardware operation; + * used for DMABUF memory types; alloc_ctx is the alloc context + * dbuf is the shared dma_buf; returns NULL on failure; + * allocator private per-buffer structure on success; + * this needs to be used for further accesses to the buffer + * @detach_dmabuf: inform the exporter of the buffer that the current DMABUF + * buffer is no longer used; the buf_priv argument is the + * allocator private per-buffer structure previously returned + * from the attach_dmabuf callback + * @map_dmabuf: request for access to the dmabuf from allocator; the allocator + * of dmabuf is informed that this driver is going to use the + * dmabuf + * @unmap_dmabuf: releases access control to the dmabuf - allocator is notified + * that this driver is done using the dmabuf for now * @vaddr: return a kernel virtual address to a given memory buffer * associated with the passed private structure or NULL if no * such mapping exists @@ -56,6 +71,8 @@ struct vb2_fileio_data; * Required ops for USERPTR types: get_userptr, put_userptr. * Required ops for MMAP types: alloc, put, num_users, mmap. * Required ops for read/write access types: alloc, put, num_users, vaddr + * Required ops for DMABUF types: attach_dmabuf, detach_dmabuf, map_dmabuf, + * unmap_dmabuf. */ struct vb2_mem_ops { void *(*alloc)(void *alloc_ctx, unsigned long size); @@ -65,6 +82,12 @@ struct vb2_mem_ops { unsigned long size, int write); void (*put_userptr)(void *buf_priv);
+ void *(*attach_dmabuf)(void *alloc_ctx, struct dma_buf *dbuf, + unsigned long size, int write); + void (*detach_dmabuf)(void *buf_priv); + int (*map_dmabuf)(void *buf_priv); + void (*unmap_dmabuf)(void *buf_priv); + void *(*vaddr)(void *buf_priv); void *(*cookie)(void *buf_priv);
@@ -75,6 +98,8 @@ struct vb2_mem_ops {
struct vb2_plane { void *mem_priv; + struct dma_buf *dbuf; + unsigned int dbuf_mapped; };
/** @@ -83,12 +108,14 @@ struct vb2_plane { * @VB2_USERPTR: driver supports USERPTR with streaming API * @VB2_READ: driver supports read() style access * @VB2_WRITE: driver supports write() style access + * @VB2_DMABUF: driver supports DMABUF with streaming API */ enum vb2_io_modes { VB2_MMAP = (1 << 0), VB2_USERPTR = (1 << 1), VB2_READ = (1 << 2), VB2_WRITE = (1 << 3), + VB2_DMABUF = (1 << 4), };
/**
Hi Tomasz,Sumit,
We have been doing performance analysis for video playback scenarios. Could get significant improvement in FPS (33% or 8~10 fps) for full HD resolution when the map/unmap sequence is not called. This is a considerable improvement in performance and would be helpful to have some logical way using e.g. "dmabuf hint" to avoid what looks like a bottleneck (map/unmap sequence).
For your reference I have I have copied the changes below. Please note this is a change for verification into the vb2 core to add an additional condition check for avoiding the calls to map/unmap if the buffer has been already mapped once.
diff --git a/drivers/media/video/videobuf2-core.c b/drivers/media/video/videobuf2-core.c index 1e0917d..e48a7fc 100644 --- a/drivers/media/video/videobuf2-core.c +++ b/drivers/media/video/videobuf2-core.c @@ -1098,12 +1098,15 @@ static int __qbuf_dmabuf(struct vb2_buffer *vb, const struct v4l2_buffer *b) * really we want to do this just before the DMA, not while queueing * the buffer(s).. */ + for (plane = 0; plane < vb->num_planes; ++plane) { - ret = call_memop(q, map_dmabuf, vb->planes[plane].mem_priv); - if (ret) { - dprintk(1, "qbuf: failed mapping dmabuf " - "memory for plane %d\n", plane); - goto err; + if( vb->planes[plane].dbuf_mapped != 1) { + ret = call_memop(q, map_dmabuf, vb->planes[plane].mem_priv); + if (ret) { + dprintk(1, "qbuf: failed mapping dmabuf " + "memory for plane %d\n", plane); + goto err; + } } vb->planes[plane].dbuf_mapped = 1; } @@ -1525,14 +1528,6 @@ int vb2_dqbuf(struct vb2_queue *q, struct v4l2_buffer *b, bool nonblocking) * really we want to do this just after DMA, not when the * buffer is dequeued.. */ - if (q->memory == V4L2_MEMORY_DMABUF) { - unsigned int i; - - for (i = 0; i < vb->num_planes; ++i) { - call_memop(q, unmap_dmabuf, vb->planes[i].mem_priv); - vb->planes[i].dbuf_mapped = 0; - } - }
switch (vb->state) { case VB2_BUF_STATE_DONE:
Hi Abhinav, Could you provide more information about the testing scenario? Was it MFC + FIMC, or MFC + DRM, or MFC + FIMC + DRM, or other?
According to the DMABUF spec map/unmap calls should be done just before a buffer is accessed by HW. In case of vb2, dma_buf_map_attachment is called just after passing the buffer to the vb2 queue. IMO, it is too early. The mapping call should be moved to __enqueue_in_driver function one day.
As I understand the DMABUF assumptions, it is exporter code to be responsible for caching mappings for dmabuf attachments.
I applied some caching policy in vb2-dma-contig exporter. The similar treat should be ported to the DRM code. I am waiting for opinions about this solution.
Regards, Tomasz Stanislawski
On 06/14/2012 06:48 PM, Abhinav Kochhar wrote:
Hi Tomasz,Sumit,
We have been doing performance analysis for video playback scenarios. Could get significant improvement in FPS (33% or 8~10 fps) for full HD resolution when the map/unmap sequence is not called. This is a considerable improvement in performance and would be helpful to have some logical way using e.g. "dmabuf hint" to avoid what looks like a bottleneck (map/unmap sequence).
For your reference I have I have copied the changes below. Please note this is a change for verification into the vb2 core to add an additional condition check for avoiding the calls to map/unmap if the buffer has been already mapped once.
diff --git a/drivers/media/video/videobuf2-core.c b/drivers/media/video/videobuf2-core.c index 1e0917d..e48a7fc 100644 --- a/drivers/media/video/videobuf2-core.c +++ b/drivers/media/video/videobuf2-core.c @@ -1098,12 +1098,15 @@ static int __qbuf_dmabuf(struct vb2_buffer *vb, const struct v4l2_buffer *b) * really we want to do this just before the DMA, not while queueing * the buffer(s).. */
- for (plane = 0; plane < vb->num_planes; ++plane) {
ret = call_memop(q, map_dmabuf, vb->planes[plane].mem_priv);
if (ret) {
dprintk(1, "qbuf: failed mapping dmabuf "
"memory for plane %d\n", plane);
goto err;
if( vb->planes[plane].dbuf_mapped != 1) {
ret = call_memop(q, map_dmabuf, vb->planes[plane].mem_priv);
if (ret) {
dprintk(1, "qbuf: failed mapping dmabuf "
"memory for plane %d\n", plane);
goto err;
} vb->planes[plane].dbuf_mapped = 1; }}
@@ -1525,14 +1528,6 @@ int vb2_dqbuf(struct vb2_queue *q, struct v4l2_buffer *b, bool nonblocking) * really we want to do this just after DMA, not when the * buffer is dequeued.. */
if (q->memory == V4L2_MEMORY_DMABUF) {
unsigned int i;
for (i = 0; i < vb->num_planes; ++i) {
call_memop(q, unmap_dmabuf, vb->planes[i].mem_priv);
vb->planes[i].dbuf_mapped = 0;
}
}
switch (vb->state) { case VB2_BUF_STATE_DONE:
Tomasz,
I've encountered an issue with this patch when userspace does several stream_on/stream_off cycles. When the user tries to qbuf a buffer after doing stream_off, we trigger the "dmabuf already pinned" warning since we didn't unmap the buffer as dqbuf was never called.
The below patch adds calls to unmap in queue_cancel, but my feeling is that we probably should be calling detach too (i.e. put_dmabuf).
Thoughts?
--Dima
Subject: [PATCH] v4l: vb2: unmap dmabufs on STREAM_OFF event
Currently, if the user issues a STREAM_OFF request and then tries to re-enqueue buffers, it will trigger a warning in the vb2 allocators as the buffer would still be mapped from before STREAM_OFF was called. The current expectation is that buffers will be unmapped in dqbuf, but that will never be called on the mapped buffers after a STREAM_OFF event.
Cc: Sumit Semwal sumit.semwal@ti.com Cc: Tomasz Stanislawski t.stanislaws@samsung.com Signed-off-by: Dima Zavin dima@android.com --- drivers/media/video/videobuf2-core.c | 22 ++++++++++++++++++++-- 1 files changed, 20 insertions(+), 2 deletions(-)
diff --git a/drivers/media/video/videobuf2-core.c b/drivers/media/video/videobuf2-core.c index b431dc6..e2a8f12 100644 --- a/drivers/media/video/videobuf2-core.c +++ b/drivers/media/video/videobuf2-core.c @@ -1592,8 +1592,26 @@ static void __vb2_queue_cancel(struct vb2_queue *q) /* * Reinitialize all buffers for next use. */ - for (i = 0; i < q->num_buffers; ++i) - q->bufs[i]->state = VB2_BUF_STATE_DEQUEUED; + for (i = 0; i < q->num_buffers; ++i) { + struct vb2_buffer *vb = q->bufs[i]; + int plane; + + vb->state = VB2_BUF_STATE_DEQUEUED; + + if (q->memory != V4L2_MEMORY_DMABUF) + continue; + + for (plane = 0; plane < vb->num_planes; ++plane) { + struct vb2_plane *p = &vb->planes[plane]; + + if (!p->mem_priv) + continue; + if (p->dbuf_mapped) { + call_memop(q, unmap_dmabuf, p->mem_priv); + p->dbuf_mapped = 0; + } + } + } }
/**
ping?
On Tue, Jun 19, 2012 at 11:12 PM, Dima Zavin dmitriyz@google.com wrote:
Tomasz,
I've encountered an issue with this patch when userspace does several stream_on/stream_off cycles. When the user tries to qbuf a buffer after doing stream_off, we trigger the "dmabuf already pinned" warning since we didn't unmap the buffer as dqbuf was never called.
The below patch adds calls to unmap in queue_cancel, but my feeling is that we probably should be calling detach too (i.e. put_dmabuf).
Thoughts?
--Dima
Subject: [PATCH] v4l: vb2: unmap dmabufs on STREAM_OFF event
Currently, if the user issues a STREAM_OFF request and then tries to re-enqueue buffers, it will trigger a warning in the vb2 allocators as the buffer would still be mapped from before STREAM_OFF was called. The current expectation is that buffers will be unmapped in dqbuf, but that will never be called on the mapped buffers after a STREAM_OFF event.
Cc: Sumit Semwal sumit.semwal@ti.com Cc: Tomasz Stanislawski t.stanislaws@samsung.com Signed-off-by: Dima Zavin dima@android.com
drivers/media/video/videobuf2-core.c | 22 ++++++++++++++++++++-- 1 files changed, 20 insertions(+), 2 deletions(-)
diff --git a/drivers/media/video/videobuf2-core.c b/drivers/media/video/videobuf2-core.c index b431dc6..e2a8f12 100644 --- a/drivers/media/video/videobuf2-core.c +++ b/drivers/media/video/videobuf2-core.c @@ -1592,8 +1592,26 @@ static void __vb2_queue_cancel(struct vb2_queue *q) /* * Reinitialize all buffers for next use. */
for (i = 0; i < q->num_buffers; ++i)
q->bufs[i]->state = VB2_BUF_STATE_DEQUEUED;
for (i = 0; i < q->num_buffers; ++i) {
struct vb2_buffer *vb = q->bufs[i];
int plane;
vb->state = VB2_BUF_STATE_DEQUEUED;
if (q->memory != V4L2_MEMORY_DMABUF)
continue;
for (plane = 0; plane < vb->num_planes; ++plane) {
struct vb2_plane *p = &vb->planes[plane];
if (!p->mem_priv)
continue;
if (p->dbuf_mapped) {
call_memop(q, unmap_dmabuf, p->mem_priv);
p->dbuf_mapped = 0;
}
}
}
}
/**
1.7.7.3
Hi Dima Zavin, Thank you for the patch and for a ping remainder :).
You are right. The unmap is missing in __vb2_queue_cancel. I will apply your fix into next version of V4L2 support for dmabuf.
Please refer to some comments below.
On 06/20/2012 08:12 AM, Dima Zavin wrote:
Tomasz,
I've encountered an issue with this patch when userspace does several stream_on/stream_off cycles. When the user tries to qbuf a buffer after doing stream_off, we trigger the "dmabuf already pinned" warning since we didn't unmap the buffer as dqbuf was never called.
The below patch adds calls to unmap in queue_cancel, but my feeling is that we probably should be calling detach too (i.e. put_dmabuf).
Thoughts?
--Dima
Subject: [PATCH] v4l: vb2: unmap dmabufs on STREAM_OFF event
Currently, if the user issues a STREAM_OFF request and then tries to re-enqueue buffers, it will trigger a warning in the vb2 allocators as the buffer would still be mapped from before STREAM_OFF was called. The current expectation is that buffers will be unmapped in dqbuf, but that will never be called on the mapped buffers after a STREAM_OFF event.
Cc: Sumit Semwal sumit.semwal@ti.com Cc: Tomasz Stanislawski t.stanislaws@samsung.com Signed-off-by: Dima Zavin dima@android.com
drivers/media/video/videobuf2-core.c | 22 ++++++++++++++++++++-- 1 files changed, 20 insertions(+), 2 deletions(-)
diff --git a/drivers/media/video/videobuf2-core.c b/drivers/media/video/videobuf2-core.c index b431dc6..e2a8f12 100644 --- a/drivers/media/video/videobuf2-core.c +++ b/drivers/media/video/videobuf2-core.c @@ -1592,8 +1592,26 @@ static void __vb2_queue_cancel(struct vb2_queue *q) /* * Reinitialize all buffers for next use. */
- for (i = 0; i < q->num_buffers; ++i)
q->bufs[i]->state = VB2_BUF_STATE_DEQUEUED;
- for (i = 0; i < q->num_buffers; ++i) {
struct vb2_buffer *vb = q->bufs[i];
int plane;
vb->state = VB2_BUF_STATE_DEQUEUED;
if (q->memory != V4L2_MEMORY_DMABUF)
continue;
for (plane = 0; plane < vb->num_planes; ++plane) {
struct vb2_plane *p = &vb->planes[plane];
if (!p->mem_priv)
continue;
is the check above really needed? No check like this is done in vb2_dqbuf.
if (p->dbuf_mapped) {
If a buffer is queued then it is also mapped, so dbuf_mapped should be always be true here (at least in theory).
call_memop(q, unmap_dmabuf, p->mem_priv);
p->dbuf_mapped = 0;
}
}
- }
} /**
Regards, Tomasz Stanislawski
Hi Dima and Tomasz,
Sorry for the late reply.
On Tuesday 26 June 2012 10:40:44 Tomasz Stanislawski wrote:
Hi Dima Zavin, Thank you for the patch and for a ping remainder :).
You are right. The unmap is missing in __vb2_queue_cancel. I will apply your fix into next version of V4L2 support for dmabuf.
Please refer to some comments below.
On 06/20/2012 08:12 AM, Dima Zavin wrote:
Tomasz,
I've encountered an issue with this patch when userspace does several stream_on/stream_off cycles. When the user tries to qbuf a buffer after doing stream_off, we trigger the "dmabuf already pinned" warning since we didn't unmap the buffer as dqbuf was never called.
The below patch adds calls to unmap in queue_cancel, but my feeling is that we probably should be calling detach too (i.e. put_dmabuf).
According to the V4L2 specification, the "VIDIOC_STREAMOFF ioctl, apart of aborting or finishing any DMA in progress, unlocks any user pointer buffers locked in physical memory, and it removes all buffers from the incoming and outgoing queues".
Detaching the buffer is thus not strictly required. At first thought I agreed with you, as not deatching the buffer might keep resources allocated for much longer than needed. For instance, an application that stops the stream and expects to resume it later will usually not free the buffers (with VIDIOC_REQBUFS(0)) between VIDIOC_STREAMOFF and VIDIOC_STREAMON. Buffer will thus be referenced for longer than needed.
However, to reuse the same buffer after restarting the stream, the application will need to keep the dmabuf fds around in order to queue them. Detaching the buffer will thus bring little benefit in terms of resource usage, as the open file handles will keep the buffer around anyway. If an application cares about that and closes all dmabuf fds after stopping the stream, I expect it to free the buffers as well.
I don't have a very strong opinion about this, if you would rather detach the buffer at stream-off time I'm fine with that.
Thoughts?
--Dima
Subject: [PATCH] v4l: vb2: unmap dmabufs on STREAM_OFF event
Currently, if the user issues a STREAM_OFF request and then tries to re-enqueue buffers, it will trigger a warning in the vb2 allocators as the buffer would still be mapped from before STREAM_OFF was called. The current expectation is that buffers will be unmapped in dqbuf, but that will never be called on the mapped buffers after a STREAM_OFF event.
Cc: Sumit Semwal sumit.semwal@ti.com Cc: Tomasz Stanislawski t.stanislaws@samsung.com Signed-off-by: Dima Zavin dima@android.com
drivers/media/video/videobuf2-core.c | 22 ++++++++++++++++++++-- 1 files changed, 20 insertions(+), 2 deletions(-)
diff --git a/drivers/media/video/videobuf2-core.c b/drivers/media/video/videobuf2-core.c index b431dc6..e2a8f12 100644 --- a/drivers/media/video/videobuf2-core.c +++ b/drivers/media/video/videobuf2-core.c @@ -1592,8 +1592,26 @@ static void __vb2_queue_cancel(struct vb2_queue *q)
/* * Reinitialize all buffers for next use. */
- for (i = 0; i < q->num_buffers; ++i)
q->bufs[i]->state = VB2_BUF_STATE_DEQUEUED;
- for (i = 0; i < q->num_buffers; ++i) {
struct vb2_buffer *vb = q->bufs[i];
int plane;
vb->state = VB2_BUF_STATE_DEQUEUED;
if (q->memory != V4L2_MEMORY_DMABUF)
continue;
Don't we need to do something similat for USERPTR buffers as well ? They don't seem to get unpinned (put_userptr) at stream-off time.
If we decide to detach the buffer as well as unmapping it, we could just call __vb2_buf_put and __vb2_buf_userptr put here. If we don't, the code might still be simplified by adding an argument to __vb2_buf_dmabuf_put to select whether to unmap and detach the buffer, or just unmap it.
for (plane = 0; plane < vb->num_planes; ++plane) {
struct vb2_plane *p = &vb->planes[plane];
if (!p->mem_priv)
continue;
is the check above really needed? No check like this is done in vb2_dqbuf.
I think the check comes from __vb2_plane_dmabuf_put. If the buffer is not queued mem_priv will be NULL. However, that might be redundant with the next check
if (p->dbuf_mapped) {
If a buffer is queued then it is also mapped, so dbuf_mapped should be always be true here (at least in theory).
The buffer might never have been queued.
call_memop(q, unmap_dmabuf, p->mem_priv);
p->dbuf_mapped = 0;
}
}
- }
} /**
On Tue 26 June 2012 11:11:06 Laurent Pinchart wrote:
Hi Dima and Tomasz,
Sorry for the late reply.
On Tuesday 26 June 2012 10:40:44 Tomasz Stanislawski wrote:
Hi Dima Zavin, Thank you for the patch and for a ping remainder :).
You are right. The unmap is missing in __vb2_queue_cancel. I will apply your fix into next version of V4L2 support for dmabuf.
Please refer to some comments below.
On 06/20/2012 08:12 AM, Dima Zavin wrote:
Tomasz,
I've encountered an issue with this patch when userspace does several stream_on/stream_off cycles. When the user tries to qbuf a buffer after doing stream_off, we trigger the "dmabuf already pinned" warning since we didn't unmap the buffer as dqbuf was never called.
The below patch adds calls to unmap in queue_cancel, but my feeling is that we probably should be calling detach too (i.e. put_dmabuf).
According to the V4L2 specification, the "VIDIOC_STREAMOFF ioctl, apart of aborting or finishing any DMA in progress, unlocks any user pointer buffers locked in physical memory, and it removes all buffers from the incoming and outgoing queues".
Correct. And what that means in practice is that after a streamoff all buffers are returned to the state they had just before STREAMON was called.
So after STREAMOFF you can immediately queue all buffers again with QBUF and call STREAMON to restart streaming. No mmap or other operations should be required. This behavior must be kept.
VIDIOC_REQBUFS() or a close() are the only two operations that will actually free the buffers completely.
In practice, a STREAMOFF is either followed by a STREAMON at a later time, or almost immediately followed by REQBUFS or close() to tear down the buffers. So I don't think the buffers should be detached at streamoff.
Regards,
Hans
Detaching the buffer is thus not strictly required. At first thought I agreed with you, as not deatching the buffer might keep resources allocated for much longer than needed. For instance, an application that stops the stream and expects to resume it later will usually not free the buffers (with VIDIOC_REQBUFS(0)) between VIDIOC_STREAMOFF and VIDIOC_STREAMON. Buffer will thus be referenced for longer than needed.
However, to reuse the same buffer after restarting the stream, the application will need to keep the dmabuf fds around in order to queue them. Detaching the buffer will thus bring little benefit in terms of resource usage, as the open file handles will keep the buffer around anyway. If an application cares about that and closes all dmabuf fds after stopping the stream, I expect it to free the buffers as well.
I don't have a very strong opinion about this, if you would rather detach the buffer at stream-off time I'm fine with that.
Thoughts?
--Dima
Subject: [PATCH] v4l: vb2: unmap dmabufs on STREAM_OFF event
Currently, if the user issues a STREAM_OFF request and then tries to re-enqueue buffers, it will trigger a warning in the vb2 allocators as the buffer would still be mapped from before STREAM_OFF was called. The current expectation is that buffers will be unmapped in dqbuf, but that will never be called on the mapped buffers after a STREAM_OFF event.
Cc: Sumit Semwal sumit.semwal@ti.com Cc: Tomasz Stanislawski t.stanislaws@samsung.com Signed-off-by: Dima Zavin dima@android.com
drivers/media/video/videobuf2-core.c | 22 ++++++++++++++++++++-- 1 files changed, 20 insertions(+), 2 deletions(-)
diff --git a/drivers/media/video/videobuf2-core.c b/drivers/media/video/videobuf2-core.c index b431dc6..e2a8f12 100644 --- a/drivers/media/video/videobuf2-core.c +++ b/drivers/media/video/videobuf2-core.c @@ -1592,8 +1592,26 @@ static void __vb2_queue_cancel(struct vb2_queue *q)
/* * Reinitialize all buffers for next use. */
- for (i = 0; i < q->num_buffers; ++i)
q->bufs[i]->state = VB2_BUF_STATE_DEQUEUED;
- for (i = 0; i < q->num_buffers; ++i) {
struct vb2_buffer *vb = q->bufs[i];
int plane;
vb->state = VB2_BUF_STATE_DEQUEUED;
if (q->memory != V4L2_MEMORY_DMABUF)
continue;
Don't we need to do something similat for USERPTR buffers as well ? They don't seem to get unpinned (put_userptr) at stream-off time.
If we decide to detach the buffer as well as unmapping it, we could just call __vb2_buf_put and __vb2_buf_userptr put here. If we don't, the code might still be simplified by adding an argument to __vb2_buf_dmabuf_put to select whether to unmap and detach the buffer, or just unmap it.
for (plane = 0; plane < vb->num_planes; ++plane) {
struct vb2_plane *p = &vb->planes[plane];
if (!p->mem_priv)
continue;
is the check above really needed? No check like this is done in vb2_dqbuf.
I think the check comes from __vb2_plane_dmabuf_put. If the buffer is not queued mem_priv will be NULL. However, that might be redundant with the next check
if (p->dbuf_mapped) {
If a buffer is queued then it is also mapped, so dbuf_mapped should be always be true here (at least in theory).
The buffer might never have been queued.
call_memop(q, unmap_dmabuf, p->mem_priv);
p->dbuf_mapped = 0;
}
}
- }
} /**
Hans and Laurent,
Thanks for the feedback.
On Tue, Jun 26, 2012 at 2:40 AM, Hans Verkuil hverkuil@xs4all.nl wrote:
On Tue 26 June 2012 11:11:06 Laurent Pinchart wrote:
Hi Dima and Tomasz,
Sorry for the late reply.
On Tuesday 26 June 2012 10:40:44 Tomasz Stanislawski wrote:
Hi Dima Zavin, Thank you for the patch and for a ping remainder :).
You are right. The unmap is missing in __vb2_queue_cancel. I will apply your fix into next version of V4L2 support for dmabuf.
Please refer to some comments below.
On 06/20/2012 08:12 AM, Dima Zavin wrote:
Tomasz,
I've encountered an issue with this patch when userspace does several stream_on/stream_off cycles. When the user tries to qbuf a buffer after doing stream_off, we trigger the "dmabuf already pinned" warning since we didn't unmap the buffer as dqbuf was never called.
The below patch adds calls to unmap in queue_cancel, but my feeling is that we probably should be calling detach too (i.e. put_dmabuf).
According to the V4L2 specification, the "VIDIOC_STREAMOFF ioctl, apart of aborting or finishing any DMA in progress, unlocks any user pointer buffers locked in physical memory, and it removes all buffers from the incoming and outgoing queues".
Correct. And what that means in practice is that after a streamoff all buffers are returned to the state they had just before STREAMON was called.
That can't be right. The buffers had to have been returned to the state just *after REQBUFS*, not just *before STREAMON*. You need to re-enqueue buffers before calling STREAMON. I assume that's what you meant?
So after STREAMOFF you can immediately queue all buffers again with QBUF and call STREAMON to restart streaming. No mmap or other operations should be required. This behavior must be kept.
VIDIOC_REQBUFS() or a close() are the only two operations that will actually free the buffers completely.
In practice, a STREAMOFF is either followed by a STREAMON at a later time, or almost immediately followed by REQBUFS or close() to tear down the buffers. So I don't think the buffers should be detached at streamoff.
I agree. I was leaning this way which is why I left it out of my patch and wanted to hear your guys' opinion as you are much more familiar with the intended behavior than I am.
Thanks!
--Dima
Regards,
Hans
Detaching the buffer is thus not strictly required. At first thought I agreed with you, as not deatching the buffer might keep resources allocated for much longer than needed. For instance, an application that stops the stream and expects to resume it later will usually not free the buffers (with VIDIOC_REQBUFS(0)) between VIDIOC_STREAMOFF and VIDIOC_STREAMON. Buffer will thus be referenced for longer than needed.
However, to reuse the same buffer after restarting the stream, the application will need to keep the dmabuf fds around in order to queue them. Detaching the buffer will thus bring little benefit in terms of resource usage, as the open file handles will keep the buffer around anyway. If an application cares about that and closes all dmabuf fds after stopping the stream, I expect it to free the buffers as well.
I don't have a very strong opinion about this, if you would rather detach the buffer at stream-off time I'm fine with that.
Thoughts?
--Dima
Subject: [PATCH] v4l: vb2: unmap dmabufs on STREAM_OFF event
Currently, if the user issues a STREAM_OFF request and then tries to re-enqueue buffers, it will trigger a warning in the vb2 allocators as the buffer would still be mapped from before STREAM_OFF was called. The current expectation is that buffers will be unmapped in dqbuf, but that will never be called on the mapped buffers after a STREAM_OFF event.
Cc: Sumit Semwal sumit.semwal@ti.com Cc: Tomasz Stanislawski t.stanislaws@samsung.com Signed-off-by: Dima Zavin dima@android.com
drivers/media/video/videobuf2-core.c | Â 22 ++++++++++++++++++++-- Â 1 files changed, 20 insertions(+), 2 deletions(-)
diff --git a/drivers/media/video/videobuf2-core.c b/drivers/media/video/videobuf2-core.c index b431dc6..e2a8f12 100644 --- a/drivers/media/video/videobuf2-core.c +++ b/drivers/media/video/videobuf2-core.c @@ -1592,8 +1592,26 @@ static void __vb2_queue_cancel(struct vb2_queue *q)
/* Â Â * Reinitialize all buffers for next use. Â Â */
- for (i = 0; i < q->num_buffers; ++i)
- q->bufs[i]->state = VB2_BUF_STATE_DEQUEUED;
- for (i = 0; i < q->num_buffers; ++i) {
- struct vb2_buffer *vb = q->bufs[i];
- int plane;
- vb->state = VB2_BUF_STATE_DEQUEUED;
- if (q->memory != V4L2_MEMORY_DMABUF)
- continue;
Don't we need to do something similat for USERPTR buffers as well ? They don't seem to get unpinned (put_userptr) at stream-off time.
If we decide to detach the buffer as well as unmapping it, we could just call __vb2_buf_put and __vb2_buf_userptr put here. If we don't, the code might still be simplified by adding an argument to __vb2_buf_dmabuf_put to select whether to unmap and detach the buffer, or just unmap it.
- for (plane = 0; plane < vb->num_planes; ++plane) {
- struct vb2_plane *p = &vb->planes[plane];
- if (!p->mem_priv)
- continue;
is the check above really needed? No check like this is done in vb2_dqbuf.
I think the check comes from __vb2_plane_dmabuf_put. If the buffer is not queued mem_priv will be NULL. However, that might be redundant with the next check
- if (p->dbuf_mapped) {
If a buffer is queued then it is also mapped, so dbuf_mapped should be always be true here (at least in theory).
The buffer might never have been queued.
- call_memop(q, unmap_dmabuf, p->mem_priv);
- p->dbuf_mapped = 0;
- }
- }
- }
}
/**
Hi Dima,
On Tuesday 26 June 2012 13:53:34 Dima Zavin wrote:
On Tue, Jun 26, 2012 at 2:40 AM, Hans Verkuil hverkuil@xs4all.nl wrote:
On Tue 26 June 2012 11:11:06 Laurent Pinchart wrote:
On Tuesday 26 June 2012 10:40:44 Tomasz Stanislawski wrote:
Hi Dima Zavin, Thank you for the patch and for a ping remainder :).
You are right. The unmap is missing in __vb2_queue_cancel. I will apply your fix into next version of V4L2 support for dmabuf.
Please refer to some comments below.
On 06/20/2012 08:12 AM, Dima Zavin wrote:
Tomasz,
I've encountered an issue with this patch when userspace does several stream_on/stream_off cycles. When the user tries to qbuf a buffer after doing stream_off, we trigger the "dmabuf already pinned" warning since we didn't unmap the buffer as dqbuf was never called.
The below patch adds calls to unmap in queue_cancel, but my feeling is that we probably should be calling detach too (i.e. put_dmabuf).
According to the V4L2 specification, the "VIDIOC_STREAMOFF ioctl, apart of aborting or finishing any DMA in progress, unlocks any user pointer buffers locked in physical memory, and it removes all buffers from the incoming and outgoing queues".
Correct. And what that means in practice is that after a streamoff all buffers are returned to the state they had just before STREAMON was called.
That can't be right. The buffers had to have been returned to the state just *after REQBUFS*, not just *before STREAMON*. You need to re-enqueue buffers before calling STREAMON. I assume that's what you meant?
Your interpretation is correct.
So after STREAMOFF you can immediately queue all buffers again with QBUF and call STREAMON to restart streaming. No mmap or other operations should be required. This behavior must be kept.
VIDIOC_REQBUFS() or a close() are the only two operations that will actually free the buffers completely.
In practice, a STREAMOFF is either followed by a STREAMON at a later time, or almost immediately followed by REQBUFS or close() to tear down the buffers. So I don't think the buffers should be detached at streamoff.
I agree. I was leaning this way which is why I left it out of my patch and wanted to hear your guys' opinion as you are much more familiar with the intended behavior than I am.
Thanks!
You're welcome. Thank you for reporting the problem and providing a patch.
Hi Laurent, Hi Dima,
On 06/27/2012 10:40 PM, Laurent Pinchart wrote:
Hi Dima,
On Tuesday 26 June 2012 13:53:34 Dima Zavin wrote:
On Tue, Jun 26, 2012 at 2:40 AM, Hans Verkuil hverkuil@xs4all.nl wrote:
On Tue 26 June 2012 11:11:06 Laurent Pinchart wrote:
On Tuesday 26 June 2012 10:40:44 Tomasz Stanislawski wrote:
Hi Dima Zavin, Thank you for the patch and for a ping remainder :).
You are right. The unmap is missing in __vb2_queue_cancel. I will apply your fix into next version of V4L2 support for dmabuf.
Please refer to some comments below.
On 06/20/2012 08:12 AM, Dima Zavin wrote:
Tomasz,
I've encountered an issue with this patch when userspace does several stream_on/stream_off cycles. When the user tries to qbuf a buffer after doing stream_off, we trigger the "dmabuf already pinned" warning since we didn't unmap the buffer as dqbuf was never called.
The below patch adds calls to unmap in queue_cancel, but my feeling is that we probably should be calling detach too (i.e. put_dmabuf).
According to the V4L2 specification, the "VIDIOC_STREAMOFF ioctl, apart of aborting or finishing any DMA in progress, unlocks any user pointer buffers locked in physical memory, and it removes all buffers from the incoming and outgoing queues".
Correct. And what that means in practice is that after a streamoff all buffers are returned to the state they had just before STREAMON was called.
That can't be right. The buffers had to have been returned to the state just *after REQBUFS*, not just *before STREAMON*. You need to re-enqueue buffers before calling STREAMON. I assume that's what you meant?
Your interpretation is correct.
So now we should decide what should be changed: the spec or vb2 ? Bringing the queue state back to *after REQBUFS* may make the next (STREAMON + QBUFs) very costly operations.
Reattaching and mapping a DMABUF might trigger DMA allocation and *will* trigger creation of IOMMU mappings. In case of a user pointer, calling next get_user_pages may cause numerous fault events and will *create* new IOMMU mapping.
Is there any need to do such a cleanup if the destruction of buffers and all caches can be explicitly executed by REQBUFS(count = 0) ?
Maybe it would be easier to change the spec by removing "apart of ... in physical memory" part?
STREAMOFF should mean stopping streaming, and that resources are no longer used. DMABUFs are unmapped but unmapping does not mean releasing. The exporter may keep the resource in its caches.
Currently, vb2 does not follow the policy from the spec. The put_userptr ops is called on: - VIDIOC_REBUFS - VIDIOC_CREATE_BUFS - vb2_queue_release() which is usually called on close() syscall
The put_userptr is not called and streamoff therefore the user pages are locked after STREAMOFF.
BTW. What does 'buffer locked' mean?
Does it mean that a buffer is pinned or referenced by a driver/HW?
Regards, Tomasz Stanislawski
So after STREAMOFF you can immediately queue all buffers again with QBUF and call STREAMON to restart streaming. No mmap or other operations should be required. This behavior must be kept.
VIDIOC_REQBUFS() or a close() are the only two operations that will actually free the buffers completely.
In practice, a STREAMOFF is either followed by a STREAMON at a later time, or almost immediately followed by REQBUFS or close() to tear down the buffers. So I don't think the buffers should be detached at streamoff.
I agree. I was leaning this way which is why I left it out of my patch and wanted to hear your guys' opinion as you are much more familiar with the intended behavior than I am.
Thanks!
You're welcome. Thank you for reporting the problem and providing a patch.
Hi Tomasz,
On Thursday 02 August 2012 18:31:13 Tomasz Stanislawski wrote:
On 06/27/2012 10:40 PM, Laurent Pinchart wrote:
On Tuesday 26 June 2012 13:53:34 Dima Zavin wrote:
On Tue, Jun 26, 2012 at 2:40 AM, Hans Verkuil hverkuil@xs4all.nl wrote:
On Tue 26 June 2012 11:11:06 Laurent Pinchart wrote:
On Tuesday 26 June 2012 10:40:44 Tomasz Stanislawski wrote:
Hi Dima Zavin, Thank you for the patch and for a ping remainder :).
You are right. The unmap is missing in __vb2_queue_cancel. I will apply your fix into next version of V4L2 support for dmabuf.
Please refer to some comments below.
On 06/20/2012 08:12 AM, Dima Zavin wrote: > Tomasz, > > I've encountered an issue with this patch when userspace does several > stream_on/stream_off cycles. When the user tries to qbuf a buffer > after doing stream_off, we trigger the "dmabuf already pinned" > warning since we didn't unmap the buffer as dqbuf was never called. > > The below patch adds calls to unmap in queue_cancel, but my feeling > is that we probably should be calling detach too (i.e. put_dmabuf).
According to the V4L2 specification, the "VIDIOC_STREAMOFF ioctl, apart of aborting or finishing any DMA in progress, unlocks any user pointer buffers locked in physical memory, and it removes all buffers from the incoming and outgoing queues".
Correct. And what that means in practice is that after a streamoff all buffers are returned to the state they had just before STREAMON was called.
That can't be right. The buffers had to have been returned to the state just *after REQBUFS*, not just *before STREAMON*. You need to re-enqueue buffers before calling STREAMON. I assume that's what you meant?
Your interpretation is correct.
So now we should decide what should be changed: the spec or vb2 ? Bringing the queue state back to *after REQBUFS* may make the next (STREAMON + QBUFs) very costly operations.
Reattaching and mapping a DMABUF might trigger DMA allocation and *will* trigger creation of IOMMU mappings. In case of a user pointer, calling next get_user_pages may cause numerous fault events and will *create* new IOMMU mapping.
Is there any need to do such a cleanup if the destruction of buffers and all caches can be explicitly executed by REQBUFS(count = 0) ?
STREAMOFF needs to pass ownership of all buffers to the application. In practice this means that buffers must then be ready to be passed to other devices, requeued to the same device, or destroyed completely.
We can't keep the buffers in the V4L2 prepared state, as queueing them would then skip cache handling. Keeping the mapping around could be done though, but would not be compliant with the V4L2 spec as the DMABUF would then not be freed until we call REQBUFS(0).
Changing the spec might be possible. I'll need to think more about this, but I'm not very fond of the way we allow a new DMABUF fd (as well as USERPTR pointer) to be associated with an existing buffer, replacing the currently associated fd/pointer. This makes the API asymetrical: it provides an explicit way to associate an fd/pointer with a buffer, but no explicit way to break that association.
It's obviously too late to change this for USERPTR, but for DMABUF we could make the buffer/fd association immutable. This would require a way to selectively destroy buffers though, or at least to explicitly break the association.
Maybe it would be easier to change the spec by removing "apart of ... in physical memory" part?
STREAMOFF should mean stopping streaming, and that resources are no longer used. DMABUFs are unmapped but unmapping does not mean releasing. The exporter may keep the resource in its caches.
If the DMABUF implementation follows the USERPTR spec, applications will expect a STREAMOFF call to release all DMABUF instances associated with the buffers. This means that a DMABUF that is only referenced by a V4L2 buffer will get destroyed by a STREAMOFF call. The more I think about it the more this sounds wrong to me. STREAMOFF has never been tasked with freeing resources. As we lack a way to selectively break the fd (or pointer) to buffer association created at buffer prepare or queue time, applications would have to call REQBUFS(0) to release all buffers, even if they will then want to start a new capture run. This might be costly (although probably not in the USERPTR and DMABUF cases), and doesn't allow to unmap DMABUF instances selectively.
Maybe an UNPREPARE ioctl would be needed ?
Currently, vb2 does not follow the policy from the spec. The put_userptr ops is called on:
- VIDIOC_REBUFS
- VIDIOC_CREATE_BUFS
- vb2_queue_release() which is usually called on close() syscall
The put_userptr is not called and streamoff therefore the user pages are locked after STREAMOFF.
BTW. What does 'buffer locked' mean?
Does it mean that a buffer is pinned or referenced by a driver/HW?
In this context I think it refers to pinning pages in memory.
On Tue, Jun 26, 2012 at 1:40 AM, Tomasz Stanislawski t.stanislaws@samsung.com wrote:
Hi Dima Zavin, Thank you for the patch and for a ping remainder :).
You are right. The unmap is missing in __vb2_queue_cancel. I will apply your fix into next version of V4L2 support for dmabuf.
Please refer to some comments below.
On 06/20/2012 08:12 AM, Dima Zavin wrote:
Tomasz,
I've encountered an issue with this patch when userspace does several stream_on/stream_off cycles. When the user tries to qbuf a buffer after doing stream_off, we trigger the "dmabuf already pinned" warning since we didn't unmap the buffer as dqbuf was never called.
The below patch adds calls to unmap in queue_cancel, but my feeling is that we probably should be calling detach too (i.e. put_dmabuf).
Thoughts?
--Dima
Subject: [PATCH] v4l: vb2: unmap dmabufs on STREAM_OFF event
Currently, if the user issues a STREAM_OFF request and then tries to re-enqueue buffers, it will trigger a warning in the vb2 allocators as the buffer would still be mapped from before STREAM_OFF was called. The current expectation is that buffers will be unmapped in dqbuf, but that will never be called on the mapped buffers after a STREAM_OFF event.
Cc: Sumit Semwal sumit.semwal@ti.com Cc: Tomasz Stanislawski t.stanislaws@samsung.com Signed-off-by: Dima Zavin dima@android.com
drivers/media/video/videobuf2-core.c | Â 22 ++++++++++++++++++++-- Â 1 files changed, 20 insertions(+), 2 deletions(-)
diff --git a/drivers/media/video/videobuf2-core.c b/drivers/media/video/videobuf2-core.c index b431dc6..e2a8f12 100644 --- a/drivers/media/video/videobuf2-core.c +++ b/drivers/media/video/videobuf2-core.c @@ -1592,8 +1592,26 @@ static void __vb2_queue_cancel(struct vb2_queue *q) Â Â Â /* Â Â Â Â * Reinitialize all buffers for next use. Â Â Â Â */
- for (i = 0; i < q->num_buffers; ++i)
- q->bufs[i]->state = VB2_BUF_STATE_DEQUEUED;
- for (i = 0; i < q->num_buffers; ++i) {
- struct vb2_buffer *vb = q->bufs[i];
- int plane;
- vb->state = VB2_BUF_STATE_DEQUEUED;
- if (q->memory != V4L2_MEMORY_DMABUF)
- continue;
- for (plane = 0; plane < vb->num_planes; ++plane) {
- struct vb2_plane *p = &vb->planes[plane];
- if (!p->mem_priv)
- continue;
is the check above really needed? No check like this is done in vb2_dqbuf.
I added it because queue_cancel gets called in release, so you never know what the state of the buffers will be. If we called req_bufs to allocate the buffer descriptors and then call release, then won't we have the vb2_buffer structs but nothing in mem_priv since we haven't attached a dmabuf yet?
- if (p->dbuf_mapped) {
If a buffer is queued then it is also mapped, so dbuf_mapped should be always be true here (at least in theory).
This loop doesn't check for what the buffer status was, it just iterates over the entire queue. The buffer may have been put into STATE_ERROR, and then you don't know if it was ever mapped, etc. It should always be safe to just follow the flag that says you mapped it and unmap it. If you think that this should always be true, we can change it to:
if (!WARN_ON(!p->dbuf_mapped))
or something like that.
Thanks!
--Dima
- call_memop(q, unmap_dmabuf, p->mem_priv);
- p->dbuf_mapped = 0;
- }
- }
- }
}
/**
Regards, Tomasz Stanislawski
From: Sumit Semwal sumit.semwal@ti.com
Adding DMABUF memory type causes videobuf to complain about not using it in some switch cases. This patch removes these warnings.
Signed-off-by: Sumit Semwal sumit.semwal@ti.com Acked-by: Laurent Pinchart laurent.pinchart@ideasonboard.com --- drivers/media/video/videobuf-core.c | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/drivers/media/video/videobuf-core.c b/drivers/media/video/videobuf-core.c index ffdf59c..3e3e55f 100644 --- a/drivers/media/video/videobuf-core.c +++ b/drivers/media/video/videobuf-core.c @@ -335,6 +335,9 @@ static void videobuf_status(struct videobuf_queue *q, struct v4l2_buffer *b, case V4L2_MEMORY_OVERLAY: b->m.offset = vb->boff; break; + case V4L2_MEMORY_DMABUF: + /* DMABUF is not handled in videobuf framework */ + break; }
b->flags = 0; @@ -411,6 +414,7 @@ int __videobuf_mmap_setup(struct videobuf_queue *q, break; case V4L2_MEMORY_USERPTR: case V4L2_MEMORY_OVERLAY: + case V4L2_MEMORY_DMABUF: /* nothing */ break; }
From: Laurent Pinchart laurent.pinchart@ideasonboard.com
Signed-off-by: Laurent Pinchart laurent.pinchart@ideasonboard.com --- drivers/media/video/videobuf2-dma-contig.c | 36 ++++++++++++++-------------- 1 file changed, 18 insertions(+), 18 deletions(-)
diff --git a/drivers/media/video/videobuf2-dma-contig.c b/drivers/media/video/videobuf2-dma-contig.c index 4b71326..a05784f 100644 --- a/drivers/media/video/videobuf2-dma-contig.c +++ b/drivers/media/video/videobuf2-dma-contig.c @@ -32,9 +32,9 @@ struct vb2_dc_buf { struct vb2_vmarea_handler handler; };
-static void vb2_dma_contig_put(void *buf_priv); +static void vb2_dc_put(void *buf_priv);
-static void *vb2_dma_contig_alloc(void *alloc_ctx, unsigned long size) +static void *vb2_dc_alloc(void *alloc_ctx, unsigned long size) { struct vb2_dc_conf *conf = alloc_ctx; struct vb2_dc_buf *buf; @@ -56,7 +56,7 @@ static void *vb2_dma_contig_alloc(void *alloc_ctx, unsigned long size) buf->size = size;
buf->handler.refcount = &buf->refcount; - buf->handler.put = vb2_dma_contig_put; + buf->handler.put = vb2_dc_put; buf->handler.arg = buf;
atomic_inc(&buf->refcount); @@ -64,7 +64,7 @@ static void *vb2_dma_contig_alloc(void *alloc_ctx, unsigned long size) return buf; }
-static void vb2_dma_contig_put(void *buf_priv) +static void vb2_dc_put(void *buf_priv) { struct vb2_dc_buf *buf = buf_priv;
@@ -75,14 +75,14 @@ static void vb2_dma_contig_put(void *buf_priv) } }
-static void *vb2_dma_contig_cookie(void *buf_priv) +static void *vb2_dc_cookie(void *buf_priv) { struct vb2_dc_buf *buf = buf_priv;
return &buf->dma_addr; }
-static void *vb2_dma_contig_vaddr(void *buf_priv) +static void *vb2_dc_vaddr(void *buf_priv) { struct vb2_dc_buf *buf = buf_priv; if (!buf) @@ -91,14 +91,14 @@ static void *vb2_dma_contig_vaddr(void *buf_priv) return buf->vaddr; }
-static unsigned int vb2_dma_contig_num_users(void *buf_priv) +static unsigned int vb2_dc_num_users(void *buf_priv) { struct vb2_dc_buf *buf = buf_priv;
return atomic_read(&buf->refcount); }
-static int vb2_dma_contig_mmap(void *buf_priv, struct vm_area_struct *vma) +static int vb2_dc_mmap(void *buf_priv, struct vm_area_struct *vma) { struct vb2_dc_buf *buf = buf_priv;
@@ -111,7 +111,7 @@ static int vb2_dma_contig_mmap(void *buf_priv, struct vm_area_struct *vma) &vb2_common_vm_ops, &buf->handler); }
-static void *vb2_dma_contig_get_userptr(void *alloc_ctx, unsigned long vaddr, +static void *vb2_dc_get_userptr(void *alloc_ctx, unsigned long vaddr, unsigned long size, int write) { struct vb2_dc_buf *buf; @@ -138,7 +138,7 @@ static void *vb2_dma_contig_get_userptr(void *alloc_ctx, unsigned long vaddr, return buf; }
-static void vb2_dma_contig_put_userptr(void *mem_priv) +static void vb2_dc_put_userptr(void *mem_priv) { struct vb2_dc_buf *buf = mem_priv;
@@ -150,14 +150,14 @@ static void vb2_dma_contig_put_userptr(void *mem_priv) }
const struct vb2_mem_ops vb2_dma_contig_memops = { - .alloc = vb2_dma_contig_alloc, - .put = vb2_dma_contig_put, - .cookie = vb2_dma_contig_cookie, - .vaddr = vb2_dma_contig_vaddr, - .mmap = vb2_dma_contig_mmap, - .get_userptr = vb2_dma_contig_get_userptr, - .put_userptr = vb2_dma_contig_put_userptr, - .num_users = vb2_dma_contig_num_users, + .alloc = vb2_dc_alloc, + .put = vb2_dc_put, + .cookie = vb2_dc_cookie, + .vaddr = vb2_dc_vaddr, + .mmap = vb2_dc_mmap, + .get_userptr = vb2_dc_get_userptr, + .put_userptr = vb2_dc_put_userptr, + .num_users = vb2_dc_num_users, }; EXPORT_SYMBOL_GPL(vb2_dma_contig_memops);
This patch removes a reference to alloc_ctx from an instance of a DMA contiguous buffer. It helps to avoid a risk of a dangling pointer if the context is released while the buffer is still valid. Moreover it removes one dereference step while accessing a device structure.
Signed-off-by: Tomasz Stanislawski t.stanislaws@samsung.com Signed-off-by: Kyungmin Park kyungmin.park@samsung.com --- drivers/media/video/videobuf2-dma-contig.c | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-)
diff --git a/drivers/media/video/videobuf2-dma-contig.c b/drivers/media/video/videobuf2-dma-contig.c index a05784f..20c95da 100644 --- a/drivers/media/video/videobuf2-dma-contig.c +++ b/drivers/media/video/videobuf2-dma-contig.c @@ -23,7 +23,7 @@ struct vb2_dc_conf { };
struct vb2_dc_buf { - struct vb2_dc_conf *conf; + struct device *dev; void *vaddr; dma_addr_t dma_addr; unsigned long size; @@ -37,22 +37,21 @@ static void vb2_dc_put(void *buf_priv); static void *vb2_dc_alloc(void *alloc_ctx, unsigned long size) { struct vb2_dc_conf *conf = alloc_ctx; + struct device *dev = conf->dev; struct vb2_dc_buf *buf;
buf = kzalloc(sizeof *buf, GFP_KERNEL); if (!buf) return ERR_PTR(-ENOMEM);
- buf->vaddr = dma_alloc_coherent(conf->dev, size, &buf->dma_addr, - GFP_KERNEL); + buf->vaddr = dma_alloc_coherent(dev, size, &buf->dma_addr, GFP_KERNEL); if (!buf->vaddr) { - dev_err(conf->dev, "dma_alloc_coherent of size %ld failed\n", - size); + dev_err(dev, "dma_alloc_coherent of size %ld failed\n", size); kfree(buf); return ERR_PTR(-ENOMEM); }
- buf->conf = conf; + buf->dev = dev; buf->size = size;
buf->handler.refcount = &buf->refcount; @@ -69,7 +68,7 @@ static void vb2_dc_put(void *buf_priv) struct vb2_dc_buf *buf = buf_priv;
if (atomic_dec_and_test(&buf->refcount)) { - dma_free_coherent(buf->conf->dev, buf->size, buf->vaddr, + dma_free_coherent(buf->dev, buf->size, buf->vaddr, buf->dma_addr); kfree(buf); }
Hi Tomasz,
Thanks for the patch.
On Thursday 14 June 2012 15:37:40 Tomasz Stanislawski wrote:
This patch removes a reference to alloc_ctx from an instance of a DMA contiguous buffer. It helps to avoid a risk of a dangling pointer if the context is released while the buffer is still valid.
Can this really happen ? All drivers except marvell-ccic seem to call vb2_dma_contig_cleanup_ctx() in their remove handler and probe cleanup path only. Freeing the context while buffers are still around would be a driver bug, and I expect drivers to destroy the queue in that case anyway.
This being said, removing the dereference step is a good idea, so I think the patch should be applied, possibly with a different commit message.
Moreover it removes one dereference step while accessing a device structure.
Signed-off-by: Tomasz Stanislawski t.stanislaws@samsung.com Signed-off-by: Kyungmin Park kyungmin.park@samsung.com
Acked-by: Laurent Pinchart laurent.pinchart@ideasonboard.com
drivers/media/video/videobuf2-dma-contig.c | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-)
diff --git a/drivers/media/video/videobuf2-dma-contig.c b/drivers/media/video/videobuf2-dma-contig.c index a05784f..20c95da 100644 --- a/drivers/media/video/videobuf2-dma-contig.c +++ b/drivers/media/video/videobuf2-dma-contig.c @@ -23,7 +23,7 @@ struct vb2_dc_conf { };
struct vb2_dc_buf {
- struct vb2_dc_conf *conf;
- struct device *dev; void *vaddr; dma_addr_t dma_addr; unsigned long size;
@@ -37,22 +37,21 @@ static void vb2_dc_put(void *buf_priv); static void *vb2_dc_alloc(void *alloc_ctx, unsigned long size) { struct vb2_dc_conf *conf = alloc_ctx;
struct device *dev = conf->dev; struct vb2_dc_buf *buf;
buf = kzalloc(sizeof *buf, GFP_KERNEL); if (!buf) return ERR_PTR(-ENOMEM);
- buf->vaddr = dma_alloc_coherent(conf->dev, size, &buf->dma_addr,
GFP_KERNEL);
- buf->vaddr = dma_alloc_coherent(dev, size, &buf->dma_addr, GFP_KERNEL); if (!buf->vaddr) {
dev_err(conf->dev, "dma_alloc_coherent of size %ld failed\n",
size);
kfree(buf); return ERR_PTR(-ENOMEM); }dev_err(dev, "dma_alloc_coherent of size %ld failed\n", size);
- buf->conf = conf;
buf->dev = dev; buf->size = size;
buf->handler.refcount = &buf->refcount;
@@ -69,7 +68,7 @@ static void vb2_dc_put(void *buf_priv) struct vb2_dc_buf *buf = buf_priv;
if (atomic_dec_and_test(&buf->refcount)) {
dma_free_coherent(buf->conf->dev, buf->size, buf->vaddr,
kfree(buf); }dma_free_coherent(buf->dev, buf->size, buf->vaddr, buf->dma_addr);
Hi Laurent,
On 06/19/2012 11:00 PM, Laurent Pinchart wrote:
Hi Tomasz,
Thanks for the patch.
On Thursday 14 June 2012 15:37:40 Tomasz Stanislawski wrote:
This patch removes a reference to alloc_ctx from an instance of a DMA contiguous buffer. It helps to avoid a risk of a dangling pointer if the context is released while the buffer is still valid.
Can this really happen ? All drivers except marvell-ccic seem to call vb2_dma_contig_cleanup_ctx() in their remove handler and probe cleanup path only. Freeing the context while buffers are still around would be a driver bug, and I expect drivers to destroy the queue in that case anyway.
This being said, removing the dereference step is a good idea, so I think the patch should be applied, possibly with a different commit message.
The problem may happen if a DMABUF sharing is used. - process A uses V4L2 queue to create a buffer - process A exports a buffer and shares it with the process B (by sockets or /proc/pid/fd) - the process A gets killed, queue is destroyed - someone call rmmod on v4l driver, alloc_ctx is freed - process B keeps reference to a buffer that has a dangling reference to alloc_ctx
The presented scenario might be a bit too pathological and artificial. Moreover it involves root privileges. But it is possible to trigger this bug. One solution might be keeping reference count in alloc_ctx but it would be easier to get rid of the reference to alloc_ctx from vb2-dma-contig buffer.
BTW. I decided to drop 'Remove unneeded allocation context structure' because Marek Szyprowski is working on extension to vb2-dma-contig that allow to create buffers with no kernel mappings. That feature involved additional parameter to alloc_ctx other than pointer to the device.
Regards, Tomasz Stanislawski
Moreover it removes one dereference step while accessing a device structure.
Signed-off-by: Tomasz Stanislawski t.stanislaws@samsung.com Signed-off-by: Kyungmin Park kyungmin.park@samsung.com
Acked-by: Laurent Pinchart laurent.pinchart@ideasonboard.com
kfree(buf); }dma_free_coherent(buf->dev, buf->size, buf->vaddr, buf->dma_addr);
Hi Tomasz,
On Wednesday 20 June 2012 13:51:06 Tomasz Stanislawski wrote:
On 06/19/2012 11:00 PM, Laurent Pinchart wrote:
On Thursday 14 June 2012 15:37:40 Tomasz Stanislawski wrote:
This patch removes a reference to alloc_ctx from an instance of a DMA contiguous buffer. It helps to avoid a risk of a dangling pointer if the context is released while the buffer is still valid.
Can this really happen ? All drivers except marvell-ccic seem to call vb2_dma_contig_cleanup_ctx() in their remove handler and probe cleanup path only. Freeing the context while buffers are still around would be a driver bug, and I expect drivers to destroy the queue in that case anyway.
This being said, removing the dereference step is a good idea, so I think the patch should be applied, possibly with a different commit message.
The problem may happen if a DMABUF sharing is used.
- process A uses V4L2 queue to create a buffer
- process A exports a buffer and shares it with the process B (by sockets or
/proc/pid/fd) - the process A gets killed, queue is destroyed
- someone call rmmod on v4l driver, alloc_ctx is freed
That's where the problem is in my opinion. As long as a buffer is in use, the queue should not get destroyed and the context should not be freed. If the driver is removed it will kfree the structure that embeds the queue, and even possible free the whole memory region that backs the buffers. We would then have much bigger trouble than just a dangling context pointer.
From a V4L2 point of view this needs to be solved for the dmabuf exporter role
only, so it's not a huge concern in the context of this patch set, but we of course need to address the issue.
- process B keeps reference to a buffer that has a dangling reference to
alloc_ctx
The presented scenario might be a bit too pathological and artificial. Moreover it involves root privileges. But it is possible to trigger this bug. One solution might be keeping reference count in alloc_ctx but it would be easier to get rid of the reference to alloc_ctx from vb2-dma-contig buffer.
BTW. I decided to drop 'Remove unneeded allocation context structure' because Marek Szyprowski is working on extension to vb2-dma-contig that allow to create buffers with no kernel mappings. That feature involved additional parameter to alloc_ctx other than pointer to the device.
OK.
From: Laurent Pinchart laurent.pinchart@ideasonboard.com
Group functions by buffer type.
Signed-off-by: Laurent Pinchart laurent.pinchart@ideasonboard.com --- drivers/media/video/videobuf2-dma-contig.c | 92 ++++++++++++++++------------ 1 file changed, 54 insertions(+), 38 deletions(-)
diff --git a/drivers/media/video/videobuf2-dma-contig.c b/drivers/media/video/videobuf2-dma-contig.c index 20c95da..daac2b2 100644 --- a/drivers/media/video/videobuf2-dma-contig.c +++ b/drivers/media/video/videobuf2-dma-contig.c @@ -25,14 +25,56 @@ struct vb2_dc_conf { struct vb2_dc_buf { struct device *dev; void *vaddr; - dma_addr_t dma_addr; unsigned long size; - struct vm_area_struct *vma; - atomic_t refcount; + dma_addr_t dma_addr; + + /* MMAP related */ struct vb2_vmarea_handler handler; + atomic_t refcount; + + /* USERPTR related */ + struct vm_area_struct *vma; };
-static void vb2_dc_put(void *buf_priv); +/*********************************************/ +/* callbacks for all buffers */ +/*********************************************/ + +static void *vb2_dc_cookie(void *buf_priv) +{ + struct vb2_dc_buf *buf = buf_priv; + + return &buf->dma_addr; +} + +static void *vb2_dc_vaddr(void *buf_priv) +{ + struct vb2_dc_buf *buf = buf_priv; + + return buf->vaddr; +} + +static unsigned int vb2_dc_num_users(void *buf_priv) +{ + struct vb2_dc_buf *buf = buf_priv; + + return atomic_read(&buf->refcount); +} + +/*********************************************/ +/* callbacks for MMAP buffers */ +/*********************************************/ + +static void vb2_dc_put(void *buf_priv) +{ + struct vb2_dc_buf *buf = buf_priv; + + if (!atomic_dec_and_test(&buf->refcount)) + return; + + dma_free_coherent(buf->dev, buf->size, buf->vaddr, buf->dma_addr); + kfree(buf); +}
static void *vb2_dc_alloc(void *alloc_ctx, unsigned long size) { @@ -63,40 +105,6 @@ static void *vb2_dc_alloc(void *alloc_ctx, unsigned long size) return buf; }
-static void vb2_dc_put(void *buf_priv) -{ - struct vb2_dc_buf *buf = buf_priv; - - if (atomic_dec_and_test(&buf->refcount)) { - dma_free_coherent(buf->dev, buf->size, buf->vaddr, - buf->dma_addr); - kfree(buf); - } -} - -static void *vb2_dc_cookie(void *buf_priv) -{ - struct vb2_dc_buf *buf = buf_priv; - - return &buf->dma_addr; -} - -static void *vb2_dc_vaddr(void *buf_priv) -{ - struct vb2_dc_buf *buf = buf_priv; - if (!buf) - return NULL; - - return buf->vaddr; -} - -static unsigned int vb2_dc_num_users(void *buf_priv) -{ - struct vb2_dc_buf *buf = buf_priv; - - return atomic_read(&buf->refcount); -} - static int vb2_dc_mmap(void *buf_priv, struct vm_area_struct *vma) { struct vb2_dc_buf *buf = buf_priv; @@ -110,6 +118,10 @@ static int vb2_dc_mmap(void *buf_priv, struct vm_area_struct *vma) &vb2_common_vm_ops, &buf->handler); }
+/*********************************************/ +/* callbacks for USERPTR buffers */ +/*********************************************/ + static void *vb2_dc_get_userptr(void *alloc_ctx, unsigned long vaddr, unsigned long size, int write) { @@ -148,6 +160,10 @@ static void vb2_dc_put_userptr(void *mem_priv) kfree(buf); }
+/*********************************************/ +/* DMA CONTIG exported functions */ +/*********************************************/ + const struct vb2_mem_ops vb2_dma_contig_memops = { .alloc = vb2_dc_alloc, .put = vb2_dc_put,
This patch introduces usage of dma_map_sg to map memory behind a userspace pointer to a device as dma-contiguous mapping.
This patch contains some of the code kindly provided by Marek Szyprowski m.szyprowski@samsung.com and Kamil Debski k.debski@samsung.com and Andrzej Pietrasiewicz andrzej.p@samsung.com. Kind thanks for bug reports from Laurent Pinchart laurent.pinchart@ideasonboard.com and Seung-Woo Kim sw0312.kim@samsung.com.
Signed-off-by: Tomasz Stanislawski t.stanislaws@samsung.com Signed-off-by: Kyungmin Park kyungmin.park@samsung.com Acked-by: Laurent Pinchart laurent.pinchart@ideasonboard.com --- drivers/media/video/videobuf2-dma-contig.c | 229 ++++++++++++++++++++++++++-- 1 file changed, 213 insertions(+), 16 deletions(-)
diff --git a/drivers/media/video/videobuf2-dma-contig.c b/drivers/media/video/videobuf2-dma-contig.c index daac2b2..94f0874 100644 --- a/drivers/media/video/videobuf2-dma-contig.c +++ b/drivers/media/video/videobuf2-dma-contig.c @@ -11,6 +11,8 @@ */
#include <linux/module.h> +#include <linux/scatterlist.h> +#include <linux/sched.h> #include <linux/slab.h> #include <linux/dma-mapping.h>
@@ -27,6 +29,8 @@ struct vb2_dc_buf { void *vaddr; unsigned long size; dma_addr_t dma_addr; + enum dma_data_direction dma_dir; + struct sg_table *dma_sgt;
/* MMAP related */ struct vb2_vmarea_handler handler; @@ -37,6 +41,44 @@ struct vb2_dc_buf { };
/*********************************************/ +/* scatterlist table functions */ +/*********************************************/ + + +static void vb2_dc_sgt_foreach_page(struct sg_table *sgt, + void (*cb)(struct page *pg)) +{ + struct scatterlist *s; + unsigned int i; + + for_each_sg(sgt->sgl, s, sgt->orig_nents, i) { + struct page *page = sg_page(s); + unsigned int n_pages = PAGE_ALIGN(s->offset + s->length) + >> PAGE_SHIFT; + unsigned int j; + + for (j = 0; j < n_pages; ++j, ++page) + cb(page); + } +} + +static unsigned long vb2_dc_get_contiguous_size(struct sg_table *sgt) +{ + struct scatterlist *s; + dma_addr_t expected = sg_dma_address(sgt->sgl); + unsigned int i; + unsigned long size = 0; + + for_each_sg(sgt->sgl, s, sgt->nents, i) { + if (sg_dma_address(s) != expected) + break; + expected = sg_dma_address(s) + sg_dma_len(s); + size += sg_dma_len(s); + } + return size; +} + +/*********************************************/ /* callbacks for all buffers */ /*********************************************/
@@ -122,42 +164,197 @@ static int vb2_dc_mmap(void *buf_priv, struct vm_area_struct *vma) /* callbacks for USERPTR buffers */ /*********************************************/
+static inline int vma_is_io(struct vm_area_struct *vma) +{ + return !!(vma->vm_flags & (VM_IO | VM_PFNMAP)); +} + +static int vb2_dc_get_user_pages(unsigned long start, struct page **pages, + int n_pages, struct vm_area_struct *vma, int write) +{ + if (vma_is_io(vma)) { + unsigned int i; + + for (i = 0; i < n_pages; ++i, start += PAGE_SIZE) { + unsigned long pfn; + int ret = follow_pfn(vma, start, &pfn); + + if (ret) { + printk(KERN_ERR "no page for address %lu\n", + start); + return ret; + } + pages[i] = pfn_to_page(pfn); + } + } else { + int n; + + n = get_user_pages(current, current->mm, start & PAGE_MASK, + n_pages, write, 1, pages, NULL); + /* negative error means that no page was pinned */ + n = max(n, 0); + if (n != n_pages) { + printk(KERN_ERR "got only %d of %d user pages\n", + n, n_pages); + while (n) + put_page(pages[--n]); + return -EFAULT; + } + } + + return 0; +} + +static void vb2_dc_put_dirty_page(struct page *page) +{ + set_page_dirty_lock(page); + put_page(page); +} + +static void vb2_dc_put_userptr(void *buf_priv) +{ + struct vb2_dc_buf *buf = buf_priv; + struct sg_table *sgt = buf->dma_sgt; + + dma_unmap_sg(buf->dev, sgt->sgl, sgt->orig_nents, buf->dma_dir); + if (!vma_is_io(buf->vma)) + vb2_dc_sgt_foreach_page(sgt, vb2_dc_put_dirty_page); + + sg_free_table(sgt); + kfree(sgt); + vb2_put_vma(buf->vma); + kfree(buf); +} + static void *vb2_dc_get_userptr(void *alloc_ctx, unsigned long vaddr, - unsigned long size, int write) + unsigned long size, int write) { + struct vb2_dc_conf *conf = alloc_ctx; struct vb2_dc_buf *buf; + unsigned long start; + unsigned long end; + unsigned long offset; + struct page **pages; + int n_pages; + int ret = 0; struct vm_area_struct *vma; - dma_addr_t dma_addr = 0; - int ret; + struct sg_table *sgt; + unsigned long contig_size;
buf = kzalloc(sizeof *buf, GFP_KERNEL); if (!buf) return ERR_PTR(-ENOMEM);
- ret = vb2_get_contig_userptr(vaddr, size, &vma, &dma_addr); + buf->dev = conf->dev; + buf->dma_dir = write ? DMA_FROM_DEVICE : DMA_TO_DEVICE; + + start = vaddr & PAGE_MASK; + offset = vaddr & ~PAGE_MASK; + end = PAGE_ALIGN(vaddr + size); + n_pages = (end - start) >> PAGE_SHIFT; + + pages = kmalloc(n_pages * sizeof pages[0], GFP_KERNEL); + if (!pages) { + ret = -ENOMEM; + printk(KERN_ERR "failed to allocate pages table\n"); + goto fail_buf; + } + + /* current->mm->mmap_sem is taken by videobuf2 core */ + vma = find_vma(current->mm, vaddr); + if (!vma) { + printk(KERN_ERR "no vma for address %lu\n", vaddr); + ret = -EFAULT; + goto fail_pages; + } + + if (vma->vm_end < vaddr + size) { + printk(KERN_ERR "vma at %lu is too small for %lu bytes\n", + vaddr, size); + ret = -EFAULT; + goto fail_pages; + } + + buf->vma = vb2_get_vma(vma); + if (!buf->vma) { + printk(KERN_ERR "failed to copy vma\n"); + ret = -ENOMEM; + goto fail_pages; + } + + /* extract page list from userspace mapping */ + ret = vb2_dc_get_user_pages(start, pages, n_pages, vma, write); if (ret) { - printk(KERN_ERR "Failed acquiring VMA for vaddr 0x%08lx\n", - vaddr); - kfree(buf); - return ERR_PTR(ret); + printk(KERN_ERR "failed to get user pages\n"); + goto fail_vma; + } + + sgt = kzalloc(sizeof *sgt, GFP_KERNEL); + if (!sgt) { + printk(KERN_ERR "failed to allocate sg table\n"); + ret = -ENOMEM; + goto fail_get_user_pages; + } + + ret = sg_alloc_table_from_pages(sgt, pages, n_pages, + offset, size, GFP_KERNEL); + if (ret) { + printk(KERN_ERR "failed to initialize sg table\n"); + goto fail_sgt; + } + + /* pages are no longer needed */ + kfree(pages); + pages = NULL; + + sgt->nents = dma_map_sg(buf->dev, sgt->sgl, sgt->orig_nents, + buf->dma_dir); + if (sgt->nents <= 0) { + printk(KERN_ERR "failed to map scatterlist\n"); + ret = -EIO; + goto fail_sgt_init; + } + + contig_size = vb2_dc_get_contiguous_size(sgt); + if (contig_size < size) { + printk(KERN_ERR "contiguous mapping is too small %lu/%lu\n", + contig_size, size); + ret = -EFAULT; + goto fail_map_sg; }
+ buf->dma_addr = sg_dma_address(sgt->sgl); buf->size = size; - buf->dma_addr = dma_addr; - buf->vma = vma; + buf->dma_sgt = sgt;
return buf; -}
-static void vb2_dc_put_userptr(void *mem_priv) -{ - struct vb2_dc_buf *buf = mem_priv; +fail_map_sg: + dma_unmap_sg(buf->dev, sgt->sgl, sgt->orig_nents, buf->dma_dir);
- if (!buf) - return; +fail_sgt_init: + if (!vma_is_io(buf->vma)) + vb2_dc_sgt_foreach_page(sgt, put_page); + sg_free_table(sgt); + +fail_sgt: + kfree(sgt);
+fail_get_user_pages: + if (pages && !vma_is_io(buf->vma)) + while (n_pages) + put_page(pages[--n_pages]); + +fail_vma: vb2_put_vma(buf->vma); + +fail_pages: + kfree(pages); /* kfree is NULL-proof */ + +fail_buf: kfree(buf); + + return ERR_PTR(ret); }
/*********************************************/
From: Marek Szyprowski m.szyprowski@samsung.com
This patch adds support for prepare/finish callbacks in VB2 allocators. These callback are used for buffer flushing.
Signed-off-by: Marek Szyprowski m.szyprowski@samsung.com Acked-by: Laurent Pinchart laurent.pinchart@ideasonboard.com --- drivers/media/video/videobuf2-core.c | 11 +++++++++++ include/media/videobuf2-core.h | 7 +++++++ 2 files changed, 18 insertions(+)
diff --git a/drivers/media/video/videobuf2-core.c b/drivers/media/video/videobuf2-core.c index f43cfa4..d60ed25 100644 --- a/drivers/media/video/videobuf2-core.c +++ b/drivers/media/video/videobuf2-core.c @@ -845,6 +845,7 @@ void vb2_buffer_done(struct vb2_buffer *vb, enum vb2_buffer_state state) { struct vb2_queue *q = vb->vb2_queue; unsigned long flags; + unsigned int plane;
if (vb->state != VB2_BUF_STATE_ACTIVE) return; @@ -855,6 +856,10 @@ void vb2_buffer_done(struct vb2_buffer *vb, enum vb2_buffer_state state) dprintk(4, "Done processing on buffer %d, state: %d\n", vb->v4l2_buf.index, vb->state);
+ /* sync buffers */ + for (plane = 0; plane < vb->num_planes; ++plane) + call_memop(q, finish, vb->planes[plane].mem_priv); + /* Add the buffer to the done buffers list */ spin_lock_irqsave(&q->done_lock, flags); vb->state = state; @@ -1137,9 +1142,15 @@ err: static void __enqueue_in_driver(struct vb2_buffer *vb) { struct vb2_queue *q = vb->vb2_queue; + unsigned int plane;
vb->state = VB2_BUF_STATE_ACTIVE; atomic_inc(&q->queued_count); + + /* sync buffers */ + for (plane = 0; plane < vb->num_planes; ++plane) + call_memop(q, prepare, vb->planes[plane].mem_priv); + q->ops->buf_queue(vb); }
diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h index 859bbaf..d079f92 100644 --- a/include/media/videobuf2-core.h +++ b/include/media/videobuf2-core.h @@ -56,6 +56,10 @@ struct vb2_fileio_data; * dmabuf * @unmap_dmabuf: releases access control to the dmabuf - allocator is notified * that this driver is done using the dmabuf for now + * @prepare: called everytime the buffer is passed from userspace to the + * driver, usefull for cache synchronisation, optional + * @finish: called everytime the buffer is passed back from the driver + * to the userspace, also optional * @vaddr: return a kernel virtual address to a given memory buffer * associated with the passed private structure or NULL if no * such mapping exists @@ -82,6 +86,9 @@ struct vb2_mem_ops { unsigned long size, int write); void (*put_userptr)(void *buf_priv);
+ void (*prepare)(void *buf_priv); + void (*finish)(void *buf_priv); + void *(*attach_dmabuf)(void *alloc_ctx, struct dma_buf *dbuf, unsigned long size, int write); void (*detach_dmabuf)(void *buf_priv);
From: Marek Szyprowski m.szyprowski@samsung.com
Add prepare/finish callbacks to vb2-dma-contig allocator.
Signed-off-by: Marek Szyprowski m.szyprowski@samsung.com --- drivers/media/video/videobuf2-dma-contig.c | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+)
diff --git a/drivers/media/video/videobuf2-dma-contig.c b/drivers/media/video/videobuf2-dma-contig.c index 94f0874..f9286d7 100644 --- a/drivers/media/video/videobuf2-dma-contig.c +++ b/drivers/media/video/videobuf2-dma-contig.c @@ -103,6 +103,28 @@ static unsigned int vb2_dc_num_users(void *buf_priv) return atomic_read(&buf->refcount); }
+static void vb2_dc_prepare(void *buf_priv) +{ + struct vb2_dc_buf *buf = buf_priv; + struct sg_table *sgt = buf->dma_sgt; + + if (!sgt) + return; + + dma_sync_sg_for_device(buf->dev, sgt->sgl, sgt->nents, buf->dma_dir); +} + +static void vb2_dc_finish(void *buf_priv) +{ + struct vb2_dc_buf *buf = buf_priv; + struct sg_table *sgt = buf->dma_sgt; + + if (!sgt) + return; + + dma_sync_sg_for_cpu(buf->dev, sgt->sgl, sgt->nents, buf->dma_dir); +} + /*********************************************/ /* callbacks for MMAP buffers */ /*********************************************/ @@ -369,6 +391,8 @@ const struct vb2_mem_ops vb2_dma_contig_memops = { .mmap = vb2_dc_mmap, .get_userptr = vb2_dc_get_userptr, .put_userptr = vb2_dc_put_userptr, + .prepare = vb2_dc_prepare, + .finish = vb2_dc_finish, .num_users = vb2_dc_num_users, }; EXPORT_SYMBOL_GPL(vb2_dma_contig_memops);
Hi Tomasz,
Thanks for the patch.
On Thursday 14 June 2012 15:37:44 Tomasz Stanislawski wrote:
From: Marek Szyprowski m.szyprowski@samsung.com
Add prepare/finish callbacks to vb2-dma-contig allocator.
Signed-off-by: Marek Szyprowski m.szyprowski@samsung.com
Acked-by: Laurent Pinchart laurent.pinchart@ideasonboard.com
drivers/media/video/videobuf2-dma-contig.c | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+)
diff --git a/drivers/media/video/videobuf2-dma-contig.c b/drivers/media/video/videobuf2-dma-contig.c index 94f0874..f9286d7 100644 --- a/drivers/media/video/videobuf2-dma-contig.c +++ b/drivers/media/video/videobuf2-dma-contig.c @@ -103,6 +103,28 @@ static unsigned int vb2_dc_num_users(void *buf_priv) return atomic_read(&buf->refcount); }
+static void vb2_dc_prepare(void *buf_priv) +{
- struct vb2_dc_buf *buf = buf_priv;
- struct sg_table *sgt = buf->dma_sgt;
- if (!sgt)
return;
- dma_sync_sg_for_device(buf->dev, sgt->sgl, sgt->nents, buf->dma_dir);
+}
+static void vb2_dc_finish(void *buf_priv) +{
- struct vb2_dc_buf *buf = buf_priv;
- struct sg_table *sgt = buf->dma_sgt;
- if (!sgt)
return;
- dma_sync_sg_for_cpu(buf->dev, sgt->sgl, sgt->nents, buf->dma_dir);
+}
/*********************************************/ /* callbacks for MMAP buffers */ /*********************************************/ @@ -369,6 +391,8 @@ const struct vb2_mem_ops vb2_dma_contig_memops = { .mmap = vb2_dc_mmap, .get_userptr = vb2_dc_get_userptr, .put_userptr = vb2_dc_put_userptr,
- .prepare = vb2_dc_prepare,
- .finish = vb2_dc_finish, .num_users = vb2_dc_num_users,
}; EXPORT_SYMBOL_GPL(vb2_dma_contig_memops);
From: Sumit Semwal sumit.semwal@ti.com
This patch makes changes for adding dma-contig as a dma_buf user. It provides function implementations for the {attach, detach, map, unmap}_dmabuf() mem_ops of DMABUF memory type.
Signed-off-by: Sumit Semwal sumit.semwal@ti.com Signed-off-by: Sumit Semwal sumit.semwal@linaro.org [author of the original patch] Signed-off-by: Tomasz Stanislawski t.stanislaws@samsung.com [integration with refactored dma-contig allocator] Acked-by: Laurent Pinchart laurent.pinchart@ideasonboard.com --- drivers/media/video/videobuf2-dma-contig.c | 120 +++++++++++++++++++++++++++- 1 file changed, 118 insertions(+), 2 deletions(-)
diff --git a/drivers/media/video/videobuf2-dma-contig.c b/drivers/media/video/videobuf2-dma-contig.c index f9286d7..040829b 100644 --- a/drivers/media/video/videobuf2-dma-contig.c +++ b/drivers/media/video/videobuf2-dma-contig.c @@ -10,6 +10,7 @@ * the Free Software Foundation. */
+#include <linux/dma-buf.h> #include <linux/module.h> #include <linux/scatterlist.h> #include <linux/sched.h> @@ -38,6 +39,9 @@ struct vb2_dc_buf {
/* USERPTR related */ struct vm_area_struct *vma; + + /* DMABUF related */ + struct dma_buf_attachment *db_attach; };
/*********************************************/ @@ -108,7 +112,8 @@ static void vb2_dc_prepare(void *buf_priv) struct vb2_dc_buf *buf = buf_priv; struct sg_table *sgt = buf->dma_sgt;
- if (!sgt) + /* DMABUF exporter will flush the cache for us */ + if (!sgt || buf->db_attach) return;
dma_sync_sg_for_device(buf->dev, sgt->sgl, sgt->nents, buf->dma_dir); @@ -119,7 +124,8 @@ static void vb2_dc_finish(void *buf_priv) struct vb2_dc_buf *buf = buf_priv; struct sg_table *sgt = buf->dma_sgt;
- if (!sgt) + /* DMABUF exporter will flush the cache for us */ + if (!sgt || buf->db_attach) return;
dma_sync_sg_for_cpu(buf->dev, sgt->sgl, sgt->nents, buf->dma_dir); @@ -380,6 +386,112 @@ fail_buf: }
/*********************************************/ +/* callbacks for DMABUF buffers */ +/*********************************************/ + +static int vb2_dc_map_dmabuf(void *mem_priv) +{ + struct vb2_dc_buf *buf = mem_priv; + struct sg_table *sgt; + unsigned long contig_size; + + if (WARN_ON(!buf->db_attach)) { + printk(KERN_ERR "trying to pin a non attached buffer\n"); + return -EINVAL; + } + + if (WARN_ON(buf->dma_sgt)) { + printk(KERN_ERR "dmabuf buffer is already pinned\n"); + return 0; + } + + /* get the associated scatterlist for this buffer */ + sgt = dma_buf_map_attachment(buf->db_attach, buf->dma_dir); + if (IS_ERR_OR_NULL(sgt)) { + printk(KERN_ERR "Error getting dmabuf scatterlist\n"); + return -EINVAL; + } + + /* checking if dmabuf is big enough to store contiguous chunk */ + contig_size = vb2_dc_get_contiguous_size(sgt); + if (contig_size < buf->size) { + printk(KERN_ERR "contiguous chunk is too small %lu/%lu b\n", + contig_size, buf->size); + dma_buf_unmap_attachment(buf->db_attach, sgt, buf->dma_dir); + return -EFAULT; + } + + buf->dma_addr = sg_dma_address(sgt->sgl); + buf->dma_sgt = sgt; + + return 0; +} + +static void vb2_dc_unmap_dmabuf(void *mem_priv) +{ + struct vb2_dc_buf *buf = mem_priv; + struct sg_table *sgt = buf->dma_sgt; + + if (WARN_ON(!buf->db_attach)) { + printk(KERN_ERR "trying to unpin a not attached buffer\n"); + return; + } + + if (WARN_ON(!sgt)) { + printk(KERN_ERR "dmabuf buffer is already unpinned\n"); + return; + } + + dma_buf_unmap_attachment(buf->db_attach, sgt, buf->dma_dir); + + buf->dma_addr = 0; + buf->dma_sgt = NULL; +} + +static void vb2_dc_detach_dmabuf(void *mem_priv) +{ + struct vb2_dc_buf *buf = mem_priv; + + /* if vb2 works correctly you should never detach mapped buffer */ + if (WARN_ON(buf->dma_addr)) + vb2_dc_unmap_dmabuf(buf); + + /* detach this attachment */ + dma_buf_detach(buf->db_attach->dmabuf, buf->db_attach); + kfree(buf); +} + +static void *vb2_dc_attach_dmabuf(void *alloc_ctx, struct dma_buf *dbuf, + unsigned long size, int write) +{ + struct vb2_dc_conf *conf = alloc_ctx; + struct vb2_dc_buf *buf; + struct dma_buf_attachment *dba; + + if (dbuf->size < size) + return ERR_PTR(-EFAULT); + + buf = kzalloc(sizeof *buf, GFP_KERNEL); + if (!buf) + return ERR_PTR(-ENOMEM); + + buf->dev = conf->dev; + /* create attachment for the dmabuf with the user device */ + dba = dma_buf_attach(dbuf, buf->dev); + if (IS_ERR(dba)) { + printk(KERN_ERR "failed to attach dmabuf\n"); + kfree(buf); + return dba; + } + + buf->dma_dir = write ? DMA_FROM_DEVICE : DMA_TO_DEVICE; + buf->size = size; + buf->db_attach = dba; + + return buf; +} + +/*********************************************/ /* DMA CONTIG exported functions */ /*********************************************/
@@ -393,6 +505,10 @@ const struct vb2_mem_ops vb2_dma_contig_memops = { .put_userptr = vb2_dc_put_userptr, .prepare = vb2_dc_prepare, .finish = vb2_dc_finish, + .map_dmabuf = vb2_dc_map_dmabuf, + .unmap_dmabuf = vb2_dc_unmap_dmabuf, + .attach_dmabuf = vb2_dc_attach_dmabuf, + .detach_dmabuf = vb2_dc_detach_dmabuf, .num_users = vb2_dc_num_users, }; EXPORT_SYMBOL_GPL(vb2_dma_contig_memops);
This patch adds support for importing DMABUF files for vmalloc allocator in Videobuf2.
Signed-off-by: Tomasz Stanislawski t.stanislaws@samsung.com Signed-off-by: Kyungmin Park kyungmin.park@samsung.com --- drivers/media/video/videobuf2-vmalloc.c | 56 +++++++++++++++++++++++++++++++ 1 file changed, 56 insertions(+)
diff --git a/drivers/media/video/videobuf2-vmalloc.c b/drivers/media/video/videobuf2-vmalloc.c index 6b5ca6c..305032f 100644 --- a/drivers/media/video/videobuf2-vmalloc.c +++ b/drivers/media/video/videobuf2-vmalloc.c @@ -29,6 +29,7 @@ struct vb2_vmalloc_buf { unsigned int n_pages; atomic_t refcount; struct vb2_vmarea_handler handler; + struct dma_buf *dbuf; };
static void vb2_vmalloc_put(void *buf_priv); @@ -206,11 +207,66 @@ static int vb2_vmalloc_mmap(void *buf_priv, struct vm_area_struct *vma) return 0; }
+/*********************************************/ +/* callbacks for DMABUF buffers */ +/*********************************************/ + +static int vb2_vmalloc_map_dmabuf(void *mem_priv) +{ + struct vb2_vmalloc_buf *buf = mem_priv; + + buf->vaddr = dma_buf_vmap(buf->dbuf); + + return buf->vaddr ? 0 : -EFAULT; +} + +static void vb2_vmalloc_unmap_dmabuf(void *mem_priv) +{ + struct vb2_vmalloc_buf *buf = mem_priv; + + dma_buf_vunmap(buf->dbuf, buf->vaddr); + buf->vaddr = NULL; +} + +static void vb2_vmalloc_detach_dmabuf(void *mem_priv) +{ + struct vb2_vmalloc_buf *buf = mem_priv; + + if (buf->vaddr) + dma_buf_vunmap(buf->dbuf, buf->vaddr); + + kfree(buf); +} + +static void *vb2_vmalloc_attach_dmabuf(void *alloc_ctx, struct dma_buf *dbuf, + unsigned long size, int write) +{ + struct vb2_vmalloc_buf *buf; + + if (dbuf->size < size) + return ERR_PTR(-EFAULT); + + buf = kzalloc(sizeof *buf, GFP_KERNEL); + if (!buf) + return ERR_PTR(-ENOMEM); + + buf->dbuf = dbuf; + buf->write = write; + buf->size = size; + + return buf; +} + + const struct vb2_mem_ops vb2_vmalloc_memops = { .alloc = vb2_vmalloc_alloc, .put = vb2_vmalloc_put, .get_userptr = vb2_vmalloc_get_userptr, .put_userptr = vb2_vmalloc_put_userptr, + .map_dmabuf = vb2_vmalloc_map_dmabuf, + .unmap_dmabuf = vb2_vmalloc_unmap_dmabuf, + .attach_dmabuf = vb2_vmalloc_attach_dmabuf, + .detach_dmabuf = vb2_vmalloc_detach_dmabuf, .vaddr = vb2_vmalloc_vaddr, .mmap = vb2_vmalloc_mmap, .num_users = vb2_vmalloc_num_users,
Hi Tomasz,
Thanks for the patch.
On Thursday 14 June 2012 15:37:46 Tomasz Stanislawski wrote:
This patch adds support for importing DMABUF files for vmalloc allocator in Videobuf2.
Signed-off-by: Tomasz Stanislawski t.stanislaws@samsung.com Signed-off-by: Kyungmin Park kyungmin.park@samsung.com
Acked-by: Laurent Pinchart laurent.pinchart@ideasonboard.com
This patch enhances VIVI driver with a support for importing a buffer from DMABUF file descriptors.
Signed-off-by: Tomasz Stanislawski t.stanislaws@samsung.com Signed-off-by: Kyungmin Park kyungmin.park@samsung.com --- drivers/media/video/Kconfig | 1 + drivers/media/video/vivi.c | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/media/video/Kconfig b/drivers/media/video/Kconfig index 99937c9..9aa7306 100644 --- a/drivers/media/video/Kconfig +++ b/drivers/media/video/Kconfig @@ -630,6 +630,7 @@ config VIDEO_VIVI depends on FRAMEBUFFER_CONSOLE || STI_CONSOLE select FONT_8x16 select VIDEOBUF2_VMALLOC + select DMA_SHARED_BUFFER default n ---help--- Enables a virtual video driver. This device shows a color bar diff --git a/drivers/media/video/vivi.c b/drivers/media/video/vivi.c index 0960d7f..05709cd 100644 --- a/drivers/media/video/vivi.c +++ b/drivers/media/video/vivi.c @@ -1422,7 +1422,7 @@ static int __init vivi_create_instance(int inst) q = &dev->vb_vidq; memset(q, 0, sizeof(dev->vb_vidq)); q->type = V4L2_BUF_TYPE_VIDEO_CAPTURE; - q->io_modes = VB2_MMAP | VB2_USERPTR | VB2_READ; + q->io_modes = VB2_MMAP | VB2_USERPTR | VB2_DMABUF | VB2_READ; q->drv_priv = dev; q->buf_struct_size = sizeof(struct vivi_buffer); q->ops = &vivi_video_qops;
This patch enhances s5p-tv with support for DMABUF importing via V4L2_MEMORY_DMABUF memory type.
Signed-off-by: Tomasz Stanislawski t.stanislaws@samsung.com Signed-off-by: Kyungmin Park kyungmin.park@samsung.com --- drivers/media/video/s5p-tv/Kconfig | 1 + drivers/media/video/s5p-tv/mixer_video.c | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/media/video/s5p-tv/Kconfig b/drivers/media/video/s5p-tv/Kconfig index f248b28..2e80126 100644 --- a/drivers/media/video/s5p-tv/Kconfig +++ b/drivers/media/video/s5p-tv/Kconfig @@ -10,6 +10,7 @@ config VIDEO_SAMSUNG_S5P_TV bool "Samsung TV driver for S5P platform (experimental)" depends on PLAT_S5P && PM_RUNTIME depends on EXPERIMENTAL + select DMA_SHARED_BUFFER default n ---help--- Say Y here to enable selecting the TV output devices for diff --git a/drivers/media/video/s5p-tv/mixer_video.c b/drivers/media/video/s5p-tv/mixer_video.c index 33fde2a..cff974a 100644 --- a/drivers/media/video/s5p-tv/mixer_video.c +++ b/drivers/media/video/s5p-tv/mixer_video.c @@ -1078,7 +1078,7 @@ struct mxr_layer *mxr_base_layer_create(struct mxr_device *mdev,
layer->vb_queue = (struct vb2_queue) { .type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE, - .io_modes = VB2_MMAP | VB2_USERPTR, + .io_modes = VB2_MMAP | VB2_USERPTR | VB2_DMABUF, .drv_priv = layer, .buf_struct_size = sizeof(struct mxr_buffer), .ops = &mxr_video_qops,
This patch enhances s5p-fimc with support for DMABUF importing via V4L2_MEMORY_DMABUF memory type.
Signed-off-by: Tomasz Stanislawski t.stanislaws@samsung.com Signed-off-by: Kyungmin Park kyungmin.park@samsung.com Acked-by: Sylwester Nawrocki s.nawrocki@samsung.com --- drivers/media/video/s5p-fimc/Kconfig | 1 + drivers/media/video/s5p-fimc/fimc-capture.c | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/media/video/s5p-fimc/Kconfig b/drivers/media/video/s5p-fimc/Kconfig index a564f7e..3106026 100644 --- a/drivers/media/video/s5p-fimc/Kconfig +++ b/drivers/media/video/s5p-fimc/Kconfig @@ -14,6 +14,7 @@ config VIDEO_S5P_FIMC depends on I2C select VIDEOBUF2_DMA_CONTIG select V4L2_MEM2MEM_DEV + select DMA_SHARED_BUFFER help This is a V4L2 driver for Samsung S5P and EXYNOS4 SoC camera host interface and video postprocessor (FIMC and FIMC-LITE) devices. diff --git a/drivers/media/video/s5p-fimc/fimc-capture.c b/drivers/media/video/s5p-fimc/fimc-capture.c index 3545745..cd27e33 100644 --- a/drivers/media/video/s5p-fimc/fimc-capture.c +++ b/drivers/media/video/s5p-fimc/fimc-capture.c @@ -1609,7 +1609,7 @@ static int fimc_register_capture_device(struct fimc_dev *fimc, q = &fimc->vid_cap.vbq; memset(q, 0, sizeof(*q)); q->type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE; - q->io_modes = VB2_MMAP | VB2_USERPTR; + q->io_modes = VB2_MMAP | VB2_USERPTR | VB2_DMABUF; q->drv_priv = fimc->vid_cap.ctx; q->ops = &fimc_capture_qops; q->mem_ops = &vb2_dma_contig_memops;
Hi Tomasz,
On Thursday 14 June 2012 15:37:34 Tomasz Stanislawski wrote:
Hello everyone, This patchset adds support for DMABUF [2] importing to V4L2 stack. The support for DMABUF exporting was moved to separate patchset due to dependency on patches for DMA mapping redesign by Marek Szyprowski [4]. This patchset depends on new scatterlist constructor [5].
There are very few remaining issues with the patch set, I think the next iteration will be the right one.
[snip]
[5] http://thread.gmane.org/gmane.linux.drivers.video-input-infrastructure/47983
What's the status of that patch ? Is it ready for v3.6 ? I'd like to see DMABUF import support in V4L2 in v3.6.
Hi Tomasz!
What is the status of this? If it is stable, then it might be a good idea to rebase to the latest for_v3.6 and make a pull request for 3.6.
Regards,
Hans
On Thu June 14 2012 15:37:34 Tomasz Stanislawski wrote:
Hello everyone, This patchset adds support for DMABUF [2] importing to V4L2 stack. The support for DMABUF exporting was moved to separate patchset due to dependency on patches for DMA mapping redesign by Marek Szyprowski [4]. This patchset depends on new scatterlist constructor [5].
v7:
- support for V4L2_MEMORY_DMABUF in v4l2-compact-ioctl32.c
- cosmetic fixes to the documentation
- added importing for vmalloc because vmap support in dmabuf for 3.5 was pull-requested
- support for dmabuf importing for VIVI
- resurrect allocation of dma-contig context
- remove reference of alloc_ctx in dma-contig buffer
- use sg_alloc_table_from_pages
- fix DMA scatterlist calls to use orig_nents instead of nents
- fix memleak in vb2_dc_sgt_foreach_page (use orig_nents instead of nents)
v6:
- fixed missing entry in v4l2_memory_names
- fixed a bug occuring after get_user_pages failure
- fixed a bug caused by using invalid vma for get_user_pages
- prepare/finish no longer call dma_sync for dmabuf buffers
v5:
- removed change of importer/exporter behaviour
- fixes vb2_dc_pages_to_sgt basing on Laurent's hints
- changed pin/unpin words to lock/unlock in Doc
v4:
- rebased on mainline 3.4-rc2
- included missing importing support for s5p-fimc and s5p-tv
- added patch for changing map/unmap for importers
- fixes to Documentation part
- coding style fixes
- pairing {map/unmap}_dmabuf in vb2-core
- fixing variable types and semantic of arguments in videobufb2-dma-contig.c
v3:
- rebased on mainline 3.4-rc1
- split 'code refactor' patch to multiple smaller patches
- squashed fixes to Sumit's patches
- patchset is no longer dependant on 'DMA mapping redesign'
- separated path for handling IO and non-IO mappings
- add documentation for DMABUF importing to V4L
- removed all DMABUF exporter related code
- removed usage of dma_get_pages extension
v2:
- extended VIDIOC_EXPBUF argument from integer memoffset to struct v4l2_exportbuffer
- added patch that breaks DMABUF spec on (un)map_atachment callcacks but allows to work with existing implementation of DMABUF prime in DRM
- all dma-contig code refactoring patches were squashed
- bugfixes
v1: List of changes since [1].
- support for DMA api extension dma_get_pages, the function is used to retrieve pages used to create DMA mapping.
- small fixes/code cleanup to videobuf2
- added prepare and finish callbacks to vb2 allocators, it is used keep consistency between dma-cpu acess to the memory (by Marek Szyprowski)
- support for exporting of DMABUF buffer in V4L2 and Videobuf2, originated from [3].
- support for dma-buf exporting in vb2-dma-contig allocator
- support for DMABUF for s5p-tv and s5p-fimc (capture interface) drivers, originated from [3]
- changed handling for userptr buffers (by Marek Szyprowski, Andrzej Pietrasiewicz)
- let mmap method to use dma_mmap_writecombine call (by Marek Szyprowski)
[1] http://thread.gmane.org/gmane.linux.drivers.video-input-infrastructure/42966... [2] https://lkml.org/lkml/2011/12/26/29 [3] http://thread.gmane.org/gmane.linux.drivers.video-input-infrastructure/36354... [4] http://thread.gmane.org/gmane.linux.kernel.cross-arch/12819 [5] http://thread.gmane.org/gmane.linux.drivers.video-input-infrastructure/47983
Laurent Pinchart (2): v4l: vb2-dma-contig: Shorten vb2_dma_contig prefix to vb2_dc v4l: vb2-dma-contig: Reorder functions
Marek Szyprowski (2): v4l: vb2: add prepare/finish callbacks to allocators v4l: vb2-dma-contig: add prepare/finish to dma-contig allocator
Sumit Semwal (4): v4l: Add DMABUF as a memory type v4l: vb2: add support for shared buffer (dma_buf) v4l: vb: remove warnings about MEMORY_DMABUF v4l: vb2-dma-contig: add support for dma_buf importing
Tomasz Stanislawski (7): Documentation: media: description of DMABUF importing in V4L2 v4l: vb2-dma-contig: remove reference of alloc_ctx from a buffer v4l: vb2-dma-contig: add support for scatterlist in userptr mode v4l: vb2-vmalloc: add support for dmabuf importing v4l: vivi: support for dmabuf importing v4l: s5p-tv: mixer: support for dmabuf importing v4l: s5p-fimc: support for dmabuf importing
Documentation/DocBook/media/v4l/compat.xml | 4 + Documentation/DocBook/media/v4l/io.xml | 179 ++++++++ .../DocBook/media/v4l/vidioc-create-bufs.xml | 3 +- Documentation/DocBook/media/v4l/vidioc-qbuf.xml | 15 + Documentation/DocBook/media/v4l/vidioc-reqbufs.xml | 47 +- drivers/media/video/Kconfig | 1 + drivers/media/video/s5p-fimc/Kconfig | 1 + drivers/media/video/s5p-fimc/fimc-capture.c | 2 +- drivers/media/video/s5p-tv/Kconfig | 1 + drivers/media/video/s5p-tv/mixer_video.c | 2 +- drivers/media/video/v4l2-compat-ioctl32.c | 16 + drivers/media/video/v4l2-ioctl.c | 1 + drivers/media/video/videobuf-core.c | 4 + drivers/media/video/videobuf2-core.c | 207 ++++++++- drivers/media/video/videobuf2-dma-contig.c | 470 +++++++++++++++++--- drivers/media/video/videobuf2-vmalloc.c | 56 +++ drivers/media/video/vivi.c | 2 +- include/linux/videodev2.h | 7 + include/media/videobuf2-core.h | 34 ++ 19 files changed, 963 insertions(+), 89 deletions(-)
On Tue July 31 2012 08:23:15 Hans Verkuil wrote:
Hi Tomasz!
What is the status of this? If it is stable, then it might be a good idea to rebase to the latest for_v3.6 and make a pull request for 3.6.
BTW, while rebasing also fix two typos in patch 9 and 15:
s/usefull/useful/
Regards,
Hans
Regards,
Hans
On Thu June 14 2012 15:37:34 Tomasz Stanislawski wrote:
Hello everyone, This patchset adds support for DMABUF [2] importing to V4L2 stack. The support for DMABUF exporting was moved to separate patchset due to dependency on patches for DMA mapping redesign by Marek Szyprowski [4]. This patchset depends on new scatterlist constructor [5].
v7:
- support for V4L2_MEMORY_DMABUF in v4l2-compact-ioctl32.c
- cosmetic fixes to the documentation
- added importing for vmalloc because vmap support in dmabuf for 3.5 was pull-requested
- support for dmabuf importing for VIVI
- resurrect allocation of dma-contig context
- remove reference of alloc_ctx in dma-contig buffer
- use sg_alloc_table_from_pages
- fix DMA scatterlist calls to use orig_nents instead of nents
- fix memleak in vb2_dc_sgt_foreach_page (use orig_nents instead of nents)
v6:
- fixed missing entry in v4l2_memory_names
- fixed a bug occuring after get_user_pages failure
- fixed a bug caused by using invalid vma for get_user_pages
- prepare/finish no longer call dma_sync for dmabuf buffers
v5:
- removed change of importer/exporter behaviour
- fixes vb2_dc_pages_to_sgt basing on Laurent's hints
- changed pin/unpin words to lock/unlock in Doc
v4:
- rebased on mainline 3.4-rc2
- included missing importing support for s5p-fimc and s5p-tv
- added patch for changing map/unmap for importers
- fixes to Documentation part
- coding style fixes
- pairing {map/unmap}_dmabuf in vb2-core
- fixing variable types and semantic of arguments in videobufb2-dma-contig.c
v3:
- rebased on mainline 3.4-rc1
- split 'code refactor' patch to multiple smaller patches
- squashed fixes to Sumit's patches
- patchset is no longer dependant on 'DMA mapping redesign'
- separated path for handling IO and non-IO mappings
- add documentation for DMABUF importing to V4L
- removed all DMABUF exporter related code
- removed usage of dma_get_pages extension
v2:
- extended VIDIOC_EXPBUF argument from integer memoffset to struct v4l2_exportbuffer
- added patch that breaks DMABUF spec on (un)map_atachment callcacks but allows to work with existing implementation of DMABUF prime in DRM
- all dma-contig code refactoring patches were squashed
- bugfixes
v1: List of changes since [1].
- support for DMA api extension dma_get_pages, the function is used to retrieve pages used to create DMA mapping.
- small fixes/code cleanup to videobuf2
- added prepare and finish callbacks to vb2 allocators, it is used keep consistency between dma-cpu acess to the memory (by Marek Szyprowski)
- support for exporting of DMABUF buffer in V4L2 and Videobuf2, originated from [3].
- support for dma-buf exporting in vb2-dma-contig allocator
- support for DMABUF for s5p-tv and s5p-fimc (capture interface) drivers, originated from [3]
- changed handling for userptr buffers (by Marek Szyprowski, Andrzej Pietrasiewicz)
- let mmap method to use dma_mmap_writecombine call (by Marek Szyprowski)
[1] http://thread.gmane.org/gmane.linux.drivers.video-input-infrastructure/42966... [2] https://lkml.org/lkml/2011/12/26/29 [3] http://thread.gmane.org/gmane.linux.drivers.video-input-infrastructure/36354... [4] http://thread.gmane.org/gmane.linux.kernel.cross-arch/12819 [5] http://thread.gmane.org/gmane.linux.drivers.video-input-infrastructure/47983
Laurent Pinchart (2): v4l: vb2-dma-contig: Shorten vb2_dma_contig prefix to vb2_dc v4l: vb2-dma-contig: Reorder functions
Marek Szyprowski (2): v4l: vb2: add prepare/finish callbacks to allocators v4l: vb2-dma-contig: add prepare/finish to dma-contig allocator
Sumit Semwal (4): v4l: Add DMABUF as a memory type v4l: vb2: add support for shared buffer (dma_buf) v4l: vb: remove warnings about MEMORY_DMABUF v4l: vb2-dma-contig: add support for dma_buf importing
Tomasz Stanislawski (7): Documentation: media: description of DMABUF importing in V4L2 v4l: vb2-dma-contig: remove reference of alloc_ctx from a buffer v4l: vb2-dma-contig: add support for scatterlist in userptr mode v4l: vb2-vmalloc: add support for dmabuf importing v4l: vivi: support for dmabuf importing v4l: s5p-tv: mixer: support for dmabuf importing v4l: s5p-fimc: support for dmabuf importing
Documentation/DocBook/media/v4l/compat.xml | 4 + Documentation/DocBook/media/v4l/io.xml | 179 ++++++++ .../DocBook/media/v4l/vidioc-create-bufs.xml | 3 +- Documentation/DocBook/media/v4l/vidioc-qbuf.xml | 15 + Documentation/DocBook/media/v4l/vidioc-reqbufs.xml | 47 +- drivers/media/video/Kconfig | 1 + drivers/media/video/s5p-fimc/Kconfig | 1 + drivers/media/video/s5p-fimc/fimc-capture.c | 2 +- drivers/media/video/s5p-tv/Kconfig | 1 + drivers/media/video/s5p-tv/mixer_video.c | 2 +- drivers/media/video/v4l2-compat-ioctl32.c | 16 + drivers/media/video/v4l2-ioctl.c | 1 + drivers/media/video/videobuf-core.c | 4 + drivers/media/video/videobuf2-core.c | 207 ++++++++- drivers/media/video/videobuf2-dma-contig.c | 470 +++++++++++++++++--- drivers/media/video/videobuf2-vmalloc.c | 56 +++ drivers/media/video/vivi.c | 2 +- include/linux/videodev2.h | 7 + include/media/videobuf2-core.h | 34 ++ 19 files changed, 963 insertions(+), 89 deletions(-)
-- 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
linaro-mm-sig@lists.linaro.org