All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFCv2 PATCH 00/14] vb2: improve dma-sg, expbuf
@ 2014-09-12 12:59 Hans Verkuil
  2014-09-12 12:59 ` [RFCv2 PATCH 01/14] vb2: introduce buf_prepare/finish_for_cpu Hans Verkuil
                   ` (13 more replies)
  0 siblings, 14 replies; 23+ messages in thread
From: Hans Verkuil @ 2014-09-12 12:59 UTC (permalink / raw)
  To: linux-media; +Cc: pawel, m.szyprowski, laurent.pinchart

Changes since v1:

- Updated commit logs of patches 1 and 3.
- Added patches for tw68 and cx23885.

The patch series adds an allocation context to dma-sg and uses that to move
dma_(un)map_sg into the vb2 framework, which is where it belongs.

Related to that is the addition of buf_prepare/finish _for_cpu variants,
where the _for_cpu ops are called when the buffer is synced for the cpu, and
the others are called when it is synced to the device.

DMABUF export support is added to dma-sg and vmalloc, so now all memory
models support DMABUF importing and exporting.

A new flag was added so drivers know when the DMA engine should be
(re)programmed. This is primarily needed for the dma-sg memory model.

Reviews are very welcome.

There is one thing I am not happy about: the addition of the 'new_cookies'
flag. The idea is that if it is set, then the driver has to setup the
DMA engine descriptors in buf_prepare(). This avoids creating the DMA
descriptors in every buf_prepare and deleting them again for every buf_finish.
The problem is that when using the new flag the cleanup of those descriptors
can't be done in buf_finish anymore since when that op is called you do not
yet know if the descriptors need to be updated. Instead the cleanup has to
happen in buf_cleanup(). The tw68 patch is a good example of that.

Unfortunately, this sequence is asymmetrical.

I cannot think of a good alternative. The only slight improvement that might
be worth doing is that the vb2 core also sets new_cookies when buf_cleanup()
needs to cleanup the descriptors. There is a corner case where buf_cleanup
doesn't need to do that (e.g. if buf_prepare returned an error), and right
now drivers need to detect that. It's probably better to move that logic
to the vb2 core.

Regards,

	Hans


^ permalink raw reply	[flat|nested] 23+ messages in thread

* [RFCv2 PATCH 01/14] vb2: introduce buf_prepare/finish_for_cpu
  2014-09-12 12:59 [RFCv2 PATCH 00/14] vb2: improve dma-sg, expbuf Hans Verkuil
@ 2014-09-12 12:59 ` Hans Verkuil
  2014-09-12 21:25   ` Laurent Pinchart
  2014-09-12 12:59 ` [RFCv2 PATCH 02/14] vb2-dma-sg: add allocation context to dma-sg Hans Verkuil
                   ` (12 subsequent siblings)
  13 siblings, 1 reply; 23+ messages in thread
From: Hans Verkuil @ 2014-09-12 12:59 UTC (permalink / raw)
  To: linux-media; +Cc: pawel, m.szyprowski, laurent.pinchart, Hans Verkuil

From: Hans Verkuil <hans.verkuil@cisco.com>

This splits the buf_prepare and buf_finish actions into two: one
called while the cpu can still access the buffer contents, and one where
the memory has been prepared for DMA and the cpu no longer can access it.

Update a few drivers that use buf_finish where they really meant
buf_finish_for_cpu.

The reason for this split is that some drivers need to modify the buffer,
either before or after the DMA has taken place, in order to e.g. add JPEG
headers or do other touch ups.

You cannot do that in buf_prepare since at that time the buffer is already
synced for DMA and the CPU shouldn't touch it. So add these extra ops to
make this explicit.

Note that the dma-sg memory model doesn't sync the buffers yet in the memop
prepare. This will change in future patches.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 drivers/media/parport/bw-qcam.c              |  4 +--
 drivers/media/pci/sta2x11/sta2x11_vip.c      |  4 +--
 drivers/media/platform/vivid/vivid-vid-cap.c |  4 +--
 drivers/media/usb/go7007/go7007-v4l2.c       |  4 +--
 drivers/media/usb/pwc/pwc-if.c               |  4 +--
 drivers/media/usb/uvc/uvc_queue.c            |  4 +--
 drivers/media/v4l2-core/videobuf2-core.c     | 29 ++++++++++++-----
 include/media/videobuf2-core.h               | 48 ++++++++++++++++++++++------
 8 files changed, 72 insertions(+), 29 deletions(-)

diff --git a/drivers/media/parport/bw-qcam.c b/drivers/media/parport/bw-qcam.c
index 67b9da1..528558f 100644
--- a/drivers/media/parport/bw-qcam.c
+++ b/drivers/media/parport/bw-qcam.c
@@ -667,7 +667,7 @@ static void buffer_queue(struct vb2_buffer *vb)
 	vb2_buffer_done(vb, VB2_BUF_STATE_DONE);
 }
 
-static void buffer_finish(struct vb2_buffer *vb)
+static void buffer_finish_for_cpu(struct vb2_buffer *vb)
 {
 	struct qcam *qcam = vb2_get_drv_priv(vb->vb2_queue);
 	void *vbuf = vb2_plane_vaddr(vb, 0);
@@ -699,7 +699,7 @@ static void buffer_finish(struct vb2_buffer *vb)
 static struct vb2_ops qcam_video_qops = {
 	.queue_setup		= queue_setup,
 	.buf_queue		= buffer_queue,
-	.buf_finish		= buffer_finish,
+	.buf_finish_for_cpu	= buffer_finish_for_cpu,
 	.wait_prepare		= vb2_ops_wait_prepare,
 	.wait_finish		= vb2_ops_wait_finish,
 };
diff --git a/drivers/media/pci/sta2x11/sta2x11_vip.c b/drivers/media/pci/sta2x11/sta2x11_vip.c
index 365bd21..bfb05cb 100644
--- a/drivers/media/pci/sta2x11/sta2x11_vip.c
+++ b/drivers/media/pci/sta2x11/sta2x11_vip.c
@@ -327,7 +327,7 @@ static void buffer_queue(struct vb2_buffer *vb)
 	}
 	spin_unlock(&vip->lock);
 }
