[PATCHv2 0/9] Support for dmabuf exporting for videobuf2

[PATCHv2 0/9] Support for dmabuf exporting for videobuf2

[Tomasz Stanislawski]

Hello everyone,
The patches adds support for DMABUF exporting to V4L2 stack.  The latest
support for DMABUF importing was posted in [1]. The exporter part is dependant
on DMA mapping redesign [2] which is expected to be merged into the mainline.
Therefore it is posted as a separate patchset. Moreover some patches depends on
vmap extension for DMABUF by Dave Airlie [3] and sg_alloc_table_from_pages
function [4]. The last patch 'v4l: vb2-dma-contig: use dma_get_sgtable' depends
on dma_get_sgtable extension to DMA api [5].

The tree with all the patches and extensions is available at:
repo: git://git.infradead.org/users/kmpark/linux-2.6-samsung
branch: media-for3.5-vb2-dmabuf-v7

Changelog:

v2:
- add documentation for DMABUF exporting
- squashed 'let mmap method to use dma_mmap_coherent call' with
  'remove vb2_mmap_pfn_range function'
- move setup of scatterlist for MMAP buffers from alloc to DMABUF export code
- use locking to serialize map/unmap of DMABUF attachments
- squash vmap/kmap, setup of sg lists, allocation in attachments into
  dma-contig exporter patch
- fix occasional failure of follow_pfn trick by using init_mm in artificial
  VMA
- add support for exporting in s5p-mfc driver
- drop all code that duplicates sg_alloc_table_from_pages
- introduce usage of dma_get_sgtable as generic solution
  to follow_pfn trick

v1:
- updated setup of VIDIOC_EXPBUF ioctl
- doc updates
- introduced workaround to avoid using dma_get_pages,
- removed caching of exported dmabuf to avoid existence of circular reference
  between dmabuf and vb2_dc_buf or resource leakage
- removed all 'change behaviour' patches
- inital support for exporting in s5p-mfs driver
- removal of vb2_mmap_pfn_range that is no longer used
- use sg_alloc_table_from_pages instead of creating sglist in vb2_dc code
- move attachment allocation to exporter's attach callback

v0: RFC
- initial version

[1] http://thread.gmane.org/gmane.linux.drivers.video-input-infrastructure/49438
[2] http://thread.gmane.org/gmane.linux.kernel.cross-arch/14098
[3] http://permalink.gmane.org/gmane.comp.video.dri.devel/69302
[4] This patchset is rebased on 3.4-rc1 plus the following patchsets:
[5] http://www.spinics.net/lists/linux-arch/msg18282.html

Marek Szyprowski (1):
  v4l: vb2-dma-contig: let mmap method to use dma_mmap_coherent call

Tomasz Stanislawski (8):
  Documentation: media: description of DMABUF exporting in V4L2
  v4l: add buffer exporting via dmabuf
  v4l: vb2: add buffer exporting via dmabuf
  v4l: vb2-dma-contig: add support for DMABUF exporting
  v4l: s5p-fimc: support for dmabuf exporting
  v4l: s5p-tv: mixer: support for dmabuf exporting
  v4l: s5p-mfc: support for dmabuf exporting
  v4l: vb2-dma-contig: use dma_get_sgtable

 Documentation/DocBook/media/v4l/compat.xml        |    3 +
 Documentation/DocBook/media/v4l/io.xml            |    3 +
 Documentation/DocBook/media/v4l/v4l2.xml          |    1 +
 Documentation/DocBook/media/v4l/vidioc-expbuf.xml |  223 ++++++++++++++++++++
 drivers/media/video/s5p-fimc/fimc-capture.c       |    9 +
 drivers/media/video/s5p-mfc/s5p_mfc_dec.c         |   18 ++
 drivers/media/video/s5p-mfc/s5p_mfc_enc.c         |   18 ++
 drivers/media/video/s5p-tv/mixer_video.c          |   10 +
 drivers/media/video/v4l2-compat-ioctl32.c         |    1 +
 drivers/media/video/v4l2-dev.c                    |    1 +
 drivers/media/video/v4l2-ioctl.c                  |    6 +
 drivers/media/video/videobuf2-core.c              |   67 ++++++
 drivers/media/video/videobuf2-dma-contig.c        |  224 ++++++++++++++++++++-
 drivers/media/video/videobuf2-memops.c            |   40 ----
 include/linux/videodev2.h                         |   26 +++
 include/media/v4l2-ioctl.h                        |    2 +
 include/media/videobuf2-core.h                    |    2 +
 include/media/videobuf2-memops.h                  |    5 -
 18 files changed, 612 insertions(+), 47 deletions(-)
 create mode 100644 Documentation/DocBook/media/v4l/vidioc-expbuf.xml

-- 
1.7.9.5

[PATCHv2 0/9] Support for dmabuf exporting for videobuf2

[Tomasz Stanislawski]

Hello everyone,
The patches adds support for DMABUF exporting to V4L2 stack.  The latest
support for DMABUF importing was posted in [1]. The exporter part is dependant
on DMA mapping redesign [2] which is expected to be merged into the mainline.
Therefore it is posted as a separate patchset. Moreover some patches depends on
vmap extension for DMABUF by Dave Airlie [3] and sg_alloc_table_from_pages
function [4]. The last patch 'v4l: vb2-dma-contig: use dma_get_sgtable' depends
on dma_get_sgtable extension to DMA api [5].

The tree with all the patches and extensions is available at:
repo: git://git.infradead.org/users/kmpark/linux-2.6-samsung
branch: media-for3.5-vb2-dmabuf-v7

Changelog:

v2:
- add documentation for DMABUF exporting
- squashed 'let mmap method to use dma_mmap_coherent call' with
  'remove vb2_mmap_pfn_range function'
- move setup of scatterlist for MMAP buffers from alloc to DMABUF export code
- use locking to serialize map/unmap of DMABUF attachments
- squash vmap/kmap, setup of sg lists, allocation in attachments into
  dma-contig exporter patch
- fix occasional failure of follow_pfn trick by using init_mm in artificial
  VMA
- add support for exporting in s5p-mfc driver
- drop all code that duplicates sg_alloc_table_from_pages
- introduce usage of dma_get_sgtable as generic solution
  to follow_pfn trick

v1:
- updated setup of VIDIOC_EXPBUF ioctl
- doc updates
- introduced workaround to avoid using dma_get_pages,
- removed caching of exported dmabuf to avoid existence of circular reference
  between dmabuf and vb2_dc_buf or resource leakage
- removed all 'change behaviour' patches
- inital support for exporting in s5p-mfs driver
- removal of vb2_mmap_pfn_range that is no longer used
- use sg_alloc_table_from_pages instead of creating sglist in vb2_dc code
- move attachment allocation to exporter's attach callback

v0: RFC
- initial version

[1] http://thread.gmane.org/gmane.linux.drivers.video-input-infrastructure/49438
[2] http://thread.gmane.org/gmane.linux.kernel.cross-arch/14098
[3] http://permalink.gmane.org/gmane.comp.video.dri.devel/69302
[4] This patchset is rebased on 3.4-rc1 plus the following patchsets:
[5] http://www.spinics.net/lists/linux-arch/msg18282.html

Marek Szyprowski (1):
  v4l: vb2-dma-contig: let mmap method to use dma_mmap_coherent call

Tomasz Stanislawski (8):
  Documentation: media: description of DMABUF exporting in V4L2
  v4l: add buffer exporting via dmabuf
  v4l: vb2: add buffer exporting via dmabuf
  v4l: vb2-dma-contig: add support for DMABUF exporting
  v4l: s5p-fimc: support for dmabuf exporting
  v4l: s5p-tv: mixer: support for dmabuf exporting
  v4l: s5p-mfc: support for dmabuf exporting
  v4l: vb2-dma-contig: use dma_get_sgtable

 Documentation/DocBook/media/v4l/compat.xml        |    3 +
 Documentation/DocBook/media/v4l/io.xml            |    3 +
 Documentation/DocBook/media/v4l/v4l2.xml          |    1 +
 Documentation/DocBook/media/v4l/vidioc-expbuf.xml |  223 ++++++++++++++++++++
 drivers/media/video/s5p-fimc/fimc-capture.c       |    9 +
 drivers/media/video/s5p-mfc/s5p_mfc_dec.c         |   18 ++
 drivers/media/video/s5p-mfc/s5p_mfc_enc.c         |   18 ++
 drivers/media/video/s5p-tv/mixer_video.c          |   10 +
 drivers/media/video/v4l2-compat-ioctl32.c         |    1 +
 drivers/media/video/v4l2-dev.c                    |    1 +
 drivers/media/video/v4l2-ioctl.c                  |    6 +
 drivers/media/video/videobuf2-core.c              |   67 ++++++
 drivers/media/video/videobuf2-dma-contig.c        |  224 ++++++++++++++++++++-
 drivers/media/video/videobuf2-memops.c            |   40 ----
 include/linux/videodev2.h                         |   26 +++
 include/media/v4l2-ioctl.h                        |    2 +
 include/media/videobuf2-core.h                    |    2 +
 include/media/videobuf2-memops.h                  |    5 -
 18 files changed, 612 insertions(+), 47 deletions(-)
 create mode 100644 Documentation/DocBook/media/v4l/vidioc-expbuf.xml

-- 
1.7.9.5

[PATCHv2 1/9] v4l: vb2-dma-contig: let mmap method to use dma_mmap_coherent call

[Tomasz Stanislawski]

From: Marek Szyprowski <m.szyprowski@samsung.com>

Let mmap method to use dma_mmap_coherent call.  This patch depends on DMA
mapping redesign patches because the usage of dma_mmap_coherent breaks
dma-contig allocator for architectures other than ARM and AVR.

Moreover, this patch removes vb2_mmap_pfn_range from videobuf2 helpers.
The function is no longer used in vb2 code.

Suggested-by: Laurent Pinchart <laurent.pinchart@ideasonboard.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 |   28 +++++++++++++++++--
 drivers/media/video/videobuf2-memops.c     |   40 ----------------------------
 include/media/videobuf2-memops.h           |    5 ----
 3 files changed, 26 insertions(+), 47 deletions(-)

diff --git a/drivers/media/video/videobuf2-dma-contig.c b/drivers/media/video/videobuf2-dma-contig.c
index 040829b..00b776c 100644
--- a/drivers/media/video/videobuf2-dma-contig.c
+++ b/drivers/media/video/videobuf2-dma-contig.c
@@ -178,14 +178,38 @@ static void *vb2_dc_alloc(void *alloc_ctx, unsigned long size)
 static int vb2_dc_mmap(void *buf_priv, struct vm_area_struct *vma)
 {
 	struct vb2_dc_buf *buf = buf_priv;
+	int ret;
 
 	if (!buf) {
 		printk(KERN_ERR "No buffer to map\n");
 		return -EINVAL;
 	}
 
-	return vb2_mmap_pfn_range(vma, buf->dma_addr, buf->size,
-				  &vb2_common_vm_ops, &buf->handler);
+	/*
+	 * dma_mmap_* uses vm_pgoff as in-buffer offset, but we want to
+	 * map whole buffer
+	 */
+	vma->vm_pgoff = 0;
+
+	ret = dma_mmap_coherent(buf->dev, vma, buf->vaddr,
+		buf->dma_addr, buf->size);
+
+	if (ret) {
+		printk(KERN_ERR "Remapping memory failed, error: %d\n", ret);
+		return ret;
+	}
+
+	vma->vm_flags		|= VM_DONTEXPAND | VM_RESERVED;
+	vma->vm_private_data	= &buf->handler;
+	vma->vm_ops		= &vb2_common_vm_ops;
+
+	vma->vm_ops->open(vma);
+
+	printk(KERN_DEBUG "%s: mapped dma addr 0x%08lx at 0x%08lx, size %ld\n",
+		__func__, (unsigned long)buf->dma_addr, vma->vm_start,
+		buf->size);
+
+	return 0;
 }
 
 /*********************************************/
