[PATCH 00/12] Support for dmabuf exporting for videobuf2

[PATCH 00/12] 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 not 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].

Changelog:
v0: (RFC)
- 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

[1] http://thread.gmane.org/gmane.linux.drivers.video-input-infrastructure/48730
[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:

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

Tomasz Stanislawski (11):
  v4l: add buffer exporting via dmabuf
  v4l: vb2: add buffer exporting via dmabuf
  v4l: vb2-dma-contig: add setup of sglist for MMAP buffers
  v4l: vb2-dma-contig: add support for DMABUF exporting
  v4l: vb2-dma-contig: add vmap/kmap 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: remove vb2_mmap_pfn_range function
  v4l: vb2-dma-contig: use sg_alloc_table_from_pages function
  v4l: vb2-dma-contig: Move allocation of dbuf attachment to attach cb

 drivers/media/video/s5p-fimc/fimc-capture.c |    9 +
 drivers/media/video/s5p-mfc/s5p_mfc_dec.c   |   13 ++
 drivers/media/video/s5p-mfc/s5p_mfc_enc.c   |   13 ++
 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  |  323 ++++++++++++++++++++++-----
 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 -
 14 files changed, 411 insertions(+), 107 deletions(-)

-- 
1.7.9.5

[PATCH 01/12] 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.

Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
 drivers/media/video/videobuf2-dma-contig.c |   28 ++++++++++++++++++++++++++--
 1 file changed, 26 insertions(+), 2 deletions(-)

diff --git a/drivers/media/video/videobuf2-dma-contig.c b/drivers/media/video/videobuf2-dma-contig.c
index 9c213bc..52b4f59 100644
--- a/drivers/media/video/videobuf2-dma-contig.c
+++ b/drivers/media/video/videobuf2-dma-contig.c
@@ -224,14 +224,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;
 }
 
 /*********************************************/
-- 
1.7.9.5

[PATCH 02/12] 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 5327ad3..45159d9 100644
--- a/drivers/media/video/v4l2-compat-ioctl32.c
+++ b/drivers/media/video/v4l2-compat-ioctl32.c
@@ -954,6 +954,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

[PATCH 03/12] 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>
---
 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

[PATCH 04/12] v4l: vb2-dma-contig: add setup of sglist for MMAP buffers

[Tomasz Stanislawski]

This patch adds the setup of sglist list for MMAP buffers.
It is needed for buffer exporting via DMABUF mechanism.

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 |   70 +++++++++++++++++++++++++++-
 1 file changed, 68 insertions(+), 2 deletions(-)

diff --git a/drivers/media/video/videobuf2-dma-contig.c b/drivers/media/video/videobuf2-dma-contig.c
index 52b4f59..ae656be 100644
--- a/drivers/media/video/videobuf2-dma-contig.c
+++ b/drivers/media/video/videobuf2-dma-contig.c
@@ -32,6 +32,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;
@@ -189,14 +190,37 @@ static void vb2_dc_put(void *buf_priv)
 	if (!atomic_dec_and_test(&buf->refcount))
 		return;
 
+	vb2_dc_release_sgtable(buf->sgt_base);
 	dma_free_coherent(buf->dev, buf->size, buf->vaddr, buf->dma_addr);
 	kfree(buf);
 }
 
+static int vb2_dc_kaddr_to_pages(unsigned long kaddr,
+	struct page **pages, unsigned int n_pages)
+{
+	unsigned int i;
+	unsigned long pfn;
+	struct vm_area_struct vma = {
+		.vm_flags = VM_IO | VM_PFNMAP,
+		.vm_mm = current->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 void *vb2_dc_alloc(void *alloc_ctx, unsigned long size)
 {
 	struct device *dev = alloc_ctx;
 	struct vb2_dc_buf *buf;
+	int ret = -ENOMEM;
+	int n_pages;
+	struct page **pages = NULL;
 
 	buf = kzalloc(sizeof *buf, GFP_KERNEL);
 	if (!buf)
@@ -205,10 +229,41 @@ static void *vb2_dc_alloc(void *alloc_ctx, unsigned long size)
 	buf->vaddr = dma_alloc_coherent(dev, size, &buf->dma_addr, GFP_KERNEL);
 	if (!buf->vaddr) {
 		dev_err(dev, "dma_alloc_coherent of size %ld failed\n", size);
-		kfree(buf);
-		return ERR_PTR(-ENOMEM);
+		goto fail_buf;
+	}
+
+	WARN_ON((unsigned long)buf->vaddr & ~PAGE_MASK);
+	WARN_ON(buf->dma_addr & ~PAGE_MASK);
+
+	n_pages = PAGE_ALIGN(size) >> PAGE_SHIFT;
+
+	pages = kmalloc(n_pages * sizeof pages[0], GFP_KERNEL);
+	if (!pages) {
+		dev_err(dev, "failed to alloc page table\n");
+		goto fail_dma;
+	}
+
+	ret = vb2_dc_kaddr_to_pages((unsigned long)buf->vaddr, pages, n_pages);
+	if (ret < 0) {
+		dev_err(dev, "failed to get buffer pages from DMA API\n");
+		goto fail_pages;
+	}
+	if (ret != n_pages) {
+		ret = -EFAULT;
+		dev_err(dev, "failed to get all pages from DMA API\n");
+		goto fail_pages;
+	}
+
+	buf->sgt_base = vb2_dc_pages_to_sgt(pages, n_pages, 0, size);
+	if (IS_ERR(buf->sgt_base)) {
+		ret = PTR_ERR(buf->sgt_base);
+		dev_err(dev, "failed to prepare sg table\n");
+		goto fail_pages;
 	}
 
+	/* pages are no longer needed */
+	kfree(pages);
+
 	buf->dev = dev;
 	buf->size = size;
 
@@ -219,6 +274,17 @@ static void *vb2_dc_alloc(void *alloc_ctx, unsigned long size)
 	atomic_inc(&buf->refcount);
 
 	return buf;
+
+fail_pages:
+	kfree(pages);
+
+fail_dma:
+	dma_free_coherent(dev, size, buf->vaddr, buf->dma_addr);
+
+fail_buf:
+	kfree(buf);
+
+	return ERR_PTR(ret);
 }
 
 static int vb2_dc_mmap(void *buf_priv, struct vm_area_struct *vma)
-- 
1.7.9.5

[PATCH 05/12] 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 |  119 ++++++++++++++++++++++++++++
 1 file changed, 119 insertions(+)

diff --git a/drivers/media/video/videobuf2-dma-contig.c b/drivers/media/video/videobuf2-dma-contig.c
index ae656be..b5826e0 100644
--- a/drivers/media/video/videobuf2-dma-contig.c
+++ b/drivers/media/video/videobuf2-dma-contig.c
@@ -325,6 +325,124 @@ 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)
+{
+	/* nothing to be done */
+	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;
+
+	dma_unmap_sg(db_attach->dev, sgt->sgl, sgt->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 dma_buf *dbuf = db_attach->dmabuf;
+	struct vb2_dc_buf *buf = dbuf->priv;
+	struct vb2_dc_attachment *attach = db_attach->priv;
+	struct sg_table *sgt;
+	struct scatterlist *rd, *wr;
+	int i, ret;
+
+	/* return previously mapped sg table */
+	if (attach)
+		return &attach->sgt;
+
+	attach = kzalloc(sizeof *attach, GFP_KERNEL);
+	if (!attach)
+		return ERR_PTR(-ENOMEM);
+
+	sgt = &attach->sgt;
+	attach->dir = dir;
+
+	/* copying the buf->base_sgt to attachment */
+	ret = sg_alloc_table(sgt, buf->sgt_base->orig_nents, GFP_KERNEL);
+	if (ret) {
+		kfree(attach);
+		return ERR_PTR(-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);
+	}
+
+	/* mapping new sglist to the client */
+	ret = dma_map_sg(db_attach->dev, sgt->sgl, sgt->orig_nents, dir);
+	if (ret <= 0) {
+		printk(KERN_ERR "failed to map scatterlist\n");
+		sg_free_table(sgt);
+		kfree(attach);
+		return ERR_PTR(-EIO);
+	}
+
+	db_attach->priv = attach;
+
+	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 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,
+	.release = vb2_dc_dmabuf_ops_release,
+};
+
+static struct dma_buf *vb2_dc_get_dmabuf(void *buf_priv)
+{
+	struct vb2_dc_buf *buf = buf_priv;
+	struct dma_buf *dbuf;
+
+	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       */
 /*********************************************/
 
@@ -621,6 +739,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

[PATCH 06/12] v4l: vb2-dma-contig: add vmap/kmap for dmabuf exporting

[Tomasz Stanislawski]

This patch adds support for vmap and kmap callbacks
for DMABUF exporter.

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

diff --git a/drivers/media/video/videobuf2-dma-contig.c b/drivers/media/video/videobuf2-dma-contig.c
index b5826e0..59ee81c 100644
--- a/drivers/media/video/videobuf2-dma-contig.c
+++ b/drivers/media/video/videobuf2-dma-contig.c
@@ -419,11 +419,28 @@ static void vb2_dc_dmabuf_ops_release(struct dma_buf *dbuf)
 	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,
 };
 
-- 
1.7.9.5

[PATCH 07/12] 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

[PATCH 08/12] 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

[PATCH 09/12] 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 |   13 +++++++++++++
 drivers/media/video/s5p-mfc/s5p_mfc_enc.c |   13 +++++++++++++
 2 files changed, 26 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..e1ebc76 100644
--- a/drivers/media/video/s5p-mfc/s5p_mfc_dec.c
+++ b/drivers/media/video/s5p-mfc/s5p_mfc_dec.c
@@ -564,6 +564,18 @@ 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);
+
+	if (eb->mem_offset < DST_QUEUE_OFF_BASE)
+		return vb2_expbuf(&ctx->vq_src, eb);
+	else
+		return vb2_expbuf(&ctx->vq_dst, eb);
+}
+
 /* Stream on */
 static int vidioc_streamon(struct file *file, void *priv,
 			   enum v4l2_buf_type type)