-static void buffer_finish(struct vb2_buffer *vb)
+static void buffer_finish_for_cpu(struct vb2_buffer *vb)
 {
 	struct sta2x11_vip *vip = vb2_get_drv_priv(vb->vb2_queue);
 	struct vip_buffer *vip_buf = to_vip_buffer(vb);
@@ -380,7 +380,7 @@ static struct vb2_ops vip_video_qops = {
 	.queue_setup		= queue_setup,
 	.buf_init		= buffer_init,
 	.buf_prepare		= buffer_prepare,
-	.buf_finish		= buffer_finish,
+	.buf_finish_for_cpu	= buffer_finish_for_cpu,
 	.buf_queue		= buffer_queue,
 	.start_streaming	= start_streaming,
 	.stop_streaming		= stop_streaming,
diff --git a/drivers/media/platform/vivid/vivid-vid-cap.c b/drivers/media/platform/vivid/vivid-vid-cap.c
index b016aed..d46f3e4 100644
--- a/drivers/media/platform/vivid/vivid-vid-cap.c
+++ b/drivers/media/platform/vivid/vivid-vid-cap.c
@@ -201,7 +201,7 @@ static int vid_cap_buf_prepare(struct vb2_buffer *vb)
 	return 0;
 }
 
-static void vid_cap_buf_finish(struct vb2_buffer *vb)
+static void vid_cap_buf_finish_for_cpu(struct vb2_buffer *vb)
 {
 	struct vivid_dev *dev = vb2_get_drv_priv(vb->vb2_queue);
 	struct v4l2_timecode *tc = &vb->v4l2_buf.timecode;
@@ -284,7 +284,7 @@ static void vid_cap_stop_streaming(struct vb2_queue *vq)
 const struct vb2_ops vivid_vid_cap_qops = {
 	.queue_setup		= vid_cap_queue_setup,
 	.buf_prepare		= vid_cap_buf_prepare,
-	.buf_finish		= vid_cap_buf_finish,
+	.buf_finish_for_cpu	= vid_cap_buf_finish_for_cpu,
 	.buf_queue		= vid_cap_buf_queue,
 	.start_streaming	= vid_cap_start_streaming,
 	.stop_streaming		= vid_cap_stop_streaming,
diff --git a/drivers/media/usb/go7007/go7007-v4l2.c b/drivers/media/usb/go7007/go7007-v4l2.c
index ec799b4..5bef286 100644
--- a/drivers/media/usb/go7007/go7007-v4l2.c
+++ b/drivers/media/usb/go7007/go7007-v4l2.c
@@ -404,7 +404,7 @@ static int go7007_buf_prepare(struct vb2_buffer *vb)
 	return 0;
 }
 
-static void go7007_buf_finish(struct vb2_buffer *vb)
+static void go7007_buf_finish_for_cpu(struct vb2_buffer *vb)
 {
 	struct vb2_queue *vq = vb->vb2_queue;
 	struct go7007 *go = vb2_get_drv_priv(vq);
@@ -478,7 +478,7 @@ static struct vb2_ops go7007_video_qops = {
 	.queue_setup    = go7007_queue_setup,
 	.buf_queue      = go7007_buf_queue,
 	.buf_prepare    = go7007_buf_prepare,
-	.buf_finish     = go7007_buf_finish,
+	.buf_finish_for_cpu = go7007_buf_finish_for_cpu,
 	.start_streaming = go7007_start_streaming,
 	.stop_streaming = go7007_stop_streaming,
 	.wait_prepare   = vb2_ops_wait_prepare,
diff --git a/drivers/media/usb/pwc/pwc-if.c b/drivers/media/usb/pwc/pwc-if.c
index 15b754d..879b455 100644
--- a/drivers/media/usb/pwc/pwc-if.c
+++ b/drivers/media/usb/pwc/pwc-if.c
@@ -614,7 +614,7 @@ static int buffer_prepare(struct vb2_buffer *vb)
 	return 0;
 }
 
-static void buffer_finish(struct vb2_buffer *vb)
+static void buffer_finish_for_cpu(struct vb2_buffer *vb)
 {
 	struct pwc_device *pdev = vb2_get_drv_priv(vb->vb2_queue);
 	struct pwc_frame_buf *buf = container_of(vb, struct pwc_frame_buf, vb);
@@ -700,7 +700,7 @@ static struct vb2_ops pwc_vb_queue_ops = {
 	.queue_setup		= queue_setup,
 	.buf_init		= buffer_init,
 	.buf_prepare		= buffer_prepare,
-	.buf_finish		= buffer_finish,
+	.buf_finish_for_cpu	= buffer_finish_for_cpu,
 	.buf_cleanup		= buffer_cleanup,
 	.buf_queue		= buffer_queue,
 	.start_streaming	= start_streaming,
diff --git a/drivers/media/usb/uvc/uvc_queue.c b/drivers/media/usb/uvc/uvc_queue.c
index 6e92d20..86a67cd 100644
--- a/drivers/media/usb/uvc/uvc_queue.c
+++ b/drivers/media/usb/uvc/uvc_queue.c
@@ -106,7 +106,7 @@ static void uvc_buffer_queue(struct vb2_buffer *vb)
 	spin_unlock_irqrestore(&queue->irqlock, flags);
 }
 
-static void uvc_buffer_finish(struct vb2_buffer *vb)
+static void uvc_buffer_finish_for_cpu(struct vb2_buffer *vb)
 {
 	struct uvc_video_queue *queue = vb2_get_drv_priv(vb->vb2_queue);
 	struct uvc_streaming *stream =
@@ -135,7 +135,7 @@ static struct vb2_ops uvc_queue_qops = {
 	.queue_setup = uvc_queue_setup,
 	.buf_prepare = uvc_buffer_prepare,
 	.buf_queue = uvc_buffer_queue,
-	.buf_finish = uvc_buffer_finish,
+	.buf_finish_for_cpu = uvc_buffer_finish_for_cpu,
 	.wait_prepare = uvc_wait_prepare,
 	.wait_finish = uvc_wait_finish,
 };
diff --git a/drivers/media/v4l2-core/videobuf2-core.c b/drivers/media/v4l2-core/videobuf2-core.c
index 7e6aff6..e5247a4 100644
--- a/drivers/media/v4l2-core/videobuf2-core.c
+++ b/drivers/media/v4l2-core/videobuf2-core.c
@@ -501,14 +501,17 @@ static int __vb2_queue_free(struct vb2_queue *q, unsigned int buffers)
 				  vb->cnt_mem_map_dmabuf != vb->cnt_mem_unmap_dmabuf ||
 				  vb->cnt_buf_queue != vb->cnt_buf_done ||
 				  vb->cnt_buf_prepare != vb->cnt_buf_finish ||
+				  vb->cnt_buf_prepare_for_cpu != vb->cnt_buf_finish_for_cpu ||
 				  vb->cnt_buf_init != vb->cnt_buf_cleanup;
 
 		if (unbalanced || debug) {
 			pr_info("vb2:   counters for queue %p, buffer %d:%s\n",
 				q, buffer, unbalanced ? " UNBALANCED!" : "");
-			pr_info("vb2:     buf_init: %u buf_cleanup: %u buf_prepare: %u buf_finish: %u\n",
-				vb->cnt_buf_init, vb->cnt_buf_cleanup,
-				vb->cnt_buf_prepare, vb->cnt_buf_finish);
+			pr_info("vb2:     buf_init: %u buf_cleanup: %u\n",
+				vb->cnt_buf_init, vb->cnt_buf_cleanup);
+			pr_info("vb2:     buf_prepare_for_cpu: %u buf_prepare: %u buf_finish: %u buf_finish_for_cpu: %u\n",
+				vb->cnt_buf_prepare_for_cpu, vb->cnt_buf_prepare,
+				vb->cnt_buf_finish, vb->cnt_buf_finish_for_cpu);
 			pr_info("vb2:     buf_queue: %u buf_done: %u\n",
 				vb->cnt_buf_queue, vb->cnt_buf_done);
 			pr_info("vb2:     alloc: %u put: %u prepare: %u finish: %u mmap: %u\n",
@@ -1192,6 +1195,8 @@ void vb2_buffer_done(struct vb2_buffer *vb, enum vb2_buffer_state state)
 	dprintk(4, "done processing on buffer %d, state: %d\n",
 			vb->v4l2_buf.index, state);
 
+	call_void_vb_qop(vb, buf_finish, vb);
+
 	/* sync buffers */
 	for (plane = 0; plane < vb->num_planes; ++plane)
 		call_void_memop(vb, finish, vb->planes[plane].mem_priv);
@@ -1622,6 +1627,12 @@ static int __buf_prepare(struct vb2_buffer *vb, const struct v4l2_buffer *b)
 	vb->v4l2_buf.timestamp.tv_usec = 0;
 	vb->v4l2_buf.sequence = 0;
 
+	ret = call_vb_qop(vb, buf_prepare_for_cpu, vb);
+	if (ret) {
+		dprintk(1, "buf_prepare_for_cpu failed\n");
+		return ret;
+	}
+
 	switch (q->memory) {
 	case V4L2_MEMORY_MMAP:
 		ret = __qbuf_mmap(vb, b);
@@ -1637,8 +1648,10 @@ static int __buf_prepare(struct vb2_buffer *vb, const struct v4l2_buffer *b)
 		ret = -EINVAL;
 	}
 
-	if (ret)
+	if (ret) {
 		dprintk(1, "buffer preparation failed: %d\n", ret);
+		call_void_vb_qop(vb, buf_finish_for_cpu, vb);
+	}
 	vb->state = ret ? VB2_BUF_STATE_DEQUEUED : VB2_BUF_STATE_PREPARED;
 
 	return ret;
@@ -2048,7 +2061,7 @@ static int vb2_internal_dqbuf(struct vb2_queue *q, struct v4l2_buffer *b, bool n
 		return -EINVAL;
 	}
 
-	call_void_vb_qop(vb, buf_finish, vb);
+	call_void_vb_qop(vb, buf_finish_for_cpu, vb);
 
 	/* Fill buffer information for the userspace */
 	__fill_v4l2_buffer(vb, b);
@@ -2076,7 +2089,7 @@ static int vb2_internal_dqbuf(struct vb2_queue *q, struct v4l2_buffer *b, bool n
  * Should be called from vidioc_dqbuf ioctl handler of a driver.
  * This function:
  * 1) verifies the passed buffer,
- * 2) calls buf_finish callback in the driver (if provided), in which
+ * 2) calls buf_finish_for_cpu callback in the driver (if provided), in which
  *    driver can perform any additional operations that may be required before
  *    returning the buffer to userspace, such as cache sync,
  * 3) the buffer struct members are filled with relevant information for
@@ -2139,7 +2152,7 @@ static void __vb2_queue_cancel(struct vb2_queue *q)
 
 	/*
 	 * Reinitialize all buffers for next use.
-	 * Make sure to call buf_finish for any queued buffers. Normally
+	 * Make sure to call buf_finish_for_cpu for any queued buffers. Normally
 	 * that's done in dqbuf, but that's not going to happen when we
 	 * cancel the whole queue. Note: this code belongs here, not in
 	 * __vb2_dqbuf() since in vb2_internal_dqbuf() there is a critical
@@ -2151,7 +2164,7 @@ static void __vb2_queue_cancel(struct vb2_queue *q)
 
 		if (vb->state != VB2_BUF_STATE_DEQUEUED) {
 			vb->state = VB2_BUF_STATE_PREPARED;
-			call_void_vb_qop(vb, buf_finish, vb);
+			call_void_vb_qop(vb, buf_finish_for_cpu, vb);
 		}
 		__vb2_dqbuf(vb);
 	}
diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h
index 5a10d8d..fff159c 100644
--- a/include/media/videobuf2-core.h
+++ b/include/media/videobuf2-core.h
@@ -227,8 +227,10 @@ struct vb2_buffer {
 	u32		cnt_mem_mmap;
 
 	u32		cnt_buf_init;
+	u32		cnt_buf_prepare_for_cpu;
 	u32		cnt_buf_prepare;
 	u32		cnt_buf_finish;
+	u32		cnt_buf_finish_for_cpu;
 	u32		cnt_buf_cleanup;
 	u32		cnt_buf_queue;
 
@@ -268,17 +270,43 @@ struct vb2_buffer {
  *			perform additional buffer-related initialization;
  *			initialization failure (return != 0) will prevent
  *			queue setup from completing successfully; optional.
- * @buf_prepare:	called every time the buffer is queued from userspace
+ * @buf_prepare_for_cpu:called every time the buffer is queued from userspace
  *			and from the VIDIOC_PREPARE_BUF ioctl; drivers may
- *			perform any initialization required before each hardware
- *			operation in this callback; drivers that support
- *			VIDIOC_CREATE_BUFS must also validate the buffer size;
- *			if an error is returned, the buffer will not be queued
- *			in driver; optional.
+ *			use this to access and modify the contents of the buffer
+ *			before it is prepared for DMA in the next step
+ *			(@buf_prepare). Drivers that support VIDIOC_CREATE_BUFS
+ *			must also validate the buffer size. If an error is
+ *			returned, the buffer will not be queued	in the driver;
+ *			optional.
+ * @buf_prepare:	called every time the buffer is queued from userspace
+ *			and from the VIDIOC_PREPARE_BUF ioctl; at this point
+ *			the buffer is prepared for DMA and the drivers may no
+ *			longer access the contents of the buffer. The driver
+ *			must perform any initialization required before each
+ *			hardware operation in this callback; drivers that
+ *			support	VIDIOC_CREATE_BUFS must also validate the
+ *			buffer size, if they haven't done that yet in
+ *			@buf_prepare_for_cpu. If an error is returned, the
+ *			buffer will not be queued in the driver; optional.
  * @buf_finish:		called before every dequeue of the buffer back to
- *			userspace; drivers may perform any operations required
- *			before userspace accesses the buffer; optional. The
- *			buffer state can be one of the following: DONE and
+ *			userspace; the contents of the buffer cannot be
+ *			accessed by the cpu at this stage as it is still setup
+ *			for DMA. Drivers may perform any operations required
+ *			before userspace accesses the buffer; optional.
+ *			The buffer state can be one of the following: DONE and
+ *			ERROR occur while streaming is in progress, and the
+ *			PREPARED state occurs when the queue has been canceled
+ *			and all pending buffers are being returned to their
+ *			default DEQUEUED state. Typically you only have to do
+ *			something if the state is VB2_BUF_STATE_DONE, since in
+ *			all other cases the buffer contents will be ignored
+ *			anyway.
+ * @buf_finish_for_cpu:	called before every dequeue of the buffer back to
+ *			userspace; at this stage the contents of the buffer is
+ *			accessible to the CPU. Drivers may perform any
+ *			operations required before userspace accesses the
+ *			buffer; optional.
+ *			The buffer state can be one of the following: DONE and
  *			ERROR occur while streaming is in progress, and the
  *			PREPARED state occurs when the queue has been canceled
  *			and all pending buffers are being returned to their
@@ -323,8 +351,10 @@ struct vb2_ops {
 	void (*wait_finish)(struct vb2_queue *q);
 
 	int (*buf_init)(struct vb2_buffer *vb);
+	int (*buf_prepare_for_cpu)(struct vb2_buffer *vb);
 	int (*buf_prepare)(struct vb2_buffer *vb);
 	void (*buf_finish)(struct vb2_buffer *vb);
+	void (*buf_finish_for_cpu)(struct vb2_buffer *vb);
 	void (*buf_cleanup)(struct vb2_buffer *vb);
 
 	int (*start_streaming)(struct vb2_queue *q, unsigned int count);
-- 
2.1.0


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [RFCv2 PATCH 02/14] vb2-dma-sg: add allocation context to dma-sg
  2014-09-12 12:59 [RFCv2 PATCH 00/14] vb2: improve dma-sg, expbuf Hans Verkuil
  2014-09-12 12:59 ` [RFCv2 PATCH 01/14] vb2: introduce buf_prepare/finish_for_cpu Hans Verkuil
@ 2014-09-12 12:59 ` Hans Verkuil
  2014-09-12 21:49   ` Laurent Pinchart
  2014-09-14  3:29   ` Pawel Osciak
  2014-09-12 12:59 ` [RFCv2 PATCH 03/14] vb2-dma-sg: add prepare/finish memops Hans Verkuil
                   ` (11 subsequent siblings)
  13 siblings, 2 replies; 23+ messages in thread
From: Hans Verkuil @ 2014-09-12 12:59 UTC (permalink / raw)
  To: linux-media; +Cc: pawel, m.szyprowski, laurent.pinchart, Hans Verkuil

From: Hans Verkuil <hans.verkuil@cisco.com>

Require that dma-sg also uses an allocation context. This is in preparation
for adding prepare/finish memops to sync the memory between DMA and CPU.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 drivers/media/pci/cx23885/cx23885-417.c         |  1 +
 drivers/media/pci/cx23885/cx23885-core.c        | 10 +++++-
 drivers/media/pci/cx23885/cx23885-dvb.c         |  1 +
 drivers/media/pci/cx23885/cx23885-vbi.c         |  1 +
 drivers/media/pci/cx23885/cx23885-video.c       |  1 +
 drivers/media/pci/cx23885/cx23885.h             |  1 +
 drivers/media/pci/saa7134/saa7134-core.c        | 18 +++++++---
 drivers/media/pci/saa7134/saa7134-ts.c          |  1 +
 drivers/media/pci/saa7134/saa7134-vbi.c         |  1 +
 drivers/media/pci/saa7134/saa7134-video.c       |  1 +
 drivers/media/pci/saa7134/saa7134.h             |  1 +
 drivers/media/pci/solo6x10/solo6x10-v4l2-enc.c  | 10 ++++++
 drivers/media/pci/solo6x10/solo6x10.h           |  1 +
 drivers/media/pci/tw68/tw68-core.c              | 15 +++++++--
 drivers/media/pci/tw68/tw68-video.c             |  1 +
 drivers/media/pci/tw68/tw68.h                   |  1 +
 drivers/media/platform/marvell-ccic/mcam-core.c |  7 ++++
 drivers/media/platform/marvell-ccic/mcam-core.h |  1 +
 drivers/media/v4l2-core/videobuf2-core.c        |  3 +-
 drivers/media/v4l2-core/videobuf2-dma-contig.c  |  4 ++-
 drivers/media/v4l2-core/videobuf2-dma-sg.c      | 44 +++++++++++++++++++++++--
 drivers/media/v4l2-core/videobuf2-vmalloc.c     |  3 +-
 include/media/videobuf2-core.h                  |  3 +-
 include/media/videobuf2-dma-sg.h                |  3 ++
 24 files changed, 118 insertions(+), 15 deletions(-)

diff --git a/drivers/media/pci/cx23885/cx23885-417.c b/drivers/media/pci/cx23885/cx23885-417.c
index 6973055..b7e143a 100644
--- a/drivers/media/pci/cx23885/cx23885-417.c
+++ b/drivers/media/pci/cx23885/cx23885-417.c
@@ -1148,6 +1148,7 @@ static int queue_setup(struct vb2_queue *q, const struct v4l2_format *fmt,
 	dev->ts1.ts_packet_count = mpeglines;
 	*num_planes = 1;
 	sizes[0] = mpeglinesize * mpeglines;
+	alloc_ctxs[0] = dev->alloc_ctx;
 	*num_buffers = mpegbufs;
 	return 0;
 }
diff --git a/drivers/media/pci/cx23885/cx23885-core.c b/drivers/media/pci/cx23885/cx23885-core.c
index cb94366b..8a36fcd 100644
--- a/drivers/media/pci/cx23885/cx23885-core.c
+++ b/drivers/media/pci/cx23885/cx23885-core.c
@@ -1997,9 +1997,14 @@ static int cx23885_initdev(struct pci_dev *pci_dev,
 	if (!pci_dma_supported(pci_dev, 0xffffffff)) {
 		printk("%s/0: Oops: no 32bit PCI DMA ???\n", dev->name);
 		err = -EIO;
-		goto fail_irq;
+		goto fail_context;
 	}
 
+	dev->alloc_ctx = vb2_dma_sg_init_ctx(&pci_dev->dev);
+	if (IS_ERR(dev->alloc_ctx)) {
+		err = -ENOMEM;
+		goto fail_context;
+	}
 	err = request_irq(pci_dev->irq, cx23885_irq,
 			  IRQF_SHARED, dev->name, dev);
 	if (err < 0) {
@@ -2028,6 +2033,8 @@ static int cx23885_initdev(struct pci_dev *pci_dev,
 	return 0;
 
 fail_irq:
+	vb2_dma_sg_cleanup_ctx(dev->alloc_ctx);
+fail_context:
 	cx23885_dev_unregister(dev);
 fail_ctrl:
 	v4l2_ctrl_handler_free(hdl);
@@ -2053,6 +2060,7 @@ static void cx23885_finidev(struct pci_dev *pci_dev)
 	free_irq(pci_dev->irq, dev);
 
 	cx23885_dev_unregister(dev);
+	vb2_dma_sg_cleanup_ctx(dev->alloc_ctx);
 	v4l2_ctrl_handler_free(&dev->ctrl_handler);
 	v4l2_device_unregister(v4l2_dev);
 	kfree(dev);
diff --git a/drivers/media/pci/cx23885/cx23885-dvb.c b/drivers/media/pci/cx23885/cx23885-dvb.c
index 332e6fa..80e2356 100644
--- a/drivers/media/pci/cx23885/cx23885-dvb.c
+++ b/drivers/media/pci/cx23885/cx23885-dvb.c
@@ -97,6 +97,7 @@ static int queue_setup(struct vb2_queue *q, const struct v4l2_format *fmt,
 	port->ts_packet_count = 32;
 	*num_planes = 1;
 	sizes[0] = port->ts_packet_size * port->ts_packet_count;
+	alloc_ctxs[0] = port->dev->alloc_ctx;
 	*num_buffers = 32;
 	return 0;
 }
diff --git a/drivers/media/pci/cx23885/cx23885-vbi.c b/drivers/media/pci/cx23885/cx23885-vbi.c
index 67b71f9..a0e5b3f 100644
--- a/drivers/media/pci/cx23885/cx23885-vbi.c
+++ b/drivers/media/pci/cx23885/cx23885-vbi.c
@@ -132,6 +132,7 @@ static int queue_setup(struct vb2_queue *q, const struct v4l2_format *fmt,
 		lines = VBI_NTSC_LINE_COUNT;
 	*num_planes = 1;
 	sizes[0] = lines * VBI_LINE_LENGTH * 2;
+	alloc_ctxs[0] = dev->alloc_ctx;
 	return 0;
 }
 
diff --git a/drivers/media/pci/cx23885/cx23885-video.c b/drivers/media/pci/cx23885/cx23885-video.c
index f0ea904..be67836 100644
--- a/drivers/media/pci/cx23885/cx23885-video.c
+++ b/drivers/media/pci/cx23885/cx23885-video.c
@@ -323,6 +323,7 @@ static int queue_setup(struct vb2_queue *q, const struct v4l2_format *fmt,
 
 	*num_planes = 1;
 	sizes[0] = (dev->fmt->depth * dev->width * dev->height) >> 3;
+	alloc_ctxs[0] = dev->alloc_ctx;
 	return 0;
 }
 
diff --git a/drivers/media/pci/cx23885/cx23885.h b/drivers/media/pci/cx23885/cx23885.h
index 0e4f406..352896e 100644
--- a/drivers/media/pci/cx23885/cx23885.h
+++ b/drivers/media/pci/cx23885/cx23885.h
@@ -412,6 +412,7 @@ struct cx23885_dev {
 	struct vb2_queue           vb2_vidq;
 	struct cx23885_dmaqueue    vbiq;
 	struct vb2_queue           vb2_vbiq;
+	void			   *alloc_ctx;
 
 	spinlock_t                 slock;
 
diff --git a/drivers/media/pci/saa7134/saa7134-core.c b/drivers/media/pci/saa7134/saa7134-core.c
index 9ff03a6..b63529a 100644
--- a/drivers/media/pci/saa7134/saa7134-core.c
+++ b/drivers/media/pci/saa7134/saa7134-core.c
@@ -995,13 +995,18 @@ static int saa7134_initdev(struct pci_dev *pci_dev,
 	saa7134_board_init1(dev);
 	saa7134_hwinit1(dev);
 
+	dev->alloc_ctx = vb2_dma_sg_init_ctx(&pci_dev->dev);
+	if (IS_ERR(dev->alloc_ctx)) {
+		err = -ENOMEM;
+		goto fail3;
+	}
 	/* get irq */
 	err = request_irq(pci_dev->irq, saa7134_irq,
 			  IRQF_SHARED, dev->name, dev);
 	if (err < 0) {
 		printk(KERN_ERR "%s: can't get IRQ %d\n",
 		       dev->name,pci_dev->irq);
-		goto fail3;
+		goto fail4;
 	}
 
 	/* wait a bit, register i2c bus */
@@ -1059,7 +1064,7 @@ static int saa7134_initdev(struct pci_dev *pci_dev,
 	if (err < 0) {
 		printk(KERN_INFO "%s: can't register video device\n",
 		       dev->name);
-		goto fail4;
+		goto fail5;
 	}
 	printk(KERN_INFO "%s: registered device %s [v4l2]\n",
 	       dev->name, video_device_node_name(dev->video_dev));
@@ -1072,7 +1077,7 @@ static int saa7134_initdev(struct pci_dev *pci_dev,
 	err = video_register_device(dev->vbi_dev,VFL_TYPE_VBI,
 				    vbi_nr[dev->nr]);
 	if (err < 0)
-		goto fail4;
+		goto fail5;
 	printk(KERN_INFO "%s: registered device %s\n",
 	       dev->name, video_device_node_name(dev->vbi_dev));
 
@@ -1083,7 +1088,7 @@ static int saa7134_initdev(struct pci_dev *pci_dev,
 		err = video_register_device(dev->radio_dev,VFL_TYPE_RADIO,
 					    radio_nr[dev->nr]);
 		if (err < 0)
-			goto fail4;
+			goto fail5;
 		printk(KERN_INFO "%s: registered device %s\n",
 		       dev->name, video_device_node_name(dev->radio_dev));
 	}
@@ -1097,10 +1102,12 @@ static int saa7134_initdev(struct pci_dev *pci_dev,
 	request_submodules(dev);
 	return 0;
 
- fail4:
+ fail5:
 	saa7134_unregister_video(dev);
 	saa7134_i2c_unregister(dev);
 	free_irq(pci_dev->irq, dev);
+ fail4:
+	vb2_dma_sg_cleanup_ctx(dev->alloc_ctx);
  fail3:
 	saa7134_hwfini(dev);
 	iounmap(dev->lmmio);
@@ -1167,6 +1174,7 @@ static void saa7134_finidev(struct pci_dev *pci_dev)
 
 	/* release resources */
 	free_irq(pci_dev->irq, dev);
+	vb2_dma_sg_cleanup_ctx(dev->alloc_ctx);
 	iounmap(dev->lmmio);
 	release_mem_region(pci_resource_start(pci_dev,0),
 			   pci_resource_len(pci_dev,0));
diff --git a/drivers/media/pci/saa7134/saa7134-ts.c b/drivers/media/pci/saa7134/saa7134-ts.c
index bd25323..8eff4a7 100644
--- a/drivers/media/pci/saa7134/saa7134-ts.c
+++ b/drivers/media/pci/saa7134/saa7134-ts.c
@@ -142,6 +142,7 @@ int saa7134_ts_queue_setup(struct vb2_queue *q, const struct v4l2_format *fmt,
 		*nbuffers = 3;
 	*nplanes = 1;
 	sizes[0] = size;
+	alloc_ctxs[0] = dev->alloc_ctx;
 	return 0;
 }
 EXPORT_SYMBOL_GPL(saa7134_ts_queue_setup);
diff --git a/drivers/media/pci/saa7134/saa7134-vbi.c b/drivers/media/pci/saa7134/saa7134-vbi.c
index c06dbe1..803e7df 100644
--- a/drivers/media/pci/saa7134/saa7134-vbi.c
+++ b/drivers/media/pci/saa7134/saa7134-vbi.c
@@ -156,6 +156,7 @@ static int queue_setup(struct vb2_queue *q, const struct v4l2_format *fmt,
 	*nbuffers = saa7134_buffer_count(size, *nbuffers);
 	*nplanes = 1;
 	sizes[0] = size;
+	alloc_ctxs[0] = dev->alloc_ctx;
 	return 0;
 }
 
diff --git a/drivers/media/pci/saa7134/saa7134-video.c b/drivers/media/pci/saa7134/saa7134-video.c
index 0cfa2ca..5e3c48d 100644
--- a/drivers/media/pci/saa7134/saa7134-video.c
+++ b/drivers/media/pci/saa7134/saa7134-video.c
@@ -932,6 +932,7 @@ static int queue_setup(struct vb2_queue *q, const struct v4l2_format *fmt,
 	*nbuffers = saa7134_buffer_count(size, *nbuffers);
 	*nplanes = 1;
 	sizes[0] = size;
+	alloc_ctxs[0] = dev->alloc_ctx;
 	return 0;
 }
 
diff --git a/drivers/media/pci/saa7134/saa7134.h b/drivers/media/pci/saa7134/saa7134.h
index e47edd4..1c21c47 100644
--- a/drivers/media/pci/saa7134/saa7134.h
+++ b/drivers/media/pci/saa7134/saa7134.h
@@ -583,6 +583,7 @@ struct saa7134_dev {
 
 
 	/* video+ts+vbi capture */
+	void			   *alloc_ctx;
 	struct saa7134_dmaqueue    video_q;
 	struct vb2_queue           video_vbq;
 	struct saa7134_dmaqueue    vbi_q;
diff --git a/drivers/media/pci/solo6x10/solo6x10-v4l2-enc.c b/drivers/media/pci/solo6x10/solo6x10-v4l2-enc.c
index 28023f9..0517fc9 100644
--- a/drivers/media/pci/solo6x10/solo6x10-v4l2-enc.c
+++ b/drivers/media/pci/solo6x10/solo6x10-v4l2-enc.c
@@ -720,7 +720,10 @@ static int solo_enc_queue_setup(struct vb2_queue *q,
 				unsigned int *num_planes, unsigned int sizes[],
 				void *alloc_ctxs[])
 {
+	struct solo_enc_dev *solo_enc = vb2_get_drv_priv(q);
+
 	sizes[0] = FRAME_BUF_SIZE;
+	alloc_ctxs[0] = solo_enc->alloc_ctx;
 	*num_planes = 1;
 
 	if (*num_buffers < MIN_VID_BUFFERS)
@@ -1263,6 +1266,11 @@ static struct solo_enc_dev *solo_enc_alloc(struct solo_dev *solo_dev,
 		return ERR_PTR(-ENOMEM);
 
 	hdl = &solo_enc->hdl;
+	solo_enc->alloc_ctx = vb2_dma_sg_init_ctx(&solo_dev->pdev->dev);
+	if (IS_ERR(solo_enc->alloc_ctx)) {
+		ret = -ENOMEM;
+		goto hdl_free;
+	}
 	v4l2_ctrl_handler_init(hdl, 10);
 	v4l2_ctrl_new_std(hdl, &solo_ctrl_ops,
 			V4L2_CID_BRIGHTNESS, 0, 255, 1, 128);
@@ -1366,6 +1374,7 @@ pci_free:
 			solo_enc->desc_items, solo_enc->desc_dma);
 hdl_free:
 	v4l2_ctrl_handler_free(hdl);
+	vb2_dma_sg_cleanup_ctx(solo_enc->alloc_ctx);
 	kfree(solo_enc);
 	return ERR_PTR(ret);
 }
@@ -1377,6 +1386,7 @@ static void solo_enc_free(struct solo_enc_dev *solo_enc)
 
 	video_unregister_device(solo_enc->vfd);
 	v4l2_ctrl_handler_free(&solo_enc->hdl);
+	vb2_dma_sg_cleanup_ctx(solo_enc->alloc_ctx);
 	kfree(solo_enc);
 }
 
diff --git a/drivers/media/pci/solo6x10/solo6x10.h b/drivers/media/pci/solo6x10/solo6x10.h
index 72017b7..bd8edfa 100644
--- a/drivers/media/pci/solo6x10/solo6x10.h
+++ b/drivers/media/pci/solo6x10/solo6x10.h
@@ -180,6 +180,7 @@ struct solo_enc_dev {
 	u32			sequence;
 	struct vb2_queue	vidq;
 	struct list_head	vidq_active;
+	void			*alloc_ctx;
 	int			desc_count;
 	int			desc_nelts;
 	struct solo_p2m_desc	*desc_items;
diff --git a/drivers/media/pci/tw68/tw68-core.c b/drivers/media/pci/tw68/tw68-core.c
index a6fb48c..c505fe0 100644
--- a/drivers/media/pci/tw68/tw68-core.c
+++ b/drivers/media/pci/tw68/tw68-core.c
@@ -304,13 +304,19 @@ static int tw68_initdev(struct pci_dev *pci_dev,
 	/* Then do any initialisation wanted before interrupts are on */
 	tw68_hw_init1(dev);
 
+	dev->alloc_ctx = vb2_dma_sg_init_ctx(&pci_dev->dev);
+	if (IS_ERR(dev->alloc_ctx)) {
+		err = -ENOMEM;
+		goto fail3;
+	}
+
 	/* get irq */
 	err = devm_request_irq(&pci_dev->dev, pci_dev->irq, tw68_irq,
 			  IRQF_SHARED | IRQF_DISABLED, dev->name, dev);
 	if (err < 0) {
 		pr_err("%s: can't get IRQ %d\n",
 		       dev->name, pci_dev->irq);
-		goto fail3;
+		goto fail4;
 	}
 
 	/*
@@ -324,7 +330,7 @@ static int tw68_initdev(struct pci_dev *pci_dev,
 	if (err < 0) {
 		pr_err("%s: can't register video device\n",
 		       dev->name);
-		goto fail4;
+		goto fail5;
 	}
 	tw_setl(TW68_INTMASK, dev->pci_irqmask);
 
@@ -333,8 +339,10 @@ static int tw68_initdev(struct pci_dev *pci_dev,
 
 	return 0;
 
-fail4:
+fail5:
 	video_unregister_device(&dev->vdev);
+fail4:
+	vb2_dma_sg_cleanup_ctx(dev->alloc_ctx);
 fail3:
 	iounmap(dev->lmmio);
 fail2:
@@ -358,6 +366,7 @@ static void tw68_finidev(struct pci_dev *pci_dev)
 	/* unregister */
 	video_unregister_device(&dev->vdev);
 	v4l2_ctrl_handler_free(&dev->hdl);
+	vb2_dma_sg_cleanup_ctx(dev->alloc_ctx);
 
 	/* release resources */
 	iounmap(dev->lmmio);
diff --git a/drivers/media/pci/tw68/tw68-video.c b/drivers/media/pci/tw68/tw68-video.c
index 5c94ac7..50dcce6 100644
--- a/drivers/media/pci/tw68/tw68-video.c
+++ b/drivers/media/pci/tw68/tw68-video.c
@@ -384,6 +384,7 @@ static int tw68_queue_setup(struct vb2_queue *q, const struct v4l2_format *fmt,
 	unsigned tot_bufs = q->num_buffers + *num_buffers;
 
 	sizes[0] = (dev->fmt->depth * dev->width * dev->height) >> 3;
+	alloc_ctxs[0] = dev->alloc_ctx;
 	/*
 	 * We allow create_bufs, but only if the sizeimage is the same as the
 	 * current sizeimage. The tw68_buffer_count calculation becomes quite
diff --git a/drivers/media/pci/tw68/tw68.h b/drivers/media/pci/tw68/tw68.h
index 2c8abe2..7a7501b 100644
--- a/drivers/media/pci/tw68/tw68.h
+++ b/drivers/media/pci/tw68/tw68.h
@@ -181,6 +181,7 @@ struct tw68_dev {
 	unsigned		field;
 	struct vb2_queue	vidq;
 	struct list_head	active;
+	void			*alloc_ctx;
 
 	/* various v4l controls */
 	const struct tw68_tvnorm *tvnorm;	/* video */
diff --git a/drivers/media/platform/marvell-ccic/mcam-core.c b/drivers/media/platform/marvell-ccic/mcam-core.c
index 7a86c77..20d53b6 100644
--- a/drivers/media/platform/marvell-ccic/mcam-core.c
+++ b/drivers/media/platform/marvell-ccic/mcam-core.c
@@ -1080,6 +1080,8 @@ static int mcam_vb_queue_setup(struct vb2_queue *vq,
 		*nbufs = minbufs;
 	if (cam->buffer_mode == B_DMA_contig)
 		alloc_ctxs[0] = cam->vb_alloc_ctx;
+	else if (cam->buffer_mode == B_DMA_sg)
+		alloc_ctxs[0] = cam->vb_alloc_ctx_sg;
 	return 0;
 }
 
@@ -1298,6 +1300,7 @@ static int mcam_setup_vb2(struct mcam_camera *cam)
 		vq->ops = &mcam_vb2_sg_ops;
 		vq->mem_ops = &vb2_dma_sg_memops;
 		vq->buf_struct_size = sizeof(struct mcam_vb_buffer);
+		cam->vb_alloc_ctx_sg = vb2_dma_sg_init_ctx(cam->dev);
 		vq->io_modes = VB2_MMAP | VB2_USERPTR;
 		cam->dma_setup = mcam_ctlr_dma_sg;
 		cam->frame_complete = mcam_dma_sg_done;
@@ -1326,6 +1329,10 @@ static void mcam_cleanup_vb2(struct mcam_camera *cam)
 	if (cam->buffer_mode == B_DMA_contig)
 		vb2_dma_contig_cleanup_ctx(cam->vb_alloc_ctx);
 #endif
+#ifdef MCAM_MODE_DMA_SG
+	if (cam->buffer_mode == B_DMA_sg)
+		vb2_dma_sg_cleanup_ctx(cam->vb_alloc_ctx_sg);
+#endif
 }
 
 
diff --git a/drivers/media/platform/marvell-ccic/mcam-core.h b/drivers/media/platform/marvell-ccic/mcam-core.h
index e0e628c..7b8c201 100644
--- a/drivers/media/platform/marvell-ccic/mcam-core.h
+++ b/drivers/media/platform/marvell-ccic/mcam-core.h
@@ -176,6 +176,7 @@ struct mcam_camera {
 	/* DMA buffers - DMA modes */
 	struct mcam_vb_buffer *vb_bufs[MAX_DMA_BUFS];
 	struct vb2_alloc_ctx *vb_alloc_ctx;
+	struct vb2_alloc_ctx *vb_alloc_ctx_sg;
 
 	/* Mode-specific ops, set at open time */
 	void (*dma_setup)(struct mcam_camera *cam);
diff --git a/drivers/media/v4l2-core/videobuf2-core.c b/drivers/media/v4l2-core/videobuf2-core.c
index e5247a4..087cd62 100644
--- a/drivers/media/v4l2-core/videobuf2-core.c
+++ b/drivers/media/v4l2-core/videobuf2-core.c
@@ -189,6 +189,7 @@ static void __vb2_queue_cancel(struct vb2_queue *q);
 static int __vb2_buf_mem_alloc(struct vb2_buffer *vb)
 {
 	struct vb2_queue *q = vb->vb2_queue;
+	int write = !V4L2_TYPE_IS_OUTPUT(q->type);
 	void *mem_priv;
 	int plane;
 
@@ -200,7 +201,7 @@ static int __vb2_buf_mem_alloc(struct vb2_buffer *vb)
 		unsigned long size = PAGE_ALIGN(q->plane_sizes[plane]);
 
 		mem_priv = call_ptr_memop(vb, alloc, q->alloc_ctx[plane],
-				      size, q->gfp_flags);
+				      size, write, q->gfp_flags);
 		if (IS_ERR_OR_NULL(mem_priv))
 			goto free;
 
diff --git a/drivers/media/v4l2-core/videobuf2-dma-contig.c b/drivers/media/v4l2-core/videobuf2-dma-contig.c
index 4a02ade..6675f12 100644
--- a/drivers/media/v4l2-core/videobuf2-dma-contig.c
+++ b/drivers/media/v4l2-core/videobuf2-dma-contig.c
@@ -155,7 +155,8 @@ static void vb2_dc_put(void *buf_priv)
 	kfree(buf);
 }
 
-static void *vb2_dc_alloc(void *alloc_ctx, unsigned long size, gfp_t gfp_flags)
+static void *vb2_dc_alloc(void *alloc_ctx, unsigned long size, int write,
+			  gfp_t gfp_flags)
 {
 	struct vb2_dc_conf *conf = alloc_ctx;
 	struct device *dev = conf->dev;
@@ -176,6 +177,7 @@ static void *vb2_dc_alloc(void *alloc_ctx, unsigned long size, gfp_t gfp_flags)
 	/* Prevent the device from being released while the buffer is used */
 	buf->dev = get_device(dev);
 	buf->size = size;
+	buf->dma_dir = write ? DMA_FROM_DEVICE : DMA_TO_DEVICE;
 
 	buf->handler.refcount = &buf->refcount;
 	buf->handler.put = vb2_dc_put;
diff --git a/drivers/media/v4l2-core/videobuf2-dma-sg.c b/drivers/media/v4l2-core/videobuf2-dma-sg.c
index adefc31..9b7a041 100644
--- a/drivers/media/v4l2-core/videobuf2-dma-sg.c
+++ b/drivers/media/v4l2-core/videobuf2-dma-sg.c
@@ -30,11 +30,17 @@ module_param(debug, int, 0644);
 			printk(KERN_DEBUG "vb2-dma-sg: " fmt, ## arg);	\
 	} while (0)
 
+struct vb2_dma_sg_conf {
+	struct device		*dev;
+};
+
 struct vb2_dma_sg_buf {
+	struct device			*dev;
 	void				*vaddr;
 	struct page			**pages;
 	int				write;
 	int				offset;
+	enum dma_data_direction		dma_dir;
 	struct sg_table			sg_table;
 	size_t				size;
 	unsigned int			num_pages;
@@ -86,22 +92,27 @@ static int vb2_dma_sg_alloc_compacted(struct vb2_dma_sg_buf *buf,
 	return 0;
 }
 
-static void *vb2_dma_sg_alloc(void *alloc_ctx, unsigned long size, gfp_t gfp_flags)
+static void *vb2_dma_sg_alloc(void *alloc_ctx, unsigned long size, int write,
+			      gfp_t gfp_flags)
 {
+	struct vb2_dma_sg_conf *conf = alloc_ctx;
 	struct vb2_dma_sg_buf *buf;
 	int ret;
 	int num_pages;
 
+	if (WARN_ON(alloc_ctx == NULL))
+		return NULL;
 	buf = kzalloc(sizeof *buf, GFP_KERNEL);
 	if (!buf)
 		return NULL;
 
 	buf->vaddr = NULL;
-	buf->write = 0;
+	buf->write = write;
 	buf->offset = 0;
 	buf->size = size;
 	/* size is already page aligned */
 	buf->num_pages = size >> PAGE_SHIFT;
+	buf->dma_dir = write ? DMA_FROM_DEVICE : DMA_TO_DEVICE;
 
 	buf->pages = kzalloc(buf->num_pages * sizeof(struct page *),
 			     GFP_KERNEL);
@@ -117,6 +128,8 @@ static void *vb2_dma_sg_alloc(void *alloc_ctx, unsigned long size, gfp_t gfp_fla
 	if (ret)
 		goto fail_table_alloc;
 
+	/* Prevent the device from being released while the buffer is used */
+	buf->dev = get_device(conf->dev);
 	buf->handler.refcount = &buf->refcount;
 	buf->handler.put = vb2_dma_sg_put;
 	buf->handler.arg = buf;
@@ -152,6 +165,7 @@ static void vb2_dma_sg_put(void *buf_priv)
 		while (--i >= 0)
 			__free_page(buf->pages[i]);
 		kfree(buf->pages);
+		put_device(buf->dev);
 		kfree(buf);
 	}
 }
@@ -164,6 +178,7 @@ static inline int vma_is_io(struct vm_area_struct *vma)
 static void *vb2_dma_sg_get_userptr(void *alloc_ctx, unsigned long vaddr,
 				    unsigned long size, int write)
 {
+	struct vb2_dma_sg_conf *conf = alloc_ctx;
 	struct vb2_dma_sg_buf *buf;
 	unsigned long first, last;
 	int num_pages_from_user;
@@ -177,6 +192,7 @@ static void *vb2_dma_sg_get_userptr(void *alloc_ctx, unsigned long vaddr,
 	buf->write = write;
 	buf->offset = vaddr & ~PAGE_MASK;
 	buf->size = size;
+	buf->dma_dir = write ? DMA_FROM_DEVICE : DMA_TO_DEVICE;
 
 	first = (vaddr           & PAGE_MASK) >> PAGE_SHIFT;
 	last  = ((vaddr + size - 1) & PAGE_MASK) >> PAGE_SHIFT;
@@ -233,6 +249,8 @@ static void *vb2_dma_sg_get_userptr(void *alloc_ctx, unsigned long vaddr,
 			buf->num_pages, buf->offset, size, 0))
 		goto userptr_fail_alloc_table_from_pages;
 
+	/* Prevent the device from being released while the buffer is used */
+	buf->dev = get_device(conf->dev);
 	return buf;
 
 userptr_fail_alloc_table_from_pages:
@@ -272,6 +290,7 @@ static void vb2_dma_sg_put_userptr(void *buf_priv)
 	}
 	kfree(buf->pages);
 	vb2_put_vma(buf->vma);
+	put_device(buf->dev);
 	kfree(buf);
 }
 
@@ -354,6 +373,27 @@ const struct vb2_mem_ops vb2_dma_sg_memops = {
 };
 EXPORT_SYMBOL_GPL(vb2_dma_sg_memops);
 
+void *vb2_dma_sg_init_ctx(struct device *dev)
+{
+	struct vb2_dma_sg_conf *conf;
+
+	conf = kzalloc(sizeof(*conf), GFP_KERNEL);
+	if (!conf)
+		return ERR_PTR(-ENOMEM);
+
+	conf->dev = dev;
+
+	return conf;
+}
+EXPORT_SYMBOL_GPL(vb2_dma_sg_init_ctx);
+
+void vb2_dma_sg_cleanup_ctx(void *alloc_ctx)
+{
+	if (!IS_ERR_OR_NULL(alloc_ctx))
+		kfree(alloc_ctx);
+}
+EXPORT_SYMBOL_GPL(vb2_dma_sg_cleanup_ctx);
+
 MODULE_DESCRIPTION("dma scatter/gather memory handling routines for videobuf2");
 MODULE_AUTHOR("Andrzej Pietrasiewicz");
 MODULE_LICENSE("GPL");
diff --git a/drivers/media/v4l2-core/videobuf2-vmalloc.c b/drivers/media/v4l2-core/videobuf2-vmalloc.c
index 313d977..d77e397 100644
--- a/drivers/media/v4l2-core/videobuf2-vmalloc.c
+++ b/drivers/media/v4l2-core/videobuf2-vmalloc.c
@@ -35,7 +35,8 @@ struct vb2_vmalloc_buf {
 
 static void vb2_vmalloc_put(void *buf_priv);
 
-static void *vb2_vmalloc_alloc(void *alloc_ctx, unsigned long size, gfp_t gfp_flags)
+static void *vb2_vmalloc_alloc(void *alloc_ctx, unsigned long size, int write,
+			       gfp_t gfp_flags)
 {
 	struct vb2_vmalloc_buf *buf;
 
diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h
index fff159c..02b96ef 100644
--- a/include/media/videobuf2-core.h
+++ b/include/media/videobuf2-core.h
@@ -82,7 +82,8 @@ struct vb2_threadio_data;
  *				  unmap_dmabuf.
  */
 struct vb2_mem_ops {
-	void		*(*alloc)(void *alloc_ctx, unsigned long size, gfp_t gfp_flags);
+	void		*(*alloc)(void *alloc_ctx, unsigned long size, int write,
+				  gfp_t gfp_flags);
 	void		(*put)(void *buf_priv);
 	struct dma_buf *(*get_dmabuf)(void *buf_priv, unsigned long flags);
 
diff --git a/include/media/videobuf2-dma-sg.h b/include/media/videobuf2-dma-sg.h
index 7b89852..14ce306 100644
--- a/include/media/videobuf2-dma-sg.h
+++ b/include/media/videobuf2-dma-sg.h
@@ -21,6 +21,9 @@ static inline struct sg_table *vb2_dma_sg_plane_desc(
 	return (struct sg_table *)vb2_plane_cookie(vb, plane_no);
 }
 
+void *vb2_dma_sg_init_ctx(struct device *dev);
+void vb2_dma_sg_cleanup_ctx(void *alloc_ctx);
+
 extern const struct vb2_mem_ops vb2_dma_sg_memops;
 
 #endif
-- 
2.1.0


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [RFCv2 PATCH 03/14] vb2-dma-sg: add prepare/finish memops
  2014-09-12 12:59 [RFCv2 PATCH 00/14] vb2: improve dma-sg, expbuf Hans Verkuil
  2014-09-12 12:59 ` [RFCv2 PATCH 01/14] vb2: introduce buf_prepare/finish_for_cpu Hans Verkuil
  2014-09-12 12:59 ` [RFCv2 PATCH 02/14] vb2-dma-sg: add allocation context to dma-sg Hans Verkuil
@ 2014-09-12 12:59 ` Hans Verkuil
  2014-09-12 21:54   ` Laurent Pinchart
  2014-09-14  3:40   ` Pawel Osciak
  2014-09-12 12:59 ` [RFCv2 PATCH 04/14] vb2: memop prepare: return errors Hans Verkuil
                   ` (10 subsequent siblings)
  13 siblings, 2 replies; 23+ messages in thread
From: Hans Verkuil @ 2014-09-12 12:59 UTC (permalink / raw)
  To: linux-media; +Cc: pawel, m.szyprowski, laurent.pinchart, Hans Verkuil

From: Hans Verkuil <hans.verkuil@cisco.com>

This moves dma_(un)map_sg to the prepare/finish memops of videobuf2-dma-sg.c.

Now that vb2-dma-sg will sync the buffers for you in the prepare/finish
memops we can drop that from the drivers that use dma-sg.

For the solo6x10 driver that was a bit more involved because it needs to
copy JPEG or MPEG headers to the buffer before returning it to userspace,
and that cannot be done in the old place since the buffer there is still
setup for DMA access, not for CPU access. However, the buf_finish_for_cpu
op is the ideal place to do this. By the time buf_finish_for_cpu is called
the buffer is available for CPU access, so copying to the buffer is fine.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 drivers/media/pci/cx23885/cx23885-417.c         |  7 +---
 drivers/media/pci/cx23885/cx23885-core.c        |  5 ---
 drivers/media/pci/cx23885/cx23885-dvb.c         |  7 +---
 drivers/media/pci/cx23885/cx23885-vbi.c         | 13 +------
 drivers/media/pci/cx23885/cx23885-video.c       | 13 +------
 drivers/media/pci/saa7134/saa7134-empress.c     |  1 -
 drivers/media/pci/saa7134/saa7134-ts.c          | 16 --------
 drivers/media/pci/saa7134/saa7134-vbi.c         | 15 --------
 drivers/media/pci/saa7134/saa7134-video.c       | 15 --------
 drivers/media/pci/saa7134/saa7134.h             |  1 -
 drivers/media/pci/solo6x10/solo6x10-v4l2-enc.c  | 50 +++++++++++--------------
 drivers/media/pci/tw68/tw68-video.c             | 12 +-----
 drivers/media/platform/marvell-ccic/mcam-core.c | 18 +--------
 drivers/media/v4l2-core/videobuf2-dma-sg.c      | 18 +++++++++
 14 files changed, 51 insertions(+), 140 deletions(-)

diff --git a/drivers/media/pci/cx23885/cx23885-417.c b/drivers/media/pci/cx23885/cx23885-417.c
index b7e143a..5d36950 100644
--- a/drivers/media/pci/cx23885/cx23885-417.c
+++ b/drivers/media/pci/cx23885/cx23885-417.c
@@ -1162,16 +1162,13 @@ static int buffer_prepare(struct vb2_buffer *vb)
 	return cx23885_buf_prepare(buf, &dev->ts1);
 }
 
-static void buffer_finish(struct vb2_buffer *vb)
+static void buffer_finish_for_cpu(struct vb2_buffer *vb)
 {
 	struct cx23885_dev *dev = vb->vb2_queue->drv_priv;
 	struct cx23885_buffer *buf = container_of(vb,
 		struct cx23885_buffer, vb);
-	struct sg_table *sgt = vb2_dma_sg_plane_desc(vb, 0);
 
 	cx23885_free_buffer(dev, buf);
-
-	dma_unmap_sg(&dev->pci->dev, sgt->sgl, sgt->nents, DMA_FROM_DEVICE);
 }
 
 static void buffer_queue(struct vb2_buffer *vb)
@@ -1227,7 +1224,7 @@ static void cx23885_stop_streaming(struct vb2_queue *q)
 static struct vb2_ops cx23885_qops = {
 	.queue_setup    = queue_setup,
 	.buf_prepare  = buffer_prepare,
-	.buf_finish = buffer_finish,
+	.buf_finish_for_cpu = buffer_finish_for_cpu,
 	.buf_queue    = buffer_queue,
 	.wait_prepare = vb2_ops_wait_prepare,
 	.wait_finish = vb2_ops_wait_finish,
diff --git a/drivers/media/pci/cx23885/cx23885-core.c b/drivers/media/pci/cx23885/cx23885-core.c
index 8a36fcd..e2e5afb 100644
--- a/drivers/media/pci/cx23885/cx23885-core.c
+++ b/drivers/media/pci/cx23885/cx23885-core.c
@@ -1453,17 +1453,12 @@ int cx23885_buf_prepare(struct cx23885_buffer *buf, struct cx23885_tsport *port)
 	struct cx23885_dev *dev = port->dev;
 	int size = port->ts_packet_size * port->ts_packet_count;
 	struct sg_table *sgt = vb2_dma_sg_plane_desc(&buf->vb, 0);
-	int rc;
 
 	dprintk(1, "%s: %p\n", __func__, buf);
 	if (vb2_plane_size(&buf->vb, 0) < size)
 		return -EINVAL;
 	vb2_set_plane_payload(&buf->vb, 0, size);
 
-	rc = dma_map_sg(&dev->pci->dev, sgt->sgl, sgt->nents, DMA_FROM_DEVICE);
-	if (!rc)
-		return -EIO;
-
 	cx23885_risc_databuffer(dev->pci, &buf->risc,
 				sgt->sgl,
 				port->ts_packet_size, port->ts_packet_count, 0);
diff --git a/drivers/media/pci/cx23885/cx23885-dvb.c b/drivers/media/pci/cx23885/cx23885-dvb.c
index 80e2356..77eb034 100644
--- a/drivers/media/pci/cx23885/cx23885-dvb.c
+++ b/drivers/media/pci/cx23885/cx23885-dvb.c
@@ -112,17 +112,14 @@ static int buffer_prepare(struct vb2_buffer *vb)
 	return cx23885_buf_prepare(buf, port);
 }
 
-static void buffer_finish(struct vb2_buffer *vb)
+static void buffer_finish_for_cpu(struct vb2_buffer *vb)
 {
 	struct cx23885_tsport *port = vb->vb2_queue->drv_priv;
 	struct cx23885_dev *dev = port->dev;
 	struct cx23885_buffer *buf = container_of(vb,
 		struct cx23885_buffer, vb);
-	struct sg_table *sgt = vb2_dma_sg_plane_desc(vb, 0);
 
 	cx23885_free_buffer(dev, buf);
-
-	dma_unmap_sg(&dev->pci->dev, sgt->sgl, sgt->nents, DMA_FROM_DEVICE);
 }
 
 static void buffer_queue(struct vb2_buffer *vb)
@@ -171,7 +168,7 @@ static void cx23885_stop_streaming(struct vb2_queue *q)
 static struct vb2_ops dvb_qops = {
 	.queue_setup    = queue_setup,
 	.buf_prepare  = buffer_prepare,
-	.buf_finish = buffer_finish,
+	.buf_finish_for_cpu = buffer_finish_for_cpu,
 	.buf_queue    = buffer_queue,
 	.wait_prepare = vb2_ops_wait_prepare,
 	.wait_finish = vb2_ops_wait_finish,
diff --git a/drivers/media/pci/cx23885/cx23885-vbi.c b/drivers/media/pci/cx23885/cx23885-vbi.c
index a0e5b3f..d7a69c6 100644
--- a/drivers/media/pci/cx23885/cx23885-vbi.c
+++ b/drivers/media/pci/cx23885/cx23885-vbi.c
@@ -143,7 +143,6 @@ static int buffer_prepare(struct vb2_buffer *vb)
 		struct cx23885_buffer, vb);
 	struct sg_table *sgt = vb2_dma_sg_plane_desc(vb, 0);
 	unsigned lines = VBI_PAL_LINE_COUNT;
-	int ret;
 
 	if (dev->tvnorm & V4L2_STD_525_60)
 		lines = VBI_NTSC_LINE_COUNT;
@@ -152,10 +151,6 @@ static int buffer_prepare(struct vb2_buffer *vb)
 		return -EINVAL;
 	vb2_set_plane_payload(vb, 0, lines * VBI_LINE_LENGTH * 2);
 
-	ret = dma_map_sg(&dev->pci->dev, sgt->sgl, sgt->nents, DMA_FROM_DEVICE);
-	if (!ret)
-		return -EIO;
-
 	cx23885_risc_vbibuffer(dev->pci, &buf->risc,
 			 sgt->sgl,
 			 0, VBI_LINE_LENGTH * lines,
@@ -164,16 +159,12 @@ static int buffer_prepare(struct vb2_buffer *vb)
 	return 0;
 }
 
-static void buffer_finish(struct vb2_buffer *vb)
+static void buffer_finish_for_cpu(struct vb2_buffer *vb)
 {
-	struct cx23885_dev *dev = vb->vb2_queue->drv_priv;
 	struct cx23885_buffer *buf = container_of(vb,
 		struct cx23885_buffer, vb);
-	struct sg_table *sgt = vb2_dma_sg_plane_desc(vb, 0);
 
 	cx23885_free_buffer(vb->vb2_queue->drv_priv, buf);
-
-	dma_unmap_sg(&dev->pci->dev, sgt->sgl, sgt->nents, DMA_FROM_DEVICE);
 }
 
 /*
@@ -263,7 +254,7 @@ static void cx23885_stop_streaming(struct vb2_queue *q)
 struct vb2_ops cx23885_vbi_qops = {
 	.queue_setup    = queue_setup,
 	.buf_prepare  = buffer_prepare,
-	.buf_finish = buffer_finish,
+	.buf_finish_for_cpu = buffer_finish_for_cpu,
 	.buf_queue    = buffer_queue,
 	.wait_prepare = vb2_ops_wait_prepare,
 	.wait_finish = vb2_ops_wait_finish,
diff --git a/drivers/media/pci/cx23885/cx23885-video.c b/drivers/media/pci/cx23885/cx23885-video.c
index be67836..d533f4e 100644
--- a/drivers/media/pci/cx23885/cx23885-video.c
+++ b/drivers/media/pci/cx23885/cx23885-video.c
@@ -335,7 +335,6 @@ static int buffer_prepare(struct vb2_buffer *vb)
 	u32 line0_offset, line1_offset;
 	struct sg_table *sgt = vb2_dma_sg_plane_desc(vb, 0);
 	int field_tff;
-	int ret;
 
 	buf->bpl = (dev->width * dev->fmt->depth) >> 3;
 
@@ -343,10 +342,6 @@ static int buffer_prepare(struct vb2_buffer *vb)
 		return -EINVAL;
 	vb2_set_plane_payload(vb, 0, dev->height * buf->bpl);
 
-	ret = dma_map_sg(&dev->pci->dev, sgt->sgl, sgt->nents, DMA_FROM_DEVICE);
-	if (!ret)
-		return -EIO;
-
 	switch (dev->field) {
 	case V4L2_FIELD_TOP:
 		cx23885_risc_buffer(dev->pci, &buf->risc,
@@ -412,16 +407,12 @@ static int buffer_prepare(struct vb2_buffer *vb)
 	return 0;
 }
 
-static void buffer_finish(struct vb2_buffer *vb)
+static void buffer_finish_for_cpu(struct vb2_buffer *vb)
 {
-	struct cx23885_dev *dev = vb->vb2_queue->drv_priv;
 	struct cx23885_buffer *buf = container_of(vb,
 		struct cx23885_buffer, vb);
-	struct sg_table *sgt = vb2_dma_sg_plane_desc(vb, 0);
 
 	cx23885_free_buffer(vb->vb2_queue->drv_priv, buf);
-
-	dma_unmap_sg(&dev->pci->dev, sgt->sgl, sgt->nents, DMA_FROM_DEVICE);
 }
 
 /*
@@ -509,7 +500,7 @@ static void cx23885_stop_streaming(struct vb2_queue *q)
 static struct vb2_ops cx23885_video_qops = {
 	.queue_setup    = queue_setup,
 	.buf_prepare  = buffer_prepare,
-	.buf_finish = buffer_finish,
+	.buf_finish_for_cpu = buffer_finish_for_cpu,
 	.buf_queue    = buffer_queue,
 	.wait_prepare = vb2_ops_wait_prepare,
 	.wait_finish = vb2_ops_wait_finish,
diff --git a/drivers/media/pci/saa7134/saa7134-empress.c b/drivers/media/pci/saa7134/saa7134-empress.c
index e4ea85f..b1cfd0e 100644
--- a/drivers/media/pci/saa7134/saa7134-empress.c
+++ b/drivers/media/pci/saa7134/saa7134-empress.c
@@ -96,7 +96,6 @@ static struct vb2_ops saa7134_empress_qops = {
 	.queue_setup	= saa7134_ts_queue_setup,
 	.buf_init	= saa7134_ts_buffer_init,
 	.buf_prepare	= saa7134_ts_buffer_prepare,
-	.buf_finish	= saa7134_ts_buffer_finish,
 	.buf_queue	= saa7134_vb2_buffer_queue,
 	.wait_prepare	= vb2_ops_wait_prepare,
 	.wait_finish	= vb2_ops_wait_finish,
diff --git a/drivers/media/pci/saa7134/saa7134-ts.c b/drivers/media/pci/saa7134/saa7134-ts.c
index 8eff4a7..2709b83 100644
--- a/drivers/media/pci/saa7134/saa7134-ts.c
+++ b/drivers/media/pci/saa7134/saa7134-ts.c
@@ -94,7 +94,6 @@ int saa7134_ts_buffer_prepare(struct vb2_buffer *vb2)
 	struct saa7134_buf *buf = container_of(vb2, struct saa7134_buf, vb2);
 	struct sg_table *dma = vb2_dma_sg_plane_desc(vb2, 0);
 	unsigned int lines, llength, size;
-	int ret;
 
 	dprintk("buffer_prepare [%p]\n", buf);
 
@@ -108,25 +107,11 @@ int saa7134_ts_buffer_prepare(struct vb2_buffer *vb2)
 	vb2_set_plane_payload(vb2, 0, size);
 	vb2->v4l2_buf.field = dev->field;
 
-	ret = dma_map_sg(&dev->pci->dev, dma->sgl, dma->nents, DMA_FROM_DEVICE);
-	if (!ret)
-		return -EIO;
 	return saa7134_pgtable_build(dev->pci, &dmaq->pt, dma->sgl, dma->nents,
 				    saa7134_buffer_startpage(buf));
 }
 EXPORT_SYMBOL_GPL(saa7134_ts_buffer_prepare);
 
-void saa7134_ts_buffer_finish(struct vb2_buffer *vb2)
-{
-	struct saa7134_dmaqueue *dmaq = vb2->vb2_queue->drv_priv;
-	struct saa7134_dev *dev = dmaq->dev;
-	struct saa7134_buf *buf = container_of(vb2, struct saa7134_buf, vb2);
-	struct sg_table *dma = vb2_dma_sg_plane_desc(&buf->vb2, 0);
-
-	dma_unmap_sg(&dev->pci->dev, dma->sgl, dma->nents, DMA_FROM_DEVICE);
-}
-EXPORT_SYMBOL_GPL(saa7134_ts_buffer_finish);
-
 int saa7134_ts_queue_setup(struct vb2_queue *q, const struct v4l2_format *fmt,
 			   unsigned int *nbuffers, unsigned int *nplanes,
 			   unsigned int sizes[], void *alloc_ctxs[])
@@ -188,7 +173,6 @@ struct vb2_ops saa7134_ts_qops = {
 	.queue_setup	= saa7134_ts_queue_setup,
 	.buf_init	= saa7134_ts_buffer_init,
 	.buf_prepare	= saa7134_ts_buffer_prepare,
-	.buf_finish	= saa7134_ts_buffer_finish,
 	.buf_queue	= saa7134_vb2_buffer_queue,
 	.wait_prepare	= vb2_ops_wait_prepare,
 	.wait_finish	= vb2_ops_wait_finish,
diff --git a/drivers/media/pci/saa7134/saa7134-vbi.c b/drivers/media/pci/saa7134/saa7134-vbi.c
index 803e7df..e188903 100644
--- a/drivers/media/pci/saa7134/saa7134-vbi.c
+++ b/drivers/media/pci/saa7134/saa7134-vbi.c
@@ -120,7 +120,6 @@ static int buffer_prepare(struct vb2_buffer *vb2)
 	struct saa7134_buf *buf = container_of(vb2, struct saa7134_buf, vb2);
 	struct sg_table *dma = vb2_dma_sg_plane_desc(&buf->vb2, 0);
 	unsigned int size;
-	int ret;
 
 	if (dma->sgl->offset) {
 		pr_err("The buffer is not page-aligned\n");
@@ -132,9 +131,6 @@ static int buffer_prepare(struct vb2_buffer *vb2)
 
 	vb2_set_plane_payload(vb2, 0, size);
 
-	ret = dma_map_sg(&dev->pci->dev, dma->sgl, dma->nents, DMA_FROM_DEVICE);
-	if (!ret)
-		return -EIO;
 	return saa7134_pgtable_build(dev->pci, &dmaq->pt, dma->sgl, dma->nents,
 				    saa7134_buffer_startpage(buf));
 }
@@ -170,21 +166,10 @@ static int buffer_init(struct vb2_buffer *vb2)
 	return 0;
 }
 
-static void buffer_finish(struct vb2_buffer *vb2)
-{
-	struct saa7134_dmaqueue *dmaq = vb2->vb2_queue->drv_priv;
-	struct saa7134_dev *dev = dmaq->dev;
-	struct saa7134_buf *buf = container_of(vb2, struct saa7134_buf, vb2);
-	struct sg_table *dma = vb2_dma_sg_plane_desc(&buf->vb2, 0);
-
-	dma_unmap_sg(&dev->pci->dev, dma->sgl, dma->nents, DMA_FROM_DEVICE);
-}
-
 struct vb2_ops saa7134_vbi_qops = {
 	.queue_setup	= queue_setup,
 	.buf_init	= buffer_init,
 	.buf_prepare	= buffer_prepare,
-	.buf_finish	= buffer_finish,
 	.buf_queue	= saa7134_vb2_buffer_queue,
 	.wait_prepare	= vb2_ops_wait_prepare,
 	.wait_finish	= vb2_ops_wait_finish,
diff --git a/drivers/media/pci/saa7134/saa7134-video.c b/drivers/media/pci/saa7134/saa7134-video.c
index 5e3c48d..15a686b 100644
--- a/drivers/media/pci/saa7134/saa7134-video.c
+++ b/drivers/media/pci/saa7134/saa7134-video.c
@@ -883,7 +883,6 @@ static int buffer_prepare(struct vb2_buffer *vb2)
 	struct saa7134_buf *buf = container_of(vb2, struct saa7134_buf, vb2);
 	struct sg_table *dma = vb2_dma_sg_plane_desc(&buf->vb2, 0);
 	unsigned int size;
-	int ret;
 
 	if (dma->sgl->offset) {
 		pr_err("The buffer is not page-aligned\n");
@@ -896,23 +895,10 @@ static int buffer_prepare(struct vb2_buffer *vb2)
 	vb2_set_plane_payload(vb2, 0, size);
 	vb2->v4l2_buf.field = dev->field;
 
-	ret = dma_map_sg(&dev->pci->dev, dma->sgl, dma->nents, DMA_FROM_DEVICE);
-	if (!ret)
-		return -EIO;
 	return saa7134_pgtable_build(dev->pci, &dmaq->pt, dma->sgl, dma->nents,
 				    saa7134_buffer_startpage(buf));
 }
 
-static void buffer_finish(struct vb2_buffer *vb2)
-{
-	struct saa7134_dmaqueue *dmaq = vb2->vb2_queue->drv_priv;
-	struct saa7134_dev *dev = dmaq->dev;
-	struct saa7134_buf *buf = container_of(vb2, struct saa7134_buf, vb2);
-	struct sg_table *dma = vb2_dma_sg_plane_desc(&buf->vb2, 0);
-
-	dma_unmap_sg(&dev->pci->dev, dma->sgl, dma->nents, DMA_FROM_DEVICE);
-}
-
 static int queue_setup(struct vb2_queue *q, const struct v4l2_format *fmt,
 			   unsigned int *nbuffers, unsigned int *nplanes,
 			   unsigned int sizes[], void *alloc_ctxs[])
@@ -1005,7 +991,6 @@ static struct vb2_ops vb2_qops = {
 	.queue_setup	= queue_setup,
 	.buf_init	= buffer_init,
 	.buf_prepare	= buffer_prepare,
-	.buf_finish	= buffer_finish,
 	.buf_queue	= saa7134_vb2_buffer_queue,
 	.wait_prepare	= vb2_ops_wait_prepare,
 	.wait_finish	= vb2_ops_wait_finish,
diff --git a/drivers/media/pci/saa7134/saa7134.h b/drivers/media/pci/saa7134/saa7134.h
index 1c21c47..0395e35 100644
--- a/drivers/media/pci/saa7134/saa7134.h
+++ b/drivers/media/pci/saa7134/saa7134.h
@@ -810,7 +810,6 @@ void saa7134_video_fini(struct saa7134_dev *dev);
 
 int saa7134_ts_buffer_init(struct vb2_buffer *vb2);
 int saa7134_ts_buffer_prepare(struct vb2_buffer *vb2);
-void saa7134_ts_buffer_finish(struct vb2_buffer *vb2);
 int saa7134_ts_queue_setup(struct vb2_queue *q, const struct v4l2_format *fmt,
 			   unsigned int *nbuffers, unsigned int *nplanes,
 			   unsigned int sizes[], void *alloc_ctxs[]);
diff --git a/drivers/media/pci/solo6x10/solo6x10-v4l2-enc.c b/drivers/media/pci/solo6x10/solo6x10-v4l2-enc.c
index 0517fc9..e2d819e 100644
--- a/drivers/media/pci/solo6x10/solo6x10-v4l2-enc.c
+++ b/drivers/media/pci/solo6x10/solo6x10-v4l2-enc.c
@@ -463,7 +463,6 @@ static int solo_fill_jpeg(struct solo_enc_dev *solo_enc,
 	struct solo_dev *solo_dev = solo_enc->solo_dev;
 	struct sg_table *vbuf = vb2_dma_sg_plane_desc(vb, 0);
 	int frame_size;
-	int ret;
 
 	vb->v4l2_buf.flags |= V4L2_BUF_FLAG_KEYFRAME;
 
@@ -473,22 +472,10 @@ static int solo_fill_jpeg(struct solo_enc_dev *solo_enc,
 	frame_size = ALIGN(vop_jpeg_size(vh) + solo_enc->jpeg_len, DMA_ALIGN);
 	vb2_set_plane_payload(vb, 0, vop_jpeg_size(vh) + solo_enc->jpeg_len);
 
-	/* may discard all previous data in vbuf->sgl */
-	if (!dma_map_sg(&solo_dev->pdev->dev, vbuf->sgl, vbuf->nents,
-			DMA_FROM_DEVICE))
-		return -ENOMEM;
-	ret = solo_send_desc(solo_enc, solo_enc->jpeg_len, vbuf,
+	return solo_send_desc(solo_enc, solo_enc->jpeg_len, vbuf,
 			     vop_jpeg_offset(vh) - SOLO_JPEG_EXT_ADDR(solo_dev),
 			     frame_size, SOLO_JPEG_EXT_ADDR(solo_dev),
 			     SOLO_JPEG_EXT_SIZE(solo_dev));
-	dma_unmap_sg(&solo_dev->pdev->dev, vbuf->sgl, vbuf->nents,
-			DMA_FROM_DEVICE);
-
-	/* add the header only after dma_unmap_sg() */
-	sg_copy_from_buffer(vbuf->sgl, vbuf->nents,
-			    solo_enc->jpeg_header, solo_enc->jpeg_len);
-
-	return ret;
 }
 
 static int solo_fill_mpeg(struct solo_enc_dev *solo_enc,
@@ -498,7 +485,6 @@ static int solo_fill_mpeg(struct solo_enc_dev *solo_enc,
 	struct sg_table *vbuf = vb2_dma_sg_plane_desc(vb, 0);
 	int frame_off, frame_size;
 	int skip = 0;
-	int ret;
 
 	if (vb2_plane_size(vb, 0) < vop_mpeg_size(vh))
 		return -EIO;
@@ -521,21 +507,9 @@ static int solo_fill_mpeg(struct solo_enc_dev *solo_enc,
 		sizeof(*vh)) % SOLO_MP4E_EXT_SIZE(solo_dev);
 	frame_size = ALIGN(vop_mpeg_size(vh) + skip, DMA_ALIGN);
 
-	/* may discard all previous data in vbuf->sgl */
-	if (!dma_map_sg(&solo_dev->pdev->dev, vbuf->sgl, vbuf->nents,
-			DMA_FROM_DEVICE))
-		return -ENOMEM;
-	ret = solo_send_desc(solo_enc, skip, vbuf, frame_off, frame_size,
+	return solo_send_desc(solo_enc, skip, vbuf, frame_off, frame_size,
 			SOLO_MP4E_EXT_ADDR(solo_dev),
 			SOLO_MP4E_EXT_SIZE(solo_dev));
-	dma_unmap_sg(&solo_dev->pdev->dev, vbuf->sgl, vbuf->nents,
-			DMA_FROM_DEVICE);
-
-	/* add the header only after dma_unmap_sg() */
-	if (!vop_type(vh))
-		sg_copy_from_buffer(vbuf->sgl, vbuf->nents,
-				    solo_enc->vop, solo_enc->vop_len);
-	return ret;
 }
 
 static int solo_enc_fillbuf(struct solo_enc_dev *solo_enc,
@@ -790,9 +764,29 @@ static void solo_enc_stop_streaming(struct vb2_queue *q)
 	solo_ring_stop(solo_enc->solo_dev);
 }
 
+static void solo_enc_buf_finish_for_cpu(struct vb2_buffer *vb)
+{
+	struct solo_enc_dev *solo_enc = vb2_get_drv_priv(vb->vb2_queue);
+	struct sg_table *vbuf = vb2_dma_sg_plane_desc(vb, 0);
+
+	switch (solo_enc->fmt) {
+	case V4L2_PIX_FMT_MPEG4:
+	case V4L2_PIX_FMT_H264:
+		if (vb->v4l2_buf.flags & V4L2_BUF_FLAG_KEYFRAME)
+			sg_copy_from_buffer(vbuf->sgl, vbuf->nents,
+					solo_enc->vop, solo_enc->vop_len);
+		break;
+	default: /* V4L2_PIX_FMT_MJPEG */
+		sg_copy_from_buffer(vbuf->sgl, vbuf->nents,
+				solo_enc->jpeg_header, solo_enc->jpeg_len);
+		break;
+	}
+}
+
 static struct vb2_ops solo_enc_video_qops = {
 	.queue_setup	= solo_enc_queue_setup,
 	.buf_queue	= solo_enc_buf_queue,
+	.buf_finish_for_cpu = solo_enc_buf_finish_for_cpu,
 	.start_streaming = solo_enc_start_streaming,
 	.stop_streaming = solo_enc_stop_streaming,
 	.wait_prepare	= vb2_ops_wait_prepare,
diff --git a/drivers/media/pci/tw68/tw68-video.c b/drivers/media/pci/tw68/tw68-video.c
index 50dcce6..13e9650 100644
--- a/drivers/media/pci/tw68/tw68-video.c
+++ b/drivers/media/pci/tw68/tw68-video.c
@@ -462,17 +462,12 @@ static int tw68_buf_prepare(struct vb2_buffer *vb)
 	struct tw68_buf *buf = container_of(vb, struct tw68_buf, vb);
 	struct sg_table *dma = vb2_dma_sg_plane_desc(vb, 0);
 	unsigned size, bpl;
-	int rc;
 
 	size = (dev->width * dev->height * dev->fmt->depth) >> 3;
 	if (vb2_plane_size(vb, 0) < size)
 		return -EINVAL;
 	vb2_set_plane_payload(vb, 0, size);
 
-	rc = dma_map_sg(&dev->pci->dev, dma->sgl, dma->nents, DMA_FROM_DEVICE);
-	if (!rc)
-		return -EIO;
-
 	bpl = (dev->width * dev->fmt->depth) >> 3;
 	switch (dev->field) {
 	case V4L2_FIELD_TOP:
@@ -502,15 +497,12 @@ static int tw68_buf_prepare(struct vb2_buffer *vb)
 	return 0;
 }
 
-static void tw68_buf_finish(struct vb2_buffer *vb)
+static void tw68_buf_finish_for_cpu(struct vb2_buffer *vb)
 {
 	struct vb2_queue *vq = vb->vb2_queue;
 	struct tw68_dev *dev = vb2_get_drv_priv(vq);
-	struct sg_table *dma = vb2_dma_sg_plane_desc(vb, 0);
 	struct tw68_buf *buf = container_of(vb, struct tw68_buf, vb);
 
-	dma_unmap_sg(&dev->pci->dev, dma->sgl, dma->nents, DMA_FROM_DEVICE);
-
 	pci_free_consistent(dev->pci, buf->size, buf->cpu, buf->dma);
 }
 
@@ -544,7 +536,7 @@ static struct vb2_ops tw68_video_qops = {
 	.queue_setup	= tw68_queue_setup,
 	.buf_queue	= tw68_buf_queue,
 	.buf_prepare	= tw68_buf_prepare,
-	.buf_finish	= tw68_buf_finish,
+	.buf_finish_for_cpu = tw68_buf_finish_for_cpu,
 	.start_streaming = tw68_start_streaming,
 	.stop_streaming = tw68_stop_streaming,
 	.wait_prepare	= vb2_ops_wait_prepare,
diff --git a/drivers/media/platform/marvell-ccic/mcam-core.c b/drivers/media/platform/marvell-ccic/mcam-core.c
index 20d53b6..d48c3d4 100644
--- a/drivers/media/platform/marvell-ccic/mcam-core.c
+++ b/drivers/media/platform/marvell-ccic/mcam-core.c
@@ -1221,17 +1221,12 @@ static int mcam_vb_sg_buf_init(struct vb2_buffer *vb)
 static int mcam_vb_sg_buf_prepare(struct vb2_buffer *vb)
 {
 	struct mcam_vb_buffer *mvb = vb_to_mvb(vb);
-	struct mcam_camera *cam = vb2_get_drv_priv(vb->vb2_queue);
 	struct sg_table *sg_table = vb2_dma_sg_plane_desc(vb, 0);
 	struct mcam_dma_desc *desc = mvb->dma_desc;
 	struct scatterlist *sg;
 	int i;
 
-	mvb->dma_desc_nent = dma_map_sg(cam->dev, sg_table->sgl,
-			sg_table->nents, DMA_FROM_DEVICE);
-	if (mvb->dma_desc_nent <= 0)
-		return -EIO;  /* Not sure what's right here */
-	for_each_sg(sg_table->sgl, sg, mvb->dma_desc_nent, i) {
+	for_each_sg(sg_table->sgl, sg, sg_table->nents, i) {
 		desc->dma_addr = sg_dma_address(sg);
 		desc->segment_len = sg_dma_len(sg);
 		desc++;
@@ -1239,16 +1234,6 @@ static int mcam_vb_sg_buf_prepare(struct vb2_buffer *vb)
 	return 0;
 }
 
-static void mcam_vb_sg_buf_finish(struct vb2_buffer *vb)
-{
-	struct mcam_camera *cam = vb2_get_drv_priv(vb->vb2_queue);
-	struct sg_table *sg_table = vb2_dma_sg_plane_desc(vb, 0);
-
-	if (sg_table)
-		dma_unmap_sg(cam->dev, sg_table->sgl,
-				sg_table->nents, DMA_FROM_DEVICE);
-}
-
 static void mcam_vb_sg_buf_cleanup(struct vb2_buffer *vb)
 {
 	struct mcam_camera *cam = vb2_get_drv_priv(vb->vb2_queue);
@@ -1265,7 +1250,6 @@ static const struct vb2_ops mcam_vb2_sg_ops = {
 	.buf_init		= mcam_vb_sg_buf_init,
 	.buf_prepare		= mcam_vb_sg_buf_prepare,
 	.buf_queue		= mcam_vb_buf_queue,
-	.buf_finish		= mcam_vb_sg_buf_finish,
 	.buf_cleanup		= mcam_vb_sg_buf_cleanup,
 	.start_streaming	= mcam_vb_start_streaming,
 	.stop_streaming		= mcam_vb_stop_streaming,
diff --git a/drivers/media/v4l2-core/videobuf2-dma-sg.c b/drivers/media/v4l2-core/videobuf2-dma-sg.c
index 9b7a041..f3bc01b 100644
--- a/drivers/media/v4l2-core/videobuf2-dma-sg.c
+++ b/drivers/media/v4l2-core/videobuf2-dma-sg.c
@@ -170,6 +170,22 @@ static void vb2_dma_sg_put(void *buf_priv)
 	}
 }
 
+static void vb2_dma_sg_prepare(void *buf_priv)
+{
+	struct vb2_dma_sg_buf *buf = buf_priv;
+	struct sg_table *sgt = &buf->sg_table;
+
+	dma_map_sg(buf->dev, sgt->sgl, sgt->nents, buf->dma_dir);
+}
+
+static void vb2_dma_sg_finish(void *buf_priv)
+{
+	struct vb2_dma_sg_buf *buf = buf_priv;
+	struct sg_table *sgt = &buf->sg_table;
+
+	dma_unmap_sg(buf->dev, sgt->sgl, sgt->nents, buf->dma_dir);
+}
+
 static inline int vma_is_io(struct vm_area_struct *vma)
 {
 	return !!(vma->vm_flags & (VM_IO | VM_PFNMAP));
@@ -366,6 +382,8 @@ const struct vb2_mem_ops vb2_dma_sg_memops = {
 	.put		= vb2_dma_sg_put,
 	.get_userptr	= vb2_dma_sg_get_userptr,
 	.put_userptr	= vb2_dma_sg_put_userptr,
+	.prepare	= vb2_dma_sg_prepare,
+	.finish		= vb2_dma_sg_finish,
 	.vaddr		= vb2_dma_sg_vaddr,
 	.mmap		= vb2_dma_sg_mmap,
 	.num_users	= vb2_dma_sg_num_users,
-- 
2.1.0


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [RFCv2 PATCH 04/14] vb2: memop prepare: return errors
  2014-09-12 12:59 [RFCv2 PATCH 00/14] vb2: improve dma-sg, expbuf Hans Verkuil
                   ` (2 preceding siblings ...)
  2014-09-12 12:59 ` [RFCv2 PATCH 03/14] vb2-dma-sg: add prepare/finish memops Hans Verkuil
@ 2014-09-12 12:59 ` Hans Verkuil
  2014-09-12 12:59 ` [RFCv2 PATCH 05/14] vb2: call memop prepare before the buf_prepare op is called Hans Verkuil
                   ` (9 subsequent siblings)
  13 siblings, 0 replies; 23+ messages in thread
From: Hans Verkuil @ 2014-09-12 12:59 UTC (permalink / raw)
  To: linux-media; +Cc: pawel, m.szyprowski, laurent.pinchart, Hans Verkuil

From: Hans Verkuil <hans.verkuil@cisco.com>

For vb2-dma-sg the dma_map_sg function can return an error. This means that
the prepare memop also needs to change so an error can be returned.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 drivers/media/v4l2-core/videobuf2-dma-contig.c | 5 +++--
 drivers/media/v4l2-core/videobuf2-dma-sg.c     | 4 ++--
 include/media/videobuf2-core.h                 | 2 +-
 3 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/media/v4l2-core/videobuf2-dma-contig.c b/drivers/media/v4l2-core/videobuf2-dma-contig.c
index 6675f12..ca870aa 100644
--- a/drivers/media/v4l2-core/videobuf2-dma-contig.c
+++ b/drivers/media/v4l2-core/videobuf2-dma-contig.c
@@ -111,16 +111,17 @@ static unsigned int vb2_dc_num_users(void *buf_priv)
 	return atomic_read(&buf->refcount);
 }
 
-static void vb2_dc_prepare(void *buf_priv)
+static int vb2_dc_prepare(void *buf_priv)
 {
 	struct vb2_dc_buf *buf = buf_priv;
 	struct sg_table *sgt = buf->dma_sgt;
 
 	/* DMABUF exporter will flush the cache for us */
 	if (!sgt || buf->db_attach)
-		return;
+		return 0;
 
 	dma_sync_sg_for_device(buf->dev, sgt->sgl, sgt->nents, buf->dma_dir);
+	return 0;
 }
 
 static void vb2_dc_finish(void *buf_priv)
diff --git a/drivers/media/v4l2-core/videobuf2-dma-sg.c b/drivers/media/v4l2-core/videobuf2-dma-sg.c
index f3bc01b..abd5252 100644
--- a/drivers/media/v4l2-core/videobuf2-dma-sg.c
+++ b/drivers/media/v4l2-core/videobuf2-dma-sg.c
@@ -170,12 +170,12 @@ static void vb2_dma_sg_put(void *buf_priv)
 	}
 }
 
-static void vb2_dma_sg_prepare(void *buf_priv)
+static int vb2_dma_sg_prepare(void *buf_priv)
 {
 	struct vb2_dma_sg_buf *buf = buf_priv;
 	struct sg_table *sgt = &buf->sg_table;
 
-	dma_map_sg(buf->dev, sgt->sgl, sgt->nents, buf->dma_dir);
+	return dma_map_sg(buf->dev, sgt->sgl, sgt->nents, buf->dma_dir) ? 0 : -EIO;
 }
 
 static void vb2_dma_sg_finish(void *buf_priv)
diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h
index 02b96ef..0ac65a6 100644
--- a/include/media/videobuf2-core.h
+++ b/include/media/videobuf2-core.h
@@ -91,7 +91,7 @@ struct vb2_mem_ops {
 					unsigned long size, int write);
 	void		(*put_userptr)(void *buf_priv);
 
-	void		(*prepare)(void *buf_priv);
+	int		(*prepare)(void *buf_priv);
 	void		(*finish)(void *buf_priv);
 
 	void		*(*attach_dmabuf)(void *alloc_ctx, struct dma_buf *dbuf,
-- 
2.1.0


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [RFCv2 PATCH 05/14] vb2: call memop prepare before the buf_prepare op is called
  2014-09-12 12:59 [RFCv2 PATCH 00/14] vb2: improve dma-sg, expbuf Hans Verkuil
                   ` (3 preceding siblings ...)
  2014-09-12 12:59 ` [RFCv2 PATCH 04/14] vb2: memop prepare: return errors Hans Verkuil
@ 2014-09-12 12:59 ` Hans Verkuil
  2014-09-12 12:59 ` [RFCv2 PATCH 06/14] vb2-dma-sg: add dmabuf import support Hans Verkuil
                   ` (8 subsequent siblings)
  13 siblings, 0 replies; 23+ messages in thread
From: Hans Verkuil @ 2014-09-12 12:59 UTC (permalink / raw)
  To: linux-media; +Cc: pawel, m.szyprowski, laurent.pinchart, Hans Verkuil

From: Hans Verkuil <hans.verkuil@cisco.com>

The prepare memop now returns an error, so we need to be able to handle that.
In addition, prepare has to be called before buf_prepare since in the dma-sg
case buf_prepare expects that the dma memory is mapped and it can use the
sg_table.

So call the prepare memop before calling buf_prepare and clean up the memory
in case of an error.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 drivers/media/v4l2-core/videobuf2-core.c | 46 +++++++++++++++++++++++---------
 1 file changed, 34 insertions(+), 12 deletions(-)

diff --git a/drivers/media/v4l2-core/videobuf2-core.c b/drivers/media/v4l2-core/videobuf2-core.c
index 087cd62..1cb3423 100644
--- a/drivers/media/v4l2-core/videobuf2-core.c
+++ b/drivers/media/v4l2-core/videobuf2-core.c
@@ -1346,13 +1346,43 @@ static void __fill_vb2_buffer(struct vb2_buffer *vb, const struct v4l2_buffer *b
 	}
 }
 
+static int __buf_memory_prepare(struct vb2_buffer *vb)
+{
+	int ret = call_vb_qop(vb, buf_prepare_for_cpu, vb);
+	unsigned int plane;
+
+	if (ret)
+		return ret;
+
+	/* sync buffers */
+	for (plane = 0; plane < vb->num_planes; ++plane) {
+		ret = call_memop(vb, prepare, vb->planes[plane].mem_priv);
+		if (ret) {
+			for (; plane; plane--)
+				call_void_memop(vb, finish, vb->planes[plane - 1].mem_priv);
+			call_void_vb_qop(vb, buf_finish_for_cpu, vb);
+			dprintk(1, "buffer memory preparation failed\n");
+			return ret;
+		}
+	}
+
+	ret = call_vb_qop(vb, buf_prepare, vb);
+	if (ret) {
+		dprintk(1, "buffer preparation failed\n");
+		for (plane = 0; plane < vb->num_planes; ++plane)
+			call_void_memop(vb, finish, vb->planes[plane].mem_priv);
+		call_void_vb_qop(vb, buf_finish_for_cpu, vb);
+	}
+	return ret;
+}
+
 /**
  * __qbuf_mmap() - handle qbuf of an MMAP buffer
  */
 static int __qbuf_mmap(struct vb2_buffer *vb, const struct v4l2_buffer *b)
 {
 	__fill_vb2_buffer(vb, b, vb->v4l2_planes);
-	return call_vb_qop(vb, buf_prepare, vb);
+	return __buf_memory_prepare(vb);
 }
 
 /**
@@ -1437,11 +1467,10 @@ static int __qbuf_userptr(struct vb2_buffer *vb, const struct v4l2_buffer *b)
 		}
 	}
 
-	ret = call_vb_qop(vb, buf_prepare, vb);
+	ret = __buf_memory_prepare(vb);
 	if (ret) {
-		dprintk(1, "buffer preparation failed\n");
 		call_void_vb_qop(vb, buf_cleanup, vb);
-		goto err;
+		return ret;
 	}
 
 	return 0;
@@ -1561,9 +1590,8 @@ static int __qbuf_dmabuf(struct vb2_buffer *vb, const struct v4l2_buffer *b)
 		}
 	}
 
-	ret = call_vb_qop(vb, buf_prepare, vb);
+	ret = __buf_memory_prepare(vb);
 	if (ret) {
-		dprintk(1, "buffer preparation failed\n");
 		call_void_vb_qop(vb, buf_cleanup, vb);
 		goto err;
 	}
@@ -1628,12 +1656,6 @@ static int __buf_prepare(struct vb2_buffer *vb, const struct v4l2_buffer *b)
 	vb->v4l2_buf.timestamp.tv_usec = 0;
 	vb->v4l2_buf.sequence = 0;
 
-	ret = call_vb_qop(vb, buf_prepare_for_cpu, vb);
-	if (ret) {
-		dprintk(1, "buf_prepare_for_cpu failed\n");
-		return ret;
-	}
-
 	switch (q->memory) {
 	case V4L2_MEMORY_MMAP:
 		ret = __qbuf_mmap(vb, b);
-- 
2.1.0


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [RFCv2 PATCH 06/14] vb2-dma-sg: add dmabuf import support
  2014-09-12 12:59 [RFCv2 PATCH 00/14] vb2: improve dma-sg, expbuf Hans Verkuil
                   ` (4 preceding siblings ...)
  2014-09-12 12:59 ` [RFCv2 PATCH 05/14] vb2: call memop prepare before the buf_prepare op is called Hans Verkuil
@ 2014-09-12 12:59 ` Hans Verkuil
  2014-09-14  8:11   ` Laurent Pinchart
  2014-09-12 12:59 ` [RFCv2 PATCH 07/14] vb2-dma-sg: add get_dmabuf Hans Verkuil
                   ` (7 subsequent siblings)
  13 siblings, 1 reply; 23+ messages in thread
From: Hans Verkuil @ 2014-09-12 12:59 UTC (permalink / raw)
  To: linux-media; +Cc: pawel, m.szyprowski, laurent.pinchart, Hans Verkuil

From: Hans Verkuil <hans.verkuil@cisco.com>

Add support for dmabuf to vb2-dma-sg.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 drivers/media/v4l2-core/videobuf2-dma-sg.c | 125 +++++++++++++++++++++++++++--
 1 file changed, 118 insertions(+), 7 deletions(-)

diff --git a/drivers/media/v4l2-core/videobuf2-dma-sg.c b/drivers/media/v4l2-core/videobuf2-dma-sg.c
index abd5252..6d922c0 100644
--- a/drivers/media/v4l2-core/videobuf2-dma-sg.c
+++ b/drivers/media/v4l2-core/videobuf2-dma-sg.c
@@ -42,11 +42,15 @@ struct vb2_dma_sg_buf {
 	int				offset;
 	enum dma_data_direction		dma_dir;
 	struct sg_table			sg_table;
+	struct sg_table			*dma_sgt;
 	size_t				size;
 	unsigned int			num_pages;
 	atomic_t			refcount;
 	struct vb2_vmarea_handler	handler;
 	struct vm_area_struct		*vma;
+
+	/* DMABUF related */
+	struct dma_buf_attachment	*db_attach;
 };
 
 static void vb2_dma_sg_put(void *buf_priv);
@@ -113,6 +117,7 @@ static void *vb2_dma_sg_alloc(void *alloc_ctx, unsigned long size, int write,
 	/* size is already page aligned */
 	buf->num_pages = size >> PAGE_SHIFT;
 	buf->dma_dir = write ? DMA_FROM_DEVICE : DMA_TO_DEVICE;
+	buf->dma_sgt = &buf->sg_table;
 
 	buf->pages = kzalloc(buf->num_pages * sizeof(struct page *),
 			     GFP_KERNEL);
@@ -123,7 +128,7 @@ static void *vb2_dma_sg_alloc(void *alloc_ctx, unsigned long size, int write,
 	if (ret)
 		goto fail_pages_alloc;
 
-	ret = sg_alloc_table_from_pages(&buf->sg_table, buf->pages,
+	ret = sg_alloc_table_from_pages(buf->dma_sgt, buf->pages,
 			buf->num_pages, 0, size, gfp_flags);
 	if (ret)
 		goto fail_table_alloc;
@@ -161,7 +166,7 @@ static void vb2_dma_sg_put(void *buf_priv)
 			buf->num_pages);
 		if (buf->vaddr)
 			vm_unmap_ram(buf->vaddr, buf->num_pages);
-		sg_free_table(&buf->sg_table);
+		sg_free_table(buf->dma_sgt);
 		while (--i >= 0)
 			__free_page(buf->pages[i]);
 		kfree(buf->pages);
@@ -173,7 +178,11 @@ static void vb2_dma_sg_put(void *buf_priv)
 static int vb2_dma_sg_prepare(void *buf_priv)
 {
 	struct vb2_dma_sg_buf *buf = buf_priv;
-	struct sg_table *sgt = &buf->sg_table;
+	struct sg_table *sgt = buf->dma_sgt;
+
+	/* DMABUF exporter will flush the cache for us */
+	if (buf->db_attach)
+		return 0;
 
 	return dma_map_sg(buf->dev, sgt->sgl, sgt->nents, buf->dma_dir) ? 0 : -EIO;
 }
@@ -181,7 +190,11 @@ static int vb2_dma_sg_prepare(void *buf_priv)
 static void vb2_dma_sg_finish(void *buf_priv)
 {
 	struct vb2_dma_sg_buf *buf = buf_priv;
-	struct sg_table *sgt = &buf->sg_table;
+	struct sg_table *sgt = buf->dma_sgt;
+
+	/* DMABUF exporter will flush the cache for us */
+	if (buf->db_attach)
+		return;
 
 	dma_unmap_sg(buf->dev, sgt->sgl, sgt->nents, buf->dma_dir);
 }
@@ -209,6 +222,7 @@ static void *vb2_dma_sg_get_userptr(void *alloc_ctx, unsigned long vaddr,
 	buf->offset = vaddr & ~PAGE_MASK;
 	buf->size = size;
 	buf->dma_dir = write ? DMA_FROM_DEVICE : DMA_TO_DEVICE;
+	buf->dma_sgt = &buf->sg_table;
 
 	first = (vaddr           & PAGE_MASK) >> PAGE_SHIFT;
 	last  = ((vaddr + size - 1) & PAGE_MASK) >> PAGE_SHIFT;
@@ -261,7 +275,7 @@ static void *vb2_dma_sg_get_userptr(void *alloc_ctx, unsigned long vaddr,
 	if (num_pages_from_user != buf->num_pages)
 		goto userptr_fail_get_user_pages;
 
-	if (sg_alloc_table_from_pages(&buf->sg_table, buf->pages,
+	if (sg_alloc_table_from_pages(buf->dma_sgt, buf->pages,
 			buf->num_pages, buf->offset, size, 0))
 		goto userptr_fail_alloc_table_from_pages;
 
@@ -297,7 +311,7 @@ static void vb2_dma_sg_put_userptr(void *buf_priv)
 	       __func__, buf->num_pages);
 	if (buf->vaddr)
 		vm_unmap_ram(buf->vaddr, buf->num_pages);
-	sg_free_table(&buf->sg_table);
+	sg_free_table(buf->dma_sgt);
 	while (--i >= 0) {
 		if (buf->write)
 			set_page_dirty_lock(buf->pages[i]);
@@ -370,11 +384,104 @@ static int vb2_dma_sg_mmap(void *buf_priv, struct vm_area_struct *vma)
 	return 0;
 }
 
+/*********************************************/
+/*       callbacks for DMABUF buffers        */
+/*********************************************/
+
+static int vb2_dma_sg_map_dmabuf(void *mem_priv)
+{
+	struct vb2_dma_sg_buf *buf = mem_priv;
+	struct sg_table *sgt;
+
+	if (WARN_ON(!buf->db_attach)) {
+		pr_err("trying to pin a non attached buffer\n");
+		return -EINVAL;
+	}
+
+	if (WARN_ON(buf->dma_sgt)) {
+		pr_err("dmabuf buffer is already pinned\n");
+		return 0;
+	}
+
+	/* get the associated scatterlist for this buffer */
+	sgt = dma_buf_map_attachment(buf->db_attach, buf->dma_dir);
+	if (IS_ERR_OR_NULL(sgt)) {
+		pr_err("Error getting dmabuf scatterlist\n");
+		return -EINVAL;
+	}
+
+	buf->dma_sgt = sgt;
+	return 0;
+}
+
+static void vb2_dma_sg_unmap_dmabuf(void *mem_priv)
+{
+	struct vb2_dma_sg_buf *buf = mem_priv;
+	struct sg_table *sgt = buf->dma_sgt;
+
+	if (WARN_ON(!buf->db_attach)) {
+		pr_err("trying to unpin a not attached buffer\n");
+		return;
+	}
+
+	if (WARN_ON(!sgt)) {
+		pr_err("dmabuf buffer is already unpinned\n");
+		return;
+	}
+
+	dma_buf_unmap_attachment(buf->db_attach, sgt, buf->dma_dir);
+
+	buf->dma_sgt = NULL;
+}
+
+static void vb2_dma_sg_detach_dmabuf(void *mem_priv)
+{
+	struct vb2_dma_sg_buf *buf = mem_priv;
+
+	/* if vb2 works correctly you should never detach mapped buffer */
+	if (WARN_ON(buf->dma_sgt))
+		vb2_dma_sg_unmap_dmabuf(buf);
+
+	/* detach this attachment */
+	dma_buf_detach(buf->db_attach->dmabuf, buf->db_attach);
+	kfree(buf);
+}
+
+static void *vb2_dma_sg_attach_dmabuf(void *alloc_ctx, struct dma_buf *dbuf,
+	unsigned long size, int write)
+{
+	struct vb2_dma_sg_conf *conf = alloc_ctx;
+	struct vb2_dma_sg_buf *buf;
+	struct dma_buf_attachment *dba;
+
+	if (dbuf->size < size)
+		return ERR_PTR(-EFAULT);
+
+	buf = kzalloc(sizeof(*buf), GFP_KERNEL);
+	if (!buf)
+		return ERR_PTR(-ENOMEM);
+
+	buf->dev = conf->dev;
+	/* create attachment for the dmabuf with the user device */
+	dba = dma_buf_attach(dbuf, buf->dev);
+	if (IS_ERR(dba)) {
+		pr_err("failed to attach dmabuf\n");
+		kfree(buf);
+		return dba;
+	}
+
+	buf->dma_dir = write ? DMA_FROM_DEVICE : DMA_TO_DEVICE;
+	buf->size = size;
+	buf->db_attach = dba;
+
+	return buf;
+}
+
 static void *vb2_dma_sg_cookie(void *buf_priv)
 {
 	struct vb2_dma_sg_buf *buf = buf_priv;
 
-	return &buf->sg_table;
+	return buf->dma_sgt;
 }
 
 const struct vb2_mem_ops vb2_dma_sg_memops = {
@@ -387,6 +494,10 @@ const struct vb2_mem_ops vb2_dma_sg_memops = {
 	.vaddr		= vb2_dma_sg_vaddr,
 	.mmap		= vb2_dma_sg_mmap,
 	.num_users	= vb2_dma_sg_num_users,
+	.map_dmabuf	= vb2_dma_sg_map_dmabuf,
+	.unmap_dmabuf	= vb2_dma_sg_unmap_dmabuf,
+	.attach_dmabuf	= vb2_dma_sg_attach_dmabuf,
+	.detach_dmabuf	= vb2_dma_sg_detach_dmabuf,
 	.cookie		= vb2_dma_sg_cookie,
 };
 EXPORT_SYMBOL_GPL(vb2_dma_sg_memops);
-- 
2.1.0


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [RFCv2 PATCH 07/14] vb2-dma-sg: add get_dmabuf
  2014-09-12 12:59 [RFCv2 PATCH 00/14] vb2: improve dma-sg, expbuf Hans Verkuil
                   ` (5 preceding siblings ...)
  2014-09-12 12:59 ` [RFCv2 PATCH 06/14] vb2-dma-sg: add dmabuf import support Hans Verkuil
@ 2014-09-12 12:59 ` Hans Verkuil
  2014-09-12 12:59 ` [RFCv2 PATCH 08/14] vb2-vmalloc: add get_dmabuf support Hans Verkuil
                   ` (6 subsequent siblings)
  13 siblings, 0 replies; 23+ messages in thread
From: Hans Verkuil @ 2014-09-12 12:59 UTC (permalink / raw)
  To: linux-media; +Cc: pawel, m.szyprowski, laurent.pinchart, Hans Verkuil

From: Hans Verkuil <hansverk@cisco.com>

Add DMABUF export support to vb2-dma-sg.

Signed-off-by: Hans Verkuil <hansverk@cisco.com>
---
 drivers/media/v4l2-core/videobuf2-dma-sg.c | 170 +++++++++++++++++++++++++++++
 1 file changed, 170 insertions(+)

diff --git a/drivers/media/v4l2-core/videobuf2-dma-sg.c b/drivers/media/v4l2-core/videobuf2-dma-sg.c
index 6d922c0..b494b49 100644
--- a/drivers/media/v4l2-core/videobuf2-dma-sg.c
+++ b/drivers/media/v4l2-core/videobuf2-dma-sg.c
@@ -385,6 +385,175 @@ static int vb2_dma_sg_mmap(void *buf_priv, struct vm_area_struct *vma)
 }
 
 /*********************************************/
+/*         DMABUF ops for exporters          */
+/*********************************************/
+
+struct vb2_dma_sg_attachment {
+	struct sg_table sgt;
+	enum dma_data_direction dir;
+};
+
+static int vb2_dma_sg_dmabuf_ops_attach(struct dma_buf *dbuf, struct device *dev,
+	struct dma_buf_attachment *dbuf_attach)
+{
+	struct vb2_dma_sg_attachment *attach;
+	unsigned int i;
+	struct scatterlist *rd, *wr;
+	struct sg_table *sgt;
+	struct vb2_dma_sg_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->dma_sgt->orig_nents, GFP_KERNEL);
+	if (ret) {
+		kfree(attach);
+		return -ENOMEM;
+	}
+
+	rd = buf->dma_sgt->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_dma_sg_dmabuf_ops_detach(struct dma_buf *dbuf,
+	struct dma_buf_attachment *db_attach)
+{
+	struct vb2_dma_sg_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_dma_sg_dmabuf_ops_map(
+	struct dma_buf_attachment *db_attach, enum dma_data_direction dir)
+{
+	struct vb2_dma_sg_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) {
+		pr_err("failed to map scatterlist\n");
+		mutex_unlock(lock);
+		return ERR_PTR(-EIO);
+	}
+
+	attach->dir = dir;
+
+	mutex_unlock(lock);
+
+	return sgt;
+}
+
+static void vb2_dma_sg_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_dma_sg_dmabuf_ops_release(struct dma_buf *dbuf)
+{
+	/* drop reference obtained in vb2_dma_sg_get_dmabuf */
+	vb2_dma_sg_put(dbuf->priv);
+}
+
+static void *vb2_dma_sg_dmabuf_ops_kmap(struct dma_buf *dbuf, unsigned long pgnum)
+{
+	struct vb2_dma_sg_buf *buf = dbuf->priv;
+
+	return buf->vaddr + pgnum * PAGE_SIZE;
+}
+
+static void *vb2_dma_sg_dmabuf_ops_vmap(struct dma_buf *dbuf)
+{
+	struct vb2_dma_sg_buf *buf = dbuf->priv;
+
+	return vb2_dma_sg_vaddr(buf);
+}
+
+static int vb2_dma_sg_dmabuf_ops_mmap(struct dma_buf *dbuf,
+	struct vm_area_struct *vma)
+{
+	return vb2_dma_sg_mmap(dbuf->priv, vma);
+}
+
+static struct dma_buf_ops vb2_dma_sg_dmabuf_ops = {
+	.attach = vb2_dma_sg_dmabuf_ops_attach,
+	.detach = vb2_dma_sg_dmabuf_ops_detach,
+	.map_dma_buf = vb2_dma_sg_dmabuf_ops_map,
+	.unmap_dma_buf = vb2_dma_sg_dmabuf_ops_unmap,
+	.kmap = vb2_dma_sg_dmabuf_ops_kmap,
+	.kmap_atomic = vb2_dma_sg_dmabuf_ops_kmap,
+	.vmap = vb2_dma_sg_dmabuf_ops_vmap,
+	.mmap = vb2_dma_sg_dmabuf_ops_mmap,
+	.release = vb2_dma_sg_dmabuf_ops_release,
+};
+
+static struct dma_buf *vb2_dma_sg_get_dmabuf(void *buf_priv, unsigned long flags)
+{
+	struct vb2_dma_sg_buf *buf = buf_priv;
+	struct dma_buf *dbuf;
+
+	if (WARN_ON(!buf->dma_sgt))
+		return NULL;
+
+	dbuf = dma_buf_export(buf, &vb2_dma_sg_dmabuf_ops, buf->size, flags, NULL);
+	if (IS_ERR(dbuf))
+		return NULL;
+
+	/* dmabuf keeps reference to vb2 buffer */
+	atomic_inc(&buf->refcount);
+
+	return dbuf;
+}
+
+/*********************************************/
 /*       callbacks for DMABUF buffers        */
 /*********************************************/
 
@@ -494,6 +663,7 @@ const struct vb2_mem_ops vb2_dma_sg_memops = {
 	.vaddr		= vb2_dma_sg_vaddr,
 	.mmap		= vb2_dma_sg_mmap,
 	.num_users	= vb2_dma_sg_num_users,
+	.get_dmabuf	= vb2_dma_sg_get_dmabuf,
 	.map_dmabuf	= vb2_dma_sg_map_dmabuf,
 	.unmap_dmabuf	= vb2_dma_sg_unmap_dmabuf,
 	.attach_dmabuf	= vb2_dma_sg_attach_dmabuf,
-- 
2.1.0


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [RFCv2 PATCH 08/14] vb2-vmalloc: add get_dmabuf support
  2014-09-12 12:59 [RFCv2 PATCH 00/14] vb2: improve dma-sg, expbuf Hans Verkuil
                   ` (6 preceding siblings ...)
  2014-09-12 12:59 ` [RFCv2 PATCH 07/14] vb2-dma-sg: add get_dmabuf Hans Verkuil
@ 2014-09-12 12:59 ` Hans Verkuil
  2014-09-12 12:59 ` [RFCv2 PATCH 09/14] vb2: replace 'write' by 'dma_dir' Hans Verkuil
                   ` (5 subsequent siblings)
  13 siblings, 0 replies; 23+ messages in thread
From: Hans Verkuil @ 2014-09-12 12:59 UTC (permalink / raw)
  To: linux-media; +Cc: pawel, m.szyprowski, laurent.pinchart, Hans Verkuil

From: Hans Verkuil <hansverk@cisco.com>

Add support for DMABUF exporting to the vb2-vmalloc implementation.

All memory models now have support for both importing and exporting of DMABUFs.

Signed-off-by: Hans Verkuil <hansverk@cisco.com>
---
 drivers/media/v4l2-core/videobuf2-vmalloc.c | 174 ++++++++++++++++++++++++++++
 1 file changed, 174 insertions(+)

diff --git a/drivers/media/v4l2-core/videobuf2-vmalloc.c b/drivers/media/v4l2-core/videobuf2-vmalloc.c
index d77e397..437fbcd 100644
--- a/drivers/media/v4l2-core/videobuf2-vmalloc.c
+++ b/drivers/media/v4l2-core/videobuf2-vmalloc.c
@@ -31,6 +31,9 @@ struct vb2_vmalloc_buf {
 	atomic_t			refcount;
 	struct vb2_vmarea_handler	handler;
 	struct dma_buf			*dbuf;
+
+	/* DMABUF related */
+	struct dma_buf_attachment	*db_attach;
 };
 
 static void vb2_vmalloc_put(void *buf_priv);
@@ -210,6 +213,176 @@ static int vb2_vmalloc_mmap(void *buf_priv, struct vm_area_struct *vma)
 }
 
 /*********************************************/
+/*         DMABUF ops for exporters          */
+/*********************************************/
+
+struct vb2_vmalloc_attachment {
+	struct sg_table sgt;
+	enum dma_data_direction dir;
+};
+
+static int vb2_vmalloc_dmabuf_ops_attach(struct dma_buf *dbuf, struct device *dev,
+	struct dma_buf_attachment *dbuf_attach)
+{
+	struct vb2_vmalloc_attachment *attach;
+	struct vb2_vmalloc_buf *buf = dbuf->priv;
+	int num_pages = PAGE_ALIGN(buf->size) / PAGE_SIZE;
+	struct sg_table *sgt;
+	struct scatterlist *sg;
+	void *vaddr = buf->vaddr;
+	int ret;
+	int i;
+
+	attach = kzalloc(sizeof(*attach), GFP_KERNEL);
+	if (!attach)
+		return -ENOMEM;
+
+	sgt = &attach->sgt;
+	ret = sg_alloc_table(sgt, num_pages, GFP_KERNEL);
+	if (ret) {
+		kfree(attach);
+		return ret;
+	}
+	for_each_sg(sgt->sgl, sg, sgt->nents, i) {
+		struct page *page = vmalloc_to_page(vaddr);
+
+		if (!page) {
+			sg_free_table(sgt);
+			kfree(attach);
+			return -ENOMEM;
+		}
+		sg_set_page(sg, page, PAGE_SIZE, 0);
+		vaddr += PAGE_SIZE;
+	}
+
+	attach->dir = DMA_NONE;
+	dbuf_attach->priv = attach;
+	return 0;
+}
+
+static void vb2_vmalloc_dmabuf_ops_detach(struct dma_buf *dbuf,
+	struct dma_buf_attachment *db_attach)
+{
+	struct vb2_vmalloc_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_vmalloc_dmabuf_ops_map(
+	struct dma_buf_attachment *db_attach, enum dma_data_direction dir)
+{
+	struct vb2_vmalloc_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) {
+		pr_err("failed to map scatterlist\n");
+		mutex_unlock(lock);
+		return ERR_PTR(-EIO);
+	}
+
+	attach->dir = dir;
+
+	mutex_unlock(lock);
+
+	return sgt;
+}
+
+static void vb2_vmalloc_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_vmalloc_dmabuf_ops_release(struct dma_buf *dbuf)
+{
+	/* drop reference obtained in vb2_vmalloc_get_dmabuf */
+	vb2_vmalloc_put(dbuf->priv);
+}
+
+static void *vb2_vmalloc_dmabuf_ops_kmap(struct dma_buf *dbuf, unsigned long pgnum)
+{
+	struct vb2_vmalloc_buf *buf = dbuf->priv;
+
+	return buf->vaddr + pgnum * PAGE_SIZE;
+}
+
+static void *vb2_vmalloc_dmabuf_ops_vmap(struct dma_buf *dbuf)
+{
+	struct vb2_vmalloc_buf *buf = dbuf->priv;
+
+	return buf->vaddr;
+}
+
+static int vb2_vmalloc_dmabuf_ops_mmap(struct dma_buf *dbuf,
+	struct vm_area_struct *vma)
+{
+	return vb2_vmalloc_mmap(dbuf->priv, vma);
+}
+
+static struct dma_buf_ops vb2_vmalloc_dmabuf_ops = {
+	.attach = vb2_vmalloc_dmabuf_ops_attach,
+	.detach = vb2_vmalloc_dmabuf_ops_detach,
+	.map_dma_buf = vb2_vmalloc_dmabuf_ops_map,
+	.unmap_dma_buf = vb2_vmalloc_dmabuf_ops_unmap,
+	.kmap = vb2_vmalloc_dmabuf_ops_kmap,
+	.kmap_atomic = vb2_vmalloc_dmabuf_ops_kmap,
+	.vmap = vb2_vmalloc_dmabuf_ops_vmap,
+	.mmap = vb2_vmalloc_dmabuf_ops_mmap,
+	.release = vb2_vmalloc_dmabuf_ops_release,
+};
+
+static struct dma_buf *vb2_vmalloc_get_dmabuf(void *buf_priv, unsigned long flags)
+{
+	struct vb2_vmalloc_buf *buf = buf_priv;
+	struct dma_buf *dbuf;
+
+	if (WARN_ON(!buf->vaddr))
+		return NULL;
+
+	dbuf = dma_buf_export(buf, &vb2_vmalloc_dmabuf_ops, buf->size, flags, NULL);
+	if (IS_ERR(dbuf))
+		return NULL;
+
+	/* dmabuf keeps reference to vb2 buffer */
+	atomic_inc(&buf->refcount);
+
+	return dbuf;
+}
+
+/*********************************************/
 /*       callbacks for DMABUF buffers        */
 /*********************************************/
 
@@ -265,6 +438,7 @@ const struct vb2_mem_ops vb2_vmalloc_memops = {
 	.put		= vb2_vmalloc_put,
 	.get_userptr	= vb2_vmalloc_get_userptr,
 	.put_userptr	= vb2_vmalloc_put_userptr,
+	.get_dmabuf	= vb2_vmalloc_get_dmabuf,
 	.map_dmabuf	= vb2_vmalloc_map_dmabuf,
 	.unmap_dmabuf	= vb2_vmalloc_unmap_dmabuf,
 	.attach_dmabuf	= vb2_vmalloc_attach_dmabuf,
-- 
2.1.0


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [RFCv2 PATCH 09/14] vb2: replace 'write' by 'dma_dir'
  2014-09-12 12:59 [RFCv2 PATCH 00/14] vb2: improve dma-sg, expbuf Hans Verkuil
                   ` (7 preceding siblings ...)
  2014-09-12 12:59 ` [RFCv2 PATCH 08/14] vb2-vmalloc: add get_dmabuf support Hans Verkuil
@ 2014-09-12 12:59 ` Hans Verkuil
  2014-09-12 22:02   ` Laurent Pinchart
  2014-09-12 12:59 ` [RFCv2 PATCH 10/14] vb2: add 'new_cookies' flag Hans Verkuil
                   ` (4 subsequent siblings)
  13 siblings, 1 reply; 23+ messages in thread
From: Hans Verkuil @ 2014-09-12 12:59 UTC (permalink / raw)
  To: linux-media; +Cc: pawel, m.szyprowski, laurent.pinchart, Hans Verkuil

From: Hans Verkuil <hans.verkuil@cisco.com>

The 'write' argument is very ambiguous. I first assumed that if it is 1,
then we're doing video output but instead it meant the reverse.

Since it is used to setup the dma_dir value anyway it is now replaced by
the correct dma_dir value which is unambiguous.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 drivers/media/v4l2-core/videobuf2-core.c       | 15 +++++----
 drivers/media/v4l2-core/videobuf2-dma-contig.c | 46 ++++++++++++++------------
 drivers/media/v4l2-core/videobuf2-dma-sg.c     | 46 ++++++++++++--------------
 drivers/media/v4l2-core/videobuf2-vmalloc.c    | 20 ++++++-----
 include/media/videobuf2-core.h                 | 11 +++---
 5 files changed, 73 insertions(+), 65 deletions(-)

diff --git a/drivers/media/v4l2-core/videobuf2-core.c b/drivers/media/v4l2-core/videobuf2-core.c
index 1cb3423..01bab25 100644
--- a/drivers/media/v4l2-core/videobuf2-core.c
+++ b/drivers/media/v4l2-core/videobuf2-core.c
@@ -189,7 +189,8 @@ static void __vb2_queue_cancel(struct vb2_queue *q);
 static int __vb2_buf_mem_alloc(struct vb2_buffer *vb)
 {
 	struct vb2_queue *q = vb->vb2_queue;
-	int write = !V4L2_TYPE_IS_OUTPUT(q->type);
+	enum dma_data_direction dma_dir =
+		V4L2_TYPE_IS_OUTPUT(q->type) ? DMA_TO_DEVICE : DMA_FROM_DEVICE;
 	void *mem_priv;
 	int plane;
 
@@ -201,7 +202,7 @@ static int __vb2_buf_mem_alloc(struct vb2_buffer *vb)
 		unsigned long size = PAGE_ALIGN(q->plane_sizes[plane]);
 
 		mem_priv = call_ptr_memop(vb, alloc, q->alloc_ctx[plane],
-				      size, write, q->gfp_flags);
+				      size, dma_dir, q->gfp_flags);
 		if (IS_ERR_OR_NULL(mem_priv))
 			goto free;
 
@@ -1395,7 +1396,8 @@ static int __qbuf_userptr(struct vb2_buffer *vb, const struct v4l2_buffer *b)
 	void *mem_priv;
 	unsigned int plane;
 	int ret;
-	int write = !V4L2_TYPE_IS_OUTPUT(q->type);
+	enum dma_data_direction dma_dir =
+		V4L2_TYPE_IS_OUTPUT(q->type) ? DMA_TO_DEVICE : DMA_FROM_DEVICE;
 	bool reacquired = vb->planes[0].mem_priv == NULL;
 
 	memset(planes, 0, sizeof(planes[0]) * vb->num_planes);
@@ -1437,7 +1439,7 @@ static int __qbuf_userptr(struct vb2_buffer *vb, const struct v4l2_buffer *b)
 		/* Acquire each plane's memory */
 		mem_priv = call_ptr_memop(vb, get_userptr, q->alloc_ctx[plane],
 				      planes[plane].m.userptr,
-				      planes[plane].length, write);
+				      planes[plane].length, dma_dir);
 		if (IS_ERR_OR_NULL(mem_priv)) {
 			dprintk(1, "failed acquiring userspace "
 						"memory for plane %d\n", plane);
@@ -1497,7 +1499,8 @@ static int __qbuf_dmabuf(struct vb2_buffer *vb, const struct v4l2_buffer *b)
 	void *mem_priv;
 	unsigned int plane;
 	int ret;
-	int write = !V4L2_TYPE_IS_OUTPUT(q->type);
+	enum dma_data_direction dma_dir =
+		V4L2_TYPE_IS_OUTPUT(q->type) ? DMA_TO_DEVICE : DMA_FROM_DEVICE;
 	bool reacquired = vb->planes[0].mem_priv == NULL;
 
 	memset(planes, 0, sizeof(planes[0]) * vb->num_planes);
@@ -1545,7 +1548,7 @@ static int __qbuf_dmabuf(struct vb2_buffer *vb, const struct v4l2_buffer *b)
 
 		/* Acquire each plane's memory */
 		mem_priv = call_ptr_memop(vb, attach_dmabuf, q->alloc_ctx[plane],
-			dbuf, planes[plane].length, write);
+			dbuf, planes[plane].length, dma_dir);
 		if (IS_ERR(mem_priv)) {
 			dprintk(1, "failed to attach dmabuf\n");
 			ret = PTR_ERR(mem_priv);
diff --git a/drivers/media/v4l2-core/videobuf2-dma-contig.c b/drivers/media/v4l2-core/videobuf2-dma-contig.c
index ca870aa..89360bd 100644
--- a/drivers/media/v4l2-core/videobuf2-dma-contig.c
+++ b/drivers/media/v4l2-core/videobuf2-dma-contig.c
@@ -156,8 +156,8 @@ static void vb2_dc_put(void *buf_priv)
 	kfree(buf);
 }
 
-static void *vb2_dc_alloc(void *alloc_ctx, unsigned long size, int write,
-			  gfp_t gfp_flags)
+static void *vb2_dc_alloc(void *alloc_ctx, unsigned long size,
+			  enum dma_data_direction dma_dir, gfp_t gfp_flags)
 {
 	struct vb2_dc_conf *conf = alloc_ctx;
 	struct device *dev = conf->dev;
@@ -178,7 +178,7 @@ static void *vb2_dc_alloc(void *alloc_ctx, unsigned long size, int write,
 	/* Prevent the device from being released while the buffer is used */
 	buf->dev = get_device(dev);
 	buf->size = size;
-	buf->dma_dir = write ? DMA_FROM_DEVICE : DMA_TO_DEVICE;
+	buf->dma_dir = dma_dir;
 
 	buf->handler.refcount = &buf->refcount;
 	buf->handler.put = vb2_dc_put;
@@ -232,7 +232,7 @@ static int vb2_dc_mmap(void *buf_priv, struct vm_area_struct *vma)
 
 struct vb2_dc_attachment {
 	struct sg_table sgt;
-	enum dma_data_direction dir;
+	enum dma_data_direction dma_dir;
 };
 
 static int vb2_dc_dmabuf_ops_attach(struct dma_buf *dbuf, struct device *dev,
@@ -267,7 +267,7 @@ static int vb2_dc_dmabuf_ops_attach(struct dma_buf *dbuf, struct device *dev,
 		wr = sg_next(wr);
 	}
 
-	attach->dir = DMA_NONE;
+	attach->dma_dir = DMA_NONE;
 	dbuf_attach->priv = attach;
 
 	return 0;
@@ -285,16 +285,16 @@ static void vb2_dc_dmabuf_ops_detach(struct dma_buf *dbuf,
 	sgt = &attach->sgt;
 
 	/* release the scatterlist cache */
-	if (attach->dir != DMA_NONE)
+	if (attach->dma_dir != DMA_NONE)
 		dma_unmap_sg(db_attach->dev, sgt->sgl, sgt->orig_nents,
-			attach->dir);
+			attach->dma_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_attachment *db_attach, enum dma_data_direction dma_dir)
 {
 	struct vb2_dc_attachment *attach = db_attach->priv;
 	/* stealing dmabuf mutex to serialize map/unmap operations */
@@ -306,27 +306,27 @@ static struct sg_table *vb2_dc_dmabuf_ops_map(
 
 	sgt = &attach->sgt;
 	/* return previously mapped sg table */
-	if (attach->dir == dir) {
+	if (attach->dma_dir == dma_dir) {
 		mutex_unlock(lock);
 		return sgt;
 	}
 
 	/* release any previous cache */
-	if (attach->dir != DMA_NONE) {
+	if (attach->dma_dir != DMA_NONE) {
 		dma_unmap_sg(db_attach->dev, sgt->sgl, sgt->orig_nents,
-			attach->dir);
-		attach->dir = DMA_NONE;
+			attach->dma_dir);
+		attach->dma_dir = DMA_NONE;
 	}
 
 	/* mapping to the client with new direction */
-	ret = dma_map_sg(db_attach->dev, sgt->sgl, sgt->orig_nents, dir);
+	ret = dma_map_sg(db_attach->dev, sgt->sgl, sgt->orig_nents, dma_dir);
 	if (ret <= 0) {
 		pr_err("failed to map scatterlist\n");
 		mutex_unlock(lock);
 		return ERR_PTR(-EIO);
 	}
 
-	attach->dir = dir;
+	attach->dma_dir = dma_dir;
 
 	mutex_unlock(lock);
 
@@ -334,7 +334,7 @@ static struct sg_table *vb2_dc_dmabuf_ops_map(
 }
 
 static void vb2_dc_dmabuf_ops_unmap(struct dma_buf_attachment *db_attach,
-	struct sg_table *sgt, enum dma_data_direction dir)
+	struct sg_table *sgt, enum dma_data_direction dma_dir)
 {
 	/* nothing to be done here */
 }
@@ -463,7 +463,8 @@ static int vb2_dc_get_user_pfn(unsigned long start, int n_pages,
 }
 
 static int vb2_dc_get_user_pages(unsigned long start, struct page **pages,
-	int n_pages, struct vm_area_struct *vma, int write)
+	int n_pages, struct vm_area_struct *vma,
+	enum dma_data_direction dma_dir)
 {
 	if (vma_is_io(vma)) {
 		unsigned int i;
@@ -485,7 +486,7 @@ static int vb2_dc_get_user_pages(unsigned long start, struct page **pages,
 		int n;
 
 		n = get_user_pages(current, current->mm, start & PAGE_MASK,
-			n_pages, write, 1, pages, NULL);
+			n_pages, dma_dir == DMA_FROM_DEVICE, 1, pages, NULL);
 		/* negative error means that no page was pinned */
 		n = max(n, 0);
 		if (n != n_pages) {
@@ -554,7 +555,7 @@ static inline dma_addr_t vb2_dc_pfn_to_dma(struct device *dev, unsigned long pfn
 #endif
 
 static void *vb2_dc_get_userptr(void *alloc_ctx, unsigned long vaddr,
-	unsigned long size, int write)
+	unsigned long size, enum dma_data_direction dma_dir)
 {
 	struct vb2_dc_conf *conf = alloc_ctx;
 	struct vb2_dc_buf *buf;
@@ -585,7 +586,7 @@ static void *vb2_dc_get_userptr(void *alloc_ctx, unsigned long vaddr,
 		return ERR_PTR(-ENOMEM);
 
 	buf->dev = conf->dev;
-	buf->dma_dir = write ? DMA_FROM_DEVICE : DMA_TO_DEVICE;
+	buf->dma_dir = dma_dir;
 
 	start = vaddr & PAGE_MASK;
 	offset = vaddr & ~PAGE_MASK;
@@ -621,7 +622,8 @@ static void *vb2_dc_get_userptr(void *alloc_ctx, unsigned long vaddr,
 	}
 
 	/* extract page list from userspace mapping */
-	ret = vb2_dc_get_user_pages(start, pages, n_pages, vma, write);
+	ret = vb2_dc_get_user_pages(start, pages, n_pages, vma,
+				    dma_dir == DMA_FROM_DEVICE);
 	if (ret) {
 		unsigned long pfn;
 		if (vb2_dc_get_user_pfn(start, n_pages, vma, &pfn) == 0) {
@@ -785,7 +787,7 @@ static void vb2_dc_detach_dmabuf(void *mem_priv)
 }
 
 static void *vb2_dc_attach_dmabuf(void *alloc_ctx, struct dma_buf *dbuf,
-	unsigned long size, int write)
+	unsigned long size, enum dma_data_direction dma_dir)
 {
 	struct vb2_dc_conf *conf = alloc_ctx;
 	struct vb2_dc_buf *buf;
@@ -807,7 +809,7 @@ static void *vb2_dc_attach_dmabuf(void *alloc_ctx, struct dma_buf *dbuf,
 		return dba;
 	}
 
-	buf->dma_dir = write ? DMA_FROM_DEVICE : DMA_TO_DEVICE;
+	buf->dma_dir = dma_dir;
 	buf->size = size;
 	buf->db_attach = dba;
 
diff --git a/drivers/media/v4l2-core/videobuf2-dma-sg.c b/drivers/media/v4l2-core/videobuf2-dma-sg.c
index b494b49..bba8446 100644
--- a/drivers/media/v4l2-core/videobuf2-dma-sg.c
+++ b/drivers/media/v4l2-core/videobuf2-dma-sg.c
@@ -38,7 +38,6 @@ struct vb2_dma_sg_buf {
 	struct device			*dev;
 	void				*vaddr;
 	struct page			**pages;
-	int				write;
 	int				offset;
 	enum dma_data_direction		dma_dir;
 	struct sg_table			sg_table;
@@ -96,8 +95,8 @@ static int vb2_dma_sg_alloc_compacted(struct vb2_dma_sg_buf *buf,
 	return 0;
 }
 
-static void *vb2_dma_sg_alloc(void *alloc_ctx, unsigned long size, int write,
-			      gfp_t gfp_flags)
+static void *vb2_dma_sg_alloc(void *alloc_ctx, unsigned long size,
+			      enum dma_data_direction dma_dir, gfp_t gfp_flags)
 {
 	struct vb2_dma_sg_conf *conf = alloc_ctx;
 	struct vb2_dma_sg_buf *buf;
@@ -111,12 +110,11 @@ static void *vb2_dma_sg_alloc(void *alloc_ctx, unsigned long size, int write,
 		return NULL;
 
 	buf->vaddr = NULL;
-	buf->write = write;
 	buf->offset = 0;
 	buf->size = size;
 	/* size is already page aligned */
 	buf->num_pages = size >> PAGE_SHIFT;
-	buf->dma_dir = write ? DMA_FROM_DEVICE : DMA_TO_DEVICE;
+	buf->dma_dir = dma_dir;
 	buf->dma_sgt = &buf->sg_table;
 
 	buf->pages = kzalloc(buf->num_pages * sizeof(struct page *),
@@ -205,7 +203,8 @@ static inline int vma_is_io(struct vm_area_struct *vma)
 }
 
 static void *vb2_dma_sg_get_userptr(void *alloc_ctx, unsigned long vaddr,
-				    unsigned long size, int write)
+				    unsigned long size,
+				    enum dma_data_direction dma_dir)
 {
 	struct vb2_dma_sg_conf *conf = alloc_ctx;
 	struct vb2_dma_sg_buf *buf;
@@ -218,10 +217,9 @@ static void *vb2_dma_sg_get_userptr(void *alloc_ctx, unsigned long vaddr,
 		return NULL;
 
 	buf->vaddr = NULL;
-	buf->write = write;
 	buf->offset = vaddr & ~PAGE_MASK;
 	buf->size = size;
-	buf->dma_dir = write ? DMA_FROM_DEVICE : DMA_TO_DEVICE;
+	buf->dma_dir = dma_dir;
 	buf->dma_sgt = &buf->sg_table;
 
 	first = (vaddr           & PAGE_MASK) >> PAGE_SHIFT;
@@ -267,7 +265,7 @@ static void *vb2_dma_sg_get_userptr(void *alloc_ctx, unsigned long vaddr,
 		num_pages_from_user = get_user_pages(current, current->mm,
 					     vaddr & PAGE_MASK,
 					     buf->num_pages,
-					     write,
+					     buf->dma_dir == DMA_FROM_DEVICE,
 					     1, /* force */
 					     buf->pages,
 					     NULL);
@@ -313,7 +311,7 @@ static void vb2_dma_sg_put_userptr(void *buf_priv)
 		vm_unmap_ram(buf->vaddr, buf->num_pages);
 	sg_free_table(buf->dma_sgt);
 	while (--i >= 0) {
-		if (buf->write)
+		if (buf->dma_dir == DMA_FROM_DEVICE)
 			set_page_dirty_lock(buf->pages[i]);
 		if (!vma_is_io(buf->vma))
 			put_page(buf->pages[i]);
@@ -390,7 +388,7 @@ static int vb2_dma_sg_mmap(void *buf_priv, struct vm_area_struct *vma)
 
 struct vb2_dma_sg_attachment {
 	struct sg_table sgt;
-	enum dma_data_direction dir;
+	enum dma_data_direction dma_dir;
 };
 
 static int vb2_dma_sg_dmabuf_ops_attach(struct dma_buf *dbuf, struct device *dev,
@@ -425,7 +423,7 @@ static int vb2_dma_sg_dmabuf_ops_attach(struct dma_buf *dbuf, struct device *dev
 		wr = sg_next(wr);
 	}
 
-	attach->dir = DMA_NONE;
+	attach->dma_dir = DMA_NONE;
 	dbuf_attach->priv = attach;
 
 	return 0;
@@ -443,16 +441,16 @@ static void vb2_dma_sg_dmabuf_ops_detach(struct dma_buf *dbuf,
 	sgt = &attach->sgt;
 
 	/* release the scatterlist cache */
-	if (attach->dir != DMA_NONE)
+	if (attach->dma_dir != DMA_NONE)
 		dma_unmap_sg(db_attach->dev, sgt->sgl, sgt->orig_nents,
-			attach->dir);
+			attach->dma_dir);
 	sg_free_table(sgt);
 	kfree(attach);
 	db_attach->priv = NULL;
 }
 
 static struct sg_table *vb2_dma_sg_dmabuf_ops_map(
-	struct dma_buf_attachment *db_attach, enum dma_data_direction dir)
+	struct dma_buf_attachment *db_attach, enum dma_data_direction dma_dir)
 {
 	struct vb2_dma_sg_attachment *attach = db_attach->priv;
 	/* stealing dmabuf mutex to serialize map/unmap operations */
@@ -464,27 +462,27 @@ static struct sg_table *vb2_dma_sg_dmabuf_ops_map(
 
 	sgt = &attach->sgt;
 	/* return previously mapped sg table */
-	if (attach->dir == dir) {
+	if (attach->dma_dir == dma_dir) {
 		mutex_unlock(lock);
 		return sgt;
 	}
 
 	/* release any previous cache */
-	if (attach->dir != DMA_NONE) {
+	if (attach->dma_dir != DMA_NONE) {
 		dma_unmap_sg(db_attach->dev, sgt->sgl, sgt->orig_nents,
-			attach->dir);
-		attach->dir = DMA_NONE;
+			attach->dma_dir);
+		attach->dma_dir = DMA_NONE;
 	}
 
 	/* mapping to the client with new direction */
-	ret = dma_map_sg(db_attach->dev, sgt->sgl, sgt->orig_nents, dir);
+	ret = dma_map_sg(db_attach->dev, sgt->sgl, sgt->orig_nents, dma_dir);
 	if (ret <= 0) {
 		pr_err("failed to map scatterlist\n");
 		mutex_unlock(lock);
 		return ERR_PTR(-EIO);
 	}
 
-	attach->dir = dir;
+	attach->dma_dir = dma_dir;
 
 	mutex_unlock(lock);
 
@@ -492,7 +490,7 @@ static struct sg_table *vb2_dma_sg_dmabuf_ops_map(
 }
 
 static void vb2_dma_sg_dmabuf_ops_unmap(struct dma_buf_attachment *db_attach,
-	struct sg_table *sgt, enum dma_data_direction dir)
+	struct sg_table *sgt, enum dma_data_direction dma_dir)
 {
 	/* nothing to be done here */
 }
@@ -617,7 +615,7 @@ static void vb2_dma_sg_detach_dmabuf(void *mem_priv)
 }
 
 static void *vb2_dma_sg_attach_dmabuf(void *alloc_ctx, struct dma_buf *dbuf,
-	unsigned long size, int write)
+	unsigned long size, enum dma_data_direction dma_dir)
 {
 	struct vb2_dma_sg_conf *conf = alloc_ctx;
 	struct vb2_dma_sg_buf *buf;
@@ -639,7 +637,7 @@ static void *vb2_dma_sg_attach_dmabuf(void *alloc_ctx, struct dma_buf *dbuf,
 		return dba;
 	}
 
-	buf->dma_dir = write ? DMA_FROM_DEVICE : DMA_TO_DEVICE;
+	buf->dma_dir = dma_dir;
 	buf->size = size;
 	buf->db_attach = dba;
 
diff --git a/drivers/media/v4l2-core/videobuf2-vmalloc.c b/drivers/media/v4l2-core/videobuf2-vmalloc.c
index 437fbcd..0f79f8d 100644
--- a/drivers/media/v4l2-core/videobuf2-vmalloc.c
+++ b/drivers/media/v4l2-core/videobuf2-vmalloc.c
@@ -25,7 +25,7 @@ struct vb2_vmalloc_buf {
 	void				*vaddr;
 	struct page			**pages;
 	struct vm_area_struct		*vma;
-	int				write;
+	enum dma_data_direction		dma_dir;
 	unsigned long			size;
 	unsigned int			n_pages;
 	atomic_t			refcount;
@@ -38,8 +38,8 @@ struct vb2_vmalloc_buf {
 
 static void vb2_vmalloc_put(void *buf_priv);
 
-static void *vb2_vmalloc_alloc(void *alloc_ctx, unsigned long size, int write,
-			       gfp_t gfp_flags)
+static void *vb2_vmalloc_alloc(void *alloc_ctx, unsigned long size,
+			       enum dma_data_direction dma_dir, gfp_t gfp_flags)
 {
 	struct vb2_vmalloc_buf *buf;
 
@@ -74,7 +74,8 @@ static void vb2_vmalloc_put(void *buf_priv)
 }
 
 static void *vb2_vmalloc_get_userptr(void *alloc_ctx, unsigned long vaddr,
-				     unsigned long size, int write)
+				     unsigned long size,
+				     enum dma_data_direction dma_dir)
 {
 	struct vb2_vmalloc_buf *buf;
 	unsigned long first, last;
@@ -86,7 +87,7 @@ static void *vb2_vmalloc_get_userptr(void *alloc_ctx, unsigned long vaddr,
 	if (!buf)
 		return NULL;
 
-	buf->write = write;
+	buf->dma_dir = dma_dir;
 	offset = vaddr & ~PAGE_MASK;
 	buf->size = size;
 
@@ -111,7 +112,8 @@ static void *vb2_vmalloc_get_userptr(void *alloc_ctx, unsigned long vaddr,
 		/* current->mm->mmap_sem is taken by videobuf2 core */
 		n_pages = get_user_pages(current, current->mm,
 					 vaddr & PAGE_MASK, buf->n_pages,
-					 write, 1, /* force */
+					 dma_dir == DMA_FROM_DEVICE,
+					 1, /* force */
 					 buf->pages, NULL);
 		if (n_pages != buf->n_pages)
 			goto fail_get_user_pages;
@@ -148,7 +150,7 @@ static void vb2_vmalloc_put_userptr(void *buf_priv)
 		if (vaddr)
 			vm_unmap_ram((void *)vaddr, buf->n_pages);
 		for (i = 0; i < buf->n_pages; ++i) {
-			if (buf->write)
+			if (buf->dma_dir == DMA_FROM_DEVICE)
 				set_page_dirty_lock(buf->pages[i]);
 			put_page(buf->pages[i]);
 		}
@@ -414,7 +416,7 @@ static void vb2_vmalloc_detach_dmabuf(void *mem_priv)
 }
 
 static void *vb2_vmalloc_attach_dmabuf(void *alloc_ctx, struct dma_buf *dbuf,
-	unsigned long size, int write)
+	unsigned long size, enum dma_data_direction dma_dir)
 {
 	struct vb2_vmalloc_buf *buf;
 
@@ -426,7 +428,7 @@ static void *vb2_vmalloc_attach_dmabuf(void *alloc_ctx, struct dma_buf *dbuf,
 		return ERR_PTR(-ENOMEM);
 
 	buf->dbuf = dbuf;
-	buf->write = write;
+	buf->dma_dir = dma_dir;
 	buf->size = size;
 
 	return buf;
diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h
index 0ac65a6..bf8bde2 100644
--- a/include/media/videobuf2-core.h
+++ b/include/media/videobuf2-core.h
@@ -82,20 +82,23 @@ struct vb2_threadio_data;
  *				  unmap_dmabuf.
  */
 struct vb2_mem_ops {
-	void		*(*alloc)(void *alloc_ctx, unsigned long size, int write,
-				  gfp_t gfp_flags);
+	void		*(*alloc)(void *alloc_ctx, unsigned long size,
+				enum dma_data_direction dma_dir,
+				gfp_t gfp_flags);
 	void		(*put)(void *buf_priv);
 	struct dma_buf *(*get_dmabuf)(void *buf_priv, unsigned long flags);
 
 	void		*(*get_userptr)(void *alloc_ctx, unsigned long vaddr,
-					unsigned long size, int write);
+					unsigned long size,
+					enum dma_data_direction dma_dir);
 	void		(*put_userptr)(void *buf_priv);
 
 	int		(*prepare)(void *buf_priv);
 	void		(*finish)(void *buf_priv);
 
 	void		*(*attach_dmabuf)(void *alloc_ctx, struct dma_buf *dbuf,
-				unsigned long size, int write);
+					  unsigned long size,
+					  enum dma_data_direction dma_dir);
 	void		(*detach_dmabuf)(void *buf_priv);
 	int		(*map_dmabuf)(void *buf_priv);
 	void		(*unmap_dmabuf)(void *buf_priv);
-- 
2.1.0


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [RFCv2 PATCH 10/14] vb2: add 'new_cookies' flag
  2014-09-12 12:59 [RFCv2 PATCH 00/14] vb2: improve dma-sg, expbuf Hans Verkuil
                   ` (8 preceding siblings ...)
  2014-09-12 12:59 ` [RFCv2 PATCH 09/14] vb2: replace 'write' by 'dma_dir' Hans Verkuil
@ 2014-09-12 12:59 ` Hans Verkuil
  2014-09-14  7:02   ` Pawel Osciak
  2014-09-12 13:00 ` [RFCv2 PATCH 11/14] tw68: only reprogram DMA engine when necessary Hans Verkuil
                   ` (3 subsequent siblings)
  13 siblings, 1 reply; 23+ messages in thread
From: Hans Verkuil @ 2014-09-12 12:59 UTC (permalink / raw)
  To: linux-media; +Cc: pawel, m.szyprowski, laurent.pinchart, Hans Verkuil

From: Hans Verkuil <hans.verkuil@cisco.com>

This flag helps drivers that need to reprogram their DMA engine whenever
a plane cookie (== DMA address or DMA scatter-gather list) changes.

Otherwise they would have to reprogram the DMA engine for every frame.

Note that it is not possible to do this in buf_init() since dma_map_sg has
to be done first, which happens just before buf_prepare() in the prepare()
memop. It is dma_map_sg that sets up the dma addresses that are needed to
configure the DMA engine.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 drivers/media/v4l2-core/videobuf2-core.c |  5 +++++
 include/media/videobuf2-core.h           | 14 ++++++++++++--
 2 files changed, 17 insertions(+), 2 deletions(-)

diff --git a/drivers/media/v4l2-core/videobuf2-core.c b/drivers/media/v4l2-core/videobuf2-core.c
index 01bab25..7217eb1 100644
--- a/drivers/media/v4l2-core/videobuf2-core.c
+++ b/drivers/media/v4l2-core/videobuf2-core.c
@@ -391,6 +391,7 @@ static int __vb2_queue_alloc(struct vb2_queue *q, enum v4l2_memory memory,
 				kfree(vb);
 				break;
 			}
+			vb->new_cookies = 1;
 		}
 
 		q->bufs[q->num_buffers + buffer] = vb;
@@ -1373,6 +1374,8 @@ static int __buf_memory_prepare(struct vb2_buffer *vb)
 		for (plane = 0; plane < vb->num_planes; ++plane)
 			call_void_memop(vb, finish, vb->planes[plane].mem_priv);
 		call_void_vb_qop(vb, buf_finish_for_cpu, vb);
+	} else {
+		vb->new_cookies = 0;
 	}
 	return ret;
 }
@@ -1467,6 +1470,7 @@ static int __qbuf_userptr(struct vb2_buffer *vb, const struct v4l2_buffer *b)
 			dprintk(1, "buffer initialization failed\n");
 			goto err;
 		}
+		vb->new_cookies = 1;
 	}
 
 	ret = __buf_memory_prepare(vb);
@@ -1591,6 +1595,7 @@ static int __qbuf_dmabuf(struct vb2_buffer *vb, const struct v4l2_buffer *b)
 			dprintk(1, "buffer initialization failed\n");
 			goto err;
 		}
+		vb->new_cookies = 1;
 	}
 
 	ret = __buf_memory_prepare(vb);
diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h
index bf8bde2..9304718 100644
--- a/include/media/videobuf2-core.h
+++ b/include/media/videobuf2-core.h
@@ -186,6 +186,11 @@ struct vb2_queue;
  * @vb2_queue:		the queue to which this driver belongs
  * @num_planes:		number of planes in the buffer
  *			on an internal driver queue
+ * @new_cookies:	the planes of the buffer have new cookie values.
+ *			This happens if a new userptr or dmabuf is used for one
+ *			or more of the buffer planes. This will change the cookie
+ *			value of those planes and you may need to reprogram the DMA
+ *			engine when buf_prepare is called.
  * @state:		current buffer state; do not change
  * @queued_entry:	entry on the queued buffers list, which holds all
  *			buffers queued from userspace
@@ -200,6 +205,7 @@ struct vb2_buffer {
 	struct vb2_queue	*vb2_queue;
 
 	unsigned int		num_planes;
+	unsigned int		new_cookies:1;
 
 /* Private: internal use only */
 	enum vb2_buffer_state	state;
@@ -290,8 +296,12 @@ struct vb2_buffer {
  *			hardware operation in this callback; drivers that
  *			support	VIDIOC_CREATE_BUFS must also validate the
  *			buffer size, if they haven't done that yet in
- *			@buf_prepare_for_cpu. If an error is returned, the
- *			buffer will not be queued in the driver; optional.
+ *			@buf_prepare_for_cpu. If one or more of the plane
+ *			cookies (see vb2_plane_cookie) are updated, then
+ *			vb->new_cookies is set to 1. If buf_prepare returns
+ *			0 (success), then new_cookies is cleared automatically.
+ *			If an error is returned, then the buffer will not be
+ *			queued in the driver; optional.
  * @buf_finish:		called before every dequeue of the buffer back to
  *			userspace; the contents of the buffer cannot be
  *			accessed by the cpu at this stage as it is still setup
-- 
2.1.0


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [RFCv2 PATCH 11/14] tw68: only reprogram DMA engine when necessary
  2014-09-12 12:59 [RFCv2 PATCH 00/14] vb2: improve dma-sg, expbuf Hans Verkuil
                   ` (9 preceding siblings ...)
  2014-09-12 12:59 ` [RFCv2 PATCH 10/14] vb2: add 'new_cookies' flag Hans Verkuil
@ 2014-09-12 13:00 ` Hans Verkuil
  2014-09-12 13:00 ` [RFCv2 PATCH 12/14] cx23885: " Hans Verkuil
                   ` (2 subsequent siblings)
  13 siblings, 0 replies; 23+ messages in thread
From: Hans Verkuil @ 2014-09-12 13:00 UTC (permalink / raw)
  To: linux-media; +Cc: pawel, m.szyprowski, laurent.pinchart, Hans Verkuil

From: Hans Verkuil <hans.verkuil@cisco.com>

Use the new 'new_cookies' flag to determine when the risc program
needs to be regenerated.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 drivers/media/pci/tw68/tw68-video.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/drivers/media/pci/tw68/tw68-video.c b/drivers/media/pci/tw68/tw68-video.c
index 13e9650..c69e6a5 100644
--- a/drivers/media/pci/tw68/tw68-video.c
+++ b/drivers/media/pci/tw68/tw68-video.c
@@ -468,6 +468,9 @@ static int tw68_buf_prepare(struct vb2_buffer *vb)
 		return -EINVAL;
 	vb2_set_plane_payload(vb, 0, size);
 
+	if (!vb->new_cookies)
+		return 0;
+
 	bpl = (dev->width * dev->fmt->depth) >> 3;
 	switch (dev->field) {
 	case V4L2_FIELD_TOP:
@@ -497,13 +500,15 @@ static int tw68_buf_prepare(struct vb2_buffer *vb)
 	return 0;
 }
 
-static void tw68_buf_finish_for_cpu(struct vb2_buffer *vb)
+static void tw68_buf_cleanup(struct vb2_buffer *vb)
 {
 	struct vb2_queue *vq = vb->vb2_queue;
 	struct tw68_dev *dev = vb2_get_drv_priv(vq);
 	struct tw68_buf *buf = container_of(vb, struct tw68_buf, vb);
 
-	pci_free_consistent(dev->pci, buf->size, buf->cpu, buf->dma);
+	if (buf->cpu)
+		pci_free_consistent(dev->pci, buf->size, buf->cpu, buf->dma);
+	buf->cpu = NULL;
 }
 
 static int tw68_start_streaming(struct vb2_queue *q, unsigned int count)
@@ -536,7 +541,7 @@ static struct vb2_ops tw68_video_qops = {
 	.queue_setup	= tw68_queue_setup,
 	.buf_queue	= tw68_buf_queue,
 	.buf_prepare	= tw68_buf_prepare,
-	.buf_finish_for_cpu = tw68_buf_finish_for_cpu,
+	.buf_cleanup	= tw68_buf_cleanup,
 	.start_streaming = tw68_start_streaming,
 	.stop_streaming = tw68_stop_streaming,
 	.wait_prepare	= vb2_ops_wait_prepare,
-- 
2.1.0


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [RFCv2 PATCH 12/14] cx23885: only reprogram DMA engine when necessary
  2014-09-12 12:59 [RFCv2 PATCH 00/14] vb2: improve dma-sg, expbuf Hans Verkuil
                   ` (10 preceding siblings ...)
  2014-09-12 13:00 ` [RFCv2 PATCH 11/14] tw68: only reprogram DMA engine when necessary Hans Verkuil
@ 2014-09-12 13:00 ` Hans Verkuil
  2014-09-12 13:00 ` [RFCv2 PATCH 13/14] saa7134: don't rebuild the page table unless new_cookies is set Hans Verkuil
  2014-09-12 13:00 ` [RFCv2 PATCH 14/14] vivid: enable vb2_expbuf support Hans Verkuil
  13 siblings, 0 replies; 23+ messages in thread
From: Hans Verkuil @ 2014-09-12 13:00 UTC (permalink / raw)
  To: linux-media; +Cc: pawel, m.szyprowski, laurent.pinchart, Hans Verkuil

From: Hans Verkuil <hans.verkuil@cisco.com>

Use the new 'new_cookies' flag to determine when the risc program
needs to be regenerated.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 drivers/media/pci/cx23885/cx23885-417.c   | 4 ++--
 drivers/media/pci/cx23885/cx23885-core.c  | 7 ++++++-
 drivers/media/pci/cx23885/cx23885-dvb.c   | 4 ++--
 drivers/media/pci/cx23885/cx23885-vbi.c   | 7 +++++--
 drivers/media/pci/cx23885/cx23885-video.c | 7 +++++--
 5 files changed, 20 insertions(+), 9 deletions(-)

diff --git a/drivers/media/pci/cx23885/cx23885-417.c b/drivers/media/pci/cx23885/cx23885-417.c
index 5d36950..c7f2c54 100644
--- a/drivers/media/pci/cx23885/cx23885-417.c
+++ b/drivers/media/pci/cx23885/cx23885-417.c
@@ -1162,7 +1162,7 @@ static int buffer_prepare(struct vb2_buffer *vb)
 	return cx23885_buf_prepare(buf, &dev->ts1);
 }
 
-static void buffer_finish_for_cpu(struct vb2_buffer *vb)
+static void buffer_cleanup(struct vb2_buffer *vb)
 {
 	struct cx23885_dev *dev = vb->vb2_queue->drv_priv;
 	struct cx23885_buffer *buf = container_of(vb,
@@ -1224,7 +1224,7 @@ static void cx23885_stop_streaming(struct vb2_queue *q)
 static struct vb2_ops cx23885_qops = {
 	.queue_setup    = queue_setup,
 	.buf_prepare  = buffer_prepare,
-	.buf_finish_for_cpu = buffer_finish_for_cpu,
+	.buf_cleanup = buffer_cleanup,
 	.buf_queue    = buffer_queue,
 	.wait_prepare = vb2_ops_wait_prepare,
 	.wait_finish = vb2_ops_wait_finish,
diff --git a/drivers/media/pci/cx23885/cx23885-core.c b/drivers/media/pci/cx23885/cx23885-core.c
index e2e5afb..6b2dac0 100644
--- a/drivers/media/pci/cx23885/cx23885-core.c
+++ b/drivers/media/pci/cx23885/cx23885-core.c
@@ -1249,7 +1249,9 @@ void cx23885_free_buffer(struct cx23885_dev *dev, struct cx23885_buffer *buf)
 	struct cx23885_riscmem *risc = &buf->risc;
 
 	BUG_ON(in_interrupt());
-	pci_free_consistent(dev->pci, risc->size, risc->cpu, risc->dma);
+	if (risc->cpu)
+		pci_free_consistent(dev->pci, risc->size, risc->cpu, risc->dma);
+	risc->cpu = NULL;
 }
 
 static void cx23885_tsport_reg_dump(struct cx23885_tsport *port)
@@ -1459,6 +1461,9 @@ int cx23885_buf_prepare(struct cx23885_buffer *buf, struct cx23885_tsport *port)
 		return -EINVAL;
 	vb2_set_plane_payload(&buf->vb, 0, size);
 
+	if (!buf->vb.new_cookies)
+		return 0;
+
 	cx23885_risc_databuffer(dev->pci, &buf->risc,
 				sgt->sgl,
 				port->ts_packet_size, port->ts_packet_count, 0);
diff --git a/drivers/media/pci/cx23885/cx23885-dvb.c b/drivers/media/pci/cx23885/cx23885-dvb.c
index 77eb034..d8fc87f 100644
--- a/drivers/media/pci/cx23885/cx23885-dvb.c
+++ b/drivers/media/pci/cx23885/cx23885-dvb.c
@@ -112,7 +112,7 @@ static int buffer_prepare(struct vb2_buffer *vb)
 	return cx23885_buf_prepare(buf, port);
 }
 
-static void buffer_finish_for_cpu(struct vb2_buffer *vb)
+static void buffer_cleanup(struct vb2_buffer *vb)
 {
 	struct cx23885_tsport *port = vb->vb2_queue->drv_priv;
 	struct cx23885_dev *dev = port->dev;
@@ -168,7 +168,7 @@ static void cx23885_stop_streaming(struct vb2_queue *q)
 static struct vb2_ops dvb_qops = {
 	.queue_setup    = queue_setup,
 	.buf_prepare  = buffer_prepare,
-	.buf_finish_for_cpu = buffer_finish_for_cpu,
+	.buf_cleanup = buffer_cleanup,
 	.buf_queue    = buffer_queue,
 	.wait_prepare = vb2_ops_wait_prepare,
 	.wait_finish = vb2_ops_wait_finish,
diff --git a/drivers/media/pci/cx23885/cx23885-vbi.c b/drivers/media/pci/cx23885/cx23885-vbi.c
index d7a69c6..b93626c 100644
--- a/drivers/media/pci/cx23885/cx23885-vbi.c
+++ b/drivers/media/pci/cx23885/cx23885-vbi.c
@@ -151,6 +151,9 @@ static int buffer_prepare(struct vb2_buffer *vb)
 		return -EINVAL;
 	vb2_set_plane_payload(vb, 0, lines * VBI_LINE_LENGTH * 2);
 
+	if (!vb->new_cookies)
+		return 0;
+
 	cx23885_risc_vbibuffer(dev->pci, &buf->risc,
 			 sgt->sgl,
 			 0, VBI_LINE_LENGTH * lines,
@@ -159,7 +162,7 @@ static int buffer_prepare(struct vb2_buffer *vb)
 	return 0;
 }
 
-static void buffer_finish_for_cpu(struct vb2_buffer *vb)
+static void buffer_cleanup(struct vb2_buffer *vb)
 {
 	struct cx23885_buffer *buf = container_of(vb,
 		struct cx23885_buffer, vb);
@@ -254,7 +257,7 @@ static void cx23885_stop_streaming(struct vb2_queue *q)
 struct vb2_ops cx23885_vbi_qops = {
 	.queue_setup    = queue_setup,
 	.buf_prepare  = buffer_prepare,
-	.buf_finish_for_cpu = buffer_finish_for_cpu,
+	.buf_cleanup = buffer_cleanup,
 	.buf_queue    = buffer_queue,
 	.wait_prepare = vb2_ops_wait_prepare,
 	.wait_finish = vb2_ops_wait_finish,
diff --git a/drivers/media/pci/cx23885/cx23885-video.c b/drivers/media/pci/cx23885/cx23885-video.c
index d533f4e..ba30fa8 100644
--- a/drivers/media/pci/cx23885/cx23885-video.c
+++ b/drivers/media/pci/cx23885/cx23885-video.c
@@ -342,6 +342,9 @@ static int buffer_prepare(struct vb2_buffer *vb)
 		return -EINVAL;
 	vb2_set_plane_payload(vb, 0, dev->height * buf->bpl);
 
+	if (!vb->new_cookies)
+		return 0;
+
 	switch (dev->field) {
 	case V4L2_FIELD_TOP:
 		cx23885_risc_buffer(dev->pci, &buf->risc,
@@ -407,7 +410,7 @@ static int buffer_prepare(struct vb2_buffer *vb)
 	return 0;
 }
 
-static void buffer_finish_for_cpu(struct vb2_buffer *vb)
+static void buffer_cleanup(struct vb2_buffer *vb)
 {
 	struct cx23885_buffer *buf = container_of(vb,
 		struct cx23885_buffer, vb);
@@ -500,7 +503,7 @@ static void cx23885_stop_streaming(struct vb2_queue *q)
 static struct vb2_ops cx23885_video_qops = {
 	.queue_setup    = queue_setup,
 	.buf_prepare  = buffer_prepare,
-	.buf_finish_for_cpu = buffer_finish_for_cpu,
+	.buf_cleanup = buffer_cleanup,
 	.buf_queue    = buffer_queue,
 	.wait_prepare = vb2_ops_wait_prepare,
 	.wait_finish = vb2_ops_wait_finish,
-- 
2.1.0


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [RFCv2 PATCH 13/14] saa7134: don't rebuild the page table unless new_cookies is set.
  2014-09-12 12:59 [RFCv2 PATCH 00/14] vb2: improve dma-sg, expbuf Hans Verkuil
                   ` (11 preceding siblings ...)
  2014-09-12 13:00 ` [RFCv2 PATCH 12/14] cx23885: " Hans Verkuil
@ 2014-09-12 13:00 ` Hans Verkuil
  2014-09-12 13:00 ` [RFCv2 PATCH 14/14] vivid: enable vb2_expbuf support Hans Verkuil
  13 siblings, 0 replies; 23+ messages in thread
From: Hans Verkuil @ 2014-09-12 13:00 UTC (permalink / raw)
  To: linux-media; +Cc: pawel, m.szyprowski, laurent.pinchart, Hans Verkuil

From: Hans Verkuil <hans.verkuil@cisco.com>

Only if new_cookies is set do you need to build the page table,
otherwise it will be unchanged.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 drivers/media/pci/saa7134/saa7134-ts.c    | 2 ++
 drivers/media/pci/saa7134/saa7134-vbi.c   | 2 ++
 drivers/media/pci/saa7134/saa7134-video.c | 2 ++
 3 files changed, 6 insertions(+)

diff --git a/drivers/media/pci/saa7134/saa7134-ts.c b/drivers/media/pci/saa7134/saa7134-ts.c
index 2709b83..4095776 100644
--- a/drivers/media/pci/saa7134/saa7134-ts.c
+++ b/drivers/media/pci/saa7134/saa7134-ts.c
@@ -107,6 +107,8 @@ int saa7134_ts_buffer_prepare(struct vb2_buffer *vb2)
 	vb2_set_plane_payload(vb2, 0, size);
 	vb2->v4l2_buf.field = dev->field;
 
+	if (!vb2->new_cookies)
+		return 0;
 	return saa7134_pgtable_build(dev->pci, &dmaq->pt, dma->sgl, dma->nents,
 				    saa7134_buffer_startpage(buf));
 }
diff --git a/drivers/media/pci/saa7134/saa7134-vbi.c b/drivers/media/pci/saa7134/saa7134-vbi.c
index e188903..5b422d0 100644
--- a/drivers/media/pci/saa7134/saa7134-vbi.c
+++ b/drivers/media/pci/saa7134/saa7134-vbi.c
@@ -131,6 +131,8 @@ static int buffer_prepare(struct vb2_buffer *vb2)
 
 	vb2_set_plane_payload(vb2, 0, size);
 
+	if (!vb2->new_cookies)
+		return 0;
 	return saa7134_pgtable_build(dev->pci, &dmaq->pt, dma->sgl, dma->nents,
 				    saa7134_buffer_startpage(buf));
 }
diff --git a/drivers/media/pci/saa7134/saa7134-video.c b/drivers/media/pci/saa7134/saa7134-video.c
index 15a686b..3ab39be 100644
--- a/drivers/media/pci/saa7134/saa7134-video.c
+++ b/drivers/media/pci/saa7134/saa7134-video.c
@@ -895,6 +895,8 @@ static int buffer_prepare(struct vb2_buffer *vb2)
 	vb2_set_plane_payload(vb2, 0, size);
 	vb2->v4l2_buf.field = dev->field;
 
+	if (!vb2->new_cookies)
+		return 0;
 	return saa7134_pgtable_build(dev->pci, &dmaq->pt, dma->sgl, dma->nents,
 				    saa7134_buffer_startpage(buf));
 }
-- 
2.1.0


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [RFCv2 PATCH 14/14] vivid: enable vb2_expbuf support.
  2014-09-12 12:59 [RFCv2 PATCH 00/14] vb2: improve dma-sg, expbuf Hans Verkuil
                   ` (12 preceding siblings ...)
  2014-09-12 13:00 ` [RFCv2 PATCH 13/14] saa7134: don't rebuild the page table unless new_cookies is set Hans Verkuil
@ 2014-09-12 13:00 ` Hans Verkuil
  13 siblings, 0 replies; 23+ messages in thread
From: Hans Verkuil @ 2014-09-12 13:00 UTC (permalink / raw)
  To: linux-media; +Cc: pawel, m.szyprowski, laurent.pinchart, Hans Verkuil

From: Hans Verkuil <hans.verkuil@cisco.com>

Now that vb2 supports DMABUF export for dma-sg and vmalloc memory
modes, we can enable the vb2_expbuf support in vivid.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 drivers/media/platform/vivid/vivid-core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/platform/vivid/vivid-core.c b/drivers/media/platform/vivid/vivid-core.c
index 2c61a62..7de8d9d 100644
--- a/drivers/media/platform/vivid/vivid-core.c
+++ b/drivers/media/platform/vivid/vivid-core.c
@@ -588,7 +588,7 @@ static const struct v4l2_ioctl_ops vivid_ioctl_ops = {
 	.vidioc_querybuf		= vb2_ioctl_querybuf,
 	.vidioc_qbuf			= vb2_ioctl_qbuf,
 	.vidioc_dqbuf			= vb2_ioctl_dqbuf,
-/* Not yet	.vidioc_expbuf		= vb2_ioctl_expbuf,*/
+	.vidioc_expbuf			= vb2_ioctl_expbuf,
 	.vidioc_streamon		= vb2_ioctl_streamon,
 	.vidioc_streamoff		= vb2_ioctl_streamoff,
 
-- 
2.1.0


^ permalink raw reply related	[flat|nested] 23+ messages in thread

* Re: [RFCv2 PATCH 01/14] vb2: introduce buf_prepare/finish_for_cpu
  2014-09-12 12:59 ` [RFCv2 PATCH 01/14] vb2: introduce buf_prepare/finish_for_cpu Hans Verkuil
@ 2014-09-12 21:25   ` Laurent Pinchart
  0 siblings, 0 replies; 23+ messages in thread
From: Laurent Pinchart @ 2014-09-12 21:25 UTC (permalink / raw)
  To: Hans Verkuil; +Cc: linux-media, pawel, m.szyprowski, Hans Verkuil

Hi Hans,

Thank you for the patch.

On Friday 12 September 2014 14:59:50 Hans Verkuil wrote:
> From: Hans Verkuil <hans.verkuil@cisco.com>
> 
> This splits the buf_prepare and buf_finish actions into two: one
> called while the cpu can still access the buffer contents, and one where
> the memory has been prepared for DMA and the cpu no longer can access it.

I don't think this applies to all drivers, or rather to all memory models. vb2 
vmalloc allows drivers to touch buffers that have been prepared, and USB 
drivers certainly expect that behaviour in order to copy the content of URBs 
to the buffer as they are received.

> Update a few drivers that use buf_finish where they really meant
> buf_finish_for_cpu.

I don't think this applies to the UVC driver. The buf_finish implementation 
doesn't touch the contents of the buffer.

> The reason for this split is that some drivers need to modify the buffer,
> either before or after the DMA has taken place, in order to e.g. add JPEG
> headers or do other touch ups.
> 
> You cannot do that in buf_prepare since at that time the buffer is already
> synced for DMA and the CPU shouldn't touch it. So add these extra ops to
> make this explicit.
> 
> Note that the dma-sg memory model doesn't sync the buffers yet in the memop
> prepare. This will change in future patches.
> 
> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
> ---
>  drivers/media/parport/bw-qcam.c              |  4 +--
>  drivers/media/pci/sta2x11/sta2x11_vip.c      |  4 +--
>  drivers/media/platform/vivid/vivid-vid-cap.c |  4 +--
>  drivers/media/usb/go7007/go7007-v4l2.c       |  4 +--
>  drivers/media/usb/pwc/pwc-if.c               |  4 +--
>  drivers/media/usb/uvc/uvc_queue.c            |  4 +--
>  drivers/media/v4l2-core/videobuf2-core.c     | 29 ++++++++++++-----
>  include/media/videobuf2-core.h               | 48 +++++++++++++++++++------
>  8 files changed, 72 insertions(+), 29 deletions(-)

-- 
Regards,

Laurent Pinchart


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [RFCv2 PATCH 02/14] vb2-dma-sg: add allocation context to dma-sg
  2014-09-12 12:59 ` [RFCv2 PATCH 02/14] vb2-dma-sg: add allocation context to dma-sg Hans Verkuil
@ 2014-09-12 21:49   ` Laurent Pinchart
  2014-09-14  3:29   ` Pawel Osciak
  1 sibling, 0 replies; 23+ messages in thread
From: Laurent Pinchart @ 2014-09-12 21:49 UTC (permalink / raw)
  To: Hans Verkuil; +Cc: linux-media, pawel, m.szyprowski, Hans Verkuil

Hi Hans,

Thank you for the patch.

On Friday 12 September 2014 14:59:51 Hans Verkuil wrote:
> From: Hans Verkuil <hans.verkuil@cisco.com>
> 
> Require that dma-sg also uses an allocation context. This is in preparation
> for adding prepare/finish memops to sync the memory between DMA and CPU.
>
> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
> ---
>  drivers/media/pci/cx23885/cx23885-417.c         |  1 +
>  drivers/media/pci/cx23885/cx23885-core.c        | 10 +++++-
>  drivers/media/pci/cx23885/cx23885-dvb.c         |  1 +
>  drivers/media/pci/cx23885/cx23885-vbi.c         |  1 +
>  drivers/media/pci/cx23885/cx23885-video.c       |  1 +
>  drivers/media/pci/cx23885/cx23885.h             |  1 +
>  drivers/media/pci/saa7134/saa7134-core.c        | 18 +++++++---
>  drivers/media/pci/saa7134/saa7134-ts.c          |  1 +
>  drivers/media/pci/saa7134/saa7134-vbi.c         |  1 +
>  drivers/media/pci/saa7134/saa7134-video.c       |  1 +
>  drivers/media/pci/saa7134/saa7134.h             |  1 +
>  drivers/media/pci/solo6x10/solo6x10-v4l2-enc.c  | 10 ++++++
>  drivers/media/pci/solo6x10/solo6x10.h           |  1 +
>  drivers/media/pci/tw68/tw68-core.c              | 15 +++++++--
>  drivers/media/pci/tw68/tw68-video.c             |  1 +
>  drivers/media/pci/tw68/tw68.h                   |  1 +
>  drivers/media/platform/marvell-ccic/mcam-core.c |  7 ++++
>  drivers/media/platform/marvell-ccic/mcam-core.h |  1 +
>  drivers/media/v4l2-core/videobuf2-core.c        |  3 +-
>  drivers/media/v4l2-core/videobuf2-dma-contig.c  |  4 ++-
>  drivers/media/v4l2-core/videobuf2-dma-sg.c      | 44 ++++++++++++++++++++--
>  drivers/media/v4l2-core/videobuf2-vmalloc.c     |  3 +-
>  include/media/videobuf2-core.h                  |  3 +-
>  include/media/videobuf2-dma-sg.h                |  3 ++
>  24 files changed, 118 insertions(+), 15 deletions(-)

[snip]

> diff --git a/drivers/media/v4l2-core/videobuf2-core.c
> b/drivers/media/v4l2-core/videobuf2-core.c index e5247a4..087cd62 100644
> --- a/drivers/media/v4l2-core/videobuf2-core.c
> +++ b/drivers/media/v4l2-core/videobuf2-core.c
> @@ -189,6 +189,7 @@ static void __vb2_queue_cancel(struct vb2_queue *q);
>  static int __vb2_buf_mem_alloc(struct vb2_buffer *vb)
>  {
>  	struct vb2_queue *q = vb->vb2_queue;
> +	int write = !V4L2_TYPE_IS_OUTPUT(q->type);
>  	void *mem_priv;
>  	int plane;
> 
> @@ -200,7 +201,7 @@ static int __vb2_buf_mem_alloc(struct vb2_buffer *vb)
>  		unsigned long size = PAGE_ALIGN(q->plane_sizes[plane]);
> 
>  		mem_priv = call_ptr_memop(vb, alloc, q->alloc_ctx[plane],
> -				      size, q->gfp_flags);
> +				      size, write, q->gfp_flags);
>  		if (IS_ERR_OR_NULL(mem_priv))
>  			goto free;
> 
> diff --git a/drivers/media/v4l2-core/videobuf2-dma-contig.c
> b/drivers/media/v4l2-core/videobuf2-dma-contig.c index 4a02ade..6675f12
> 100644
> --- a/drivers/media/v4l2-core/videobuf2-dma-contig.c
> +++ b/drivers/media/v4l2-core/videobuf2-dma-contig.c
> @@ -155,7 +155,8 @@ static void vb2_dc_put(void *buf_priv)
>  	kfree(buf);
>  }
> 
> -static void *vb2_dc_alloc(void *alloc_ctx, unsigned long size, gfp_t
> gfp_flags)
> +static void *vb2_dc_alloc(void *alloc_ctx, unsigned long size, int write,
> +			  gfp_t gfp_flags)
>  {
>  	struct vb2_dc_conf *conf = alloc_ctx;
>  	struct device *dev = conf->dev;
> @@ -176,6 +177,7 @@ static void *vb2_dc_alloc(void *alloc_ctx, unsigned long
> size, gfp_t gfp_flags) /* Prevent the device from being released while the
> buffer is used */ buf->dev = get_device(dev);
>  	buf->size = size;
> +	buf->dma_dir = write ? DMA_FROM_DEVICE : DMA_TO_DEVICE;

This is an unrelated bug fix, it should be split to a separate patch.

>  	buf->handler.refcount = &buf->refcount;
>  	buf->handler.put = vb2_dc_put;
> diff --git a/drivers/media/v4l2-core/videobuf2-dma-sg.c
> b/drivers/media/v4l2-core/videobuf2-dma-sg.c index adefc31..9b7a041 100644
> --- a/drivers/media/v4l2-core/videobuf2-dma-sg.c
> +++ b/drivers/media/v4l2-core/videobuf2-dma-sg.c
> @@ -30,11 +30,17 @@ module_param(debug, int, 0644);
>  			printk(KERN_DEBUG "vb2-dma-sg: " fmt, ## arg);	\
>  	} while (0)
> 
> +struct vb2_dma_sg_conf {
> +	struct device		*dev;
> +};
> +
>  struct vb2_dma_sg_buf {
> +	struct device			*dev;
>  	void				*vaddr;
>  	struct page			**pages;
>  	int				write;
>  	int				offset;
> +	enum dma_data_direction		dma_dir;
>  	struct sg_table			sg_table;
>  	size_t				size;
>  	unsigned int			num_pages;
> @@ -86,22 +92,27 @@ static int vb2_dma_sg_alloc_compacted(struct
> vb2_dma_sg_buf *buf, return 0;
>  }
> 
> -static void *vb2_dma_sg_alloc(void *alloc_ctx, unsigned long size, gfp_t
> gfp_flags)
> +static void *vb2_dma_sg_alloc(void *alloc_ctx, unsigned long size, int
> write,
> +			      gfp_t gfp_flags)
>  {
> +	struct vb2_dma_sg_conf *conf = alloc_ctx;
>  	struct vb2_dma_sg_buf *buf;
>  	int ret;
>  	int num_pages;
> 
> +	if (WARN_ON(alloc_ctx == NULL))
> +		return NULL;
>  	buf = kzalloc(sizeof *buf, GFP_KERNEL);
>  	if (!buf)
>  		return NULL;
> 
>  	buf->vaddr = NULL;
> -	buf->write = 0;
> +	buf->write = write;
>  	buf->offset = 0;
>  	buf->size = size;
>  	/* size is already page aligned */
>  	buf->num_pages = size >> PAGE_SHIFT;
> +	buf->dma_dir = write ? DMA_FROM_DEVICE : DMA_TO_DEVICE;
> 
>  	buf->pages = kzalloc(buf->num_pages * sizeof(struct page *),
>  			     GFP_KERNEL);
> @@ -117,6 +128,8 @@ static void *vb2_dma_sg_alloc(void *alloc_ctx, unsigned
> long size, gfp_t gfp_fla if (ret)
>  		goto fail_table_alloc;
> 
> +	/* Prevent the device from being released while the buffer is used */
> +	buf->dev = get_device(conf->dev);
>  	buf->handler.refcount = &buf->refcount;
>  	buf->handler.put = vb2_dma_sg_put;
>  	buf->handler.arg = buf;
> @@ -152,6 +165,7 @@ static void vb2_dma_sg_put(void *buf_priv)
>  		while (--i >= 0)
>  			__free_page(buf->pages[i]);
>  		kfree(buf->pages);
> +		put_device(buf->dev);
>  		kfree(buf);
>  	}
>  }
> @@ -164,6 +178,7 @@ static inline int vma_is_io(struct vm_area_struct *vma)
>  static void *vb2_dma_sg_get_userptr(void *alloc_ctx, unsigned long vaddr,
>  				    unsigned long size, int write)
>  {
> +	struct vb2_dma_sg_conf *conf = alloc_ctx;
>  	struct vb2_dma_sg_buf *buf;
>  	unsigned long first, last;
>  	int num_pages_from_user;
> @@ -177,6 +192,7 @@ static void *vb2_dma_sg_get_userptr(void *alloc_ctx,
> unsigned long vaddr, buf->write = write;
>  	buf->offset = vaddr & ~PAGE_MASK;
>  	buf->size = size;
> +	buf->dma_dir = write ? DMA_FROM_DEVICE : DMA_TO_DEVICE;
> 
>  	first = (vaddr           & PAGE_MASK) >> PAGE_SHIFT;
>  	last  = ((vaddr + size - 1) & PAGE_MASK) >> PAGE_SHIFT;
> @@ -233,6 +249,8 @@ static void *vb2_dma_sg_get_userptr(void *alloc_ctx,
> unsigned long vaddr, buf->num_pages, buf->offset, size, 0))
>  		goto userptr_fail_alloc_table_from_pages;
> 
> +	/* Prevent the device from being released while the buffer is used */
> +	buf->dev = get_device(conf->dev);
>  	return buf;
> 
>  userptr_fail_alloc_table_from_pages:
> @@ -272,6 +290,7 @@ static void vb2_dma_sg_put_userptr(void *buf_priv)
>  	}
>  	kfree(buf->pages);
>  	vb2_put_vma(buf->vma);
> +	put_device(buf->dev);
>  	kfree(buf);
>  }
> 
> @@ -354,6 +373,27 @@ const struct vb2_mem_ops vb2_dma_sg_memops = {
>  };
>  EXPORT_SYMBOL_GPL(vb2_dma_sg_memops);
> 
> +void *vb2_dma_sg_init_ctx(struct device *dev)
> +{
> +	struct vb2_dma_sg_conf *conf;
> +
> +	conf = kzalloc(sizeof(*conf), GFP_KERNEL);
> +	if (!conf)
> +		return ERR_PTR(-ENOMEM);
> +
> +	conf->dev = dev;

This is unrelated to this patch, but given that the dc and dma-sg allocation 
contexts only need to store a struct device pointer, wouldn't it be simpler to 
teach vb2 about struct device ?

> +
> +	return conf;
> +}
> +EXPORT_SYMBOL_GPL(vb2_dma_sg_init_ctx);
> +
> +void vb2_dma_sg_cleanup_ctx(void *alloc_ctx)
> +{
> +	if (!IS_ERR_OR_NULL(alloc_ctx))
> +		kfree(alloc_ctx);
> +}
> +EXPORT_SYMBOL_GPL(vb2_dma_sg_cleanup_ctx);
> +
>  MODULE_DESCRIPTION("dma scatter/gather memory handling routines for
> videobuf2"); MODULE_AUTHOR("Andrzej Pietrasiewicz");
>  MODULE_LICENSE("GPL");
> diff --git a/drivers/media/v4l2-core/videobuf2-vmalloc.c
> b/drivers/media/v4l2-core/videobuf2-vmalloc.c index 313d977..d77e397 100644
> --- a/drivers/media/v4l2-core/videobuf2-vmalloc.c
> +++ b/drivers/media/v4l2-core/videobuf2-vmalloc.c
> @@ -35,7 +35,8 @@ struct vb2_vmalloc_buf {
> 
>  static void vb2_vmalloc_put(void *buf_priv);
> 
> -static void *vb2_vmalloc_alloc(void *alloc_ctx, unsigned long size, gfp_t
> gfp_flags)
> +static void *vb2_vmalloc_alloc(void *alloc_ctx, unsigned long size, int
> write,
> +			       gfp_t gfp_flags)
>  {
>  	struct vb2_vmalloc_buf *buf;
> 
> diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h
> index fff159c..02b96ef 100644
> --- a/include/media/videobuf2-core.h
> +++ b/include/media/videobuf2-core.h
> @@ -82,7 +82,8 @@ struct vb2_threadio_data;
>   *				  unmap_dmabuf.
>   */
>  struct vb2_mem_ops {
> -	void		*(*alloc)(void *alloc_ctx, unsigned long size, gfp_t gfp_flags);
> +	void		*(*alloc)(void *alloc_ctx, unsigned long size, int write,

I know that we're already using a write flag in other parts of the API, but I 
find it a bit confusing. I think we should either document what the write flag 
means in the comment block above this structure, or replace it with a more 
explicit dma direction (which the alloc function will need to compute anyway).

> +				  gfp_t gfp_flags);
>  	void		(*put)(void *buf_priv);
>  	struct dma_buf *(*get_dmabuf)(void *buf_priv, unsigned long flags);
> 
> diff --git a/include/media/videobuf2-dma-sg.h
> b/include/media/videobuf2-dma-sg.h index 7b89852..14ce306 100644
> --- a/include/media/videobuf2-dma-sg.h
> +++ b/include/media/videobuf2-dma-sg.h
> @@ -21,6 +21,9 @@ static inline struct sg_table *vb2_dma_sg_plane_desc(
>  	return (struct sg_table *)vb2_plane_cookie(vb, plane_no);
>  }
> 
> +void *vb2_dma_sg_init_ctx(struct device *dev);
> +void vb2_dma_sg_cleanup_ctx(void *alloc_ctx);
> +
>  extern const struct vb2_mem_ops vb2_dma_sg_memops;
> 
>  #endif

-- 
Regards,

Laurent Pinchart


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [RFCv2 PATCH 03/14] vb2-dma-sg: add prepare/finish memops
  2014-09-12 12:59 ` [RFCv2 PATCH 03/14] vb2-dma-sg: add prepare/finish memops Hans Verkuil
@ 2014-09-12 21:54   ` Laurent Pinchart
  2014-09-14  3:40   ` Pawel Osciak
  1 sibling, 0 replies; 23+ messages in thread
From: Laurent Pinchart @ 2014-09-12 21:54 UTC (permalink / raw)
  To: Hans Verkuil; +Cc: linux-media, pawel, m.szyprowski, Hans Verkuil

Hi Hans,

Thank you for the patch.

On Friday 12 September 2014 14:59:52 Hans Verkuil wrote:
> From: Hans Verkuil <hans.verkuil@cisco.com>
> 
> This moves dma_(un)map_sg to the prepare/finish memops of
> videobuf2-dma-sg.c.
> 
> Now that vb2-dma-sg will sync the buffers for you in the prepare/finish
> memops we can drop that from the drivers that use dma-sg.
> 
> For the solo6x10 driver that was a bit more involved because it needs to
> copy JPEG or MPEG headers to the buffer before returning it to userspace,
> and that cannot be done in the old place since the buffer there is still
> setup for DMA access, not for CPU access. However, the buf_finish_for_cpu
> op is the ideal place to do this. By the time buf_finish_for_cpu is called
> the buffer is available for CPU access, so copying to the buffer is fine.
> 
> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
> ---
>  drivers/media/pci/cx23885/cx23885-417.c         |  7 +---
>  drivers/media/pci/cx23885/cx23885-core.c        |  5 ---
>  drivers/media/pci/cx23885/cx23885-dvb.c         |  7 +---
>  drivers/media/pci/cx23885/cx23885-vbi.c         | 13 +------
>  drivers/media/pci/cx23885/cx23885-video.c       | 13 +------
>  drivers/media/pci/saa7134/saa7134-empress.c     |  1 -
>  drivers/media/pci/saa7134/saa7134-ts.c          | 16 --------
>  drivers/media/pci/saa7134/saa7134-vbi.c         | 15 --------
>  drivers/media/pci/saa7134/saa7134-video.c       | 15 --------
>  drivers/media/pci/saa7134/saa7134.h             |  1 -
>  drivers/media/pci/solo6x10/solo6x10-v4l2-enc.c  | 50 ++++++++++------------
>  drivers/media/pci/tw68/tw68-video.c             | 12 +-----
>  drivers/media/platform/marvell-ccic/mcam-core.c | 18 +--------
>  drivers/media/v4l2-core/videobuf2-dma-sg.c      | 18 +++++++++
>  14 files changed, 51 insertions(+), 140 deletions(-)

[snip]

> diff --git a/drivers/media/v4l2-core/videobuf2-dma-sg.c
> b/drivers/media/v4l2-core/videobuf2-dma-sg.c index 9b7a041..f3bc01b 100644
> --- a/drivers/media/v4l2-core/videobuf2-dma-sg.c
> +++ b/drivers/media/v4l2-core/videobuf2-dma-sg.c
> @@ -170,6 +170,22 @@ static void vb2_dma_sg_put(void *buf_priv)
>  	}
>  }
> 
> +static void vb2_dma_sg_prepare(void *buf_priv)
> +{
> +	struct vb2_dma_sg_buf *buf = buf_priv;
> +	struct sg_table *sgt = &buf->sg_table;
> +
> +	dma_map_sg(buf->dev, sgt->sgl, sgt->nents, buf->dma_dir);

Shouldn't you use dma_sync_sg_for_device (and dma_sync_sg_for_cpu below) 
instead ? Mapping/unmapping the buffer can be costly when an IOMMU is 
involved.

> +}
> +
> +static void vb2_dma_sg_finish(void *buf_priv)
> +{
> +	struct vb2_dma_sg_buf *buf = buf_priv;
> +	struct sg_table *sgt = &buf->sg_table;
> +
> +	dma_unmap_sg(buf->dev, sgt->sgl, sgt->nents, buf->dma_dir);
> +}
> +
>  static inline int vma_is_io(struct vm_area_struct *vma)
>  {
>  	return !!(vma->vm_flags & (VM_IO | VM_PFNMAP));
> @@ -366,6 +382,8 @@ const struct vb2_mem_ops vb2_dma_sg_memops = {
>  	.put		= vb2_dma_sg_put,
>  	.get_userptr	= vb2_dma_sg_get_userptr,
>  	.put_userptr	= vb2_dma_sg_put_userptr,
> +	.prepare	= vb2_dma_sg_prepare,
> +	.finish		= vb2_dma_sg_finish,
>  	.vaddr		= vb2_dma_sg_vaddr,
>  	.mmap		= vb2_dma_sg_mmap,
>  	.num_users	= vb2_dma_sg_num_users,

-- 
Regards,

Laurent Pinchart


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [RFCv2 PATCH 09/14] vb2: replace 'write' by 'dma_dir'
  2014-09-12 12:59 ` [RFCv2 PATCH 09/14] vb2: replace 'write' by 'dma_dir' Hans Verkuil
@ 2014-09-12 22:02   ` Laurent Pinchart
  0 siblings, 0 replies; 23+ messages in thread
From: Laurent Pinchart @ 2014-09-12 22:02 UTC (permalink / raw)
  To: Hans Verkuil; +Cc: linux-media, pawel, m.szyprowski, Hans Verkuil

Hi Hans,

Thank you for the patch.

On Friday 12 September 2014 14:59:58 Hans Verkuil wrote:
> From: Hans Verkuil <hans.verkuil@cisco.com>
> 
> The 'write' argument is very ambiguous. I first assumed that if it is 1,
> then we're doing video output but instead it meant the reverse.
> 
> Since it is used to setup the dma_dir value anyway it is now replaced by
> the correct dma_dir value which is unambiguous.
> 
> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>

Given my comments to one of your earlier patches, I can only agree with you 
here :-)

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

However, how about moving this patch up in the series, before adding the write 
argument to the alloc function ?

> ---
>  drivers/media/v4l2-core/videobuf2-core.c       | 15 +++++----
>  drivers/media/v4l2-core/videobuf2-dma-contig.c | 46 ++++++++++++-----------
>  drivers/media/v4l2-core/videobuf2-dma-sg.c     | 46 +++++++++++------------
>  drivers/media/v4l2-core/videobuf2-vmalloc.c    | 20 ++++++-----
>  include/media/videobuf2-core.h                 | 11 +++---
>  5 files changed, 73 insertions(+), 65 deletions(-)

-- 
Regards,

Laurent Pinchart


^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [RFCv2 PATCH 02/14] vb2-dma-sg: add allocation context to dma-sg
  2014-09-12 12:59 ` [RFCv2 PATCH 02/14] vb2-dma-sg: add allocation context to dma-sg Hans Verkuil
  2014-09-12 21:49   ` Laurent Pinchart
@ 2014-09-14  3:29   ` Pawel Osciak
  1 sibling, 0 replies; 23+ messages in thread
From: Pawel Osciak @ 2014-09-14  3:29 UTC (permalink / raw)
  To: Hans Verkuil; +Cc: LMML, Marek Szyprowski, Laurent Pinchart, Hans Verkuil

Hi Hans,
Thank you for working on this!

On Fri, Sep 12, 2014 at 8:59 PM, Hans Verkuil <hverkuil@xs4all.nl> wrote:
> From: Hans Verkuil <hans.verkuil@cisco.com>
>
> Require that dma-sg also uses an allocation context. This is in preparation
> for adding prepare/finish memops to sync the memory between DMA and CPU.
>
> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>

[...]

> diff --git a/drivers/media/pci/cx23885/cx23885-core.c b/drivers/media/pci/cx23885/cx23885-core.c
> index cb94366b..8a36fcd 100644
> --- a/drivers/media/pci/cx23885/cx23885-core.c
> +++ b/drivers/media/pci/cx23885/cx23885-core.c
> @@ -1997,9 +1997,14 @@ static int cx23885_initdev(struct pci_dev *pci_dev,
>         if (!pci_dma_supported(pci_dev, 0xffffffff)) {
>                 printk("%s/0: Oops: no 32bit PCI DMA ???\n", dev->name);
>                 err = -EIO;
> -               goto fail_irq;
> +               goto fail_context;
>         }
>
> +       dev->alloc_ctx = vb2_dma_sg_init_ctx(&pci_dev->dev);
> +       if (IS_ERR(dev->alloc_ctx)) {
> +               err = -ENOMEM;

err = PTR_ERR(dev->alloc_ctx) ?

> +               goto fail_context;
> +       }
>         err = request_irq(pci_dev->irq, cx23885_irq,
>                           IRQF_SHARED, dev->name, dev);
>         if (err < 0) {
> @@ -2028,6 +2033,8 @@ static int cx23885_initdev(struct pci_dev *pci_dev,
>         return 0;
>
>  fail_irq:
> +       vb2_dma_sg_cleanup_ctx(dev->alloc_ctx);
> +fail_context:
>         cx23885_dev_unregister(dev);
>  fail_ctrl:
>         v4l2_ctrl_handler_free(hdl);
> @@ -2053,6 +2060,7 @@ static void cx23885_finidev(struct pci_dev *pci_dev)
>         free_irq(pci_dev->irq, dev);
>
>         cx23885_dev_unregister(dev);
> +       vb2_dma_sg_cleanup_ctx(dev->alloc_ctx);
>         v4l2_ctrl_handler_free(&dev->ctrl_handler);
>         v4l2_device_unregister(v4l2_dev);
>         kfree(dev);

[...]

> diff --git a/drivers/media/platform/marvell-ccic/mcam-core.h b/drivers/media/platform/marvell-ccic/mcam-core.h
> index e0e628c..7b8c201 100644
> --- a/drivers/media/platform/marvell-ccic/mcam-core.h
> +++ b/drivers/media/platform/marvell-ccic/mcam-core.h
> @@ -176,6 +176,7 @@ struct mcam_camera {
>         /* DMA buffers - DMA modes */
>         struct mcam_vb_buffer *vb_bufs[MAX_DMA_BUFS];
>         struct vb2_alloc_ctx *vb_alloc_ctx;
> +       struct vb2_alloc_ctx *vb_alloc_ctx_sg;

Should this be under #ifdef MCAM_MODE_DMA_SG?

>
>         /* Mode-specific ops, set at open time */
>         void (*dma_setup)(struct mcam_camera *cam);

[...]

> diff --git a/drivers/media/v4l2-core/videobuf2-vmalloc.c b/drivers/media/v4l2-core/videobuf2-vmalloc.c
> index 313d977..d77e397 100644
> --- a/drivers/media/v4l2-core/videobuf2-vmalloc.c
> +++ b/drivers/media/v4l2-core/videobuf2-vmalloc.c
> @@ -35,7 +35,8 @@ struct vb2_vmalloc_buf {
>
>  static void vb2_vmalloc_put(void *buf_priv);
>
> -static void *vb2_vmalloc_alloc(void *alloc_ctx, unsigned long size, gfp_t gfp_flags)
> +static void *vb2_vmalloc_alloc(void *alloc_ctx, unsigned long size, int write,
> +                              gfp_t gfp_flags)

I agree with Laurent that "write" is confusing, this could be a
direction flag, but the dma direction allows bidirectional, which we
would not be using. So I would personally prefer a binary flag or
enum. So perhaps we should keep this, only documenting it please.

-- 
Best regards,
Pawel Osciak

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [RFCv2 PATCH 03/14] vb2-dma-sg: add prepare/finish memops
  2014-09-12 12:59 ` [RFCv2 PATCH 03/14] vb2-dma-sg: add prepare/finish memops Hans Verkuil
  2014-09-12 21:54   ` Laurent Pinchart
@ 2014-09-14  3:40   ` Pawel Osciak
  1 sibling, 0 replies; 23+ messages in thread
From: Pawel Osciak @ 2014-09-14  3:40 UTC (permalink / raw)
  To: Hans Verkuil; +Cc: LMML, Marek Szyprowski, Laurent Pinchart, Hans Verkuil

Hi Hans,
Thank you for working on this.

On Fri, Sep 12, 2014 at 8:59 PM, Hans Verkuil <hverkuil@xs4all.nl> wrote:
> From: Hans Verkuil <hans.verkuil@cisco.com>
>
> This moves dma_(un)map_sg to the prepare/finish memops of videobuf2-dma-sg.c.

I agree with Laurent. dma_map and unmap should be done when we alloc
(or get) and put a buffer, while prepare and finish should only sync.

-- 
Best regards,
Pawel Osciak

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [RFCv2 PATCH 10/14] vb2: add 'new_cookies' flag
  2014-09-12 12:59 ` [RFCv2 PATCH 10/14] vb2: add 'new_cookies' flag Hans Verkuil
@ 2014-09-14  7:02   ` Pawel Osciak
  0 siblings, 0 replies; 23+ messages in thread
From: Pawel Osciak @ 2014-09-14  7:02 UTC (permalink / raw)
  To: Hans Verkuil; +Cc: LMML, Marek Szyprowski, Laurent Pinchart, Hans Verkuil

Hi Hans,

On Fri, Sep 12, 2014 at 8:59 PM, Hans Verkuil <hverkuil@xs4all.nl> wrote:
> From: Hans Verkuil <hans.verkuil@cisco.com>
>
> This flag helps drivers that need to reprogram their DMA engine whenever
> a plane cookie (== DMA address or DMA scatter-gather list) changes.
>
> Otherwise they would have to reprogram the DMA engine for every frame.
>
> Note that it is not possible to do this in buf_init() since dma_map_sg has
> to be done first, which happens just before buf_prepare() in the prepare()
> memop. It is dma_map_sg that sets up the dma addresses that are needed to
> configure the DMA engine.

Perhaps I'm missing something, but couldn't we just dma_map_sg on
allocation/get in dma-sg and unmap on put?

-- 
Best regards,
Pawel Osciak

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [RFCv2 PATCH 06/14] vb2-dma-sg: add dmabuf import support
  2014-09-12 12:59 ` [RFCv2 PATCH 06/14] vb2-dma-sg: add dmabuf import support Hans Verkuil
@ 2014-09-14  8:11   ` Laurent Pinchart
  0 siblings, 0 replies; 23+ messages in thread
From: Laurent Pinchart @ 2014-09-14  8:11 UTC (permalink / raw)
  To: Hans Verkuil; +Cc: linux-media, pawel, m.szyprowski, Hans Verkuil

Hi Hans,

Thank you for the patch.

On Friday 12 September 2014 14:59:55 Hans Verkuil wrote:
> From: Hans Verkuil <hans.verkuil@cisco.com>
> 
> Add support for dmabuf to vb2-dma-sg.
> 
> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
> ---
>  drivers/media/v4l2-core/videobuf2-dma-sg.c | 125 ++++++++++++++++++++++++--
>  1 file changed, 118 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/media/v4l2-core/videobuf2-dma-sg.c
> b/drivers/media/v4l2-core/videobuf2-dma-sg.c index abd5252..6d922c0 100644
> --- a/drivers/media/v4l2-core/videobuf2-dma-sg.c
> +++ b/drivers/media/v4l2-core/videobuf2-dma-sg.c
> @@ -42,11 +42,15 @@ struct vb2_dma_sg_buf {
>  	int				offset;
>  	enum dma_data_direction		dma_dir;
>  	struct sg_table			sg_table;
> +	struct sg_table			*dma_sgt;
>  	size_t				size;
>  	unsigned int			num_pages;
>  	atomic_t			refcount;
>  	struct vb2_vmarea_handler	handler;
>  	struct vm_area_struct		*vma;
> +
> +	/* DMABUF related */
> +	struct dma_buf_attachment	*db_attach;
>  };
> 
>  static void vb2_dma_sg_put(void *buf_priv);
> @@ -113,6 +117,7 @@ static void *vb2_dma_sg_alloc(void *alloc_ctx, unsigned
> long size, int write, /* size is already page aligned */
>  	buf->num_pages = size >> PAGE_SHIFT;
>  	buf->dma_dir = write ? DMA_FROM_DEVICE : DMA_TO_DEVICE;
> +	buf->dma_sgt = &buf->sg_table;
> 
>  	buf->pages = kzalloc(buf->num_pages * sizeof(struct page *),
>  			     GFP_KERNEL);
> @@ -123,7 +128,7 @@ static void *vb2_dma_sg_alloc(void *alloc_ctx, unsigned
> long size, int write, if (ret)
>  		goto fail_pages_alloc;
> 
> -	ret = sg_alloc_table_from_pages(&buf->sg_table, buf->pages,
> +	ret = sg_alloc_table_from_pages(buf->dma_sgt, buf->pages,
>  			buf->num_pages, 0, size, gfp_flags);
>  	if (ret)
>  		goto fail_table_alloc;
> @@ -161,7 +166,7 @@ static void vb2_dma_sg_put(void *buf_priv)
>  			buf->num_pages);
>  		if (buf->vaddr)
>  			vm_unmap_ram(buf->vaddr, buf->num_pages);
> -		sg_free_table(&buf->sg_table);
> +		sg_free_table(buf->dma_sgt);
>  		while (--i >= 0)
>  			__free_page(buf->pages[i]);
>  		kfree(buf->pages);
> @@ -173,7 +178,11 @@ static void vb2_dma_sg_put(void *buf_priv)
>  static int vb2_dma_sg_prepare(void *buf_priv)
>  {
>  	struct vb2_dma_sg_buf *buf = buf_priv;
> -	struct sg_table *sgt = &buf->sg_table;
> +	struct sg_table *sgt = buf->dma_sgt;
> +
> +	/* DMABUF exporter will flush the cache for us */

This isn't only about cache flushing. "DMABUF exporter will sync the memory 
for us" would be more accurate. Same comment for vb2_dma_sg_finish.

> +	if (buf->db_attach)
> +		return 0;
> 
>  	return dma_map_sg(buf->dev, sgt->sgl, sgt->nents, buf->dma_dir) ? 0 :
> -EIO; }
> @@ -181,7 +190,11 @@ static int vb2_dma_sg_prepare(void *buf_priv)
>  static void vb2_dma_sg_finish(void *buf_priv)
>  {
>  	struct vb2_dma_sg_buf *buf = buf_priv;
> -	struct sg_table *sgt = &buf->sg_table;
> +	struct sg_table *sgt = buf->dma_sgt;
> +
> +	/* DMABUF exporter will flush the cache for us */
> +	if (buf->db_attach)
> +		return;
> 
>  	dma_unmap_sg(buf->dev, sgt->sgl, sgt->nents, buf->dma_dir);
>  }
> @@ -209,6 +222,7 @@ static void *vb2_dma_sg_get_userptr(void *alloc_ctx,
> unsigned long vaddr, buf->offset = vaddr & ~PAGE_MASK;
>  	buf->size = size;
>  	buf->dma_dir = write ? DMA_FROM_DEVICE : DMA_TO_DEVICE;
> +	buf->dma_sgt = &buf->sg_table;
> 
>  	first = (vaddr           & PAGE_MASK) >> PAGE_SHIFT;
>  	last  = ((vaddr + size - 1) & PAGE_MASK) >> PAGE_SHIFT;
> @@ -261,7 +275,7 @@ static void *vb2_dma_sg_get_userptr(void *alloc_ctx,
> unsigned long vaddr, if (num_pages_from_user != buf->num_pages)
>  		goto userptr_fail_get_user_pages;
> 
> -	if (sg_alloc_table_from_pages(&buf->sg_table, buf->pages,
> +	if (sg_alloc_table_from_pages(buf->dma_sgt, buf->pages,
>  			buf->num_pages, buf->offset, size, 0))
>  		goto userptr_fail_alloc_table_from_pages;
> 
> @@ -297,7 +311,7 @@ static void vb2_dma_sg_put_userptr(void *buf_priv)
>  	       __func__, buf->num_pages);
>  	if (buf->vaddr)
>  		vm_unmap_ram(buf->vaddr, buf->num_pages);
> -	sg_free_table(&buf->sg_table);
> +	sg_free_table(buf->dma_sgt);
>  	while (--i >= 0) {
>  		if (buf->write)
>  			set_page_dirty_lock(buf->pages[i]);
> @@ -370,11 +384,104 @@ static int vb2_dma_sg_mmap(void *buf_priv, struct
> vm_area_struct *vma) return 0;
>  }
> 
> +/*********************************************/
> +/*       callbacks for DMABUF buffers        */
> +/*********************************************/
> +
> +static int vb2_dma_sg_map_dmabuf(void *mem_priv)
> +{
> +	struct vb2_dma_sg_buf *buf = mem_priv;
> +	struct sg_table *sgt;
> +
> +	if (WARN_ON(!buf->db_attach)) {
> +		pr_err("trying to pin a non attached buffer\n");
> +		return -EINVAL;
> +	}
> +
> +	if (WARN_ON(buf->dma_sgt)) {
> +		pr_err("dmabuf buffer is already pinned\n");
> +		return 0;
> +	}
> +
> +	/* get the associated scatterlist for this buffer */
> +	sgt = dma_buf_map_attachment(buf->db_attach, buf->dma_dir);
> +	if (IS_ERR_OR_NULL(sgt)) {

dma_buf_map_attachment() never returns NULL, you can use IS_ERR() and return 
PTR_ERR(sgt).

> +		pr_err("Error getting dmabuf scatterlist\n");
> +		return -EINVAL;
> +	}
> +
> +	buf->dma_sgt = sgt;
> +	return 0;
> +}
> +
> +static void vb2_dma_sg_unmap_dmabuf(void *mem_priv)
> +{
> +	struct vb2_dma_sg_buf *buf = mem_priv;
> +	struct sg_table *sgt = buf->dma_sgt;
> +
> +	if (WARN_ON(!buf->db_attach)) {
> +		pr_err("trying to unpin a not attached buffer\n");
> +		return;
> +	}
> +
> +	if (WARN_ON(!sgt)) {
> +		pr_err("dmabuf buffer is already unpinned\n");
> +		return;
> +	}
> +
> +	dma_buf_unmap_attachment(buf->db_attach, sgt, buf->dma_dir);
> +
> +	buf->dma_sgt = NULL;
> +}
> +
> +static void vb2_dma_sg_detach_dmabuf(void *mem_priv)
> +{
> +	struct vb2_dma_sg_buf *buf = mem_priv;
> +
> +	/* if vb2 works correctly you should never detach mapped buffer */
> +	if (WARN_ON(buf->dma_sgt))
> +		vb2_dma_sg_unmap_dmabuf(buf);
> +
> +	/* detach this attachment */
> +	dma_buf_detach(buf->db_attach->dmabuf, buf->db_attach);
> +	kfree(buf);
> +}
> +
> +static void *vb2_dma_sg_attach_dmabuf(void *alloc_ctx, struct dma_buf
> *dbuf, +	unsigned long size, int write)
> +{
> +	struct vb2_dma_sg_conf *conf = alloc_ctx;
> +	struct vb2_dma_sg_buf *buf;
> +	struct dma_buf_attachment *dba;
> +
> +	if (dbuf->size < size)
> +		return ERR_PTR(-EFAULT);
> +
> +	buf = kzalloc(sizeof(*buf), GFP_KERNEL);
> +	if (!buf)
> +		return ERR_PTR(-ENOMEM);
> +
> +	buf->dev = conf->dev;
> +	/* create attachment for the dmabuf with the user device */
> +	dba = dma_buf_attach(dbuf, buf->dev);
> +	if (IS_ERR(dba)) {
> +		pr_err("failed to attach dmabuf\n");
> +		kfree(buf);
> +		return dba;
> +	}
> +
> +	buf->dma_dir = write ? DMA_FROM_DEVICE : DMA_TO_DEVICE;
> +	buf->size = size;
> +	buf->db_attach = dba;
> +
> +	return buf;
> +}
> +
>  static void *vb2_dma_sg_cookie(void *buf_priv)
>  {
>  	struct vb2_dma_sg_buf *buf = buf_priv;
> 
> -	return &buf->sg_table;
> +	return buf->dma_sgt;
>  }
> 
>  const struct vb2_mem_ops vb2_dma_sg_memops = {
> @@ -387,6 +494,10 @@ const struct vb2_mem_ops vb2_dma_sg_memops = {
>  	.vaddr		= vb2_dma_sg_vaddr,
>  	.mmap		= vb2_dma_sg_mmap,
>  	.num_users	= vb2_dma_sg_num_users,
> +	.map_dmabuf	= vb2_dma_sg_map_dmabuf,
> +	.unmap_dmabuf	= vb2_dma_sg_unmap_dmabuf,
> +	.attach_dmabuf	= vb2_dma_sg_attach_dmabuf,
> +	.detach_dmabuf	= vb2_dma_sg_detach_dmabuf,
>  	.cookie		= vb2_dma_sg_cookie,
>  };
>  EXPORT_SYMBOL_GPL(vb2_dma_sg_memops);

-- 
Regards,

Laurent Pinchart


^ permalink raw reply	[flat|nested] 23+ messages in thread

end of thread, other threads:[~2014-09-14  8:11 UTC | newest]

Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-09-12 12:59 [RFCv2 PATCH 00/14] vb2: improve dma-sg, expbuf Hans Verkuil
2014-09-12 12:59 ` [RFCv2 PATCH 01/14] vb2: introduce buf_prepare/finish_for_cpu Hans Verkuil
2014-09-12 21:25   ` Laurent Pinchart
2014-09-12 12:59 ` [RFCv2 PATCH 02/14] vb2-dma-sg: add allocation context to dma-sg Hans Verkuil
2014-09-12 21:49   ` Laurent Pinchart
2014-09-14  3:29   ` Pawel Osciak
2014-09-12 12:59 ` [RFCv2 PATCH 03/14] vb2-dma-sg: add prepare/finish memops Hans Verkuil
2014-09-12 21:54   ` Laurent Pinchart
2014-09-14  3:40   ` Pawel Osciak
2014-09-12 12:59 ` [RFCv2 PATCH 04/14] vb2: memop prepare: return errors Hans Verkuil
2014-09-12 12:59 ` [RFCv2 PATCH 05/14] vb2: call memop prepare before the buf_prepare op is called Hans Verkuil
2014-09-12 12:59 ` [RFCv2 PATCH 06/14] vb2-dma-sg: add dmabuf import support Hans Verkuil
2014-09-14  8:11   ` Laurent Pinchart
2014-09-12 12:59 ` [RFCv2 PATCH 07/14] vb2-dma-sg: add get_dmabuf Hans Verkuil
2014-09-12 12:59 ` [RFCv2 PATCH 08/14] vb2-vmalloc: add get_dmabuf support Hans Verkuil
2014-09-12 12:59 ` [RFCv2 PATCH 09/14] vb2: replace 'write' by 'dma_dir' Hans Verkuil
2014-09-12 22:02   ` Laurent Pinchart
2014-09-12 12:59 ` [RFCv2 PATCH 10/14] vb2: add 'new_cookies' flag Hans Verkuil
2014-09-14  7:02   ` Pawel Osciak
2014-09-12 13:00 ` [RFCv2 PATCH 11/14] tw68: only reprogram DMA engine when necessary Hans Verkuil
2014-09-12 13:00 ` [RFCv2 PATCH 12/14] cx23885: " Hans Verkuil
2014-09-12 13:00 ` [RFCv2 PATCH 13/14] saa7134: don't rebuild the page table unless new_cookies is set Hans Verkuil
2014-09-12 13:00 ` [RFCv2 PATCH 14/14] vivid: enable vb2_expbuf support Hans Verkuil

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.