diff --git a/drivers/media/video/videobuf2-memops.c b/drivers/media/video/videobuf2-memops.c
index 504cd4c..81c1ad8 100644
--- a/drivers/media/video/videobuf2-memops.c
+++ b/drivers/media/video/videobuf2-memops.c
@@ -137,46 +137,6 @@ int vb2_get_contig_userptr(unsigned long vaddr, unsigned long size,
 EXPORT_SYMBOL_GPL(vb2_get_contig_userptr);
 
 /**
- * vb2_mmap_pfn_range() - map physical pages to userspace
- * @vma:	virtual memory region for the mapping
- * @paddr:	starting physical address of the memory to be mapped
- * @size:	size of the memory to be mapped
- * @vm_ops:	vm operations to be assigned to the created area
- * @priv:	private data to be associated with the area
- *
- * Returns 0 on success.
- */
-int vb2_mmap_pfn_range(struct vm_area_struct *vma, unsigned long paddr,
-				unsigned long size,
-				const struct vm_operations_struct *vm_ops,
-				void *priv)
-{
-	int ret;
-
-	size = min_t(unsigned long, vma->vm_end - vma->vm_start, size);
-
-	vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot);
-	ret = remap_pfn_range(vma, vma->vm_start, paddr >> PAGE_SHIFT,
-				size, vma->vm_page_prot);
-	if (ret) {
-		printk(KERN_ERR "Remapping memory failed, error: %d\n", ret);
-		return ret;
-	}
-
-	vma->vm_flags		|= VM_DONTEXPAND | VM_RESERVED;
-	vma->vm_private_data	= priv;
-	vma->vm_ops		= vm_ops;
-
-	vma->vm_ops->open(vma);
-
-	pr_debug("%s: mapped paddr 0x%08lx at 0x%08lx, size %ld\n",
-			__func__, paddr, vma->vm_start, size);
-
-	return 0;
-}
-EXPORT_SYMBOL_GPL(vb2_mmap_pfn_range);
-
-/**
  * vb2_common_vm_open() - increase refcount of the vma
  * @vma:	virtual memory region for the mapping
  *
diff --git a/include/media/videobuf2-memops.h b/include/media/videobuf2-memops.h
index 84e1f6c..f05444c 100644
--- a/include/media/videobuf2-memops.h
+++ b/include/media/videobuf2-memops.h
@@ -33,11 +33,6 @@ extern const struct vm_operations_struct vb2_common_vm_ops;
 int vb2_get_contig_userptr(unsigned long vaddr, unsigned long size,
 			   struct vm_area_struct **res_vma, dma_addr_t *res_pa);
 
-int vb2_mmap_pfn_range(struct vm_area_struct *vma, unsigned long paddr,
-				unsigned long size,
-				const struct vm_operations_struct *vm_ops,
-				void *priv);
-
 struct vm_area_struct *vb2_get_vma(struct vm_area_struct *vma);
 void vb2_put_vma(struct vm_area_struct *vma);
 
-- 
1.7.9.5

[PATCHv2 2/9] Documentation: media: description of DMABUF exporting in V4L2

[Tomasz Stanislawski]

This patch adds description and usage examples for exporting
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        |    3 +
 Documentation/DocBook/media/v4l/io.xml            |    3 +
 Documentation/DocBook/media/v4l/v4l2.xml          |    1 +
 Documentation/DocBook/media/v4l/vidioc-expbuf.xml |  223 +++++++++++++++++++++
 4 files changed, 230 insertions(+)
 create mode 100644 Documentation/DocBook/media/v4l/vidioc-expbuf.xml

diff --git a/Documentation/DocBook/media/v4l/compat.xml b/Documentation/DocBook/media/v4l/compat.xml
index 07a311f..7773450 100644
--- a/Documentation/DocBook/media/v4l/compat.xml
+++ b/Documentation/DocBook/media/v4l/compat.xml
@@ -2591,6 +2591,9 @@ ioctls.</para>
 	  <para>Importing DMABUF file descriptors as a new IO method described
 	  in <xref linkend="dmabuf" />.</para>
         </listitem>
+        <listitem>
+	  <para>Exporting DMABUF files using &VIDIOC-EXPBUF; ioctl.</para>
+        </listitem>
       </itemizedlist>
     </section>
 
diff --git a/Documentation/DocBook/media/v4l/io.xml b/Documentation/DocBook/media/v4l/io.xml
index f55b0ab..7a0dfc9 100644
--- a/Documentation/DocBook/media/v4l/io.xml
+++ b/Documentation/DocBook/media/v4l/io.xml
@@ -488,6 +488,9 @@ 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>Refer to <link linked="vidioc-expbuf"> DMABUF exporting </link> for
+details about exporting a V4L2 buffers as DMABUF file descriptors.</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
diff --git a/Documentation/DocBook/media/v4l/v4l2.xml b/Documentation/DocBook/media/v4l/v4l2.xml
index 015c561..8f650d2 100644
--- a/Documentation/DocBook/media/v4l/v4l2.xml
+++ b/Documentation/DocBook/media/v4l/v4l2.xml
@@ -561,6 +561,7 @@ and discussions on the V4L mailing list.</revremark>
     &sub-log-status;
     &sub-overlay;
     &sub-qbuf;
+    &sub-expbuf;
     &sub-querybuf;
     &sub-querycap;
     &sub-queryctrl;
diff --git a/Documentation/DocBook/media/v4l/vidioc-expbuf.xml b/Documentation/DocBook/media/v4l/vidioc-expbuf.xml
new file mode 100644
index 0000000..30ebf67
--- /dev/null
+++ b/Documentation/DocBook/media/v4l/vidioc-expbuf.xml
@@ -0,0 +1,223 @@
+<refentry id="vidioc-expbuf">
+
+  <refmeta>
+    <refentrytitle>ioctl VIDIOC_EXPBUF</refentrytitle>
+    &manvol;
+  </refmeta>
+
+  <refnamediv>
+    <refname>VIDIOC_EXPBUF</refname>
+    <refpurpose>Export a buffer as a DMABUF file descriptor.</refpurpose>
+  </refnamediv>
+
+  <refsynopsisdiv>
+    <funcsynopsis>
+      <funcprototype>
+	<funcdef>int <function>ioctl</function></funcdef>
+	<paramdef>int <parameter>fd</parameter></paramdef>
+	<paramdef>int <parameter>request</parameter></paramdef>
+	<paramdef>struct v4l2_exportbuffer *<parameter>argp</parameter></paramdef>
+      </funcprototype>
+    </funcsynopsis>
+  </refsynopsisdiv>
+
+  <refsect1>
+    <title>Arguments</title>
+
+    <variablelist>
+      <varlistentry>
+	<term><parameter>fd</parameter></term>
+	<listitem>
+	  <para>&fd;</para>
+	</listitem>
+      </varlistentry>
+      <varlistentry>
+	<term><parameter>request</parameter></term>
+	<listitem>
+	  <para>VIDIOC_EXPBUF</para>
+	</listitem>
+      </varlistentry>
+      <varlistentry>
+	<term><parameter>argp</parameter></term>
+	<listitem>
+	  <para></para>
+	</listitem>
+      </varlistentry>
+    </variablelist>
+  </refsect1>
+
+  <refsect1>
+    <title>Description</title>
+
+    <note>
+      <title>Experimental</title>
+      <para>This is an <link linkend="experimental"> experimental </link>
+      interface and may change in the future.</para>
+    </note>
+
+<para>This ioctl is an extension to the <link linkend="mmap">memory
+mapping</link> I/O method therefore it is available only for
+<constant>V4L2_MEMORY_MMAP</constant> buffers.  It can be used to export a
+buffer as DMABUF file at any time after buffers have been allocated with the
+&VIDIOC-REQBUFS; ioctl.</para>
+
+<para>Prior to exporting an application calls <link
+linkend="vidioc-querybuf">VIDIOC_QUERYBUF</link> to obtain memory offsets. When
+using the <link linkend="planar-apis">multi-planar API</link> every plane has
+own offset.</para>
+
+<para>To export a buffer, the application fills &v4l2-exportbuffer;.  The
+<structfield> mem_offset </structfield> field is set to the offset obtained
+from <constant> VIDIOC_QUERYBUF </constant>.  Additional flags may be posted in
+the <structfield> flags </structfield> field.  Refer to manual for open syscall
+for details. Currently only O_CLOEXEC is guaranteed to be supported.  All other
+fields must be set to zero.  In a case of multi-planar API, every plane is
+exported separately using multiple <constant> VIDIOC_EXPBUF </constant>
+calls.</para>
+
+<para> After calling <constant>VIDIOC_EXPBUF</constant> the <structfield> fd
+</structfield> field will be set by a driver.  This is a DMABUF file
+descriptor. The application may pass it to other API. Refer to <link
+linkend="dmabuf">DMABUF importing</link> for details about importing DMABUF
+files into V4L2 nodes. A developer is encouraged to close a DMABUF file when it
+is no longer used.  </para>
+
+  </refsect1>
+  <refsect1>
+   <section>
+      <title>Examples</title>
+
+      <example>
+	<title>Exporting a buffer.</title>
+	<programlisting>
+int buffer_export(int v4lfd, &v4l2-buf-type; bt, int index, int *dmafd)
+{
+	&v4l2-buffer; buf;
+	&v4l2-exportbuffer; expbuf;
+
+	memset(&amp;buf, 0, sizeof buf);
+	buf.type = bt;
+	buf.memory = V4L2_MEMORY_MMAP;
+	buf.index = index;
+
+	if (ioctl (v4lfd, &VIDIOC-QUERYBUF;, &amp;buf) == -1) {
+		perror ("VIDIOC_QUERYBUF");
+		return -1;
+	}
+
+	memset(&amp;expbuf, 0, sizeof expbuf);
+	expbuf.mem_offset = buf.m.offset;
+	if (ioctl (v4lfd, &VIDIOC-EXPBUF;, &amp;expbuf) == -1) {
+		perror ("VIDIOC_EXPBUF");
+		return -1;
+	}
+
+	*dmafd = expbuf.fd;
+
+	return 0;
+}
+        </programlisting>
+      </example>
+
+      <example>
+	<title>Exporting a buffer using multi plane API.</title>
+	<programlisting>
+int buffer_export_mp(int v4lfd, &v4l2-buf-type; bt, int index,
+	int dmafd[], int n_planes)
+{
+	&v4l2-buffer; buf;
+	&v4l2-plane; planes[VIDEO_MAX_PLANES];
+	int i;
+
+	memset(&amp;buf, 0, sizeof buf);
+	buf.type = bt;
+	buf.memory = V4L2_MEMORY_MMAP;
+	buf.index = index;
+	buf.m.planes = planes;
+	buf.length = n_planes;
+	memset(&amp;planes, 0, sizeof planes);
+
+	if (ioctl (v4lfd, &VIDIOC-QUERYBUF;, &amp;buf) == -1) {
+		perror ("VIDIOC_QUERYBUF");
+		return -1;
+	}
+
+	for (i = 0; i &lt; n_planes; ++i) {
+		&v4l2-exportbuffer; expbuf;
+
+		memset(&amp;expbuf, 0, sizeof expbuf);
+		expbuf.mem_offset = plane[i].m.offset;
+		if (ioctl (v4lfd, &VIDIOC-EXPBUF;, &amp;expbuf) == -1) {
+			perror ("VIDIOC_EXPBUF");
+			while (i)
+				close(dmafd[--i]);
+			return -1;
+		}
+		dmafd[i] = expbuf.fd;
+	}
+
+	return 0;
+}
+        </programlisting>
+      </example>
+   </section>
+  </refsect1>
+
+  <refsect1>
+    <table pgwide="1" frame="none" id="v4l2-exportbuffer">
+      <title>struct <structname>v4l2_exportbuffer</structname></title>
+      <tgroup cols="3">
+	&cs-str;
+	<tbody valign="top">
+	  <row>
+	    <entry>__u32</entry>
+	    <entry><structfield>fd</structfield></entry>
+	    <entry>The DMABUF file descriptor associated with a buffer. Set by
+		a driver.</entry>
+	  </row>
+	  <row>
+	    <entry>__u32</entry>
+	    <entry><structfield>reserved0</structfield></entry>
+	    <entry>Reserved field for future use. Must be set to zero.</entry>
+	  </row>
+	  <row>
+	    <entry>__u32</entry>
+	    <entry><structfield>mem_offset</structfield></entry>
+	    <entry>Buffer memory offset as returned by <constant>
+VIDIOC_QUERYBUF </constant> in &v4l2-buffer;<structfield> ::m.offset
+</structfield> (for single-plane formats) or &v4l2-plane;<structfield>
+::m.offset </structfield> (for multi-planar formats)</entry>
+	  </row>
+	  <row>
+	    <entry>__u32</entry>
+	    <entry><structfield>flags</structfield></entry>
+	    <entry>Flags for newly created file, currently only <constant>
+O_CLOEXEC </constant> is supported, refer to manual of open syscall for more
+details.</entry>
+	  </row>
+	  <row>
+	    <entry>__u32</entry>
+	    <entry><structfield>reserved[12]</structfield></entry>
+	    <entry>Reserved field for future use. Must be set to zero.</entry>
+	  </row>
+	</tbody>
+      </tgroup>
+    </table>
+
+  </refsect1>
+
+  <refsect1>
+    &return-value;
+    <variablelist>
+      <varlistentry>
+	<term><errorcode>EINVAL</errorcode></term>
+	<listitem>
+	  <para>A queue is not in MMAP mode or DMABUF exporting is not
+supported or <structfield> flag </structfield> or <structfield> mem_offset
+</structfield> fields are invalid.</para>
+	</listitem>
+      </varlistentry>
+    </variablelist>
+  </refsect1>
+
+</refentry>
-- 
1.7.9.5

[PATCHv2 3/9] v4l: add buffer exporting via dmabuf

[Tomasz Stanislawski]

This patch adds extension to V4L2 api. It allow to export a mmap buffer as file
descriptor. New ioctl VIDIOC_EXPBUF is added. It takes a buffer offset used by
mmap and return a file descriptor on success.

Signed-off-by: Tomasz Stanislawski <t.stanislaws@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 drivers/media/video/v4l2-compat-ioctl32.c |    1 +
 drivers/media/video/v4l2-dev.c            |    1 +
 drivers/media/video/v4l2-ioctl.c          |    6 ++++++
 include/linux/videodev2.h                 |   26 ++++++++++++++++++++++++++
 include/media/v4l2-ioctl.h                |    2 ++
 5 files changed, 36 insertions(+)

diff --git a/drivers/media/video/v4l2-compat-ioctl32.c b/drivers/media/video/v4l2-compat-ioctl32.c
index d33ab18..141e745 100644
--- a/drivers/media/video/v4l2-compat-ioctl32.c
+++ b/drivers/media/video/v4l2-compat-ioctl32.c
@@ -970,6 +970,7 @@ long v4l2_compat_ioctl32(struct file *file, unsigned int cmd, unsigned long arg)
 	case VIDIOC_S_FBUF32:
 	case VIDIOC_OVERLAY32:
 	case VIDIOC_QBUF32:
+	case VIDIOC_EXPBUF:
 	case VIDIOC_DQBUF32:
 	case VIDIOC_STREAMON32:
 	case VIDIOC_STREAMOFF32:
diff --git a/drivers/media/video/v4l2-dev.c b/drivers/media/video/v4l2-dev.c
index 5ccbd46..6bf6307 100644
--- a/drivers/media/video/v4l2-dev.c
+++ b/drivers/media/video/v4l2-dev.c
@@ -597,6 +597,7 @@ static void determine_valid_ioctls(struct video_device *vdev)
 	SET_VALID_IOCTL(ops, VIDIOC_REQBUFS, vidioc_reqbufs);
 	SET_VALID_IOCTL(ops, VIDIOC_QUERYBUF, vidioc_querybuf);
 	SET_VALID_IOCTL(ops, VIDIOC_QBUF, vidioc_qbuf);
+	SET_VALID_IOCTL(ops, VIDIOC_EXPBUF, vidioc_expbuf);
 	SET_VALID_IOCTL(ops, VIDIOC_DQBUF, vidioc_dqbuf);
 	SET_VALID_IOCTL(ops, VIDIOC_OVERLAY, vidioc_overlay);
 	SET_VALID_IOCTL(ops, VIDIOC_G_FBUF, vidioc_g_fbuf);
diff --git a/drivers/media/video/v4l2-ioctl.c b/drivers/media/video/v4l2-ioctl.c
index 31fc2ad..a73b14e 100644
--- a/drivers/media/video/v4l2-ioctl.c
+++ b/drivers/media/video/v4l2-ioctl.c
@@ -212,6 +212,7 @@ static struct v4l2_ioctl_info v4l2_ioctls[] = {
 	IOCTL_INFO(VIDIOC_S_FBUF, INFO_FL_PRIO),
 	IOCTL_INFO(VIDIOC_OVERLAY, INFO_FL_PRIO),
 	IOCTL_INFO(VIDIOC_QBUF, 0),
+	IOCTL_INFO(VIDIOC_EXPBUF, 0),
 	IOCTL_INFO(VIDIOC_DQBUF, 0),
 	IOCTL_INFO(VIDIOC_STREAMON, INFO_FL_PRIO),
 	IOCTL_INFO(VIDIOC_STREAMOFF, INFO_FL_PRIO),
@@ -957,6 +958,11 @@ static long __video_do_ioctl(struct file *file,
 			dbgbuf(cmd, vfd, p);
 		break;
 	}
+	case VIDIOC_EXPBUF:
+	{
+		ret = ops->vidioc_expbuf(file, fh, arg);
+		break;
+	}
 	case VIDIOC_DQBUF:
 	{
 		struct v4l2_buffer *p = arg;
diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h
index 51b20f4..e8893a5 100644
--- a/include/linux/videodev2.h
+++ b/include/linux/videodev2.h
@@ -684,6 +684,31 @@ struct v4l2_buffer {
 #define V4L2_BUF_FLAG_NO_CACHE_INVALIDATE	0x0800
 #define V4L2_BUF_FLAG_NO_CACHE_CLEAN		0x1000
 
+/**
+ * struct v4l2_exportbuffer - export of video buffer as DMABUF file descriptor
+ *
+ * @fd:		file descriptor associated with DMABUF (set by driver)
+ * @mem_offset:	buffer memory offset as returned by VIDIOC_QUERYBUF in struct
+ *		v4l2_buffer::m.offset (for single-plane formats) or
+ *		v4l2_plane::m.offset (for multi-planar formats)
+ * @flags:	flags for newly created file, currently only O_CLOEXEC is
+ *		supported, refer to manual of open syscall for more details
+ *
+ * Contains data used for exporting a video buffer as DMABUF file descriptor.
+ * The buffer is identified by a 'cookie' returned by VIDIOC_QUERYBUF
+ * (identical to the cookie used to mmap() the buffer to userspace). All
+ * reserved fields must be set to zero. The field reserved0 is expected to
+ * become a structure 'type' allowing an alternative layout of the structure
+ * content. Therefore this field should not be used for any other extensions.
+ */
+struct v4l2_exportbuffer {
+	__u32		fd;
+	__u32		reserved0;
+	__u32		mem_offset;
+	__u32		flags;
+	__u32		reserved[12];
+};
+
 /*
  *	O V E R L A Y   P R E V I E W
  */