@@ -739,6 +751,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..887f1aa 100644
--- a/drivers/media/video/s5p-mfc/s5p_mfc_enc.c
+++ b/drivers/media/video/s5p-mfc/s5p_mfc_enc.c
@@ -1141,6 +1141,18 @@ 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);
+
+	if (eb->mem_offset < DST_QUEUE_OFF_BASE)
+		return vb2_expbuf(&ctx->vq_src, eb);
+	else
+		return vb2_expbuf(&ctx->vq_dst, eb);
+}
+
 /* Stream on */
 static int vidioc_streamon(struct file *file, void *priv,
 			   enum v4l2_buf_type type)
@@ -1486,6 +1498,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

[PATCH 10/12] v4l: vb2: remove vb2_mmap_pfn_range function

[Tomasz Stanislawski]

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>
---
 drivers/media/video/videobuf2-memops.c |   40 --------------------------------
 include/media/videobuf2-memops.h       |    5 ----
 2 files changed, 45 deletions(-)

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

[PATCH 11/12] v4l: vb2-dma-contig: use sg_alloc_table_from_pages function

[Tomasz Stanislawski]

This patch makes use of sg_alloc_table_from_pages to simplify
handling of sg tables.

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 |   90 ++++++++--------------------
 1 file changed, 25 insertions(+), 65 deletions(-)

diff --git a/drivers/media/video/videobuf2-dma-contig.c b/drivers/media/video/videobuf2-dma-contig.c
index 59ee81c..b5caf1d 100644
--- a/drivers/media/video/videobuf2-dma-contig.c
+++ b/drivers/media/video/videobuf2-dma-contig.c
@@ -32,7 +32,7 @@ struct vb2_dc_buf {
 	/* MMAP related */
 	struct vb2_vmarea_handler	handler;
 	atomic_t			refcount;
-	struct sg_table			*sgt_base;
+	struct sg_table			sgt_base;
 
 	/* USERPTR related */
 	struct vm_area_struct		*vma;
@@ -45,57 +45,6 @@ struct vb2_dc_buf {
 /*        scatterlist table functions        */
 /*********************************************/
 
-static struct sg_table *vb2_dc_pages_to_sgt(struct page **pages,
-	unsigned int n_pages, unsigned long offset, unsigned long size)
-{
-	struct sg_table *sgt;
-	unsigned int chunks;
-	unsigned int i;
-	unsigned int cur_page;
-	int ret;
-	struct scatterlist *s;
-
-	sgt = kzalloc(sizeof *sgt, GFP_KERNEL);
-	if (!sgt)
-		return ERR_PTR(-ENOMEM);
-
-	/* compute number of chunks */
-	chunks = 1;
-	for (i = 1; i < n_pages; ++i)
-		if (pages[i] != pages[i - 1] + 1)
-			++chunks;
-
-	ret = sg_alloc_table(sgt, chunks, GFP_KERNEL);
-	if (ret) {
-		kfree(sgt);
-		return ERR_PTR(-ENOMEM);
-	}
-
-	/* merging chunks and putting them into the scatterlist */
-	cur_page = 0;
-	for_each_sg(sgt->sgl, s, sgt->orig_nents, i) {
-		unsigned long chunk_size;
-		unsigned int j;
-
-		for (j = cur_page + 1; j < n_pages; ++j)
-			if (pages[j] != pages[j - 1] + 1)
-				break;
-
-		chunk_size = ((j - cur_page) << PAGE_SHIFT) - offset;
-		sg_set_page(s, pages[cur_page], min(size, chunk_size), offset);
-		size -= chunk_size;
-		offset = 0;
-		cur_page = j;
-	}
-
-	return sgt;
-}
-
-static void vb2_dc_release_sgtable(struct sg_table *sgt)
-{
-	sg_free_table(sgt);
-	kfree(sgt);
-}
 
 static void vb2_dc_sgt_foreach_page(struct sg_table *sgt,
 	void (*cb)(struct page *pg))
@@ -190,7 +139,7 @@ static void vb2_dc_put(void *buf_priv)
 	if (!atomic_dec_and_test(&buf->refcount))
 		return;
 
-	vb2_dc_release_sgtable(buf->sgt_base);
+	sg_free_table(&buf->sgt_base);
 	dma_free_coherent(buf->dev, buf->size, buf->vaddr, buf->dma_addr);
 	kfree(buf);
 }
@@ -254,9 +203,9 @@ static void *vb2_dc_alloc(void *alloc_ctx, unsigned long size)
 		goto fail_pages;
 	}
 
-	buf->sgt_base = vb2_dc_pages_to_sgt(pages, n_pages, 0, size);
-	if (IS_ERR(buf->sgt_base)) {
-		ret = PTR_ERR(buf->sgt_base);
+	ret = sg_alloc_table_from_pages(&buf->sgt_base,
+		pages, n_pages, 0, size, GFP_KERNEL);
+	if (ret) {
 		dev_err(dev, "failed to prepare sg table\n");
 		goto fail_pages;
 	}
@@ -379,13 +328,13 @@ static struct sg_table *vb2_dc_dmabuf_ops_map(
 	attach->dir = dir;
 
 	/* copying the buf->base_sgt to attachment */
-	ret = sg_alloc_table(sgt, buf->sgt_base->orig_nents, GFP_KERNEL);
+	ret = sg_alloc_table(sgt, buf->sgt_base.orig_nents, GFP_KERNEL);
 	if (ret) {
 		kfree(attach);
 		return ERR_PTR(-ENOMEM);
 	}
 
-	rd = buf->sgt_base->sgl;
+	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);
@@ -519,7 +468,8 @@ static void vb2_dc_put_userptr(void *buf_priv)
 	if (!vma_is_io(buf->vma))
 		vb2_dc_sgt_foreach_page(sgt, vb2_dc_put_dirty_page);
 
-	vb2_dc_release_sgtable(sgt);
+	sg_free_table(sgt);
+	kfree(sgt);
 	vb2_put_vma(buf->vma);
 	kfree(buf);
 }
@@ -586,13 +536,20 @@ static void *vb2_dc_get_userptr(void *alloc_ctx, unsigned long vaddr,
 		goto fail_vma;
 	}
 
-	sgt = vb2_dc_pages_to_sgt(pages, n_pages, offset, size);
-	if (IS_ERR(sgt)) {
-		printk(KERN_ERR "failed to create scatterlist table\n");
+	sgt = kzalloc(sizeof *sgt, GFP_KERNEL);
+	if (!sgt) {
+		printk(KERN_ERR "failed to allocate sg table\n");
 		ret = -ENOMEM;
 		goto fail_get_user_pages;
 	}
 
+	ret = sg_alloc_table_from_pages(sgt, pages, n_pages,
+		offset, size, GFP_KERNEL);
+	if (ret) {
+		printk(KERN_ERR "failed to initialize sg table\n");
+		goto fail_sgt;
+	}
+
 	/* pages are no longer needed */
 	kfree(pages);
 	pages = NULL;
@@ -602,7 +559,7 @@ static void *vb2_dc_get_userptr(void *alloc_ctx, unsigned long vaddr,
 	if (sgt->nents <= 0) {
 		printk(KERN_ERR "failed to map scatterlist\n");
 		ret = -EIO;
-		goto fail_sgt;
+		goto fail_sgt_init;
 	}
 
 	contig_size = vb2_dc_get_contiguous_size(sgt);
@@ -622,10 +579,13 @@ static void *vb2_dc_get_userptr(void *alloc_ctx, unsigned long vaddr,
 fail_map_sg:
 	dma_unmap_sg(buf->dev, sgt->sgl, sgt->nents, buf->dma_dir);
 
-fail_sgt:
+fail_sgt_init:
 	if (!vma_is_io(buf->vma))
 		vb2_dc_sgt_foreach_page(sgt, put_page);
-	vb2_dc_release_sgtable(sgt);
+	sg_free_table(sgt);
+
+fail_sgt:
+	kfree(sgt);
 
 fail_get_user_pages:
 	if (pages && !vma_is_io(buf->vma))
-- 
1.7.9.5

[PATCH 12/12] v4l: vb2-dma-contig: Move allocation of dbuf attachment to attach cb

[Tomasz Stanislawski]

The allocation of dma_buf_attachment is moved to attach callback.
The initialization is left in map callback.

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 |   39 ++++++++++++++++++----------
 1 file changed, 26 insertions(+), 13 deletions(-)

diff --git a/drivers/media/video/videobuf2-dma-contig.c b/drivers/media/video/videobuf2-dma-contig.c
index b5caf1d..3bf7c45 100644
--- a/drivers/media/video/videobuf2-dma-contig.c
+++ b/drivers/media/video/videobuf2-dma-contig.c
@@ -285,7 +285,15 @@ struct vb2_dc_attachment {
 static int vb2_dc_dmabuf_ops_attach(struct dma_buf *dbuf, struct device *dev,
 	struct dma_buf_attachment *dbuf_attach)
 {
-	/* nothing to be done */
+	struct vb2_dc_attachment *attach;
+
+	attach = kzalloc(sizeof *attach, GFP_KERNEL);
+	if (!attach)
+		return -ENOMEM;
+
+	attach->dir = DMA_NONE;
+	dbuf_attach->priv = attach;
+
 	return 0;
 }
 
@@ -300,7 +308,9 @@ static void vb2_dc_dmabuf_ops_detach(struct dma_buf *dbuf,
 
 	sgt = &attach->sgt;
 
-	dma_unmap_sg(db_attach->dev, sgt->sgl, sgt->nents, attach->dir);
+	/* checking if scaterlist was ever mapped */
+	if (attach->dir != DMA_NONE)
+		dma_unmap_sg(db_attach->dev, sgt->sgl, sgt->nents, attach->dir);
 	sg_free_table(sgt);
 	kfree(attach);
 	db_attach->priv = NULL;
@@ -314,25 +324,28 @@ static struct sg_table *vb2_dc_dmabuf_ops_map(
 	struct vb2_dc_attachment *attach = db_attach->priv;
 	struct sg_table *sgt;
 	struct scatterlist *rd, *wr;
-	int i, ret;
+	int ret;
+	unsigned int i;
+
+	if (WARN_ON(dir == DMA_NONE))
+		return ERR_PTR(-EINVAL);
 
 	/* return previously mapped sg table */
-	if (attach)
+	if (attach->dir == dir)
 		return &attach->sgt;
 
-	attach = kzalloc(sizeof *attach, GFP_KERNEL);
-	if (!attach)
-		return ERR_PTR(-ENOMEM);
+	/* reattaching is not allowed */
+	if (WARN_ON(attach->dir != DMA_NONE))
+		return ERR_PTR(-EBUSY);
 
 	sgt = &attach->sgt;
-	attach->dir = dir;
 
-	/* copying the buf->base_sgt to attachment */
+	/* 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);
+	if (ret)
 		return ERR_PTR(-ENOMEM);
-	}
 
 	rd = buf->sgt_base.sgl;
 	wr = sgt->sgl;
@@ -347,10 +360,10 @@ static struct sg_table *vb2_dc_dmabuf_ops_map(
 	if (ret <= 0) {
 		printk(KERN_ERR "failed to map scatterlist\n");
 		sg_free_table(sgt);
-		kfree(attach);
 		return ERR_PTR(-EIO);
 	}
 
+	attach->dir = dir;
 	db_attach->priv = attach;
 
 	return sgt;
-- 
1.7.9.5

Re: [Linaro-mm-sig] [PATCH 00/12] Support for dmabuf exporting for videobuf2

[Sangwook Lee]

[-- Attachment #1.1: Type: text/plain, Size: 3204 bytes --]

Hi Tomasz

On 23 May 2012 14:07, Tomasz Stanislawski <t.stanislaws@samsung.com> wrote:

> 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 not 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].
>

Do you have your own git ?

Thanks
Sangwook


> Changelog:
> v0: (RFC)
> - 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
>
> [1]
> http://thread.gmane.org/gmane.linux.drivers.video-input-infrastructure/48730
> [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:
>
> Marek Szyprowski (1):
>  v4l: vb2-dma-contig: let mmap method to use dma_mmap_coherent call
>
> Tomasz Stanislawski (11):
>  v4l: add buffer exporting via dmabuf
>  v4l: vb2: add buffer exporting via dmabuf
>  v4l: vb2-dma-contig: add setup of sglist for MMAP buffers
>  v4l: vb2-dma-contig: add support for DMABUF exporting
>  v4l: vb2-dma-contig: add vmap/kmap 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: remove vb2_mmap_pfn_range function
>  v4l: vb2-dma-contig: use sg_alloc_table_from_pages function
>  v4l: vb2-dma-contig: Move allocation of dbuf attachment to attach cb
>
>  drivers/media/video/s5p-fimc/fimc-capture.c |    9 +
>  drivers/media/video/s5p-mfc/s5p_mfc_dec.c   |   13 ++
>  drivers/media/video/s5p-mfc/s5p_mfc_enc.c   |   13 ++
>  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  |  323
> ++++++++++++++++++++++-----
>  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 -
>  14 files changed, 411 insertions(+), 107 deletions(-)
>
> --
> 1.7.9.5
>
>
> _______________________________________________
> Linaro-mm-sig mailing list
> Linaro-mm-sig@lists.linaro.org
> http://lists.linaro.org/mailman/listinfo/linaro-mm-sig
>

[-- Attachment #1.2: Type: text/html, Size: 4355 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 03/12] v4l: vb2: add buffer exporting via dmabuf

[Laurent Pinchart]

Hi Tomasz,

Thanks for the patch.

On Wednesday 23 May 2012 15:07:26 Tomasz Stanislawski wrote:
> This patch adds extension to videobuf2-core. It allow to export a mmap

s/allow/allows/

> 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>

-- 
Regards,

Laurent Pinchart

Re: [PATCH 01/12] v4l: vb2-dma-contig: let mmap method to use dma_mmap_coherent call

[Laurent Pinchart]

Hi Tomasz,

On Wednesday 23 May 2012 15:07:24 Tomasz Stanislawski wrote:
> 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.
> 
> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>

Could you please squash 10/12 into this patch ? Then, for both,

Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

-- 
Regards,

Laurent Pinchart

Re: [PATCH 02/12] v4l: add buffer exporting via dmabuf

[Laurent Pinchart]

Hi Tomasz,

Thanks for the patch.

On Wednesday 23 May 2012 15:07:25 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>

Both the API proposal and the patch look good to me. I'll ack this along with 
the update to the V4L2 documentation ;-)

> ---
>  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 5327ad3..45159d9 100644
> --- a/drivers/media/video/v4l2-compat-ioctl32.c
> +++ b/drivers/media/video/v4l2-compat-ioctl32.c
> @@ -954,6 +954,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);
-- 
Regards,

Laurent Pinchart

Re: [PATCH 11/12] v4l: vb2-dma-contig: use sg_alloc_table_from_pages function

[Laurent Pinchart]

Hi Tomasz,

Thanks for the patch.

On Wednesday 23 May 2012 15:07:34 Tomasz Stanislawski wrote:
> This patch makes use of sg_alloc_table_from_pages to simplify
> handling of sg tables.

Would you mind moving this patch before 04/12, to avoid introducing a 
vb2_dc_pages_to_sgt() user only to remove it in this patch ? Otherwise this 
looks good.

> 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 |   90 +++++++------------------
>  1 file changed, 25 insertions(+), 65 deletions(-)
> 
> diff --git a/drivers/media/video/videobuf2-dma-contig.c
> b/drivers/media/video/videobuf2-dma-contig.c index 59ee81c..b5caf1d 100644
> --- a/drivers/media/video/videobuf2-dma-contig.c
> +++ b/drivers/media/video/videobuf2-dma-contig.c
> @@ -32,7 +32,7 @@ struct vb2_dc_buf {
>  	/* MMAP related */
>  	struct vb2_vmarea_handler	handler;
>  	atomic_t			refcount;
> -	struct sg_table			*sgt_base;
> +	struct sg_table			sgt_base;
> 
>  	/* USERPTR related */
>  	struct vm_area_struct		*vma;
> @@ -45,57 +45,6 @@ struct vb2_dc_buf {
>  /*        scatterlist table functions        */
>  /*********************************************/
> 
> -static struct sg_table *vb2_dc_pages_to_sgt(struct page **pages,
> -	unsigned int n_pages, unsigned long offset, unsigned long size)
> -{
> -	struct sg_table *sgt;
> -	unsigned int chunks;
> -	unsigned int i;
> -	unsigned int cur_page;
> -	int ret;
> -	struct scatterlist *s;
> -
> -	sgt = kzalloc(sizeof *sgt, GFP_KERNEL);
> -	if (!sgt)
> -		return ERR_PTR(-ENOMEM);
> -
> -	/* compute number of chunks */
> -	chunks = 1;
> -	for (i = 1; i < n_pages; ++i)
> -		if (pages[i] != pages[i - 1] + 1)
> -			++chunks;
> -
> -	ret = sg_alloc_table(sgt, chunks, GFP_KERNEL);
> -	if (ret) {
> -		kfree(sgt);
> -		return ERR_PTR(-ENOMEM);
> -	}
> -
> -	/* merging chunks and putting them into the scatterlist */
> -	cur_page = 0;
> -	for_each_sg(sgt->sgl, s, sgt->orig_nents, i) {
> -		unsigned long chunk_size;
> -		unsigned int j;
> -
> -		for (j = cur_page + 1; j < n_pages; ++j)
> -			if (pages[j] != pages[j - 1] + 1)
> -				break;
> -
> -		chunk_size = ((j - cur_page) << PAGE_SHIFT) - offset;
> -		sg_set_page(s, pages[cur_page], min(size, chunk_size), offset);
> -		size -= chunk_size;
> -		offset = 0;
> -		cur_page = j;
> -	}
> -
> -	return sgt;
> -}
> -
> -static void vb2_dc_release_sgtable(struct sg_table *sgt)
> -{
> -	sg_free_table(sgt);
> -	kfree(sgt);
> -}
> 
>  static void vb2_dc_sgt_foreach_page(struct sg_table *sgt,
>  	void (*cb)(struct page *pg))
> @@ -190,7 +139,7 @@ static void vb2_dc_put(void *buf_priv)
>  	if (!atomic_dec_and_test(&buf->refcount))
>  		return;
> 
> -	vb2_dc_release_sgtable(buf->sgt_base);
> +	sg_free_table(&buf->sgt_base);
>  	dma_free_coherent(buf->dev, buf->size, buf->vaddr, buf->dma_addr);
>  	kfree(buf);
>  }
> @@ -254,9 +203,9 @@ static void *vb2_dc_alloc(void *alloc_ctx, unsigned long
> size) goto fail_pages;
>  	}
> 
> -	buf->sgt_base = vb2_dc_pages_to_sgt(pages, n_pages, 0, size);
> -	if (IS_ERR(buf->sgt_base)) {
> -		ret = PTR_ERR(buf->sgt_base);
> +	ret = sg_alloc_table_from_pages(&buf->sgt_base,
> +		pages, n_pages, 0, size, GFP_KERNEL);
> +	if (ret) {
>  		dev_err(dev, "failed to prepare sg table\n");
>  		goto fail_pages;
>  	}
> @@ -379,13 +328,13 @@ static struct sg_table *vb2_dc_dmabuf_ops_map(
>  	attach->dir = dir;
> 
>  	/* copying the buf->base_sgt to attachment */
> -	ret = sg_alloc_table(sgt, buf->sgt_base->orig_nents, GFP_KERNEL);
> +	ret = sg_alloc_table(sgt, buf->sgt_base.orig_nents, GFP_KERNEL);
>  	if (ret) {
>  		kfree(attach);
>  		return ERR_PTR(-ENOMEM);
>  	}
> 
> -	rd = buf->sgt_base->sgl;
> +	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);
> @@ -519,7 +468,8 @@ static void vb2_dc_put_userptr(void *buf_priv)
>  	if (!vma_is_io(buf->vma))
>  		vb2_dc_sgt_foreach_page(sgt, vb2_dc_put_dirty_page);
> 
> -	vb2_dc_release_sgtable(sgt);
> +	sg_free_table(sgt);
> +	kfree(sgt);
>  	vb2_put_vma(buf->vma);
>  	kfree(buf);
>  }
> @@ -586,13 +536,20 @@ static void *vb2_dc_get_userptr(void *alloc_ctx,
> unsigned long vaddr, goto fail_vma;
>  	}
> 
> -	sgt = vb2_dc_pages_to_sgt(pages, n_pages, offset, size);
> -	if (IS_ERR(sgt)) {
> -		printk(KERN_ERR "failed to create scatterlist table\n");
> +	sgt = kzalloc(sizeof *sgt, GFP_KERNEL);
> +	if (!sgt) {
> +		printk(KERN_ERR "failed to allocate sg table\n");
>  		ret = -ENOMEM;
>  		goto fail_get_user_pages;
>  	}
> 
> +	ret = sg_alloc_table_from_pages(sgt, pages, n_pages,
> +		offset, size, GFP_KERNEL);
> +	if (ret) {
> +		printk(KERN_ERR "failed to initialize sg table\n");
> +		goto fail_sgt;
> +	}
> +
>  	/* pages are no longer needed */
>  	kfree(pages);
>  	pages = NULL;
> @@ -602,7 +559,7 @@ static void *vb2_dc_get_userptr(void *alloc_ctx,
> unsigned long vaddr, if (sgt->nents <= 0) {
>  		printk(KERN_ERR "failed to map scatterlist\n");
>  		ret = -EIO;
> -		goto fail_sgt;
> +		goto fail_sgt_init;
>  	}
> 
>  	contig_size = vb2_dc_get_contiguous_size(sgt);
> @@ -622,10 +579,13 @@ static void *vb2_dc_get_userptr(void *alloc_ctx,
> unsigned long vaddr, fail_map_sg:
>  	dma_unmap_sg(buf->dev, sgt->sgl, sgt->nents, buf->dma_dir);
> 
> -fail_sgt:
> +fail_sgt_init:
>  	if (!vma_is_io(buf->vma))
>  		vb2_dc_sgt_foreach_page(sgt, put_page);
> -	vb2_dc_release_sgtable(sgt);
> +	sg_free_table(sgt);
> +
> +fail_sgt:
> +	kfree(sgt);
> 
>  fail_get_user_pages:
>  	if (pages && !vma_is_io(buf->vma))
-- 
Regards,

Laurent Pinchart

Re: [PATCH 04/12] v4l: vb2-dma-contig: add setup of sglist for MMAP buffers

[Laurent Pinchart]

Hi Tomasz,

Thanks for the patch.

On Wednesday 23 May 2012 15:07:27 Tomasz Stanislawski wrote:
> This patch adds the setup of sglist list for MMAP buffers.
> It is needed for buffer exporting via DMABUF mechanism.
> 
> 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 |   70 ++++++++++++++++++++++++-
>  1 file changed, 68 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/media/video/videobuf2-dma-contig.c
> b/drivers/media/video/videobuf2-dma-contig.c index 52b4f59..ae656be 100644
> --- a/drivers/media/video/videobuf2-dma-contig.c
> +++ b/drivers/media/video/videobuf2-dma-contig.c
> @@ -32,6 +32,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;
> @@ -189,14 +190,37 @@ static void vb2_dc_put(void *buf_priv)
>  	if (!atomic_dec_and_test(&buf->refcount))
>  		return;
> 
> +	vb2_dc_release_sgtable(buf->sgt_base);
>  	dma_free_coherent(buf->dev, buf->size, buf->vaddr, buf->dma_addr);
>  	kfree(buf);
>  }
> 
> +static int vb2_dc_kaddr_to_pages(unsigned long kaddr,
> +	struct page **pages, unsigned int n_pages)
> +{
> +	unsigned int i;
> +	unsigned long pfn;
> +	struct vm_area_struct vma = {
> +		.vm_flags = VM_IO | VM_PFNMAP,
> +		.vm_mm = current->mm,
> +	};
> +
> +	for (i = 0; i < n_pages; ++i, kaddr += PAGE_SIZE) {

The follow_pfn() kerneldoc mentions that it looks up a PFN for a user address. 
The only users I've found in the kernel sources pass a user address. Is it 
legal to use it for kernel addresses ?

> +		if (follow_pfn(&vma, kaddr, &pfn))
> +			break;
> +		pages[i] = pfn_to_page(pfn);
> +	}
> +
> +	return i;
> +}
> +
>  static void *vb2_dc_alloc(void *alloc_ctx, unsigned long size)
>  {
>  	struct device *dev = alloc_ctx;
>  	struct vb2_dc_buf *buf;
> +	int ret = -ENOMEM;
> +	int n_pages;
> +	struct page **pages = NULL;
> 
>  	buf = kzalloc(sizeof *buf, GFP_KERNEL);
>  	if (!buf)
> @@ -205,10 +229,41 @@ static void *vb2_dc_alloc(void *alloc_ctx, unsigned
> long size) buf->vaddr = dma_alloc_coherent(dev, size, &buf->dma_addr,
> GFP_KERNEL); if (!buf->vaddr) {
>  		dev_err(dev, "dma_alloc_coherent of size %ld failed\n", size);
> -		kfree(buf);
> -		return ERR_PTR(-ENOMEM);
> +		goto fail_buf;
> +	}
> +
> +	WARN_ON((unsigned long)buf->vaddr & ~PAGE_MASK);
> +	WARN_ON(buf->dma_addr & ~PAGE_MASK);
> +
> +	n_pages = PAGE_ALIGN(size) >> PAGE_SHIFT;
> +
> +	pages = kmalloc(n_pages * sizeof pages[0], GFP_KERNEL);
> +	if (!pages) {
> +		dev_err(dev, "failed to alloc page table\n");
> +		goto fail_dma;
> +	}
> +
> +	ret = vb2_dc_kaddr_to_pages((unsigned long)buf->vaddr, pages, n_pages);
> +	if (ret < 0) {
> +		dev_err(dev, "failed to get buffer pages from DMA API\n");
> +		goto fail_pages;
> +	}
> +	if (ret != n_pages) {
> +		ret = -EFAULT;
> +		dev_err(dev, "failed to get all pages from DMA API\n");
> +		goto fail_pages;
> +	}
> +
> +	buf->sgt_base = vb2_dc_pages_to_sgt(pages, n_pages, 0, size);
> +	if (IS_ERR(buf->sgt_base)) {
> +		ret = PTR_ERR(buf->sgt_base);
> +		dev_err(dev, "failed to prepare sg table\n");
> +		goto fail_pages;
>  	}
> 
> +	/* pages are no longer needed */
> +	kfree(pages);
> +
>  	buf->dev = dev;
>  	buf->size = size;
> 
> @@ -219,6 +274,17 @@ static void *vb2_dc_alloc(void *alloc_ctx, unsigned
> long size) atomic_inc(&buf->refcount);
> 
>  	return buf;
> +
> +fail_pages:
> +	kfree(pages);
> +
> +fail_dma:

You can merge the fail_pages and fail_dma labels, as kfree(NULL) is valid.

> +	dma_free_coherent(dev, size, buf->vaddr, buf->dma_addr);
> +
> +fail_buf:
> +	kfree(buf);
> +
> +	return ERR_PTR(ret);
>  }
> 
>  static int vb2_dc_mmap(void *buf_priv, struct vm_area_struct *vma)

-- 
Regards,

Laurent Pinchart

Re: [PATCH 04/12] v4l: vb2-dma-contig: add setup of sglist for MMAP buffers

[Tomasz Stanislawski]

Hi Laurent,
Thank your for your comments.

On 06/06/2012 10:06 AM, Laurent Pinchart wrote:
> Hi Tomasz,
> 
> Thanks for the patch.
> 
> On Wednesday 23 May 2012 15:07:27 Tomasz Stanislawski wrote:
>> This patch adds the setup of sglist list for MMAP buffers.
>> It is needed for buffer exporting via DMABUF mechanism.
>>
>> 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 |   70 ++++++++++++++++++++++++-
>>  1 file changed, 68 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/media/video/videobuf2-dma-contig.c
>> b/drivers/media/video/videobuf2-dma-contig.c index 52b4f59..ae656be 100644
>> --- a/drivers/media/video/videobuf2-dma-contig.c
>> +++ b/drivers/media/video/videobuf2-dma-contig.c
>> @@ -32,6 +32,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;
>> @@ -189,14 +190,37 @@ static void vb2_dc_put(void *buf_priv)
>>  	if (!atomic_dec_and_test(&buf->refcount))
>>  		return;
>>
>> +	vb2_dc_release_sgtable(buf->sgt_base);
>>  	dma_free_coherent(buf->dev, buf->size, buf->vaddr, buf->dma_addr);
>>  	kfree(buf);
>>  }
>>
>> +static int vb2_dc_kaddr_to_pages(unsigned long kaddr,
>> +	struct page **pages, unsigned int n_pages)
>> +{
>> +	unsigned int i;
>> +	unsigned long pfn;
>> +	struct vm_area_struct vma = {
>> +		.vm_flags = VM_IO | VM_PFNMAP,
>> +		.vm_mm = current->mm,
>> +	};
>> +
>> +	for (i = 0; i < n_pages; ++i, kaddr += PAGE_SIZE) {
> 
> The follow_pfn() kerneldoc mentions that it looks up a PFN for a user address. 
> The only users I've found in the kernel sources pass a user address. Is it 
> legal to use it for kernel addresses ?
> 

It is not completely legal :). As I understand the mm code,
the follow_pfn works only for IO/PFN mappings.
This is the typical case (every case?) of mappings
created by dma_alloc_coherent.

In order to make this function work for a kernel pointer,
one has to create an artificial VMA that has IO/PFN bits on.

This solution is a hack-around for dma_get_pages (aka dma_get_sgtable).
This way the dependency on dma_get_pages was broken giving a small
hope of merging vb2 exporting.

Marek prepared a patchset 'ARM: DMA-mapping: new extensions for
buffer sharing' that adds dma buffers with no kernel mappings
and dma_get_sgtable function.

However this patchset is still in a RFC state.

I have prepared a patch that removes vb2_dc_kaddr_to_pages
and substitutes it with dma_get_pages. It will become
a part of vb2-exporter patches just after dma_get_sgtable
is merged (or at least acked by major maintainers).

>> +		if (follow_pfn(&vma, kaddr, &pfn))
>> +			break;
>> +		pages[i] = pfn_to_page(pfn);
>> +	}
>> +
>> +	return i;
>> +}
>> +
>>  static void *vb2_dc_alloc(void *alloc_ctx, unsigned long size)
>>  {
>>  	struct device *dev = alloc_ctx;
>>  	struct vb2_dc_buf *buf;
>> +	int ret = -ENOMEM;
>> +	int n_pages;
>> +	struct page **pages = NULL;
>>
>>  	buf = kzalloc(sizeof *buf, GFP_KERNEL);
>>  	if (!buf)
>> @@ -205,10 +229,41 @@ static void *vb2_dc_alloc(void *alloc_ctx, unsigned
>> long size) buf->vaddr = dma_alloc_coherent(dev, size, &buf->dma_addr,
>> GFP_KERNEL); if (!buf->vaddr) {
>>  		dev_err(dev, "dma_alloc_coherent of size %ld failed\n", size);
>> -		kfree(buf);
>> -		return ERR_PTR(-ENOMEM);
>> +		goto fail_buf;
>> +	}
>> +
>> +	WARN_ON((unsigned long)buf->vaddr & ~PAGE_MASK);
>> +	WARN_ON(buf->dma_addr & ~PAGE_MASK);
>> +
>> +	n_pages = PAGE_ALIGN(size) >> PAGE_SHIFT;
>> +
>> +	pages = kmalloc(n_pages * sizeof pages[0], GFP_KERNEL);
>> +	if (!pages) {
>> +		dev_err(dev, "failed to alloc page table\n");
>> +		goto fail_dma;
>> +	}
>> +
>> +	ret = vb2_dc_kaddr_to_pages((unsigned long)buf->vaddr, pages, n_pages);
>> +	if (ret < 0) {
>> +		dev_err(dev, "failed to get buffer pages from DMA API\n");
>> +		goto fail_pages;
>> +	}
>> +	if (ret != n_pages) {
>> +		ret = -EFAULT;
>> +		dev_err(dev, "failed to get all pages from DMA API\n");
>> +		goto fail_pages;
>> +	}
>> +
>> +	buf->sgt_base = vb2_dc_pages_to_sgt(pages, n_pages, 0, size);
>> +	if (IS_ERR(buf->sgt_base)) {
>> +		ret = PTR_ERR(buf->sgt_base);
>> +		dev_err(dev, "failed to prepare sg table\n");
>> +		goto fail_pages;
>>  	}
>>
>> +	/* pages are no longer needed */
>> +	kfree(pages);
>> +
>>  	buf->dev = dev;
>>  	buf->size = size;
>>
>> @@ -219,6 +274,17 @@ static void *vb2_dc_alloc(void *alloc_ctx, unsigned
>> long size) atomic_inc(&buf->refcount);
>>
>>  	return buf;
>> +
>> +fail_pages:
>> +	kfree(pages);
>> +
>> +fail_dma:
> 
> You can merge the fail_pages and fail_dma labels, as kfree(NULL) is valid.
> 

Yes, I can. But there are two reasons for not doing that:
- first: calling a dummy kfree introduces a negligible but non-zero overhead
- second: the fail-path becomes more difficult to understand

Regards,
Tomasz Stanislawski

>> +	dma_free_coherent(dev, size, buf->vaddr, buf->dma_addr);
>> +
>> +fail_buf:
>> +	kfree(buf);
>> +
>> +	return ERR_PTR(ret);
>>  }
>>
>>  static int vb2_dc_mmap(void *buf_priv, struct vm_area_struct *vma)
> 

Re: [PATCH 04/12] v4l: vb2-dma-contig: add setup of sglist for MMAP buffers

[Laurent Pinchart]

Hi Tomasz,

On Wednesday 06 June 2012 13:56:42 Tomasz Stanislawski wrote:
> On 06/06/2012 10:06 AM, Laurent Pinchart wrote:
> > On Wednesday 23 May 2012 15:07:27 Tomasz Stanislawski wrote:
> >> This patch adds the setup of sglist list for MMAP buffers.
> >> It is needed for buffer exporting via DMABUF mechanism.
> >> 
> >> 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 |   70 +++++++++++++++++++++-
> >>  1 file changed, 68 insertions(+), 2 deletions(-)
> >> 
> >> diff --git a/drivers/media/video/videobuf2-dma-contig.c
> >> b/drivers/media/video/videobuf2-dma-contig.c index 52b4f59..ae656be
> >> 100644
> >> --- a/drivers/media/video/videobuf2-dma-contig.c
> >> +++ b/drivers/media/video/videobuf2-dma-contig.c

[snip]

> >> +static int vb2_dc_kaddr_to_pages(unsigned long kaddr,
> >> +	struct page **pages, unsigned int n_pages)
> >> +{
> >> +	unsigned int i;
> >> +	unsigned long pfn;
> >> +	struct vm_area_struct vma = {
> >> +		.vm_flags = VM_IO | VM_PFNMAP,
> >> +		.vm_mm = current->mm,
> >> +	};
> >> +
> >> +	for (i = 0; i < n_pages; ++i, kaddr += PAGE_SIZE) {
> > 
> > The follow_pfn() kerneldoc mentions that it looks up a PFN for a user
> > address. The only users I've found in the kernel sources pass a user
> > address. Is it legal to use it for kernel addresses ?
> 
> It is not completely legal :). As I understand the mm code, the follow_pfn
> works only for IO/PFN mappings. This is the typical case (every case?) of
> mappings created by dma_alloc_coherent.
> 
> In order to make this function work for a kernel pointer, one has to create
> an artificial VMA that has IO/PFN bits on.
> 
> This solution is a hack-around for dma_get_pages (aka dma_get_sgtable). This
> way the dependency on dma_get_pages was broken giving a small hope of
> merging vb2 exporting.
> 
> Marek prepared a patchset 'ARM: DMA-mapping: new extensions for buffer
> sharing' that adds dma buffers with no kernel mappings and dma_get_sgtable
> function.
> 
> However this patchset is still in a RFC state.

That's totally understood :-) I'm fine with keeping the hack for now until the 
dma_get_sgtable() gets in a usable/mergeable state, please just mention it in 
the code with something like

/* HACK: This is a temporary workaround until the dma_get_sgtable() function 
becomes available. */

> I have prepared a patch that removes vb2_dc_kaddr_to_pages and substitutes
> it with dma_get_pages. It will become a part of vb2-exporter patches just
> after dma_get_sgtable is merged (or at least acked by major maintainers).

-- 
Regards,

Laurent Pinchart

Re: [PATCH 04/12] v4l: vb2-dma-contig: add setup of sglist for MMAP buffers

[Subash Patel]

[-- Attachment #1: Type: text/plain, Size: 3062 bytes --]

Hello Tomasz,

On 06/07/2012 06:06 AM, Laurent Pinchart wrote:
> Hi Tomasz,
>
> On Wednesday 06 June 2012 13:56:42 Tomasz Stanislawski wrote:
>> On 06/06/2012 10:06 AM, Laurent Pinchart wrote:
>>> On Wednesday 23 May 2012 15:07:27 Tomasz Stanislawski wrote:
>>>> This patch adds the setup of sglist list for MMAP buffers.
>>>> It is needed for buffer exporting via DMABUF mechanism.
>>>>
>>>> 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 |   70 +++++++++++++++++++++-
>>>>   1 file changed, 68 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/drivers/media/video/videobuf2-dma-contig.c
>>>> b/drivers/media/video/videobuf2-dma-contig.c index 52b4f59..ae656be
>>>> 100644
>>>> --- a/drivers/media/video/videobuf2-dma-contig.c
>>>> +++ b/drivers/media/video/videobuf2-dma-contig.c
>
> [snip]
>
>>>> +static int vb2_dc_kaddr_to_pages(unsigned long kaddr,
>>>> +	struct page **pages, unsigned int n_pages)
>>>> +{
>>>> +	unsigned int i;
>>>> +	unsigned long pfn;
>>>> +	struct vm_area_struct vma = {
>>>> +		.vm_flags = VM_IO | VM_PFNMAP,
>>>> +		.vm_mm = current->mm,
>>>> +	};
>>>> +
>>>> +	for (i = 0; i<  n_pages; ++i, kaddr += PAGE_SIZE) {
>>>
>>> The follow_pfn() kerneldoc mentions that it looks up a PFN for a user
>>> address. The only users I've found in the kernel sources pass a user
>>> address. Is it legal to use it for kernel addresses ?
>>
>> It is not completely legal :). As I understand the mm code, the follow_pfn
>> works only for IO/PFN mappings. This is the typical case (every case?) of
>> mappings created by dma_alloc_coherent.
>>
>> In order to make this function work for a kernel pointer, one has to create
>> an artificial VMA that has IO/PFN bits on.
>>
>> This solution is a hack-around for dma_get_pages (aka dma_get_sgtable). This
>> way the dependency on dma_get_pages was broken giving a small hope of
>> merging vb2 exporting.
>>
>> Marek prepared a patchset 'ARM: DMA-mapping: new extensions for buffer
>> sharing' that adds dma buffers with no kernel mappings and dma_get_sgtable
>> function.
>>
>> However this patchset is still in a RFC state.
>
> That's totally understood :-) I'm fine with keeping the hack for now until the
> dma_get_sgtable() gets in a usable/mergeable state, please just mention it in
> the code with something like
>
> /* HACK: This is a temporary workaround until the dma_get_sgtable() function
> becomes available. */
>
>> I have prepared a patch that removes vb2_dc_kaddr_to_pages and substitutes
>> it with dma_get_pages. It will become a part of vb2-exporter patches just
>> after dma_get_sgtable is merged (or at least acked by major maintainers).
>
The above function call (because of follow_pfn) doesn't succeed for all 
the allocated pages. Hence I created a patch(attached) which is based on 
[1] series. One can apply it for using your present patch-set in the 
meantime.

Regards,
Subash
[1] http://www.spinics.net/lists/kernel/msg1343092.html

[-- Attachment #2: 0001-v4l2-vb2-use-dma_get_sgtable.patch --]
[-- Type: text/x-patch, Size: 3182 bytes --]

>From f9b2eace2eef0038a1830e5e6dd55f3bb6017e1a Mon Sep 17 00:00:00 2001
From: Subash Patel <subash.ramaswamy@linaro.org>
Date: Thu, 7 Jun 2012 19:49:10 +0530
Subject: [PATCH] v4l2: vb2: use dma_get_sgtable

This is patch to remove vb2_dc_kaddr_to_pages() function with dma_get_sgtable()
in the patch set posted by Tomasz. It was observed that the former function
fails to get the pages, as follow_pfn() can fail for remapped kernel va provided
by the dma-mapping sub-system.

As Tomasz mentioned, the later call was temporary patch until dma-mapping author
finalizes the implementation of dma_get_sgtable(). One can use this patch to use
this vb2 patch-set for his/her work in the meantime.

Signed-off-by: Subash Patel <subash.ramaswamy@linaro.org>
---
 drivers/media/video/videobuf2-dma-contig.c |   53 ++-------------------------
 1 files changed, 4 insertions(+), 49 deletions(-)

diff --git a/drivers/media/video/videobuf2-dma-contig.c b/drivers/media/video/videobuf2-dma-contig.c
index e8da7f1..1b9023a 100644
--- a/drivers/media/video/videobuf2-dma-contig.c
+++ b/drivers/media/video/videobuf2-dma-contig.c
@@ -143,32 +143,11 @@ static void vb2_dc_put(void *buf_priv)
 	kfree(buf);
 }
 
-static int vb2_dc_kaddr_to_pages(unsigned long kaddr,
-	struct page **pages, unsigned int n_pages)
-{
-	unsigned int i;
-	unsigned long pfn;
-	struct vm_area_struct vma = {
-		.vm_flags = VM_IO | VM_PFNMAP,
-		.vm_mm = current->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 void *vb2_dc_alloc(void *alloc_ctx, unsigned long size)
 {
 	struct device *dev = alloc_ctx;
 	struct vb2_dc_buf *buf;
 	int ret = -ENOMEM;
-	int n_pages;
-	struct page **pages = NULL;
 
 	buf = kzalloc(sizeof *buf, GFP_KERNEL);
 	if (!buf)
@@ -183,35 +162,14 @@ static void *vb2_dc_alloc(void *alloc_ctx, unsigned long size)
 	WARN_ON((unsigned long)buf->vaddr & ~PAGE_MASK);
 	WARN_ON(buf->dma_addr & ~PAGE_MASK);
 
-	n_pages = PAGE_ALIGN(size) >> PAGE_SHIFT;
+	ret = dma_get_sgtable(dev, &buf->sgt_base, buf->vaddr,
+						buf->dma_addr, size, NULL);
 
-	pages = kmalloc(n_pages * sizeof pages[0], GFP_KERNEL);
-	if (!pages) {
-		dev_err(dev, "failed to alloc page table\n");
-		goto fail_dma;
-	}
-
-	ret = vb2_dc_kaddr_to_pages((unsigned long)buf->vaddr, pages, n_pages);
 	if (ret < 0) {
-		dev_err(dev, "failed to get buffer pages from DMA API\n");
-		goto fail_pages;
-	}
-	if (ret != n_pages) {
-		ret = -EFAULT;
-		dev_err(dev, "failed to get all pages from DMA API\n");
-		goto fail_pages;
-	}
-
-	ret = sg_alloc_table_from_pages(&buf->sgt_base,
-		pages, n_pages, 0, size, GFP_KERNEL);
-	if (ret) {
-		dev_err(dev, "failed to prepare sg table\n");
-		goto fail_pages;
+		dev_err(dev, "failed to get the SGT for the allocated pages\n");
+		goto fail_dma;
 	}
 
-	/* pages are no longer needed */
-	kfree(pages);
-
 	buf->dev = dev;
 	buf->size = size;
 
@@ -223,9 +181,6 @@ static void *vb2_dc_alloc(void *alloc_ctx, unsigned long size)
 
 	return buf;
 
-fail_pages:
-	kfree(pages);
-
 fail_dma:
 	dma_free_coherent(dev, size, buf->vaddr, buf->dma_addr);
 
-- 
1.7.5.4

Re: [PATCH 04/12] v4l: vb2-dma-contig: add setup of sglist for MMAP buffers

[Tomasz Stanislawski]

Hi Laurent and Subash,

I confirm the issue found by Subash. The function
vb2_dc_kaddr_to_pages does fail for some occasions.
The failures are rather strange like 'got 95 of
150 pages'. It took me some time to find the reason
of the problem.

I found that dma_alloc_coherent for iommu an ARM does
use ioremap_page_range to map a buffer to the kernel
space. The mapping is done by updating the page-table.

The problem is that any process has a different first-level
page-table. The ioremap_page_range updates only the table
for init process. The PT present in current->mm shares
a majority of entries of 1st-level PT at kernel range
(above 0xc0000000) but *not all*. That is why
vb2_dc_kaddr_to_pages worked for small buffers and
occasionally failed for larger buffers.

I found two ways to fix this problem.
a) use &init_mm instead of current->mm while
   creating an artificial vma
b) access the dma memory by calling
   *((volatile int *)kaddr) = 0;
   before calling follow_pfn
   This way a fault is generated and the PT is
   updated by copying entries from init_mm.

What do you think about presented solutions?

Regards,
Tomasz Stanislawski



On 06/07/2012 04:28 PM, Subash Patel wrote:
> Hello Tomasz,
> 
> On 06/07/2012 06:06 AM, Laurent Pinchart wrote:
>> Hi Tomasz,
>>
>> On Wednesday 06 June 2012 13:56:42 Tomasz Stanislawski wrote:
>>> On 06/06/2012 10:06 AM, Laurent Pinchart wrote:
>>>> On Wednesday 23 May 2012 15:07:27 Tomasz Stanislawski wrote:
>>>>> This patch adds the setup of sglist list for MMAP buffers.
>>>>> It is needed for buffer exporting via DMABUF mechanism.
>>>>>
>>>>> 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 |   70 +++++++++++++++++++++-
>>>>>   1 file changed, 68 insertions(+), 2 deletions(-)
>>>>>
>>>>> diff --git a/drivers/media/video/videobuf2-dma-contig.c
>>>>> b/drivers/media/video/videobuf2-dma-contig.c index 52b4f59..ae656be
>>>>> 100644
>>>>> --- a/drivers/media/video/videobuf2-dma-contig.c
>>>>> +++ b/drivers/media/video/videobuf2-dma-contig.c
>>
>> [snip]
>>
>>>>> +static int vb2_dc_kaddr_to_pages(unsigned long kaddr,
>>>>> +    struct page **pages, unsigned int n_pages)
>>>>> +{
>>>>> +    unsigned int i;
>>>>> +    unsigned long pfn;
>>>>> +    struct vm_area_struct vma = {
>>>>> +        .vm_flags = VM_IO | VM_PFNMAP,
>>>>> +        .vm_mm = current->mm,
>>>>> +    };
>>>>> +
>>>>> +    for (i = 0; i<  n_pages; ++i, kaddr += PAGE_SIZE) {
>>>>
>>>> The follow_pfn() kerneldoc mentions that it looks up a PFN for a user
>>>> address. The only users I've found in the kernel sources pass a user
>>>> address. Is it legal to use it for kernel addresses ?
>>>
>>> It is not completely legal :). As I understand the mm code, the follow_pfn
>>> works only for IO/PFN mappings. This is the typical case (every case?) of
>>> mappings created by dma_alloc_coherent.
>>>
>>> In order to make this function work for a kernel pointer, one has to create
>>> an artificial VMA that has IO/PFN bits on.
>>>
>>> This solution is a hack-around for dma_get_pages (aka dma_get_sgtable). This
>>> way the dependency on dma_get_pages was broken giving a small hope of
>>> merging vb2 exporting.
>>>
>>> Marek prepared a patchset 'ARM: DMA-mapping: new extensions for buffer
>>> sharing' that adds dma buffers with no kernel mappings and dma_get_sgtable
>>> function.
>>>
>>> However this patchset is still in a RFC state.
>>
>> That's totally understood :-) I'm fine with keeping the hack for now until the
>> dma_get_sgtable() gets in a usable/mergeable state, please just mention it in
>> the code with something like
>>
>> /* HACK: This is a temporary workaround until the dma_get_sgtable() function
>> becomes available. */
>>
>>> I have prepared a patch that removes vb2_dc_kaddr_to_pages and substitutes
>>> it with dma_get_pages. It will become a part of vb2-exporter patches just
>>> after dma_get_sgtable is merged (or at least acked by major maintainers).
>>
> The above function call (because of follow_pfn) doesn't succeed for all the allocated pages. Hence I created a patch(attached)
> which is based on [1] series. One can apply it for using your present patch-set in the meantime.
> 
> Regards,
> Subash
> [1] http://www.spinics.net/lists/kernel/msg1343092.html

Re: [PATCH 04/12] v4l: vb2-dma-contig: add setup of sglist for MMAP buffers

[Laurent Pinchart]

Hi Tomasz,

On Friday 08 June 2012 16:31:31 Tomasz Stanislawski wrote:
> Hi Laurent and Subash,
> 
> I confirm the issue found by Subash. The function vb2_dc_kaddr_to_pages does
> fail for some occasions. The failures are rather strange like 'got 95 of
> 150 pages'. It took me some time to find the reason of the problem.
> 
> I found that dma_alloc_coherent for iommu an ARM does use ioremap_page_range
> to map a buffer to the kernel space. The mapping is done by updating the
> page-table.
> 
> The problem is that any process has a different first-level page-table. The
> ioremap_page_range updates only the table for init process. The PT present
> in current->mm shares a majority of entries of 1st-level PT at kernel range
> (above 0xc0000000) but *not all*. That is why vb2_dc_kaddr_to_pages worked
> for small buffers and occasionally failed for larger buffers.
> 
> I found two ways to fix this problem.
> a) use &init_mm instead of current->mm while creating an artificial vma
> b) access the dma memory by calling
>    *((volatile int *)kaddr) = 0;
>    before calling follow_pfn
>    This way a fault is generated and the PT is
>    updated by copying entries from init_mm.
> 
> What do you think about presented solutions?

Just to be sure, this is a hack until dma_get_sgtable is available, and it 
won't make it to mainline, right ?  In that case using init_mm seem easier.

-- 
Regards,

Laurent Pinchart

Re: [PATCH 04/12] v4l: vb2-dma-contig: add setup of sglist for MMAP buffers

[Subash Patel]

Hi Laurent, Tomasz,

On 06/10/2012 11:28 PM, Laurent Pinchart wrote:
> Hi Tomasz,
>
> On Friday 08 June 2012 16:31:31 Tomasz Stanislawski wrote:
>> Hi Laurent and Subash,
>>
>> I confirm the issue found by Subash. The function vb2_dc_kaddr_to_pages does
>> fail for some occasions. The failures are rather strange like 'got 95 of
>> 150 pages'. It took me some time to find the reason of the problem.
>>
>> I found that dma_alloc_coherent for iommu an ARM does use ioremap_page_range
>> to map a buffer to the kernel space. The mapping is done by updating the
>> page-table.
>>
>> The problem is that any process has a different first-level page-table. The
>> ioremap_page_range updates only the table for init process. The PT present
>> in current->mm shares a majority of entries of 1st-level PT at kernel range
>> (above 0xc0000000) but *not all*. That is why vb2_dc_kaddr_to_pages worked
>> for small buffers and occasionally failed for larger buffers.
>>
>> I found two ways to fix this problem.
>> a) use&init_mm instead of current->mm while creating an artificial vma
>> b) access the dma memory by calling
>>     *((volatile int *)kaddr) = 0;
>>     before calling follow_pfn
>>     This way a fault is generated and the PT is
>>     updated by copying entries from init_mm.
>>
>> What do you think about presented solutions?
>
> Just to be sure, this is a hack until dma_get_sgtable is available, and it
> won't make it to mainline, right ?  In that case using init_mm seem easier.
Although I agree adding a hack for timebeing, why not use the 
dma_get_sgtable() RFC itself to solve this in clean way? The hacks 
anyways cannot go into mainline when vb2 patches get merged.

Regards,
Subash
>