@@ -2553,6 +2578,7 @@ struct v4l2_create_buffers {
 #define VIDIOC_S_FBUF		 _IOW('V', 11, struct v4l2_framebuffer)
 #define VIDIOC_OVERLAY		 _IOW('V', 14, int)
 #define VIDIOC_QBUF		_IOWR('V', 15, struct v4l2_buffer)
+#define VIDIOC_EXPBUF		_IOWR('V', 16, struct v4l2_exportbuffer)
 #define VIDIOC_DQBUF		_IOWR('V', 17, struct v4l2_buffer)
 #define VIDIOC_STREAMON		 _IOW('V', 18, int)
 #define VIDIOC_STREAMOFF	 _IOW('V', 19, int)
diff --git a/include/media/v4l2-ioctl.h b/include/media/v4l2-ioctl.h
index d8b76f7..ccd1faa 100644
--- a/include/media/v4l2-ioctl.h
+++ b/include/media/v4l2-ioctl.h
@@ -119,6 +119,8 @@ struct v4l2_ioctl_ops {
 	int (*vidioc_reqbufs) (struct file *file, void *fh, struct v4l2_requestbuffers *b);
 	int (*vidioc_querybuf)(struct file *file, void *fh, struct v4l2_buffer *b);
 	int (*vidioc_qbuf)    (struct file *file, void *fh, struct v4l2_buffer *b);
+	int (*vidioc_expbuf)  (struct file *file, void *fh,
+				struct v4l2_exportbuffer *e);
 	int (*vidioc_dqbuf)   (struct file *file, void *fh, struct v4l2_buffer *b);
 
 	int (*vidioc_create_bufs)(struct file *file, void *fh, struct v4l2_create_buffers *b);
-- 
1.7.9.5

Re: [PATCHv2 3/9] v4l: add buffer exporting via dmabuf

[Hans Verkuil]

On Thu June 14 2012 16:32:23 Tomasz Stanislawski wrote:
> This patch adds extension to V4L2 api. It allow to export a mmap buffer as file
> descriptor. New ioctl VIDIOC_EXPBUF is added. It takes a buffer offset used by
> mmap and return a file descriptor on success.
> 
> Signed-off-by: Tomasz Stanislawski <t.stanislaws@samsung.com>
> Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> ---
>  drivers/media/video/v4l2-compat-ioctl32.c |    1 +
>  drivers/media/video/v4l2-dev.c            |    1 +
>  drivers/media/video/v4l2-ioctl.c          |    6 ++++++
>  include/linux/videodev2.h                 |   26 ++++++++++++++++++++++++++
>  include/media/v4l2-ioctl.h                |    2 ++
>  5 files changed, 36 insertions(+)
> 
> diff --git a/drivers/media/video/v4l2-compat-ioctl32.c b/drivers/media/video/v4l2-compat-ioctl32.c
> index d33ab18..141e745 100644
> --- a/drivers/media/video/v4l2-compat-ioctl32.c
> +++ b/drivers/media/video/v4l2-compat-ioctl32.c
> @@ -970,6 +970,7 @@ long v4l2_compat_ioctl32(struct file *file, unsigned int cmd, unsigned long arg)
>  	case VIDIOC_S_FBUF32:
>  	case VIDIOC_OVERLAY32:
>  	case VIDIOC_QBUF32:
> +	case VIDIOC_EXPBUF:
>  	case VIDIOC_DQBUF32:
>  	case VIDIOC_STREAMON32:
>  	case VIDIOC_STREAMOFF32:
> diff --git a/drivers/media/video/v4l2-dev.c b/drivers/media/video/v4l2-dev.c
> index 5ccbd46..6bf6307 100644
> --- a/drivers/media/video/v4l2-dev.c
> +++ b/drivers/media/video/v4l2-dev.c
> @@ -597,6 +597,7 @@ static void determine_valid_ioctls(struct video_device *vdev)
>  	SET_VALID_IOCTL(ops, VIDIOC_REQBUFS, vidioc_reqbufs);
>  	SET_VALID_IOCTL(ops, VIDIOC_QUERYBUF, vidioc_querybuf);
>  	SET_VALID_IOCTL(ops, VIDIOC_QBUF, vidioc_qbuf);
> +	SET_VALID_IOCTL(ops, VIDIOC_EXPBUF, vidioc_expbuf);
>  	SET_VALID_IOCTL(ops, VIDIOC_DQBUF, vidioc_dqbuf);
>  	SET_VALID_IOCTL(ops, VIDIOC_OVERLAY, vidioc_overlay);
>  	SET_VALID_IOCTL(ops, VIDIOC_G_FBUF, vidioc_g_fbuf);
> diff --git a/drivers/media/video/v4l2-ioctl.c b/drivers/media/video/v4l2-ioctl.c
> index 31fc2ad..a73b14e 100644
> --- a/drivers/media/video/v4l2-ioctl.c
> +++ b/drivers/media/video/v4l2-ioctl.c
> @@ -212,6 +212,7 @@ static struct v4l2_ioctl_info v4l2_ioctls[] = {
>  	IOCTL_INFO(VIDIOC_S_FBUF, INFO_FL_PRIO),
>  	IOCTL_INFO(VIDIOC_OVERLAY, INFO_FL_PRIO),
>  	IOCTL_INFO(VIDIOC_QBUF, 0),
> +	IOCTL_INFO(VIDIOC_EXPBUF, 0),
>  	IOCTL_INFO(VIDIOC_DQBUF, 0),
>  	IOCTL_INFO(VIDIOC_STREAMON, INFO_FL_PRIO),
>  	IOCTL_INFO(VIDIOC_STREAMOFF, INFO_FL_PRIO),
> @@ -957,6 +958,11 @@ static long __video_do_ioctl(struct file *file,
>  			dbgbuf(cmd, vfd, p);
>  		break;
>  	}
> +	case VIDIOC_EXPBUF:
> +	{
> +		ret = ops->vidioc_expbuf(file, fh, arg);
> +		break;
> +	}
>  	case VIDIOC_DQBUF:
>  	{
>  		struct v4l2_buffer *p = arg;
> diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h
> index 51b20f4..e8893a5 100644
> --- a/include/linux/videodev2.h
> +++ b/include/linux/videodev2.h
> @@ -684,6 +684,31 @@ struct v4l2_buffer {
>  #define V4L2_BUF_FLAG_NO_CACHE_INVALIDATE	0x0800
>  #define V4L2_BUF_FLAG_NO_CACHE_CLEAN		0x1000
>  
> +/**
> + * struct v4l2_exportbuffer - export of video buffer as DMABUF file descriptor
> + *
> + * @fd:		file descriptor associated with DMABUF (set by driver)
> + * @mem_offset:	buffer memory offset as returned by VIDIOC_QUERYBUF in struct
> + *		v4l2_buffer::m.offset (for single-plane formats) or
> + *		v4l2_plane::m.offset (for multi-planar formats)
> + * @flags:	flags for newly created file, currently only O_CLOEXEC is
> + *		supported, refer to manual of open syscall for more details
> + *
> + * Contains data used for exporting a video buffer as DMABUF file descriptor.
> + * The buffer is identified by a 'cookie' returned by VIDIOC_QUERYBUF
> + * (identical to the cookie used to mmap() the buffer to userspace). All
> + * reserved fields must be set to zero. The field reserved0 is expected to
> + * become a structure 'type' allowing an alternative layout of the structure
> + * content. Therefore this field should not be used for any other extensions.
> + */
> +struct v4l2_exportbuffer {
> +	__u32		fd;
> +	__u32		reserved0;
> +	__u32		mem_offset;

This should be a union identical to the m union in v4l2_plane, together with a
u32 memory field. That way you can just copy memory and m from v4l2_plane/buffer
and call expbuf. If new memory types are added in the future, then you don't need
to change this struct.

For that matter, wouldn't it be useful to support exporting a userptr buffer at
some point in the future?

BTW, this patch series needs to be rebased to the latest for_v3.6. Quite a few
core things have changed when it comes to adding new ioctls.

Regards,

	Hans

> +	__u32		flags;
> +	__u32		reserved[12];
> +};
> +
>  /*
>   *	O V E R L A Y   P R E V I E W
>   */
> @@ -2553,6 +2578,7 @@ struct v4l2_create_buffers {
>  #define VIDIOC_S_FBUF		 _IOW('V', 11, struct v4l2_framebuffer)
>  #define VIDIOC_OVERLAY		 _IOW('V', 14, int)
>  #define VIDIOC_QBUF		_IOWR('V', 15, struct v4l2_buffer)
> +#define VIDIOC_EXPBUF		_IOWR('V', 16, struct v4l2_exportbuffer)
>  #define VIDIOC_DQBUF		_IOWR('V', 17, struct v4l2_buffer)
>  #define VIDIOC_STREAMON		 _IOW('V', 18, int)
>  #define VIDIOC_STREAMOFF	 _IOW('V', 19, int)
> diff --git a/include/media/v4l2-ioctl.h b/include/media/v4l2-ioctl.h
> index d8b76f7..ccd1faa 100644
> --- a/include/media/v4l2-ioctl.h
> +++ b/include/media/v4l2-ioctl.h
> @@ -119,6 +119,8 @@ struct v4l2_ioctl_ops {
>  	int (*vidioc_reqbufs) (struct file *file, void *fh, struct v4l2_requestbuffers *b);
>  	int (*vidioc_querybuf)(struct file *file, void *fh, struct v4l2_buffer *b);
>  	int (*vidioc_qbuf)    (struct file *file, void *fh, struct v4l2_buffer *b);
> +	int (*vidioc_expbuf)  (struct file *file, void *fh,
> +				struct v4l2_exportbuffer *e);
>  	int (*vidioc_dqbuf)   (struct file *file, void *fh, struct v4l2_buffer *b);
>  
>  	int (*vidioc_create_bufs)(struct file *file, void *fh, struct v4l2_create_buffers *b);
> 

Re: [PATCHv2 3/9] v4l: add buffer exporting via dmabuf

[Laurent Pinchart]

Hi Hans,

On Tuesday 31 July 2012 08:33:56 Hans Verkuil wrote:
> On Thu June 14 2012 16:32:23 Tomasz Stanislawski wrote:
> > This patch adds extension to V4L2 api. It allow to export a mmap buffer as
> > file descriptor. New ioctl VIDIOC_EXPBUF is added. It takes a buffer
> > offset used by mmap and return a file descriptor on success.
> > 
> > Signed-off-by: Tomasz Stanislawski <t.stanislaws@samsung.com>
> > Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
> > ---
> > 
> >  drivers/media/video/v4l2-compat-ioctl32.c |    1 +
> >  drivers/media/video/v4l2-dev.c            |    1 +
> >  drivers/media/video/v4l2-ioctl.c          |    6 ++++++
> >  include/linux/videodev2.h                 |   26
> >  ++++++++++++++++++++++++++
> >  include/media/v4l2-ioctl.h                |    2 ++
> >  5 files changed, 36 insertions(+)
> > 
> > diff --git a/drivers/media/video/v4l2-compat-ioctl32.c
> > b/drivers/media/video/v4l2-compat-ioctl32.c index d33ab18..141e745 100644
> > --- a/drivers/media/video/v4l2-compat-ioctl32.c
> > +++ b/drivers/media/video/v4l2-compat-ioctl32.c
> > @@ -970,6 +970,7 @@ long v4l2_compat_ioctl32(struct file *file, unsigned
> > int cmd, unsigned long arg)> 
> >  	case VIDIOC_S_FBUF32:
> >  	case VIDIOC_OVERLAY32:
> > 
> >  	case VIDIOC_QBUF32:
> > +	case VIDIOC_EXPBUF:
> >  	case VIDIOC_DQBUF32:
> >  	case VIDIOC_STREAMON32:
> > 
> >  	case VIDIOC_STREAMOFF32:
> > diff --git a/drivers/media/video/v4l2-dev.c
> > b/drivers/media/video/v4l2-dev.c index 5ccbd46..6bf6307 100644
> > --- a/drivers/media/video/v4l2-dev.c
> > +++ b/drivers/media/video/v4l2-dev.c
> > @@ -597,6 +597,7 @@ static void determine_valid_ioctls(struct video_device
> > *vdev)> 
> >  	SET_VALID_IOCTL(ops, VIDIOC_REQBUFS, vidioc_reqbufs);
> >  	SET_VALID_IOCTL(ops, VIDIOC_QUERYBUF, vidioc_querybuf);
> >  	SET_VALID_IOCTL(ops, VIDIOC_QBUF, vidioc_qbuf);
> > 
> > +	SET_VALID_IOCTL(ops, VIDIOC_EXPBUF, vidioc_expbuf);
> > 
> >  	SET_VALID_IOCTL(ops, VIDIOC_DQBUF, vidioc_dqbuf);
> >  	SET_VALID_IOCTL(ops, VIDIOC_OVERLAY, vidioc_overlay);
> >  	SET_VALID_IOCTL(ops, VIDIOC_G_FBUF, vidioc_g_fbuf);
> > 
> > diff --git a/drivers/media/video/v4l2-ioctl.c
> > b/drivers/media/video/v4l2-ioctl.c index 31fc2ad..a73b14e 100644
> > --- a/drivers/media/video/v4l2-ioctl.c
> > +++ b/drivers/media/video/v4l2-ioctl.c
> > @@ -212,6 +212,7 @@ static struct v4l2_ioctl_info v4l2_ioctls[] = {
> > 
> >  	IOCTL_INFO(VIDIOC_S_FBUF, INFO_FL_PRIO),
> >  	IOCTL_INFO(VIDIOC_OVERLAY, INFO_FL_PRIO),
> >  	IOCTL_INFO(VIDIOC_QBUF, 0),
> > 
> > +	IOCTL_INFO(VIDIOC_EXPBUF, 0),
> > 
> >  	IOCTL_INFO(VIDIOC_DQBUF, 0),
> >  	IOCTL_INFO(VIDIOC_STREAMON, INFO_FL_PRIO),
> >  	IOCTL_INFO(VIDIOC_STREAMOFF, INFO_FL_PRIO),
> > 
> > @@ -957,6 +958,11 @@ static long __video_do_ioctl(struct file *file,
> > 
> >  			dbgbuf(cmd, vfd, p);
> >  		
> >  		break;
> >  	
> >  	}
> > 
> > +	case VIDIOC_EXPBUF:
> > +	{
> > +		ret = ops->vidioc_expbuf(file, fh, arg);
> > +		break;
> > +	}
> > 
> >  	case VIDIOC_DQBUF:
> >  	{
> >  	
> >  		struct v4l2_buffer *p = arg;
> > 
> > diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h
> > index 51b20f4..e8893a5 100644
> > --- a/include/linux/videodev2.h
> > +++ b/include/linux/videodev2.h
> > @@ -684,6 +684,31 @@ struct v4l2_buffer {
> > 
> >  #define V4L2_BUF_FLAG_NO_CACHE_INVALIDATE	0x0800
> >  #define V4L2_BUF_FLAG_NO_CACHE_CLEAN		0x1000
> > 
> > +/**
> > + * struct v4l2_exportbuffer - export of video buffer as DMABUF file
> > descriptor + *
> > + * @fd:		file descriptor associated with DMABUF (set by driver)
> > + * @mem_offset:	buffer memory offset as returned by VIDIOC_QUERYBUF in
> > struct + *		v4l2_buffer::m.offset (for single-plane formats) or
> > + *		v4l2_plane::m.offset (for multi-planar formats)
> > + * @flags:	flags for newly created file, currently only O_CLOEXEC is
> > + *		supported, refer to manual of open syscall for more details
> > + *
> > + * Contains data used for exporting a video buffer as DMABUF file
> > descriptor. + * The buffer is identified by a 'cookie' returned by
> > VIDIOC_QUERYBUF + * (identical to the cookie used to mmap() the buffer to
> > userspace). All + * reserved fields must be set to zero. The field
> > reserved0 is expected to + * become a structure 'type' allowing an
> > alternative layout of the structure + * content. Therefore this field
> > should not be used for any other extensions. + */
> > +struct v4l2_exportbuffer {
> > +	__u32		fd;
> > +	__u32		reserved0;
> > +	__u32		mem_offset;
> 
> This should be a union identical to the m union in v4l2_plane, together with
> a u32 memory field. That way you can just copy memory and m from
> v4l2_plane/buffer and call expbuf. If new memory types are added in the
> future, then you don't need to change this struct.

OK, reserved0 could be replace by __u32 memory. Could we have other dma-buf 
export types in the future not corresponding to a memory type ? I don't see 
any use case right now though.

> For that matter, wouldn't it be useful to support exporting a userptr buffer
> at some point in the future?

Shouldn't USERPTR usage be discouraged once we get dma-buf support ?

> BTW, this patch series needs to be rebased to the latest for_v3.6. Quite a
> few core things have changed when it comes to adding new ioctls.

-- 
Regards,

Laurent Pinchart

Re: [PATCHv2 3/9] v4l: add buffer exporting via dmabuf

[Hans Verkuil]

On Tue 31 July 2012 13:56:14 Laurent Pinchart wrote:
> Hi Hans,
> 
> On Tuesday 31 July 2012 08:33:56 Hans Verkuil wrote:
> > On Thu June 14 2012 16:32:23 Tomasz Stanislawski wrote:
> > > +/**
> > > + * struct v4l2_exportbuffer - export of video buffer as DMABUF file
> > > descriptor + *
> > > + * @fd:		file descriptor associated with DMABUF (set by driver)
> > > + * @mem_offset:	buffer memory offset as returned by VIDIOC_QUERYBUF in
> > > struct + *		v4l2_buffer::m.offset (for single-plane formats) or
> > > + *		v4l2_plane::m.offset (for multi-planar formats)
> > > + * @flags:	flags for newly created file, currently only O_CLOEXEC is
> > > + *		supported, refer to manual of open syscall for more details
> > > + *
> > > + * Contains data used for exporting a video buffer as DMABUF file
> > > descriptor. + * The buffer is identified by a 'cookie' returned by
> > > VIDIOC_QUERYBUF + * (identical to the cookie used to mmap() the buffer to
> > > userspace). All + * reserved fields must be set to zero. The field
> > > reserved0 is expected to + * become a structure 'type' allowing an
> > > alternative layout of the structure + * content. Therefore this field
> > > should not be used for any other extensions. + */
> > > +struct v4l2_exportbuffer {
> > > +	__u32		fd;
> > > +	__u32		reserved0;
> > > +	__u32		mem_offset;
> > 
> > This should be a union identical to the m union in v4l2_plane, together with
> > a u32 memory field. That way you can just copy memory and m from
> > v4l2_plane/buffer and call expbuf. If new memory types are added in the
> > future, then you don't need to change this struct.
> 
> OK, reserved0 could be replace by __u32 memory. Could we have other dma-buf 
> export types in the future not corresponding to a memory type ? I don't see 
> any use case right now though.

The memory type should be all you need. And the union is also needed since the
userptr value is unsigned long, thus ensuring that you have 64-bits available
for pointer types in the future. That's really my main point: the union should
have the same size as the union in v4l2_buffer/plane, allowing for the same
size pointers or whatever to be added in the future.

> > For that matter, wouldn't it be useful to support exporting a userptr buffer
> > at some point in the future?
> 
> Shouldn't USERPTR usage be discouraged once we get dma-buf support ?

Why? It's perfectly fine to use it and it's not going away.

I'm not saying that we should support exporting a userptr buffer as a dmabuf fd,
but I'm just wondering if that is possible at all and how difficult it would be.

Regards,

	Hans

> 
> > BTW, this patch series needs to be rebased to the latest for_v3.6. Quite a
> > few core things have changed when it comes to adding new ioctls.
> 
> 

Re: [PATCHv2 3/9] v4l: add buffer exporting via dmabuf

[Rob Clark]

On Tue, Jul 31, 2012 at 7:11 AM, Hans Verkuil <hverkuil@xs4all.nl> wrote:
>> > For that matter, wouldn't it be useful to support exporting a userptr buffer
>> > at some point in the future?
>>
>> Shouldn't USERPTR usage be discouraged once we get dma-buf support ?
>
> Why? It's perfectly fine to use it and it's not going away.
>
> I'm not saying that we should support exporting a userptr buffer as a dmabuf fd,
> but I'm just wondering if that is possible at all and how difficult it would be.

it seems not terribly safe, since you don't really have much control
over where the memory comes from w/ userptr.  I'm more in favor of
discouraging usage of userptr

BR,
-R

Re: [PATCHv2 3/9] v4l: add buffer exporting via dmabuf

[Tomasz Stanislawski]

Hi Hans,

On 07/31/2012 02:11 PM, Hans Verkuil wrote:
> On Tue 31 July 2012 13:56:14 Laurent Pinchart wrote:
>> Hi Hans,
>>
>> On Tuesday 31 July 2012 08:33:56 Hans Verkuil wrote:
>>> On Thu June 14 2012 16:32:23 Tomasz Stanislawski wrote:
>>>> +/**
>>>> + * struct v4l2_exportbuffer - export of video buffer as DMABUF file
>>>> descriptor + *
>>>> + * @fd:		file descriptor associated with DMABUF (set by driver)
>>>> + * @mem_offset:	buffer memory offset as returned by VIDIOC_QUERYBUF in
>>>> struct + *		v4l2_buffer::m.offset (for single-plane formats) or
>>>> + *		v4l2_plane::m.offset (for multi-planar formats)
>>>> + * @flags:	flags for newly created file, currently only O_CLOEXEC is
>>>> + *		supported, refer to manual of open syscall for more details
>>>> + *
>>>> + * Contains data used for exporting a video buffer as DMABUF file
>>>> descriptor. + * The buffer is identified by a 'cookie' returned by
>>>> VIDIOC_QUERYBUF + * (identical to the cookie used to mmap() the buffer to
>>>> userspace). All + * reserved fields must be set to zero. The field
>>>> reserved0 is expected to + * become a structure 'type' allowing an
>>>> alternative layout of the structure + * content. Therefore this field
>>>> should not be used for any other extensions. + */
>>>> +struct v4l2_exportbuffer {
>>>> +	__u32		fd;
>>>> +	__u32		reserved0;
>>>> +	__u32		mem_offset;
>>>
>>> This should be a union identical to the m union in v4l2_plane, together with
>>> a u32 memory field. That way you can just copy memory and m from
>>> v4l2_plane/buffer and call expbuf. If new memory types are added in the
>>> future, then you don't need to change this struct.
>>
>> OK, reserved0 could be replace by __u32 memory. Could we have other dma-buf 
>> export types in the future not corresponding to a memory type ? I don't see 
>> any use case right now though.
> 
> The memory type should be all you need. And the union is also needed since the
> userptr value is unsigned long, thus ensuring that you have 64-bits available
> for pointer types in the future. That's really my main point: the union should
> have the same size as the union in v4l2_buffer/plane, allowing for the same
> size pointers or whatever to be added in the future.
> 

I do not see any good point in using v4l2_plane. What would be the meaning
of bytesused, length, data_offset in case of DMABUF exporting?

The field reserved0 was introduced to be replaced by __u32 memory if other means
of buffer description would be needed. The reserved fields at the end of
the structure could be used for auxiliary parameters other then offset and flags.
The flags field is expected to be used by all exporting types therefore the
layout could be reorganized to:

struct v4l2_exportbuffers {
	__u32	fd;
	__u32	flags;
	__u32	reserved0[2]; /* place for '__u32 memory' + forcing 64 bit alignment */
	__u32	mem_offset; /* what do you thing about using 64-bit field? */
	__u32	reserved1[11];
};

What is your opinion about this idea?

>>> For that matter, wouldn't it be useful to support exporting a userptr buffer
>>> at some point in the future?
>>
>> Shouldn't USERPTR usage be discouraged once we get dma-buf support ?
> 
> Why? It's perfectly fine to use it and it's not going away.
> 
> I'm not saying that we should support exporting a userptr buffer as a dmabuf fd,
> but I'm just wondering if that is possible at all and how difficult it would be.

It would be difficult. Currently there is no safe/portable way to transform
a userptr into a scatterlist mappable for other devices. The most trouble
some examples are userspace-mapping of IO memory like framebuffers.
How reference management is going to work if there are no struct pages
describing mapped memory?

The VB2 uses a workaround by keeping a copy of vma that is used to call
open/close callbacks. I am not sure how reliable this solution is.

Who knows, maybe in future someone will introduce a mechanism for creation of
DMABUF descriptor from a userptr without any help of client APIs.
In such a case, it will be the part of DMABUF api not V4L2 :).

Regards,
Tomasz Stanislawski

> 
> Regards,
> 
> 	Hans
> 
>>
>>> BTW, this patch series needs to be rebased to the latest for_v3.6. Quite a
>>> few core things have changed when it comes to adding new ioctls.
>>
>>

Re: [PATCHv2 3/9] v4l: add buffer exporting via dmabuf

[Laurent Pinchart]

Hi Tomasz,

On Wednesday 01 August 2012 10:01:45 Tomasz Stanislawski wrote:
> On 07/31/2012 02:11 PM, Hans Verkuil wrote:
> > On Tue 31 July 2012 13:56:14 Laurent Pinchart wrote:
> >> On Tuesday 31 July 2012 08:33:56 Hans Verkuil wrote:
> >>> On Thu June 14 2012 16:32:23 Tomasz Stanislawski wrote:
> >>>> +/**
> >>>> + * struct v4l2_exportbuffer - export of video buffer as DMABUF file
> >>>> descriptor + *
> >>>> + * @fd:		file descriptor associated with DMABUF (set by driver)
> >>>> + * @mem_offset:	buffer memory offset as returned by VIDIOC_QUERYBUF 
in
> >>>> struct + *		v4l2_buffer::m.offset (for single-plane formats) or
> >>>> + *		v4l2_plane::m.offset (for multi-planar formats)
> >>>> + * @flags:	flags for newly created file, currently only O_CLOEXEC is
> >>>> + *		supported, refer to manual of open syscall for more details
> >>>> + *
> >>>> + * Contains data used for exporting a video buffer as DMABUF file
> >>>> descriptor. + * The buffer is identified by a 'cookie' returned by
> >>>> VIDIOC_QUERYBUF + * (identical to the cookie used to mmap() the buffer
> >>>> to
> >>>> userspace). All + * reserved fields must be set to zero. The field
> >>>> reserved0 is expected to + * become a structure 'type' allowing an
> >>>> alternative layout of the structure + * content. Therefore this field
> >>>> should not be used for any other extensions. + */
> >>>> +struct v4l2_exportbuffer {
> >>>> +	__u32		fd;
> >>>> +	__u32		reserved0;
> >>>> +	__u32		mem_offset;
> >>> 
> >>> This should be a union identical to the m union in v4l2_plane, together
> >>> with a u32 memory field. That way you can just copy memory and m from
> >>> v4l2_plane/buffer and call expbuf. If new memory types are added in the
> >>> future, then you don't need to change this struct.
> >> 
> >> OK, reserved0 could be replace by __u32 memory. Could we have other
> >> dma-buf
> >> export types in the future not corresponding to a memory type ? I don't
> >> see
> >> any use case right now though.
> > 
> > The memory type should be all you need. And the union is also needed since
> > the userptr value is unsigned long, thus ensuring that you have 64-bits
> > available for pointer types in the future. That's really my main point:
> > the union should have the same size as the union in v4l2_buffer/plane,
> > allowing for the same size pointers or whatever to be added in the
> > future.
> 
> I do not see any good point in using v4l2_plane. What would be the meaning
> of bytesused, length, data_offset in case of DMABUF exporting?

I don't think Hans meant using v4l2_plane directly, but to use the same union 
as in v4l2_plane.

> The field reserved0 was introduced to be replaced by __u32 memory if other
> means of buffer description would be needed. The reserved fields at the end
> of the structure could be used for auxiliary parameters other then offset
> and flags. The flags field is expected to be used by all exporting types
> therefore the layout could be reorganized to:
> 
> struct v4l2_exportbuffers {
> 	__u32	fd;
> 	__u32	flags;
> 	__u32	reserved0[2]; /* place for '__u32 memory' + forcing 64 bit 
alignment
> */ __u32	mem_offset; /* what do you thing about using 64-bit field? */
> __u32	reserved1[11];
> };
> 
> What is your opinion about this idea?
> 
> >>> For that matter, wouldn't it be useful to support exporting a userptr
> >>> buffer at some point in the future?
> >> 
> >> Shouldn't USERPTR usage be discouraged once we get dma-buf support ?
> > 
> > Why? It's perfectly fine to use it and it's not going away.
> > 
> > I'm not saying that we should support exporting a userptr buffer as a
> > dmabuf fd, but I'm just wondering if that is possible at all and how
> > difficult it would be.
> It would be difficult. Currently there is no safe/portable way to transform
> a userptr into a scatterlist mappable for other devices. The most trouble
> some examples are userspace-mapping of IO memory like framebuffers.
> How reference management is going to work if there are no struct pages
> describing mapped memory?
> 
> The VB2 uses a workaround by keeping a copy of vma that is used to call
> open/close callbacks. I am not sure how reliable this solution is.
> 
> Who knows, maybe in future someone will introduce a mechanism for creation
> of DMABUF descriptor from a userptr without any help of client APIs.
> In such a case, it will be the part of DMABUF api not V4L2 :).

-- 
Regards,

Laurent Pinchart

Re: [PATCHv2 3/9] v4l: add buffer exporting via dmabuf

[Hans Verkuil]

On Wed 1 August 2012 10:01:45 Tomasz Stanislawski wrote:
> Hi Hans,
> 
> On 07/31/2012 02:11 PM, Hans Verkuil wrote:
> > On Tue 31 July 2012 13:56:14 Laurent Pinchart wrote:
> >> Hi Hans,
> >>
> >> On Tuesday 31 July 2012 08:33:56 Hans Verkuil wrote:
> >>> On Thu June 14 2012 16:32:23 Tomasz Stanislawski wrote:
> >>>> +/**
> >>>> + * struct v4l2_exportbuffer - export of video buffer as DMABUF file
> >>>> descriptor + *
> >>>> + * @fd:		file descriptor associated with DMABUF (set by driver)
> >>>> + * @mem_offset:	buffer memory offset as returned by VIDIOC_QUERYBUF in
> >>>> struct + *		v4l2_buffer::m.offset (for single-plane formats) or
> >>>> + *		v4l2_plane::m.offset (for multi-planar formats)
> >>>> + * @flags:	flags for newly created file, currently only O_CLOEXEC is
> >>>> + *		supported, refer to manual of open syscall for more details
> >>>> + *
> >>>> + * Contains data used for exporting a video buffer as DMABUF file
> >>>> descriptor. + * The buffer is identified by a 'cookie' returned by
> >>>> VIDIOC_QUERYBUF + * (identical to the cookie used to mmap() the buffer to
> >>>> userspace). All + * reserved fields must be set to zero. The field
> >>>> reserved0 is expected to + * become a structure 'type' allowing an
> >>>> alternative layout of the structure + * content. Therefore this field
> >>>> should not be used for any other extensions. + */
> >>>> +struct v4l2_exportbuffer {
> >>>> +	__u32		fd;
> >>>> +	__u32		reserved0;
> >>>> +	__u32		mem_offset;
> >>>
> >>> This should be a union identical to the m union in v4l2_plane, together with
> >>> a u32 memory field. That way you can just copy memory and m from
> >>> v4l2_plane/buffer and call expbuf. If new memory types are added in the
> >>> future, then you don't need to change this struct.
> >>
> >> OK, reserved0 could be replace by __u32 memory. Could we have other dma-buf 
> >> export types in the future not corresponding to a memory type ? I don't see 
> >> any use case right now though.
> > 
> > The memory type should be all you need. And the union is also needed since the
> > userptr value is unsigned long, thus ensuring that you have 64-bits available
> > for pointer types in the future. That's really my main point: the union should
> > have the same size as the union in v4l2_buffer/plane, allowing for the same
> > size pointers or whatever to be added in the future.
> > 
> 
> I do not see any good point in using v4l2_plane. What would be the meaning
> of bytesused, length, data_offset in case of DMABUF exporting?
> 
> The field reserved0 was introduced to be replaced by __u32 memory if other means
> of buffer description would be needed. The reserved fields at the end of
> the structure could be used for auxiliary parameters other then offset and flags.
> The flags field is expected to be used by all exporting types therefore the
> layout could be reorganized to:
> 
> struct v4l2_exportbuffers {
> 	__u32	fd;
> 	__u32	flags;
> 	__u32	reserved0[2]; /* place for '__u32 memory' + forcing 64 bit alignment */
> 	__u32	mem_offset; /* what do you thing about using 64-bit field? */
> 	__u32	reserved1[11];
> };
> 
> What is your opinion about this idea?

You're missing the point of my argument. How does v4l2_buffer work currently: you
have a memory field and a union. The memory field determines which field of the
union is to be used. In order to be able to use VIDIOC_EXPBUF you need to be
able to add new memory types in the future. Currently only MMAP makes sense here,
so all you need is the offset, but in order to be able to support future memory
types you need to make sure that you can extend v4l2_exportbuffers with the
largest possible value that v4l2_buffers can contain in the union, and that's
an unsigned long/pointer. That won't fit in the current proposal since there is only
a u32.

So I would propose this:

struct v4l2_exportbuffers {
	__u32	fd;
	__u32	memory;
	union {
		__u32 mem_offset;
		void *reserved;	/* ensure that we can handle pointers in the future */
	} m;
	__u32	flags;
	__u32	reserved1[11];
};

That way an application can just do:

	struct v4l2_buffer buf;
	struct v4l2_exportbuffers expbuf;

	expbuf.memory = buf.memory;
	memcpy(&expbuf.m, &buf.m, sizeof(expbuf.m));

and VIDIOC_EXPBUF will return an error if expbuf.memory != V4L2_MEMORY_MMAP.

I was actually wondering whether it might not be an idea to pass a v4l2_buffer
directly to VIDIOC_EXPBUF. In order to support that you would have to add fd
fields to v4l2_buffer and v4l2_plane and those would be filled in by VIDIOC_EXPBUF.
For the flags field in exportbuffers you would have to add new V4L2_BUF_FLAG_ flags.

That way you don't need to introduce a new struct and all planes are also
automatically exported. It's just an idea...

> 
> >>> For that matter, wouldn't it be useful to support exporting a userptr buffer
> >>> at some point in the future?
> >>
> >> Shouldn't USERPTR usage be discouraged once we get dma-buf support ?
> > 
> > Why? It's perfectly fine to use it and it's not going away.
> > 
> > I'm not saying that we should support exporting a userptr buffer as a dmabuf fd,
> > but I'm just wondering if that is possible at all and how difficult it would be.
> 
> It would be difficult. Currently there is no safe/portable way to transform
> a userptr into a scatterlist mappable for other devices. The most trouble
> some examples are userspace-mapping of IO memory like framebuffers.
> How reference management is going to work if there are no struct pages
> describing mapped memory?
> 
> The VB2 uses a workaround by keeping a copy of vma that is used to call
> open/close callbacks. I am not sure how reliable this solution is.
> 
> Who knows, maybe in future someone will introduce a mechanism for creation of
> DMABUF descriptor from a userptr without any help of client APIs.
> In such a case, it will be the part of DMABUF api not V4L2 :).

OK, thanks for the explanation!

Regards,

	Hans

Re: [PATCHv2 3/9] v4l: add buffer exporting via dmabuf

[Tomasz Stanislawski]

Hi Hans,

>>
>> I do not see any good point in using v4l2_plane. What would be the meaning
>> of bytesused, length, data_offset in case of DMABUF exporting?
>>
>> The field reserved0 was introduced to be replaced by __u32 memory if other means
>> of buffer description would be needed. The reserved fields at the end of
>> the structure could be used for auxiliary parameters other then offset and flags.
>> The flags field is expected to be used by all exporting types therefore the
>> layout could be reorganized to:
>>
>> struct v4l2_exportbuffers {
>> 	__u32	fd;
>> 	__u32	flags;
>> 	__u32	reserved0[2]; /* place for '__u32 memory' + forcing 64 bit alignment */
>> 	__u32	mem_offset; /* what do you thing about using 64-bit field? */
>> 	__u32	reserved1[11];
>> };
>>
>> What is your opinion about this idea?
> 
> You're missing the point of my argument. How does v4l2_buffer work currently: you
> have a memory field and a union. The memory field determines which field of the
> union is to be used. In order to be able to use VIDIOC_EXPBUF you need to be
> able to add new memory types in the future. Currently only MMAP makes sense here,
> so all you need is the offset, but in order to be able to support future memory
> types you need to make sure that you can extend v4l2_exportbuffers with the
> largest possible value that v4l2_buffers can contain in the union, and that's
> an unsigned long/pointer. That won't fit in the current proposal since there is only
> a u32.
> 
> So I would propose this:
> 
> struct v4l2_exportbuffers {
> 	__u32	fd;
> 	__u32	memory;
> 	union {
> 		__u32 mem_offset;
> 		void *reserved;	/* ensure that we can handle pointers in the future */
> 	} m;
> 	__u32	flags;
> 	__u32	reserved1[11];
> };

The layout about prevents adding any auxiliary fields other then mem_offset if
expbuf.memory == V4L2_MEMORY_MMAP. This could be fixed by the layout below
(it might be considered ugly by some people):

struct v4l2_exportbuffers {
	__u32	fd;
	__u32	flags;
	__u32	memory;
	__u32	reserved;
	union {
		struct v4l2_exportbyoffset {
			__u32	mem_offset;
			__u32	reserved[11];
		} byoffset;
		struct v4l2_exportbyuserptr {
			__u64	userptr;
			__u32	reserved[10];
		} byuserptr;
		__u32	reserved[12];
	};
};

Notice that the structure above in binary compatible with:

struct v4l2_exportbuffers {
 	__u32	fd;
 	__u32	flags;
 	__u32	reserved0[2];
 	__u32	mem_offset;
 	__u32	reserved1[11];
};

> 
> That way an application can just do:
> 
> 	struct v4l2_buffer buf;
> 	struct v4l2_exportbuffers expbuf;
> 
> 	expbuf.memory = buf.memory;
> 	memcpy(&expbuf.m, &buf.m, sizeof(expbuf.m));
> 
> and VIDIOC_EXPBUF will return an error if expbuf.memory != V4L2_MEMORY_MMAP.

The other question is if we should use V4L2_MEMORY_MMAP as expbuf.memory.
I think that new enums should be introduced for this purpose. Exporting is
very different from buffer requesting or queuing. One could envision
extension to VIDIOC_EXPBUF for exporting a buffer as entity different then
DMABUF file. In such a case, the fd and flags should go to a separate union.
This argument supports *not* using any v4l2_buffer related structs for VIDIOC_EXPBUF.
It should use its own structures. Likely, no extra structs are needed at the moment.

> 
> I was actually wondering whether it might not be an idea to pass a v4l2_buffer
> directly to VIDIOC_EXPBUF. In order to support that you would have to add fd
> fields to v4l2_buffer and v4l2_plane and those would be filled in by VIDIOC_EXPBUF.
> For the flags field in exportbuffers you would have to add new V4L2_BUF_FLAG_ flags.
> 
> That way you don't need to introduce a new struct and all planes are also
> automatically exported. It's just an idea...

One may not want to create DMABUF descriptors for all the planes.
The number of file descriptors is limited for the process.
Why creating file descriptor if they are going to closed or
(even worse) not used?

The mmap is called on each plane separately. So why VIDIOC_EXPBUF should
behave differently?

Regards,
Tomasz Stanislawski

Re: [PATCHv2 3/9] v4l: add buffer exporting via dmabuf

[Rémi Denis-Courmont]

Le mardi 31 juillet 2012 14:56:14 Laurent Pinchart, vous avez écrit :
> > For that matter, wouldn't it be useful to support exporting a userptr
> > buffer at some point in the future?
> 
> Shouldn't USERPTR usage be discouraged once we get dma-buf support ?

USERPTR, where available, is currently the only way to perform zero-copy from 
kernel to userspace. READWRITE does not support zero-copy at all. MMAP only 
supports zero-copy if userspace knows a boundary on the number of concurrent 
buffers *and* the device can deal with that number of buffers; in general, 
MMAP requires memory copying.

I am not sure DMABUF even supports transmitting data efficiently to userspace. 
In my understanding, it's meant for transmitting data between DSP's bypassing 
userspace entirely, in other words the exact opposite of what USERBUF does.

-- 
Rémi Denis-Courmont
http://www.remlab.net/
http://fi.linkedin.com/in/remidenis

Re: [PATCHv2 3/9] v4l: add buffer exporting via dmabuf

[Rob Clark]

On Tue, Jul 31, 2012 at 8:39 AM, Rémi Denis-Courmont <remi@remlab.net> wrote:
> Le mardi 31 juillet 2012 14:56:14 Laurent Pinchart, vous avez écrit :
>> > For that matter, wouldn't it be useful to support exporting a userptr
>> > buffer at some point in the future?
>>
>> Shouldn't USERPTR usage be discouraged once we get dma-buf support ?
>
> USERPTR, where available, is currently the only way to perform zero-copy from
> kernel to userspace. READWRITE does not support zero-copy at all. MMAP only
> supports zero-copy if userspace knows a boundary on the number of concurrent
> buffers *and* the device can deal with that number of buffers; in general,
> MMAP requires memory copying.

hmm, this sounds like the problem is device pre-allocating buffers?
Anyways, last time I looked, the vb2 core supported changing dmabuf fd
each time you QBUF, in a similar way to what you can do w/ userptr.
So that seems to get you the advantages you miss w/ mmap without the
pitfalls of userptr.

> I am not sure DMABUF even supports transmitting data efficiently to userspace.
> In my understanding, it's meant for transmitting data between DSP's bypassing
> userspace entirely, in other words the exact opposite of what USERBUF does.

well, dmabuf's can be mmap'd.. so it is more a matter of where the
buffer gets allocated, malloc() or from some driver (v4l2 or other).
There are a *ton* of ways userspace allocated memory can go badly,
esp. if the hw has special requirements about memory (GFP_DMA32 in a
system w/ PAE/LPAE, certain ranges of memory, certain alignment of
memory, etc).

BR,
-R

> --
> Rémi Denis-Courmont
> http://www.remlab.net/
> http://fi.linkedin.com/in/remidenis

Re: [PATCHv2 3/9] v4l: add buffer exporting via dmabuf

[Rémi Denis-Courmont]

Le mardi 31 juillet 2012 17:03:52 Rob Clark, vous avez écrit :
> On Tue, Jul 31, 2012 at 8:39 AM, Rémi Denis-Courmont <remi@remlab.net> 
wrote:
> > Le mardi 31 juillet 2012 14:56:14 Laurent Pinchart, vous avez écrit :
> >> > For that matter, wouldn't it be useful to support exporting a userptr
> >> > buffer at some point in the future?
> >> 
> >> Shouldn't USERPTR usage be discouraged once we get dma-buf support ?
> > 
> > USERPTR, where available, is currently the only way to perform zero-copy
> > from kernel to userspace. READWRITE does not support zero-copy at all.
> > MMAP only supports zero-copy if userspace knows a boundary on the number
> > of concurrent buffers *and* the device can deal with that number of
> > buffers; in general, MMAP requires memory copying.
> 
> hmm, this sounds like the problem is device pre-allocating buffers?

Basically, yes.

> Anyways, last time I looked, the vb2 core supported changing dmabuf fd
> each time you QBUF, in a similar way to what you can do w/ userptr.
> So that seems to get you the advantages you miss w/ mmap without the
> pitfalls of userptr.

It might work albeit with a higher system calls count overhead.

But what about libv4l2 transparent format conversion? Emulated USERBUF, with  
MMAP in the back-end would provide by far the least overhead. I don't see how 
DMABUF would work there.

-- 
Rémi Denis-Courmont
http://www.remlab.net/
http://fi.linkedin.com/in/remidenis

Re: [PATCHv2 3/9] v4l: add buffer exporting via dmabuf

[Laurent Pinchart]

Hi Rémi,

On Tuesday 31 July 2012 16:39:00 Rémi Denis-Courmont wrote:
> Le mardi 31 juillet 2012 14:56:14 Laurent Pinchart, vous avez écrit :
> > > For that matter, wouldn't it be useful to support exporting a userptr
> > > buffer at some point in the future?
> > 
> > Shouldn't USERPTR usage be discouraged once we get dma-buf support ?
> 
> USERPTR, where available, is currently the only way to perform zero-copy
> from kernel to userspace. READWRITE does not support zero-copy at all. MMAP
> only supports zero-copy if userspace knows a boundary on the number of
> concurrent buffers *and* the device can deal with that number of buffers;
> in general, MMAP requires memory copying.

Could you please share your use case(s) with us ?

> I am not sure DMABUF even supports transmitting data efficiently to
> userspace. In my understanding, it's meant for transmitting data between
> DSP's bypassing userspace entirely, in other words the exact opposite of
> what USERBUF does.

-- 
Regards,

Laurent Pinchart

Re: [PATCHv2 3/9] v4l: add buffer exporting via dmabuf

[Rémi Denis-Courmont]

Le mardi 31 juillet 2012 19:28:12 Laurent Pinchart, vous avez écrit :
> Hi Rémi,
> 
> On Tuesday 31 July 2012 16:39:00 Rémi Denis-Courmont wrote:
> > Le mardi 31 juillet 2012 14:56:14 Laurent Pinchart, vous avez écrit :
> > > > For that matter, wouldn't it be useful to support exporting a userptr
> > > > buffer at some point in the future?
> > > 
> > > Shouldn't USERPTR usage be discouraged once we get dma-buf support ?
> > 
> > USERPTR, where available, is currently the only way to perform zero-copy
> > from kernel to userspace. READWRITE does not support zero-copy at all.
> > MMAP only supports zero-copy if userspace knows a boundary on the number
> > of concurrent buffers *and* the device can deal with that number of
> > buffers; in general, MMAP requires memory copying.
> 
> Could you please share your use case(s) with us ?

I want to receive the video buffers in user space for processing. Typically 
"processing" is software encoding or conversion. That's what virtually any V4L 
application does on the desktop...

-- 
Rémi Denis-Courmont
http://www.remlab.net/
http://fi.linkedin.com/in/remidenis

Re: [PATCHv2 3/9] v4l: add buffer exporting via dmabuf

[Laurent Pinchart]

Hi Rémi,

On Tuesday 31 July 2012 21:39:40 Rémi Denis-Courmont wrote:
> Le mardi 31 juillet 2012 19:28:12 Laurent Pinchart, vous avez écrit :
> > On Tuesday 31 July 2012 16:39:00 Rémi Denis-Courmont wrote:
> > > Le mardi 31 juillet 2012 14:56:14 Laurent Pinchart, vous avez écrit :
> > > > > For that matter, wouldn't it be useful to support exporting a
> > > > > userptr
> > > > > buffer at some point in the future?
> > > > 
> > > > Shouldn't USERPTR usage be discouraged once we get dma-buf support ?
> > > 
> > > USERPTR, where available, is currently the only way to perform zero-copy
> > > from kernel to userspace. READWRITE does not support zero-copy at all.
> > > MMAP only supports zero-copy if userspace knows a boundary on the number
> > > of concurrent buffers *and* the device can deal with that number of
> > > buffers; in general, MMAP requires memory copying.
> > 
> > Could you please share your use case(s) with us ?
> 
> I want to receive the video buffers in user space for processing. Typically
> "processing" is software encoding or conversion. That's what virtually any
> V4L application does on the desktop...

But what prevents you from using MMAP ?

-- 
Regards,

Laurent Pinchart

Re: [PATCHv2 3/9] v4l: add buffer exporting via dmabuf

[Rémi Denis-Courmont]

On Tue, 31 Jul 2012 23:52:35 +0200, Laurent Pinchart

<laurent.pinchart@ideasonboard.com> wrote:

>> I want to receive the video buffers in user space for processing.

>> Typically

>> "processing" is software encoding or conversion. That's what virtually

>> any

>> V4L application does on the desktop...

> 

> But what prevents you from using MMAP ?



As I wrote several times earlier, MMAP uses fixed number of buffers. In

some tightly controlled media pipeline with low latency, it might work.



But in general, the V4L element in the pipeline does not know how fast the

downstream element(s) will consume the buffers. Thus it has to copy from

the MMAP buffers into anonymous user memory pending processing. Then any

dequeued buffer can be requeued as soon as possible. In theory, it might

also be that, even though the latency is known, the number of required

buffers exceeds the maximum MMAP buffers count of the V4L device. Either

way, user space ends up doing memory copy from MMAP to custom buffers.



This problem does not exist with USERBUF - the V4L2 element can simply

allocate a new buffer for each dequeued buffer.



By the way, this was already discussed a few months ago for the exact same

DMABUF patch series...



-- 

Rémi Denis-Courmont

Sent from my collocated server

Re: [PATCHv2 3/9] v4l: add buffer exporting via dmabuf

[Rémi Denis-Courmont]

On Tue, 31 Jul 2012 23:52:35 +0200, Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
>> I want to receive the video buffers in user space for processing.
>> Typically
>> "processing" is software encoding or conversion. That's what virtually
>> any
>> V4L application does on the desktop...
> 
> But what prevents you from using MMAP ?

As I wrote several times earlier, MMAP uses fixed number of buffers. In
some tightly controlled media pipeline with low latency, it might work.

But in general, the V4L element in the pipeline does not know how fast the
downstream element(s) will consume the buffers. Thus it has to copy from
the MMAP buffers into anonymous user memory pending processing. Then any
dequeued buffer can be requeued as soon as possible. In theory, it might
also be that, even though the latency is known, the number of required
buffers exceeds the maximum MMAP buffers count of the V4L device. Either
way, user space ends up doing memory copy from MMAP to custom buffers.

This problem does not exist with USERBUF - the V4L2 element can simply
allocate a new buffer for each dequeued buffer.

By the way, this was already discussed a few months ago for the exact same
DMABUF patch series...

-- 
Rémi Denis-Courmont
Sent from my collocated server

Re: [PATCHv2 3/9] v4l: add buffer exporting via dmabuf

[Laurent Pinchart]

Hi Rémi,

On Wednesday 01 August 2012 10:37:02 Rémi Denis-Courmont wrote:
> On Tue, 31 Jul 2012 23:52:35 +0200, Laurent Pinchart wrote:
> >> I want to receive the video buffers in user space for processing.
> >> Typically "processing" is software encoding or conversion. That's what
> >> virtually any V4L application does on the desktop...
> > 
> > But what prevents you from using MMAP ?
> 
> As I wrote several times earlier, MMAP uses fixed number of buffers. In
> some tightly controlled media pipeline with low latency, it might work.

Sorry about making you repeat.

> But in general, the V4L element in the pipeline does not know how fast the
> downstream element(s) will consume the buffers. Thus it has to copy from
> the MMAP buffers into anonymous user memory pending processing. Then any
> dequeued buffer can be requeued as soon as possible. In theory, it might
> also be that, even though the latency is known, the number of required
> buffers exceeds the maximum MMAP buffers count of the V4L device. Either
> way, user space ends up doing memory copy from MMAP to custom buffers.
> 
> This problem does not exist with USERBUF - the V4L2 element can simply
> allocate a new buffer for each dequeued buffer.

What about using the CREATE_BUFS ioctl to add new MMAP buffers at runtime ?

> By the way, this was already discussed a few months ago for the exact same
> DMABUF patch series...

-- 
Regards,

Laurent Pinchart

Re: [PATCHv2 3/9] v4l: add buffer exporting via dmabuf

[Rémi Denis-Courmont]

Le mercredi 1 août 2012 14:35:03 Laurent Pinchart, vous avez écrit :
> > But in general, the V4L element in the pipeline does not know how fast
> > the downstream element(s) will consume the buffers. Thus it has to copy
> > from the MMAP buffers into anonymous user memory pending processing.
> > Then any dequeued buffer can be requeued as soon as possible. In theory,
> > it might also be that, even though the latency is known, the number of
> > required buffers exceeds the maximum MMAP buffers count of the V4L
> > device. Either way, user space ends up doing memory copy from MMAP to
> > custom buffers.
> > 
> > This problem does not exist with USERBUF - the V4L2 element can simply
> > allocate a new buffer for each dequeued buffer.
> 
> What about using the CREATE_BUFS ioctl to add new MMAP buffers at runtime ?

Does CREATE_BUFS always work while already streaming has already started? If 
it depends on the driver, it's kinda helpless.

What's the guaranteed minimum buffer count? It seems in any case, MMAP has a 
hard limit of 32 buffers (at least videobuf2 has), though one might argue this 
should be more than enough.

-- 
Rémi Denis-Courmont
http://www.remlab.net/
http://fi.linkedin.com/in/remidenis

Re: [PATCHv2 3/9] v4l: add buffer exporting via dmabuf

[Hans Verkuil]

On Wed August 1 2012 22:49:57 Rémi Denis-Courmont wrote:
> Le mercredi 1 août 2012 14:35:03 Laurent Pinchart, vous avez écrit :
> > > But in general, the V4L element in the pipeline does not know how fast
> > > the downstream element(s) will consume the buffers. Thus it has to copy
> > > from the MMAP buffers into anonymous user memory pending processing.
> > > Then any dequeued buffer can be requeued as soon as possible. In theory,
> > > it might also be that, even though the latency is known, the number of
> > > required buffers exceeds the maximum MMAP buffers count of the V4L
> > > device. Either way, user space ends up doing memory copy from MMAP to
> > > custom buffers.
> > > 
> > > This problem does not exist with USERBUF - the V4L2 element can simply
> > > allocate a new buffer for each dequeued buffer.
> > 
> > What about using the CREATE_BUFS ioctl to add new MMAP buffers at runtime ?
> 
> Does CREATE_BUFS always work while already streaming has already started? If 
> it depends on the driver, it's kinda helpless.

Yes, it does. It's one of the reasons it exists in the first place. But there
are currently only a handful of drivers that implement it. I hope that as
more and more drivers are converted to vb2 that the availability of create_bufs
will increase.

> What's the guaranteed minimum buffer count? It seems in any case, MMAP has a 
> hard limit of 32 buffers (at least videobuf2 has), though one might argue this 
> should be more than enough.

Minimum or maximum? The maximum is 32, that's hardcoded in the V4L2 core.
Although drivers may force a lower maximum if they want. I have no idea
whether there are drivers that do that. There probably are.

The minimum is usually between 1 and 3, depending on hardware limitations.

Regards,

	Hans

Re: [PATCHv2 3/9] v4l: add buffer exporting via dmabuf

[Rémi Denis-Courmont]

Le jeudi 2 août 2012 09:35:58 Hans Verkuil, vous avez écrit :
> On Wed August 1 2012 22:49:57 Rémi Denis-Courmont wrote:
> > > What about using the CREATE_BUFS ioctl to add new MMAP buffers at
> > > runtime ?
> > 
> > Does CREATE_BUFS always work while already streaming has already started?
> > If it depends on the driver, it's kinda helpless.
> 
> Yes, it does. It's one of the reasons it exists in the first place. But
> there are currently only a handful of drivers that implement it. I hope
> that as more and more drivers are converted to vb2 that the availability
> of create_bufs will increase.

That's contradictory. If most drivers do not support it, then it won't work 
during streaming.

> > What's the guaranteed minimum buffer count? It seems in any case, MMAP
> > has a hard limit of 32 buffers (at least videobuf2 has), though one
> > might argue this should be more than enough.
> 
> Minimum or maximum? The maximum is 32, that's hardcoded in the V4L2 core.
> Although drivers may force a lower maximum if they want. I have no idea
> whether there are drivers that do that. There probably are.

The smallest of the maxima of all drivers.

> The minimum is usually between 1 and 3, depending on hardware limitations.

And that's clearly insufficient without memory copy to userspace buffers.

It does not seem to me that CREATE_BUFS+MMAP is a useful replacement for 
REQBUFS+USERBUF then.

-- 
Rémi Denis-Courmont
http://www.remlab.net/
http://fi.linkedin.com/in/remidenis

Re: [PATCHv2 3/9] v4l: add buffer exporting via dmabuf

[Hans Verkuil]

On Thu August 2 2012 08:56:43 Rémi Denis-Courmont wrote:
> Le jeudi 2 août 2012 09:35:58 Hans Verkuil, vous avez écrit :
> > On Wed August 1 2012 22:49:57 Rémi Denis-Courmont wrote:
> > > > What about using the CREATE_BUFS ioctl to add new MMAP buffers at
> > > > runtime ?
> > > 
> > > Does CREATE_BUFS always work while already streaming has already started?
> > > If it depends on the driver, it's kinda helpless.
> > 
> > Yes, it does. It's one of the reasons it exists in the first place. But
> > there are currently only a handful of drivers that implement it. I hope
> > that as more and more drivers are converted to vb2 that the availability
> > of create_bufs will increase.
> 
> That's contradictory. If most drivers do not support it, then it won't work 
> during streaming.

IF create_bufs is implemented in the driver, THEN you can use it during streaming.
I.e., it will never return EBUSY as an error due to the fact that streaming is in
progress.

Obviously it won't work if the driver didn't implement it in the first place.

> 
> > > What's the guaranteed minimum buffer count? It seems in any case, MMAP
> > > has a hard limit of 32 buffers (at least videobuf2 has), though one
> > > might argue this should be more than enough.
> > 
> > Minimum or maximum? The maximum is 32, that's hardcoded in the V4L2 core.
> > Although drivers may force a lower maximum if they want. I have no idea
> > whether there are drivers that do that. There probably are.
> 
> The smallest of the maxima of all drivers.

I've no idea. Most will probably abide by the 32 maximum, but without analyzing
all drivers I can't guarantee it.

> > The minimum is usually between 1 and 3, depending on hardware limitations.
> 
> And that's clearly insufficient without memory copy to userspace buffers.
> 
> It does not seem to me that CREATE_BUFS+MMAP is a useful replacement for 
> REQBUFS+USERBUF then.

Just to put your mind at rest: USERPTR mode will *not* disappear or be deprecated
in any way. It's been there for a long time, it's in heavy use, it's easy to use
and it will not be turned into a second class citizen, because it isn't. Just
because there is a new dmabuf mode available doesn't mean that everything should
be done as a mmap+dmabuf thing.

Regards,

	Hans

Re: [PATCHv2 3/9] v4l: add buffer exporting via dmabuf

[Laurent Pinchart]

Hi Hans,

On Thursday 02 August 2012 09:08:18 Hans Verkuil wrote:
> On Thu August 2 2012 08:56:43 Rémi Denis-Courmont wrote:
> > Le jeudi 2 août 2012 09:35:58 Hans Verkuil, vous avez écrit :
> > > On Wed August 1 2012 22:49:57 Rémi Denis-Courmont wrote:
> > > > > What about using the CREATE_BUFS ioctl to add new MMAP buffers at
> > > > > runtime ?
> > > > 
> > > > Does CREATE_BUFS always work while already streaming has already
> > > > started? If it depends on the driver, it's kinda helpless.
> > > 
> > > Yes, it does. It's one of the reasons it exists in the first place. But
> > > there are currently only a handful of drivers that implement it. I hope
> > > that as more and more drivers are converted to vb2 that the availability
> > > of create_bufs will increase.
> > 
> > That's contradictory. If most drivers do not support it, then it won't
> > work during streaming.
> 
> IF create_bufs is implemented in the driver, THEN you can use it during
> streaming. I.e., it will never return EBUSY as an error due to the fact
> that streaming is in progress.
> 
> Obviously it won't work if the driver didn't implement it in the first
> place.
>
> > > > What's the guaranteed minimum buffer count? It seems in any case, MMAP
> > > > has a hard limit of 32 buffers (at least videobuf2 has), though one
> > > > might argue this should be more than enough.
> > > 
> > > Minimum or maximum? The maximum is 32, that's hardcoded in the V4L2
> > > core. Although drivers may force a lower maximum if they want. I have no
> > > idea whether there are drivers that do that. There probably are.
> > 
> > The smallest of the maxima of all drivers.
> 
> I've no idea. Most will probably abide by the 32 maximum, but without
> analyzing all drivers I can't guarantee it.
> 
> > > The minimum is usually between 1 and 3, depending on hardware
> > > limitations.
> > 
> > And that's clearly insufficient without memory copy to userspace buffers.
> > 
> > It does not seem to me that CREATE_BUFS+MMAP is a useful replacement for
> > REQBUFS+USERBUF then.
> 
> Just to put your mind at rest: USERPTR mode will *not* disappear or be
> deprecated in any way. It's been there for a long time, it's in heavy use,
> it's easy to use and it will not be turned into a second class citizen,
> because it isn't. Just because there is a new dmabuf mode available doesn't
> mean that everything should be done as a mmap+dmabuf thing.

I disagree with this. Not everything should obviously be done with MMAP + 
DMABUF, but for buffer sharing between devices, we should encourage 
application developers to use DMABUF instead of USERPTR.

-- 
Regards,

Laurent Pinchart

Re: [PATCHv2 3/9] v4l: add buffer exporting via dmabuf

[Laurent Pinchart]

Hi Rémi,

On Thursday 02 August 2012 09:56:43 Rémi Denis-Courmont wrote:
> Le jeudi 2 août 2012 09:35:58 Hans Verkuil, vous avez écrit :
> > On Wed August 1 2012 22:49:57 Rémi Denis-Courmont wrote:
> > > > What about using the CREATE_BUFS ioctl to add new MMAP buffers at
> > > > runtime ?
> > > 
> > > Does CREATE_BUFS always work while already streaming has already
> > > started?
> > > If it depends on the driver, it's kinda helpless.
> > 
> > Yes, it does. It's one of the reasons it exists in the first place. But
> > there are currently only a handful of drivers that implement it. I hope
> > that as more and more drivers are converted to vb2 that the availability
> > of create_bufs will increase.
> 
> That's contradictory. If most drivers do not support it, then it won't work
> during streaming.
> 
> > > What's the guaranteed minimum buffer count? It seems in any case, MMAP
> > > has a hard limit of 32 buffers (at least videobuf2 has), though one
> > > might argue this should be more than enough.
> > 
> > Minimum or maximum? The maximum is 32, that's hardcoded in the V4L2 core.
> > Although drivers may force a lower maximum if they want. I have no idea
> > whether there are drivers that do that. There probably are.
> 
> The smallest of the maxima of all drivers.
> 
> > The minimum is usually between 1 and 3, depending on hardware limitations.
> 
> And that's clearly insufficient without memory copy to userspace buffers.

That's the minimum number of buffers *required* by the hardware. You can add 
up to 32 buffers, I'm not aware of any driver that would prevent that.

> It does not seem to me that CREATE_BUFS+MMAP is a useful replacement for
> REQBUFS+USERBUF then.

-- 
Regards,

Laurent Pinchart

Re: [PATCHv2 3/9] v4l: add buffer exporting via dmabuf

[Sakari Ailus]

Hi Hans and Rémi,

On Thu, Aug 02, 2012 at 08:35:58AM +0200, Hans Verkuil wrote:
...
> Minimum or maximum? The maximum is 32, that's hardcoded in the V4L2 core.

As far as I understand, V4L1 did have that limitation, as well as videobuf1
and 2 and a number of other drivers, but it's not found in the V4L2 core
itself. While I'm not aware of a driver that'd allow creating more buffers
than that the changes required to support more would be likely limited to
videobuf2.

Regards,

-- 
Sakari Ailus
e-mail: sakari.ailus@iki.fi	jabber/XMPP/Gmail: sailus@retiisi.org.uk

Re: [PATCHv2 3/9] v4l: add buffer exporting via dmabuf

[Hans Verkuil]

On Wed 8 August 2012 11:35:38 Sakari Ailus wrote:
> Hi Hans and Rémi,
> 
> On Thu, Aug 02, 2012 at 08:35:58AM +0200, Hans Verkuil wrote:
> ...
> > Minimum or maximum? The maximum is 32, that's hardcoded in the V4L2 core.
> 
> As far as I understand, V4L1 did have that limitation, as well as videobuf1
> and 2 and a number of other drivers, but it's not found in the V4L2 core
> itself. While I'm not aware of a driver that'd allow creating more buffers
> than that the changes required to support more would be likely limited to
> videobuf2.

You are correct. It does not touch the v4l2 core, just videobuf and videobuf2.
Although the define is in videodev2.h as well, so it's something that apps
might use as well. But frankly, 32 is a very generous maximum anyway.

Only in special cases can I imagine needing more buffers (frame slicing,
high-speed capture).

Regards,

	Hans

[PATCHv2 4/9] v4l: vb2: add buffer exporting via dmabuf

[Tomasz Stanislawski]

This patch adds extension to videobuf2-core. It allow to export a mmap buffer
as a file descriptor.

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-core.c |   67 ++++++++++++++++++++++++++++++++++
 include/media/videobuf2-core.h       |    2 +
 2 files changed, 69 insertions(+)

diff --git a/drivers/media/video/videobuf2-core.c b/drivers/media/video/videobuf2-core.c
index d60ed25..923165a 100644
--- a/drivers/media/video/videobuf2-core.c
+++ b/drivers/media/video/videobuf2-core.c
@@ -1730,6 +1730,73 @@ static int __find_plane_by_offset(struct vb2_queue *q, unsigned long off,
 }
 
 /**
+ * vb2_expbuf() - Export a buffer as a file descriptor
+ * @q:		videobuf2 queue
+ * @eb:		export buffer structure passed from userspace to vidioc_expbuf
+ *		handler in driver
+ *
+ * The return values from this function are intended to be directly returned
+ * from vidioc_expbuf handler in driver.
+ */
+int vb2_expbuf(struct vb2_queue *q, struct v4l2_exportbuffer *eb)
+{
+	struct vb2_buffer *vb = NULL;
+	struct vb2_plane *vb_plane;
+	unsigned int buffer, plane;
+	int ret;
+	struct dma_buf *dbuf;
+
+	if (q->memory != V4L2_MEMORY_MMAP) {
+		dprintk(1, "Queue is not currently set up for mmap\n");
+		return -EINVAL;
+	}
+
+	if (!q->mem_ops->get_dmabuf) {
+		dprintk(1, "Queue does not support DMA buffer exporting\n");
+		return -EINVAL;
+	}
+
+	if (eb->flags & ~O_CLOEXEC) {
+		dprintk(1, "Queue does support only O_CLOEXEC flag\n");
+		return -EINVAL;
+	}
+
+	/*
+	 * Find the plane corresponding to the offset passed by userspace.
+	 */
+	ret = __find_plane_by_offset(q, eb->mem_offset, &buffer, &plane);
+	if (ret) {
+		dprintk(1, "invalid offset %u\n", eb->mem_offset);
+		return ret;
+	}
+
+	vb = q->bufs[buffer];
+	vb_plane = &vb->planes[plane];
+
+	dbuf = call_memop(q, get_dmabuf, vb_plane->mem_priv);
+	if (IS_ERR_OR_NULL(dbuf)) {
+		dprintk(1, "Failed to export buffer %d, plane %d\n",
+			buffer, plane);
+		return -EINVAL;
+	}
+
+	ret = dma_buf_fd(dbuf, eb->flags);
+	if (ret < 0) {
+		dprintk(3, "buffer %d, plane %d failed to export (%d)\n",
+			buffer, plane, ret);
+		dma_buf_put(dbuf);
+		return ret;
+	}
+
+	dprintk(3, "buffer %d, plane %d exported as %d descriptor\n",
+		buffer, plane, ret);
+	eb->fd = ret;
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(vb2_expbuf);
+
+/**
  * vb2_mmap() - map video buffers into application address space
  * @q:		videobuf2 queue
  * @vma:	vma passed to the mmap file operation handler in the driver
diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h
index d079f92..fe01f95 100644
--- a/include/media/videobuf2-core.h
+++ b/include/media/videobuf2-core.h
@@ -81,6 +81,7 @@ struct vb2_fileio_data;
 struct vb2_mem_ops {
 	void		*(*alloc)(void *alloc_ctx, unsigned long size);
 	void		(*put)(void *buf_priv);
+	struct dma_buf *(*get_dmabuf)(void *buf_priv);
 
 	void		*(*get_userptr)(void *alloc_ctx, unsigned long vaddr,
 					unsigned long size, int write);
@@ -350,6 +351,7 @@ int vb2_queue_init(struct vb2_queue *q);
 void vb2_queue_release(struct vb2_queue *q);
 
 int vb2_qbuf(struct vb2_queue *q, struct v4l2_buffer *b);
+int vb2_expbuf(struct vb2_queue *q, struct v4l2_exportbuffer *eb);
 int vb2_dqbuf(struct vb2_queue *q, struct v4l2_buffer *b, bool nonblocking);
 
 int vb2_streamon(struct vb2_queue *q, enum v4l2_buf_type type);
-- 
1.7.9.5

[PATCHv2 5/9] v4l: vb2-dma-contig: add support for DMABUF exporting

[Tomasz Stanislawski]

This patch adds support for exporting a dma-contig buffer using
DMABUF interface.

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 |  248 ++++++++++++++++++++++++++++
 1 file changed, 248 insertions(+)

diff --git a/drivers/media/video/videobuf2-dma-contig.c b/drivers/media/video/videobuf2-dma-contig.c
index 00b776c..a845ff7 100644
--- a/drivers/media/video/videobuf2-dma-contig.c
+++ b/drivers/media/video/videobuf2-dma-contig.c
@@ -36,6 +36,7 @@ struct vb2_dc_buf {
 	/* MMAP related */
 	struct vb2_vmarea_handler	handler;
 	atomic_t			refcount;
+	struct sg_table			*sgt_base;
 
 	/* USERPTR related */
 	struct vm_area_struct		*vma;
@@ -142,6 +143,10 @@ static void vb2_dc_put(void *buf_priv)
 	if (!atomic_dec_and_test(&buf->refcount))
 		return;
 
+	if (buf->sgt_base) {
+		sg_free_table(buf->sgt_base);
+		kfree(buf->sgt_base);
+	}
 	dma_free_coherent(buf->dev, buf->size, buf->vaddr, buf->dma_addr);
 	kfree(buf);
 }
@@ -213,6 +218,248 @@ static int vb2_dc_mmap(void *buf_priv, struct vm_area_struct *vma)
 }
 
 /*********************************************/
+/*         DMABUF ops for exporters          */
+/*********************************************/
+
+struct vb2_dc_attachment {
+	struct sg_table sgt;
+	enum dma_data_direction dir;
+};
+
+static int vb2_dc_dmabuf_ops_attach(struct dma_buf *dbuf, struct device *dev,
+	struct dma_buf_attachment *dbuf_attach)
+{
+	struct vb2_dc_attachment *attach;
+	unsigned int i;
+	struct scatterlist *rd, *wr;
+	struct sg_table *sgt;
+	struct vb2_dc_buf *buf = dbuf->priv;
+	int ret;
+
+	attach = kzalloc(sizeof *attach, GFP_KERNEL);
+	if (!attach)
+		return -ENOMEM;
+
+	sgt = &attach->sgt;
+	/* Copy the buf->base_sgt scatter list to the attachment, as we can't
+	 * map the same scatter list to multiple attachments at the same time.
+	 */
+	ret = sg_alloc_table(sgt, buf->sgt_base->orig_nents, GFP_KERNEL);
+	if (ret) {
+		kfree(attach);
+		return -ENOMEM;
+	}
+
+	rd = buf->sgt_base->sgl;
+	wr = sgt->sgl;
+	for (i = 0; i < sgt->orig_nents; ++i) {
+		sg_set_page(wr, sg_page(rd), rd->length, rd->offset);
+		rd = sg_next(rd);
+		wr = sg_next(wr);
+	}
+
+	attach->dir = DMA_NONE;
+	dbuf_attach->priv = attach;
+
+	return 0;
+}
+
+static void vb2_dc_dmabuf_ops_detach(struct dma_buf *dbuf,
+	struct dma_buf_attachment *db_attach)
+{
+	struct vb2_dc_attachment *attach = db_attach->priv;
+	struct sg_table *sgt;
+
+	if (!attach)
+		return;
+
+	sgt = &attach->sgt;
+
+	/* release the scatterlist cache */
+	if (attach->dir != DMA_NONE)
+		dma_unmap_sg(db_attach->dev, sgt->sgl, sgt->orig_nents,
+			attach->dir);
+	sg_free_table(sgt);
+	kfree(attach);
+	db_attach->priv = NULL;
+}
+
+static struct sg_table *vb2_dc_dmabuf_ops_map(
+	struct dma_buf_attachment *db_attach, enum dma_data_direction dir)
+{
+	struct vb2_dc_attachment *attach = db_attach->priv;
+	/* stealing dmabuf mutex to serialize map/unmap operations */
+	struct mutex *lock = &db_attach->dmabuf->lock;
+	struct sg_table *sgt;
+	int ret;
+
+	mutex_lock(lock);
+
+	sgt = &attach->sgt;
+	/* return previously mapped sg table */
+	if (attach->dir == dir) {
+		mutex_unlock(lock);
+		return sgt;
+	}
+
+	/* release any previous cache */
+	if (attach->dir != DMA_NONE) {
+		dma_unmap_sg(db_attach->dev, sgt->sgl, sgt->orig_nents,
+			attach->dir);
+		attach->dir = DMA_NONE;
+	}
+
+	/* mapping to the client with new direction */
+	ret = dma_map_sg(db_attach->dev, sgt->sgl, sgt->orig_nents, dir);
+	if (ret <= 0) {
+		printk(KERN_ERR "failed to map scatterlist\n");
+		mutex_unlock(lock);
+		return ERR_PTR(-EIO);
+	}
+
+	attach->dir = dir;
+
+	mutex_unlock(lock);
+
+	return sgt;
+}
+
+static void vb2_dc_dmabuf_ops_unmap(struct dma_buf_attachment *db_attach,
+	struct sg_table *sgt, enum dma_data_direction dir)
+{
+	/* nothing to be done here */
+}
+
+static void vb2_dc_dmabuf_ops_release(struct dma_buf *dbuf)
+{
+	/* drop reference obtained in vb2_dc_get_dmabuf */
+	vb2_dc_put(dbuf->priv);
+}
+
+static void *vb2_dc_dmabuf_ops_kmap(struct dma_buf *dbuf, unsigned long pgnum)
+{
+	struct vb2_dc_buf *buf = dbuf->priv;
+
+	return buf->vaddr + pgnum * PAGE_SIZE;
+}
+
+static void *vb2_dc_dmabuf_ops_vmap(struct dma_buf *dbuf)
+{
+	struct vb2_dc_buf *buf = dbuf->priv;
+
+	return buf->vaddr;
+}
+
+static struct dma_buf_ops vb2_dc_dmabuf_ops = {
+	.attach = vb2_dc_dmabuf_ops_attach,
+	.detach = vb2_dc_dmabuf_ops_detach,
+	.map_dma_buf = vb2_dc_dmabuf_ops_map,
+	.unmap_dma_buf = vb2_dc_dmabuf_ops_unmap,
+	.kmap = vb2_dc_dmabuf_ops_kmap,
+	.kmap_atomic = vb2_dc_dmabuf_ops_kmap,
+	.vmap = vb2_dc_dmabuf_ops_vmap,
+	.release = vb2_dc_dmabuf_ops_release,
+};
+
+/**
+ * vb2_dc_kaddr_to_pages() - extract list of struct pages from a kernel
+ * pointer.  This function is a workaround to extract pages from a pointer
+ * returned by dma_alloc_coherent. The pages are obtained by creating an
+ * artificial vma and using follow_pfn to do a page walk to find a PFN
+ */
+static int vb2_dc_kaddr_to_pages(unsigned long kaddr,
+	struct page **pages, unsigned int n_pages)
+{
+	unsigned int i;
+	unsigned long pfn;
+	/* create an artificial VMA */
+	struct vm_area_struct vma = {
+		.vm_flags = VM_IO | VM_PFNMAP,
+		.vm_mm = &init_mm,
+	};
+
+	for (i = 0; i < n_pages; ++i, kaddr += PAGE_SIZE) {
+		if (follow_pfn(&vma, kaddr, &pfn))
+			break;
+		pages[i] = pfn_to_page(pfn);
+	}
+
+	return i;
+}
+
+static struct sg_table *vb2_dc_get_base_sgt(struct vb2_dc_buf *buf)
+{
+	int n_pages;
+	struct page **pages = NULL;
+	int ret;
+	struct sg_table *sgt;
+
+	n_pages = PAGE_ALIGN(buf->size) >> PAGE_SHIFT;
+
+	pages = kmalloc(n_pages * sizeof pages[0], GFP_KERNEL);
+	if (!pages) {
+		dev_err(buf->dev, "failed to alloc page table\n");
+		return ERR_PTR(-ENOMEM);
+	}
+
+	ret = vb2_dc_kaddr_to_pages((unsigned long)buf->vaddr, pages, n_pages);
+	if (ret < 0) {
+		dev_err(buf->dev, "failed to get buffer pages from DMA API\n");
+		kfree(pages);
+		return ERR_PTR(ret);
+	}
+	if (ret != n_pages) {
+		dev_err(buf->dev, "got only %d of %d pages from DMA API\n",
+			ret, n_pages);
+		kfree(pages);
+		return ERR_PTR(-EFAULT);
+	}
+
+	sgt = kmalloc(sizeof *sgt, GFP_KERNEL);
+	if (!sgt) {
+		dev_err(buf->dev, "failed to alloc sg table\n");
+		kfree(pages);
+		return ERR_PTR(-ENOMEM);
+	}
+
+	ret = sg_alloc_table_from_pages(sgt, pages, n_pages, 0,
+		buf->size, GFP_KERNEL);
+	/* failure or not, pages are no longer needed */
+	kfree(pages);
+	if (ret) {
+		dev_err(buf->dev, "failed to covert pages to sg table\n");
+		kfree(sgt);
+		return ERR_PTR(ret);
+	}
+
+	return sgt;
+}
+
+static struct dma_buf *vb2_dc_get_dmabuf(void *buf_priv)
+{
+	struct vb2_dc_buf *buf = buf_priv;
+	struct dma_buf *dbuf;
+	struct sg_table *sgt = buf->sgt_base;
+
+	if (!sgt)
+		sgt = vb2_dc_get_base_sgt(buf);
+	if (WARN_ON(IS_ERR(sgt)))
+		return NULL;
+
+	/* cache base sgt for future use */
+	buf->sgt_base = sgt;
+
+	dbuf = dma_buf_export(buf, &vb2_dc_dmabuf_ops, buf->size, 0);
+	if (IS_ERR(dbuf))
+		return NULL;
+
+	/* dmabuf keeps reference to vb2 buffer */
+	atomic_inc(&buf->refcount);
+
+	return dbuf;
+}
+
+/*********************************************/
 /*       callbacks for USERPTR buffers       */
 /*********************************************/
 
@@ -522,6 +769,7 @@ static void *vb2_dc_attach_dmabuf(void *alloc_ctx, struct dma_buf *dbuf,
 const struct vb2_mem_ops vb2_dma_contig_memops = {
 	.alloc		= vb2_dc_alloc,
 	.put		= vb2_dc_put,
+	.get_dmabuf	= vb2_dc_get_dmabuf,
 	.cookie		= vb2_dc_cookie,
 	.vaddr		= vb2_dc_vaddr,
 	.mmap		= vb2_dc_mmap,
-- 
1.7.9.5

[PATCHv2 6/9] v4l: s5p-fimc: support for dmabuf exporting

[Tomasz Stanislawski]

This patch enhances s5p-fimc with support for DMABUF exporting via
VIDIOC_EXPBUF ioctl.

Signed-off-by: Tomasz Stanislawski <t.stanislaws@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 drivers/media/video/s5p-fimc/fimc-capture.c |    9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/media/video/s5p-fimc/fimc-capture.c b/drivers/media/video/s5p-fimc/fimc-capture.c
index cd27e33..52c9b36 100644
--- a/drivers/media/video/s5p-fimc/fimc-capture.c
+++ b/drivers/media/video/s5p-fimc/fimc-capture.c
@@ -1101,6 +1101,14 @@ static int fimc_cap_qbuf(struct file *file, void *priv,
 	return vb2_qbuf(&fimc->vid_cap.vbq, buf);
 }
 
+static int fimc_cap_expbuf(struct file *file, void *priv,
+			  struct v4l2_exportbuffer *eb)
+{
+	struct fimc_dev *fimc = video_drvdata(file);
+
+	return vb2_expbuf(&fimc->vid_cap.vbq, eb);
+}
+
 static int fimc_cap_dqbuf(struct file *file, void *priv,
 			   struct v4l2_buffer *buf)
 {
@@ -1225,6 +1233,7 @@ static const struct v4l2_ioctl_ops fimc_capture_ioctl_ops = {
 
 	.vidioc_qbuf			= fimc_cap_qbuf,
 	.vidioc_dqbuf			= fimc_cap_dqbuf,
+	.vidioc_expbuf			= fimc_cap_expbuf,
 
 	.vidioc_prepare_buf		= fimc_cap_prepare_buf,
 	.vidioc_create_bufs		= fimc_cap_create_bufs,
-- 
1.7.9.5

[PATCHv2 7/9] v4l: s5p-tv: mixer: support for dmabuf exporting

[Tomasz Stanislawski]

This patch enhances s5p-tv with support for DMABUF exporting via
VIDIOC_EXPBUF ioctl.

Signed-off-by: Tomasz Stanislawski <t.stanislaws@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
---
 drivers/media/video/s5p-tv/mixer_video.c |   10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/media/video/s5p-tv/mixer_video.c b/drivers/media/video/s5p-tv/mixer_video.c
index cff974a..d8def5b 100644
--- a/drivers/media/video/s5p-tv/mixer_video.c
+++ b/drivers/media/video/s5p-tv/mixer_video.c
@@ -697,6 +697,15 @@ static int mxr_dqbuf(struct file *file, void *priv, struct v4l2_buffer *p)
 	return vb2_dqbuf(&layer->vb_queue, p, file->f_flags & O_NONBLOCK);
 }
 
+static int mxr_expbuf(struct file *file, void *priv,
+	struct v4l2_exportbuffer *eb)
+{
+	struct mxr_layer *layer = video_drvdata(file);
+
+	mxr_dbg(layer->mdev, "%s:%d\n", __func__, __LINE__);
+	return vb2_expbuf(&layer->vb_queue, eb);
+}
+
 static int mxr_streamon(struct file *file, void *priv, enum v4l2_buf_type i)
 {
 	struct mxr_layer *layer = video_drvdata(file);
@@ -724,6 +733,7 @@ static const struct v4l2_ioctl_ops mxr_ioctl_ops = {
 	.vidioc_querybuf = mxr_querybuf,
 	.vidioc_qbuf = mxr_qbuf,
 	.vidioc_dqbuf = mxr_dqbuf,
+	.vidioc_expbuf = mxr_expbuf,
 	/* Streaming control */
 	.vidioc_streamon = mxr_streamon,
 	.vidioc_streamoff = mxr_streamoff,
-- 
1.7.9.5

[PATCHv2 8/9] v4l: s5p-mfc: support for dmabuf exporting

[Tomasz Stanislawski]

This patch enhances s5p-mfc with support for DMABUF exporting via
VIDIOC_EXPBUF ioctl.

Signed-off-by: Tomasz Stanislawski <t.stanislaws@samsung.com>
Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
CC: Kamil Debski <k.debski@samsung.com>
---
 drivers/media/video/s5p-mfc/s5p_mfc_dec.c |   18 ++++++++++++++++++
 drivers/media/video/s5p-mfc/s5p_mfc_enc.c |   18 ++++++++++++++++++
 2 files changed, 36 insertions(+)

diff --git a/drivers/media/video/s5p-mfc/s5p_mfc_dec.c b/drivers/media/video/s5p-mfc/s5p_mfc_dec.c
index c25ec02..8344ce5 100644
--- a/drivers/media/video/s5p-mfc/s5p_mfc_dec.c
+++ b/drivers/media/video/s5p-mfc/s5p_mfc_dec.c
@@ -564,6 +564,23 @@ static int vidioc_dqbuf(struct file *file, void *priv, struct v4l2_buffer *buf)
 	return -EINVAL;
 }
 
+/* Export DMA buffer */
+static int vidioc_expbuf(struct file *file, void *priv,
+	struct v4l2_exportbuffer *eb)
+{
+	struct s5p_mfc_ctx *ctx = fh_to_ctx(priv);
+	int ret;
+
+	if (eb->mem_offset < DST_QUEUE_OFF_BASE)
+		return vb2_expbuf(&ctx->vq_src, eb);
+
+	eb->mem_offset -= DST_QUEUE_OFF_BASE;
+	ret = vb2_expbuf(&ctx->vq_dst, eb);
+	eb->mem_offset += DST_QUEUE_OFF_BASE;
+
+	return ret;
+}
+
 /* Stream on */
 static int vidioc_streamon(struct file *file, void *priv,
 			   enum v4l2_buf_type type)
@@ -739,6 +756,7 @@ static const struct v4l2_ioctl_ops s5p_mfc_dec_ioctl_ops = {
 	.vidioc_querybuf = vidioc_querybuf,
 	.vidioc_qbuf = vidioc_qbuf,
 	.vidioc_dqbuf = vidioc_dqbuf,
+	.vidioc_expbuf = vidioc_expbuf,
 	.vidioc_streamon = vidioc_streamon,
 	.vidioc_streamoff = vidioc_streamoff,
 	.vidioc_g_crop = vidioc_g_crop,
diff --git a/drivers/media/video/s5p-mfc/s5p_mfc_enc.c b/drivers/media/video/s5p-mfc/s5p_mfc_enc.c
index acedb20..db110c5 100644
--- a/drivers/media/video/s5p-mfc/s5p_mfc_enc.c
+++ b/drivers/media/video/s5p-mfc/s5p_mfc_enc.c
@@ -1141,6 +1141,23 @@ static int vidioc_dqbuf(struct file *file, void *priv, struct v4l2_buffer *buf)
 	return -EINVAL;
 }
 
+/* Export DMA buffer */
+static int vidioc_expbuf(struct file *file, void *priv,
+	struct v4l2_exportbuffer *eb)
+{
+	struct s5p_mfc_ctx *ctx = fh_to_ctx(priv);
+	int ret;
+
+	if (eb->mem_offset < DST_QUEUE_OFF_BASE)
+		return vb2_expbuf(&ctx->vq_src, eb);
+
+	eb->mem_offset -= DST_QUEUE_OFF_BASE;
+	ret = vb2_expbuf(&ctx->vq_dst, eb);
+	eb->mem_offset += DST_QUEUE_OFF_BASE;
+
+	return ret;
+}
+
 /* Stream on */
 static int vidioc_streamon(struct file *file, void *priv,
 			   enum v4l2_buf_type type)
@@ -1486,6 +1503,7 @@ static const struct v4l2_ioctl_ops s5p_mfc_enc_ioctl_ops = {
 	.vidioc_querybuf = vidioc_querybuf,
 	.vidioc_qbuf = vidioc_qbuf,
 	.vidioc_dqbuf = vidioc_dqbuf,
+	.vidioc_expbuf = vidioc_expbuf,
 	.vidioc_streamon = vidioc_streamon,
 	.vidioc_streamoff = vidioc_streamoff,
 	.vidioc_s_parm = vidioc_s_parm,
-- 
1.7.9.5

[PATCHv2 9/9] v4l: vb2-dma-contig: use dma_get_sgtable

[Tomasz Stanislawski]

This patch removes a workaround for extraction of struct pages
from DMA buffer. The method of using follow_pfn for artificial
VMA is dropped in favour of dma_get_sgtable function.

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 |   60 ++--------------------------
 1 file changed, 4 insertions(+), 56 deletions(-)

diff --git a/drivers/media/video/videobuf2-dma-contig.c b/drivers/media/video/videobuf2-dma-contig.c
index a845ff7..73297b7 100644
--- a/drivers/media/video/videobuf2-dma-contig.c
+++ b/drivers/media/video/videobuf2-dma-contig.c
@@ -361,73 +361,21 @@ static struct dma_buf_ops vb2_dc_dmabuf_ops = {
 	.release = vb2_dc_dmabuf_ops_release,
 };
 
-/**
- * vb2_dc_kaddr_to_pages() - extract list of struct pages from a kernel
- * pointer.  This function is a workaround to extract pages from a pointer
- * returned by dma_alloc_coherent. The pages are obtained by creating an
- * artificial vma and using follow_pfn to do a page walk to find a PFN
- */
-static int vb2_dc_kaddr_to_pages(unsigned long kaddr,
-	struct page **pages, unsigned int n_pages)
-{
-	unsigned int i;
-	unsigned long pfn;
-	/* create an artificial VMA */
-	struct vm_area_struct vma = {
-		.vm_flags = VM_IO | VM_PFNMAP,
-		.vm_mm = &init_mm,
-	};
-
-	for (i = 0; i < n_pages; ++i, kaddr += PAGE_SIZE) {
-		if (follow_pfn(&vma, kaddr, &pfn))
-			break;
-		pages[i] = pfn_to_page(pfn);
-	}
-
-	return i;
-}
-
 static struct sg_table *vb2_dc_get_base_sgt(struct vb2_dc_buf *buf)
 {
-	int n_pages;
-	struct page **pages = NULL;
 	int ret;
 	struct sg_table *sgt;
 
-	n_pages = PAGE_ALIGN(buf->size) >> PAGE_SHIFT;
-
-	pages = kmalloc(n_pages * sizeof pages[0], GFP_KERNEL);
-	if (!pages) {
-		dev_err(buf->dev, "failed to alloc page table\n");
-		return ERR_PTR(-ENOMEM);
-	}
-
-	ret = vb2_dc_kaddr_to_pages((unsigned long)buf->vaddr, pages, n_pages);
-	if (ret < 0) {
-		dev_err(buf->dev, "failed to get buffer pages from DMA API\n");
-		kfree(pages);
-		return ERR_PTR(ret);
-	}
-	if (ret != n_pages) {
-		dev_err(buf->dev, "got only %d of %d pages from DMA API\n",
-			ret, n_pages);
-		kfree(pages);
-		return ERR_PTR(-EFAULT);
-	}
-
 	sgt = kmalloc(sizeof *sgt, GFP_KERNEL);
 	if (!sgt) {
 		dev_err(buf->dev, "failed to alloc sg table\n");
-		kfree(pages);
 		return ERR_PTR(-ENOMEM);
 	}
 
-	ret = sg_alloc_table_from_pages(sgt, pages, n_pages, 0,
-		buf->size, GFP_KERNEL);
-	/* failure or not, pages are no longer needed */
-	kfree(pages);
-	if (ret) {
-		dev_err(buf->dev, "failed to covert pages to sg table\n");
+	ret = dma_get_sgtable(buf->dev, sgt, buf->vaddr, buf->dma_addr,
+		buf->size, NULL);
+	if (ret < 0) {
+		dev_err(buf->dev, "failed to get scatterlist from DMA API\n");
 		kfree(sgt);
 		return ERR_PTR(ret);
 	}
-- 
1.7.9.5