linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 1/4] v4l: vb2: add prepare/finish callbacks to allocators
@ 2012-09-24 10:58 Federico Vaga
  2012-09-24 10:58 ` [PATCH v3 2/4] videobuf2-dma-streaming: new videobuf2 memory allocator Federico Vaga
                   ` (4 more replies)
  0 siblings, 5 replies; 32+ messages in thread
From: Federico Vaga @ 2012-09-24 10:58 UTC (permalink / raw)
  To: Mauro Carvalho Chehab, Pawel Osciak, Marek Szyprowski, Hans Verkuil
  Cc: Giancarlo Asnaghi, linux-media, linux-kernel, Jonathan Corbet,
	Federico Vaga

This patch adds support for prepare/finish callbacks in VB2 allocators.
These callback are used for buffer flushing.

Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Acked-by: Federico Vaga <federico.vaga@gmail.com>
---
 drivers/media/v4l2-core/videobuf2-core.c | 11 +++++++++++
 include/media/videobuf2-core.h           |  7 +++++++
 2 file modificati, 18 inserzioni(+)

diff --git a/drivers/media/v4l2-core/videobuf2-core.c b/drivers/media/v4l2-core/videobuf2-core.c
index 4da3df6..079fa79 100644
--- a/drivers/media/v4l2-core/videobuf2-core.c
+++ b/drivers/media/v4l2-core/videobuf2-core.c
@@ -790,6 +790,7 @@ void vb2_buffer_done(struct vb2_buffer *vb, enum vb2_buffer_state state)
 {
 	struct vb2_queue *q = vb->vb2_queue;
 	unsigned long flags;
+	unsigned int plane;
 
 	if (vb->state != VB2_BUF_STATE_ACTIVE)
 		return;
@@ -800,6 +801,10 @@ void vb2_buffer_done(struct vb2_buffer *vb, enum vb2_buffer_state state)
 	dprintk(4, "Done processing on buffer %d, state: %d\n",
 			vb->v4l2_buf.index, vb->state);
 
+	/* sync buffers */
+	for (plane = 0; plane < vb->num_planes; ++plane)
+		call_memop(q, finish, vb->planes[plane].mem_priv);
+
 	/* Add the buffer to the done buffers list */
 	spin_lock_irqsave(&q->done_lock, flags);
 	vb->state = state;
@@ -975,9 +980,15 @@ static int __qbuf_mmap(struct vb2_buffer *vb, const struct v4l2_buffer *b)
 static void __enqueue_in_driver(struct vb2_buffer *vb)
 {
 	struct vb2_queue *q = vb->vb2_queue;
+	unsigned int plane;
 
 	vb->state = VB2_BUF_STATE_ACTIVE;
 	atomic_inc(&q->queued_count);
+
+	/* sync buffers */
+	for (plane = 0; plane < vb->num_planes; ++plane)
+		call_memop(q, prepare, vb->planes[plane].mem_priv);
+
 	q->ops->buf_queue(vb);
 }
 
diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h
index 8dd9b6c..2508609 100644
--- a/include/media/videobuf2-core.h
+++ b/include/media/videobuf2-core.h
@@ -41,6 +41,10 @@ struct vb2_fileio_data;
  *		 argument to other ops in this structure
  * @put_userptr: inform the allocator that a USERPTR buffer will no longer
  *		 be used
+ * @prepare:	called every time the buffer is passed from userspace to the
+ *		driver, usefull for cache synchronisation, optional
+ * @finish:	called every time the buffer is passed back from the driver
+ *		to the userspace, also optional
  * @vaddr:	return a kernel virtual address to a given memory buffer
  *		associated with the passed private structure or NULL if no
  *		such mapping exists
@@ -65,6 +69,9 @@ struct vb2_mem_ops {
 					unsigned long size, int write);
 	void		(*put_userptr)(void *buf_priv);
 
+	void		(*prepare)(void *buf_priv);
+	void		(*finish)(void *buf_priv);
+
 	void		*(*vaddr)(void *buf_priv);
 	void		*(*cookie)(void *buf_priv);
 
-- 
1.7.11.4


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

* [PATCH v3 2/4] videobuf2-dma-streaming: new videobuf2 memory allocator
  2012-09-24 10:58 [PATCH v3 1/4] v4l: vb2: add prepare/finish callbacks to allocators Federico Vaga
@ 2012-09-24 10:58 ` Federico Vaga
  2012-09-24 12:44   ` Marek Szyprowski
  2012-09-24 10:58 ` [PATCH v3 3/4] sta2x11_vip: convert to videobuf2 and control framework Federico Vaga
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 32+ messages in thread
From: Federico Vaga @ 2012-09-24 10:58 UTC (permalink / raw)
  To: Mauro Carvalho Chehab, Pawel Osciak, Marek Szyprowski, Hans Verkuil
  Cc: Giancarlo Asnaghi, linux-media, linux-kernel, Jonathan Corbet,
	Federico Vaga

The DMA streaming allocator is similar to the DMA contig but it use the
DMA streaming interface (dma_map_single, dma_unmap_single). The
allocator allocates buffers and immediately map the memory for DMA
transfer. For each buffer prepare/finish it does a DMA synchronization.

Signed-off-by: Federico Vaga <federico.vaga@gmail.com>
---
 drivers/media/v4l2-core/Kconfig                   |   5 +
 drivers/media/v4l2-core/Makefile                  |   1 +
 drivers/media/v4l2-core/videobuf2-dma-streaming.c | 205 ++++++++++++++++++++++
 include/media/videobuf2-dma-streaming.h           |  32 ++++
 4 file modificati, 243 inserzioni(+)
 create mode 100644 drivers/media/v4l2-core/videobuf2-dma-streaming.c
 create mode 100644 include/media/videobuf2-dma-streaming.h

diff --git a/drivers/media/v4l2-core/Kconfig b/drivers/media/v4l2-core/Kconfig
index 0c54e19..60548a7 100644
--- a/drivers/media/v4l2-core/Kconfig
+++ b/drivers/media/v4l2-core/Kconfig
@@ -79,3 +79,8 @@ config VIDEOBUF2_DMA_SG
 	#depends on HAS_DMA
 	select VIDEOBUF2_CORE
 	select VIDEOBUF2_MEMOPS
+
+config VIDEOBUF2_DMA_STREAMING
+	select VIDEOBUF2_CORE
+	select VIDEOBUF2_MEMOPS
+	tristate
diff --git a/drivers/media/v4l2-core/Makefile b/drivers/media/v4l2-core/Makefile
index c2d61d4..0b2756f 100644
--- a/drivers/media/v4l2-core/Makefile
+++ b/drivers/media/v4l2-core/Makefile
@@ -28,6 +28,7 @@ obj-$(CONFIG_VIDEOBUF2_MEMOPS) += videobuf2-memops.o
 obj-$(CONFIG_VIDEOBUF2_VMALLOC) += videobuf2-vmalloc.o
 obj-$(CONFIG_VIDEOBUF2_DMA_CONTIG) += videobuf2-dma-contig.o
 obj-$(CONFIG_VIDEOBUF2_DMA_SG) += videobuf2-dma-sg.o
+obj-$(CONFIG_VIDEOBUF2_DMA_STREAMING) += videobuf2-dma-streaming.o
 
 ccflags-y += -I$(srctree)/drivers/media/dvb-core
 ccflags-y += -I$(srctree)/drivers/media/dvb-frontends
diff --git a/drivers/media/v4l2-core/videobuf2-dma-streaming.c b/drivers/media/v4l2-core/videobuf2-dma-streaming.c
new file mode 100644
index 0000000..c839e05
--- /dev/null
+++ b/drivers/media/v4l2-core/videobuf2-dma-streaming.c
@@ -0,0 +1,205 @@
+/*
+ * videobuf2-dma-streaming.c - DMA streaming memory allocator for videobuf2
+ *
+ * Copyright (C) 2012 Federico Vaga <federico.vaga@gmail.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/module.h>
+#include <linux/slab.h>
+#include <linux/pagemap.h>
+#include <linux/dma-mapping.h>
+
+#include <media/videobuf2-core.h>
+#include <media/videobuf2-dma-streaming.h>
+#include <media/videobuf2-memops.h>
+
+struct vb2_streaming_conf {
+	struct device			*dev;
+};
+struct vb2_streaming_buf {
+	struct vb2_streaming_conf	*conf;
+	void				*vaddr;
+
+	dma_addr_t			dma_handle;
+
+	unsigned long			size;
+	struct vm_area_struct		*vma;
+
+	atomic_t			refcount;
+	struct vb2_vmarea_handler	handler;
+};
+
+static void vb2_dma_streaming_put(void *buf_priv)
+{
+	struct vb2_streaming_buf *buf = buf_priv;
+
+	if (atomic_dec_and_test(&buf->refcount)) {
+		dma_unmap_single(buf->conf->dev, buf->dma_handle, buf->size,
+				 DMA_FROM_DEVICE);
+		free_pages_exact(buf->vaddr, buf->size);
+		kfree(buf);
+	}
+
+}
+
+static void *vb2_dma_streaming_alloc(void *alloc_ctx, unsigned long size)
+{
+	struct vb2_streaming_conf *conf = alloc_ctx;
+	struct vb2_streaming_buf *buf;
+	int err;
+
+	buf = kzalloc(sizeof(struct vb2_streaming_buf), GFP_KERNEL);
+	if (!buf)
+		return ERR_PTR(-ENOMEM);
+	buf->vaddr = alloc_pages_exact(size, GFP_KERNEL | GFP_DMA);
+	if (!buf->vaddr) {
+		err = -ENOMEM;
+		goto out;
+	}
+	buf->dma_handle = dma_map_single(conf->dev, buf->vaddr, size,
+					 DMA_FROM_DEVICE);
+	err = dma_mapping_error(conf->dev, buf->dma_handle);
+	if (err) {
+		dev_err(conf->dev, "dma_map_single failed\n");
+
+		free_pages_exact(buf->vaddr, size);
+		buf->vaddr = NULL;
+		goto out_pages;
+	}
+	buf->conf = conf;
+	buf->size = size;
+	buf->handler.refcount = &buf->refcount;
+	buf->handler.put = vb2_dma_streaming_put;
+	buf->handler.arg = buf;
+
+	atomic_inc(&buf->refcount);
+	return buf;
+
+out_pages:
+	free_pages_exact(buf->vaddr, buf->size);
+out:
+	kfree(buf);
+	return ERR_PTR(err);
+}
+
+static void *vb2_dma_streaming_cookie(void *buf_priv)
+{
+	struct vb2_streaming_buf *buf = buf_priv;
+
+	return &buf->dma_handle;
+}
+
+static void *vb2_dma_streaming_vaddr(void *buf_priv)
+{
+	struct vb2_streaming_buf *buf = buf_priv;
+
+	if (!buf)
+		return NULL;
+	return buf->vaddr;
+}
+
+static unsigned int vb2_dma_streaming_num_users(void *buf_priv)
+{
+	struct vb2_streaming_buf *buf = buf_priv;
+
+	return atomic_read(&buf->refcount);
+}
+
+static int vb2_dma_streaming_mmap(void *buf_priv, struct vm_area_struct *vma)
+{
+	struct vb2_streaming_buf *buf = buf_priv;
+	unsigned long pos, start = vma->vm_start;
+	unsigned long size;
+	struct page *page;
+	int err;
+
+	/* Try to remap memory */
+	size = vma->vm_end - vma->vm_start;
+	size = (size < buf->size) ? size : buf->size;
+	pos = (unsigned long)buf->vaddr;
+
+	while (size > 0) {
+		page = virt_to_page((void *)pos);
+		if (!page) {
+			dev_err(buf->conf->dev, "mmap: virt_to_page failed\n");
+			return -ENOMEM;
+		}
+		err = vm_insert_page(vma, start, page);
+		if (err) {
+			dev_err(buf->conf->dev, "mmap: insert failed %d\n", err);
+			return -ENOMEM;
+		}
+		start += PAGE_SIZE;
+		pos += PAGE_SIZE;
+
+		if (size > PAGE_SIZE)
+			size -= PAGE_SIZE;
+		else
+			size = 0;
+	}
+
+
+	vma->vm_ops = &vb2_common_vm_ops;
+	vma->vm_flags |= VM_DONTEXPAND;
+	vma->vm_private_data = &buf->handler;
+
+	vma->vm_ops->open(vma);
+
+	return 0;
+}
+
+static void vb2_dma_streaming_prepare(void *buf_priv)
+{
+	struct vb2_streaming_buf *buf = buf_priv;
+
+	dma_sync_single_for_device(buf->conf->dev, buf->dma_handle,
+				   buf->size, DMA_FROM_DEVICE);
+}
+
+static void vb2_dma_streaming_finish(void *buf_priv)
+{
+	struct vb2_streaming_buf *buf = buf_priv;
+
+	dma_sync_single_for_cpu(buf->conf->dev, buf->dma_handle,
+				buf->size, DMA_FROM_DEVICE);
+}
+
+const struct vb2_mem_ops vb2_dma_streaming_memops = {
+	.alloc		= vb2_dma_streaming_alloc,
+	.put		= vb2_dma_streaming_put,
+	.cookie		= vb2_dma_streaming_cookie,
+	.vaddr		= vb2_dma_streaming_vaddr,
+	.mmap		= vb2_dma_streaming_mmap,
+	.num_users	= vb2_dma_streaming_num_users,
+	.prepare	= vb2_dma_streaming_prepare,
+	.finish		= vb2_dma_streaming_finish,
+};
+EXPORT_SYMBOL_GPL(vb2_dma_streaming_memops);
+
+void *vb2_dma_streaming_init_ctx(struct device *dev)
+{
+	struct vb2_streaming_conf *conf;
+
+	conf = kmalloc(sizeof(struct vb2_streaming_conf), GFP_KERNEL);
+	if (!conf)
+		return ERR_PTR(-ENOMEM);
+
+	conf->dev = dev;
+
+	return conf;
+}
+EXPORT_SYMBOL_GPL(vb2_dma_streaming_init_ctx);
+
+void vb2_dma_streaming_cleanup_ctx(void *alloc_ctx)
+{
+	kfree(alloc_ctx);
+}
+EXPORT_SYMBOL_GPL(vb2_dma_streaming_cleanup_ctx);
+
+MODULE_DESCRIPTION("DMA-streaming memory allocator for videobuf2");
+MODULE_AUTHOR("Federico Vaga <federico.vaga@gmail.com>");
+MODULE_LICENSE("GPL v2");
diff --git a/include/media/videobuf2-dma-streaming.h b/include/media/videobuf2-dma-streaming.h
new file mode 100644
index 0000000..2a62d93
--- /dev/null
+++ b/include/media/videobuf2-dma-streaming.h
@@ -0,0 +1,32 @@
+/*
+ * videobuf2-dma-streaming.h - DMA streaming memory allocator for videobuf2
+ *
+ * Copyright (C) 2012 Federico Vaga
+ *
+ * Author: Federico Vaga <federico.vaga@gmail.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation.
+ */
+
+#ifndef _MEDIA_VIDEOBUF2_DMA_STREAMING_H
+#define _MEDIA_VIDEOBUF2_DMA_STREAMING_H
+
+#include <media/videobuf2-core.h>
+#include <linux/dma-mapping.h>
+
+void *vb2_dma_streaming_init_ctx(struct device *dev);
+void vb2_dma_streaming_cleanup_ctx(void *alloc_ctx);
+
+extern const struct vb2_mem_ops vb2_dma_streaming_memops;
+
+static inline dma_addr_t
+vb2_dma_streaming_plane_paddr(struct vb2_buffer *vb, unsigned int plane_no)
+{
+	dma_addr_t *dma_addr = vb2_plane_cookie(vb, plane_no);
+
+	return *dma_addr;
+}
+
+#endif
-- 
1.7.11.4


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

* [PATCH v3 3/4] sta2x11_vip: convert to videobuf2 and control framework
  2012-09-24 10:58 [PATCH v3 1/4] v4l: vb2: add prepare/finish callbacks to allocators Federico Vaga
  2012-09-24 10:58 ` [PATCH v3 2/4] videobuf2-dma-streaming: new videobuf2 memory allocator Federico Vaga
@ 2012-09-24 10:58 ` Federico Vaga
  2012-12-04 16:15   ` Mauro Carvalho Chehab
  2012-09-24 10:58 ` [PATCH v3 4/4] adv7180: remove {query/g_/s_}ctrl Federico Vaga
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 32+ messages in thread
From: Federico Vaga @ 2012-09-24 10:58 UTC (permalink / raw)
  To: Mauro Carvalho Chehab, Pawel Osciak, Marek Szyprowski, Hans Verkuil
  Cc: Giancarlo Asnaghi, linux-media, linux-kernel, Jonathan Corbet,
	Federico Vaga

This patch re-write the driver and use the videobuf2
interface instead of the old videobuf. Moreover, it uses also
the control framework which allows the driver to inherit
controls from its subdevice (ADV7180)

Signed-off-by: Federico Vaga <federico.vaga@gmail.com>
Acked-by: Giancarlo Asnaghi <giancarlo.asnaghi@st.com>
---
 drivers/media/pci/sta2x11/Kconfig       |    2 +-
 drivers/media/pci/sta2x11/sta2x11_vip.c | 1238 ++++++++++---------------------
 2 file modificati, 407 inserzioni(+), 833 rimozioni(-)

diff --git a/drivers/media/pci/sta2x11/Kconfig b/drivers/media/pci/sta2x11/Kconfig
index 6749f67..654339f 100644
--- a/drivers/media/pci/sta2x11/Kconfig
+++ b/drivers/media/pci/sta2x11/Kconfig
@@ -2,7 +2,7 @@ config STA2X11_VIP
 	tristate "STA2X11 VIP Video For Linux"
 	depends on STA2X11
 	select VIDEO_ADV7180 if MEDIA_SUBDRV_AUTOSELECT
-	select VIDEOBUF_DMA_CONTIG
+	select VIDEOBUF2_DMA_STREAMING
 	depends on PCI && VIDEO_V4L2 && VIRT_TO_BUS
 	help
 	  Say Y for support for STA2X11 VIP (Video Input Port) capture
diff --git a/drivers/media/pci/sta2x11/sta2x11_vip.c b/drivers/media/pci/sta2x11/sta2x11_vip.c
index 4c10205..b9ff926 100644
--- a/drivers/media/pci/sta2x11/sta2x11_vip.c
+++ b/drivers/media/pci/sta2x11/sta2x11_vip.c
@@ -1,6 +1,7 @@
 /*
  * This is the driver for the STA2x11 Video Input Port.
  *
+ * Copyright (C) 2012       ST Microelectronics
  * Copyright (C) 2010       WindRiver Systems, Inc.
  *
  * This program is free software; you can redistribute it and/or modify it
@@ -19,36 +20,30 @@
  * The full GNU General Public License is included in this distribution in
  * the file called "COPYING".
  *
- * Author: Andreas Kies <andreas.kies@windriver.com>
- *		Vlad Lungu <vlad.lungu@windriver.com>
- *
  */
 
 #include <linux/types.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/init.h>
-#include <linux/vmalloc.h>
-
 #include <linux/videodev2.h>
-
 #include <linux/kmod.h>
-
 #include <linux/pci.h>
 #include <linux/interrupt.h>
-#include <linux/mutex.h>
 #include <linux/io.h>
 #include <linux/gpio.h>
 #include <linux/i2c.h>
 #include <linux/delay.h>
 #include <media/v4l2-common.h>
 #include <media/v4l2-device.h>
+#include <media/v4l2-ctrls.h>
 #include <media/v4l2-ioctl.h>
-#include <media/videobuf-dma-contig.h>
+#include <media/v4l2-fh.h>
+#include <media/v4l2-event.h>
+#include <media/videobuf2-dma-streaming.h>
 
 #include "sta2x11_vip.h"
 
-#define DRV_NAME "sta2x11_vip"
 #define DRV_VERSION "1.3"
 
 #ifndef PCI_DEVICE_ID_STMICRO_VIP
@@ -63,8 +58,8 @@
 #define DVP_TFS		0x08
 #define DVP_BFO		0x0C
 #define DVP_BFS		0x10
-#define DVP_VTP         0x14
-#define DVP_VBP         0x18
+#define DVP_VTP		0x14
+#define DVP_VBP		0x18
 #define DVP_VMP		0x1C
 #define DVP_ITM		0x98
 #define DVP_ITS		0x9C
@@ -84,43 +79,20 @@
 
 #define DVP_HLFLN_SD	0x00000001
 
-#define REG_WRITE(vip, reg, value) iowrite32((value), (vip->iomem)+(reg))
-#define REG_READ(vip, reg) ioread32((vip->iomem)+(reg))
-
 #define SAVE_COUNT 8
 #define AUX_COUNT 3
 #define IRQ_COUNT 1
 
-/**
- * struct sta2x11_vip - All internal data for one instance of device
- * @v4l2_dev: device registered in v4l layer
- * @video_dev: properties of our device
- * @pdev: PCI device
- * @adapter: contains I2C adapter information
- * @register_save_area: All relevant register are saved here during suspend
- * @decoder: contains information about video DAC
- * @format: pixel format, fixed UYVY
- * @std: video standard (e.g. PAL/NTSC)
- * @input: input line for video signal ( 0 or 1 )
- * @users: Number of open of device ( max. 1 )
- * @disabled: Device is in power down state
- * @mutex: ensures exclusive opening of device
- * @slock: for excluse acces of registers
- * @vb_vidq: queue maintained by videobuf layer
- * @capture: linked list of capture buffer
- * @active: struct videobuf_buffer currently beingg filled
- * @started: device is ready to capture frame
- * @closing: device will be shut down
- * @tcount: Number of top frames
- * @bcount: Number of bottom frames
- * @overflow: Number of FIFO overflows
- * @mem_spare: small buffer of unused frame
- * @dma_spare: dma addres of mem_spare
- * @iomem: hardware base address
- * @config: I2C and gpio config from platform
- *
- * All non-local data is accessed via this structure.
- */
+
+struct vip_buffer {
+	struct vb2_buffer	vb;
+	struct list_head	list;
+	dma_addr_t		dma;
+};
+static inline struct vip_buffer *to_vip_buffer(struct vb2_buffer *vb2)
+{
+	return container_of(vb2, struct vip_buffer, vb);
+}
 
 struct sta2x11_vip {
 	struct v4l2_device v4l2_dev;
@@ -129,21 +101,27 @@ struct sta2x11_vip {
 	struct i2c_adapter *adapter;
 	unsigned int register_save_area[IRQ_COUNT + SAVE_COUNT + AUX_COUNT];
 	struct v4l2_subdev *decoder;
-	struct v4l2_pix_format format;
-	v4l2_std_id std;
-	unsigned int input;
-	int users;
-	int disabled;
-	struct mutex mutex;	/* exclusive access during open */
-	spinlock_t slock;	/* spin lock for hardware and queue access */
-	struct videobuf_queue vb_vidq;
-	struct list_head capture;
-	struct videobuf_buffer *active;
-	int started, closing, tcount, bcount;
+	struct v4l2_ctrl_handler ctrl_hdl;
+
+
+	struct v4l2_pix_format format;	/* pixel format, fixed UYVY */
+	v4l2_std_id std;	/* Video standard (PAL/NTSC)*/
+	unsigned int input;	/* Input line (0 or 1) */
+	int disabled; /* 1 disabled 0 enabled */
+	spinlock_t slock; /* spin lock for hardware */
+
+	struct vb2_alloc_ctx *alloc_ctx;
+	struct vb2_queue vb_vidq; /* queue maintaned by videobuf2 */
+	struct list_head buffer_list; /* list of buffers */
+	unsigned int sequence;
+	struct vip_buffer *active; /* current active buffer */
+	spinlock_t lock; /* Used in videobuf2 callback */
+
+	/* Interrupt counters */
+	int tcount, bcount;
 	int overflow;
-	void *mem_spare;
-	dma_addr_t dma_spare;
-	void *iomem;
+
+	void *iomem;	/* I/O Memory */
 	struct vip_config *config;
 };
 
@@ -206,360 +184,212 @@ static struct v4l2_pix_format formats_60[] = {
 	 .colorspace = V4L2_COLORSPACE_SMPTE170M},
 };
 
-/**
- * buf_setup - Get size and number of video buffer
- * @vq: queue in videobuf
- * @count: Number of buffers (1..MAX_FRAMES).
- *		0 use default value.
- * @size:  size of buffer in bytes
- *
- * returns size and number of buffers
- * a preset value of 0 returns the default number.
- * return value: 0, always succesfull.
- */
-static int buf_setup(struct videobuf_queue *vq, unsigned int *count,
-		     unsigned int *size)
+/* Write VIP register */
+static inline void reg_write(struct sta2x11_vip *vip, unsigned int reg, u32 val)
 {
-	struct sta2x11_vip *vip = vq->priv_data;
-
-	*size = vip->format.width * vip->format.height * 2;
-	if (0 == *count || MAX_FRAMES < *count)
-		*count = MAX_FRAMES;
-	return 0;
-};
-
-/**
- * buf_prepare - prepare buffer for usage
- * @vq: queue in videobuf layer
- * @vb: buffer to be prepared
- * @field: type of video data (interlaced/non-interlaced)
- *
- * Allocate or realloc buffer
- * return value: 0, successful.
- *
- * -EINVAL, supplied buffer is too small.
- *
- *  other, buffer could not be locked.
- */
-static int buf_prepare(struct videobuf_queue *vq, struct videobuf_buffer *vb,
-		       enum v4l2_field field)
+	iowrite32((val), (vip->iomem)+(reg));
+}
+/* Read VIP register */
+static inline u32 reg_read(struct sta2x11_vip *vip, unsigned int reg)
 {
-	struct sta2x11_vip *vip = vq->priv_data;
-	int ret;
-
-	vb->size = vip->format.width * vip->format.height * 2;
-	if ((0 != vb->baddr) && (vb->bsize < vb->size))
-		return -EINVAL;
-	vb->width = vip->format.width;
-	vb->height = vip->format.height;
-	vb->field = field;
-
-	if (VIDEOBUF_NEEDS_INIT == vb->state) {
-		ret = videobuf_iolock(vq, vb, NULL);
-		if (ret)
-			goto fail;
-	}
-	vb->state = VIDEOBUF_PREPARED;
-	return 0;
-fail:
-	videobuf_dma_contig_free(vq, vb);
-	vb->state = VIDEOBUF_NEEDS_INIT;
-	return ret;
+	return  ioread32((vip->iomem)+(reg));
 }
-
-/**
- * buf_queu - queue buffer for filling
- * @vq: queue in videobuf layer
- * @vb: buffer to be queued
- *
- * if capturing is already running, the buffer will be queued. Otherwise
- * capture is started and the buffer is used directly.
- */
-static void buf_queue(struct videobuf_queue *vq, struct videobuf_buffer *vb)
+/* Start DMA acquisition */
+static void start_dma(struct sta2x11_vip *vip, struct vip_buffer *vip_buf)
 {
-	struct sta2x11_vip *vip = vq->priv_data;
-	u32 dma;
+	unsigned long offset = 0;
+
+	if (vip->format.field == V4L2_FIELD_INTERLACED)
+		offset = vip->format.width * 2;
 
-	vb->state = VIDEOBUF_QUEUED;
+	spin_lock_irq(&vip->slock);
+	/* Enable acquisition */
+	reg_write(vip, DVP_CTL, reg_read(vip, DVP_CTL) | DVP_CTL_ENA);
+	/* Set Top and Bottom Field memory address */
+	reg_write(vip, DVP_VTP, (u32)vip_buf->dma);
+	reg_write(vip, DVP_VBP, (u32)vip_buf->dma + offset);
+	spin_unlock_irq(&vip->slock);
+}
 
-	if (vip->active) {
-		list_add_tail(&vb->queue, &vip->capture);
+/* Fetch the next buffer to activate */
+static void vip_active_buf_next(struct sta2x11_vip *vip)
+{
+	/* Get the next buffer */
+	spin_lock(&vip->lock);
+	if (list_empty(&vip->buffer_list)) {/* No available buffer */
+		spin_unlock(&vip->lock);
 		return;
 	}
-
-	vip->started = 1;
+	vip->active = list_first_entry(&vip->buffer_list,
+				       struct vip_buffer,
+				       list);
+	/* Reset Top and Bottom counter */
 	vip->tcount = 0;
 	vip->bcount = 0;
-	vip->active = vb;
-	vb->state = VIDEOBUF_ACTIVE;
+	spin_unlock(&vip->lock);
+	if (vb2_is_streaming(&vip->vb_vidq)) {	/* streaming is on */
+		start_dma(vip, vip->active);	/* start dma capture */
+	}
+}
 
-	dma = videobuf_to_dma_contig(vb);
 
-	REG_WRITE(vip, DVP_TFO, (0 << 16) | (0));
-	/* despite of interlace mode, upper and lower frames start at zero */
-	REG_WRITE(vip, DVP_BFO, (0 << 16) | (0));
+/* Videobuf2 Operations */
+static int queue_setup(struct vb2_queue *vq, const struct v4l2_format *fmt,
+		       unsigned int *nbuffers, unsigned int *nplanes,
+		       unsigned int sizes[], void *alloc_ctxs[])
+{
+	struct sta2x11_vip *vip = vb2_get_drv_priv(vq);
 
-	switch (vip->format.field) {
-	case V4L2_FIELD_INTERLACED:
-		REG_WRITE(vip, DVP_TFS,
-			  ((vip->format.height / 2 - 1) << 16) |
-			  (2 * vip->format.width - 1));
-		REG_WRITE(vip, DVP_BFS, ((vip->format.height / 2 - 1) << 16) |
-			  (2 * vip->format.width - 1));
-		REG_WRITE(vip, DVP_VTP, dma);
-		REG_WRITE(vip, DVP_VBP, dma + vip->format.width * 2);
-		REG_WRITE(vip, DVP_VMP, 4 * vip->format.width);
-		break;
-	case V4L2_FIELD_TOP:
-		REG_WRITE(vip, DVP_TFS,
-			  ((vip->format.height - 1) << 16) |
-			  (2 * vip->format.width - 1));
-		REG_WRITE(vip, DVP_BFS, ((0) << 16) |
-			  (2 * vip->format.width - 1));
-		REG_WRITE(vip, DVP_VTP, dma);
-		REG_WRITE(vip, DVP_VBP, dma);
-		REG_WRITE(vip, DVP_VMP, 2 * vip->format.width);
-		break;
-	case V4L2_FIELD_BOTTOM:
-		REG_WRITE(vip, DVP_TFS, ((0) << 16) |
-			  (2 * vip->format.width - 1));
-		REG_WRITE(vip, DVP_BFS,
-			  ((vip->format.height) << 16) |
-			  (2 * vip->format.width - 1));
-		REG_WRITE(vip, DVP_VTP, dma);
-		REG_WRITE(vip, DVP_VBP, dma);
-		REG_WRITE(vip, DVP_VMP, 2 * vip->format.width);
-		break;
+	if (!(*nbuffers) || *nbuffers < MAX_FRAMES)
+		*nbuffers = MAX_FRAMES;
 
-	default:
-		pr_warning("VIP: unknown field format\n");
-		return;
-	}
+	*nplanes = 1;
+	sizes[0] = vip->format.sizeimage;
+	alloc_ctxs[0] = vip->alloc_ctx;
 
-	REG_WRITE(vip, DVP_CTL, DVP_CTL_ENA);
-}
+	vip->sequence = 0;
+	vip->active = NULL;
+	vip->tcount = 0;
+	vip->bcount = 0;
 
-/**
- * buff_release - release buffer
- * @vq: queue in videobuf layer
- * @vb: buffer to be released
- *
- * release buffer in videobuf layer
- */
-static void buf_release(struct videobuf_queue *vq, struct videobuf_buffer *vb)
+	return 0;
+};
+static int buffer_init(struct vb2_buffer *vb)
 {
+	struct vip_buffer *vip_buf = to_vip_buffer(vb);
 
-	videobuf_dma_contig_free(vq, vb);
-	vb->state = VIDEOBUF_NEEDS_INIT;
+	vip_buf->dma = vb2_dma_streaming_plane_paddr(vb, 0);
+	INIT_LIST_HEAD(&vip_buf->list);
+	return 0;
 }
 
-static struct videobuf_queue_ops vip_qops = {
-	.buf_setup = buf_setup,
-	.buf_prepare = buf_prepare,
-	.buf_queue = buf_queue,
-	.buf_release = buf_release,
-};
-
-/**
- * vip_open - open video device
- * @file: descriptor of device
- *
- * open device, make sure it is only opened once.
- * return value: 0, no error.
- *
- * -EBUSY, device is already opened
- *
- * -ENOMEM, no memory for auxiliary DMA buffer
- */
-static int vip_open(struct file *file)
+static int buffer_prepare(struct vb2_buffer *vb)
 {
-	struct video_device *dev = video_devdata(file);
-	struct sta2x11_vip *vip = video_get_drvdata(dev);
+	struct sta2x11_vip *vip = vb2_get_drv_priv(vb->vb2_queue);
+	struct vip_buffer *vip_buf = to_vip_buffer(vb);
+	unsigned long size;
+
+	size = vip->format.sizeimage;
+	if (vb2_plane_size(vb, 0) < size) {
+		v4l2_err(&vip->v4l2_dev, "buffer too small (%lu < %lu)\n",
+			 vb2_plane_size(vb, 0), size);
+		return -EINVAL;
+	}
 
-	mutex_lock(&vip->mutex);
-	vip->users++;
+	vb2_set_plane_payload(&vip_buf->vb, 0, size);
 
-	if (vip->users > 1) {
-		vip->users--;
-		mutex_unlock(&vip->mutex);
-		return -EBUSY;
+	return 0;
+}
+static void buffer_queue(struct vb2_buffer *vb)
+{
+	struct sta2x11_vip *vip = vb2_get_drv_priv(vb->vb2_queue);
+	struct vip_buffer *vip_buf = to_vip_buffer(vb);
+
+	spin_lock(&vip->lock);
+	list_add_tail(&vip_buf->list, &vip->buffer_list);
+	if (!vip->active) {	/* No active buffer, active the first one */
+		vip->active = list_first_entry(&vip->buffer_list,
+					       struct vip_buffer,
+					       list);
+		if (vb2_is_streaming(&vip->vb_vidq))	/* streaming is on */
+			start_dma(vip, vip_buf);	/* start dma capture */
 	}
+	spin_unlock(&vip->lock);
+}
+static int buffer_finish(struct vb2_buffer *vb)
+{
+	struct sta2x11_vip *vip = vb2_get_drv_priv(vb->vb2_queue);
+	struct vip_buffer *vip_buf = to_vip_buffer(vb);
 
-	file->private_data = dev;
-	vip->overflow = 0;
-	vip->started = 0;
-	vip->closing = 0;
-	vip->active = NULL;
+	/* Buffer handled, remove it from the list */
+	spin_lock(&vip->lock);
+	list_del_init(&vip_buf->list);
+	spin_unlock(&vip->lock);
 
-	INIT_LIST_HEAD(&vip->capture);
-	vip->mem_spare = dma_alloc_coherent(&vip->pdev->dev, 64,
-					    &vip->dma_spare, GFP_KERNEL);
-	if (!vip->mem_spare) {
-		vip->users--;
-		mutex_unlock(&vip->mutex);
-		return -ENOMEM;
-	}
+	vip_active_buf_next(vip);
 
-	mutex_unlock(&vip->mutex);
-	videobuf_queue_dma_contig_init_cached(&vip->vb_vidq,
-					      &vip_qops,
-					      &vip->pdev->dev,
-					      &vip->slock,
-					      V4L2_BUF_TYPE_VIDEO_CAPTURE,
-					      V4L2_FIELD_INTERLACED,
-					      sizeof(struct videobuf_buffer),
-					      vip, NULL);
-	REG_READ(vip, DVP_ITS);
-	REG_WRITE(vip, DVP_HLFLN, DVP_HLFLN_SD);
-	REG_WRITE(vip, DVP_ITM, DVP_IT_VSB | DVP_IT_VST);
-	REG_WRITE(vip, DVP_CTL, DVP_CTL_RST);
-	REG_WRITE(vip, DVP_CTL, 0);
-	REG_READ(vip, DVP_ITS);
 	return 0;
 }
 
-/**
- * vip_close - close video device
- * @file: descriptor of device
- *
- * close video device, wait until all pending operations are finished
- * ( maximum FRAME_MAX buffers pending )
- * Turn off interrupts.
- *
- * return value: 0, always succesful.
- */
-static int vip_close(struct file *file)
+static int start_streaming(struct vb2_queue *vq, unsigned int count)
 {
-	struct video_device *dev = video_devdata(file);
-	struct sta2x11_vip *vip = video_get_drvdata(dev);
+	struct sta2x11_vip *vip = vb2_get_drv_priv(vq);
 
-	vip->closing = 1;
-	if (vip->active)
-		videobuf_waiton(&vip->vb_vidq, vip->active, 0, 0);
 	spin_lock_irq(&vip->slock);
-
-	REG_WRITE(vip, DVP_ITM, 0);
-	REG_WRITE(vip, DVP_CTL, DVP_CTL_RST);
-	REG_WRITE(vip, DVP_CTL, 0);
-	REG_READ(vip, DVP_ITS);
-
-	vip->started = 0;
-	vip->active = NULL;
-
+	/* Enable interrupt VSYNC Top and Bottom*/
+	reg_write(vip, DVP_ITM, DVP_IT_VSB | DVP_IT_VST);
 	spin_unlock_irq(&vip->slock);
 
-	videobuf_stop(&vip->vb_vidq);
-	videobuf_mmap_free(&vip->vb_vidq);
+	if (count)
+		start_dma(vip, vip->active);
 
-	dma_free_coherent(&vip->pdev->dev, 64, vip->mem_spare, vip->dma_spare);
-	file->private_data = NULL;
-	mutex_lock(&vip->mutex);
-	vip->users--;
-	mutex_unlock(&vip->mutex);
 	return 0;
 }
 
-/**
- * vip_read - read from video input
- * @file: descriptor of device
- * @data: user buffer
- * @count: number of bytes to be read
- * @ppos: position within stream
- *
- * read video data from video device.
- * handling is done in generic videobuf layer
- * return value: provided by videobuf layer
- */
-static ssize_t vip_read(struct file *file, char __user *data,
-			size_t count, loff_t *ppos)
+/* abort streaming and wait for last buffer */
+static int stop_streaming(struct vb2_queue *vq)
 {
-	struct video_device *dev = file->private_data;
-	struct sta2x11_vip *vip = video_get_drvdata(dev);
-
-	return videobuf_read_stream(&vip->vb_vidq, data, count, ppos, 0,
-				    file->f_flags & O_NONBLOCK);
+	struct sta2x11_vip *vip = vb2_get_drv_priv(vq);
+	struct vip_buffer *vip_buf, *node;
+
+	/* Disable acquisition */
+	reg_write(vip, DVP_CTL, reg_read(vip, DVP_CTL) & ~DVP_CTL_ENA);
+	/* Disable all interrupts */
+	reg_write(vip, DVP_ITM, 0);
+
+	/* Release all active buffers */
+	spin_lock(&vip->lock);
+	list_for_each_entry_safe(vip_buf, node, &vip->buffer_list, list) {
+		vb2_buffer_done(&vip_buf->vb, VB2_BUF_STATE_ERROR);
+		list_del(&vip_buf->list);
+	}
+	spin_unlock(&vip->lock);
+	return 0;
 }
 
-/**
- * vip_mmap - map user buffer
- * @file: descriptor of device
- * @vma: user buffer
- *
- * map user space buffer into kernel mode, including DMA address.
- * handling is done in generic videobuf layer.
- * return value: provided by videobuf layer
- */
-static int vip_mmap(struct file *file, struct vm_area_struct *vma)
-{
-	struct video_device *dev = file->private_data;
-	struct sta2x11_vip *vip = video_get_drvdata(dev);
+static struct vb2_ops vip_video_qops = {
+	.queue_setup		= queue_setup,
+	.buf_init		= buffer_init,
+	.buf_prepare		= buffer_prepare,
+	.buf_finish		= buffer_finish,
+	.buf_queue		= buffer_queue,
+	.start_streaming	= start_streaming,
+	.stop_streaming		= stop_streaming,
+};
 
-	return videobuf_mmap_mapper(&vip->vb_vidq, vma);
-}
 
-/**
- * vip_poll - poll for event
- * @file: descriptor of device
- * @wait: contains events to be waited for
- *
- * wait for event related to video device.
- * handling is done in generic videobuf layer.
- * return value: provided by videobuf layer
- */
-static unsigned int vip_poll(struct file *file, struct poll_table_struct *wait)
-{
-	struct video_device *dev = file->private_data;
-	struct sta2x11_vip *vip = video_get_drvdata(dev);
+/* File Operations */
+static const struct v4l2_file_operations vip_fops = {
+	.owner = THIS_MODULE,
+	.open = v4l2_fh_open,
+	.release = vb2_fop_release,
+	.unlocked_ioctl = video_ioctl2,
+	.read = vb2_fop_read,
+	.mmap = vb2_fop_mmap,
+	.poll = vb2_fop_poll
+};
 
-	return videobuf_poll_stream(file, &vip->vb_vidq, wait);
-}
 
-/**
- * vidioc_querycap - return capabilities of device
- * @file: descriptor of device (not used)
- * @priv: points to current videodevice
- * @cap: contains return values
- *
- * the capabilities of the device are returned
- *
- * return value: 0, no error.
- */
+/* V4L2 ioctl Operations */
 static int vidioc_querycap(struct file *file, void *priv,
 			   struct v4l2_capability *cap)
 {
-	struct video_device *dev = priv;
-	struct sta2x11_vip *vip = video_get_drvdata(dev);
+	struct sta2x11_vip *vip = video_drvdata(file);
 
-	memset(cap, 0, sizeof(struct v4l2_capability));
-	strcpy(cap->driver, DRV_NAME);
-	strcpy(cap->card, DRV_NAME);
-	cap->version = 0;
+	strcpy(cap->driver, KBUILD_MODNAME);
+	strcpy(cap->card, KBUILD_MODNAME);
 	snprintf(cap->bus_info, sizeof(cap->bus_info), "PCI:%s",
 		 pci_name(vip->pdev));
-	cap->capabilities = V4L2_CAP_VIDEO_CAPTURE | V4L2_CAP_READWRITE |
-	    V4L2_CAP_STREAMING;
+	cap->device_caps = V4L2_CAP_VIDEO_CAPTURE | V4L2_CAP_READWRITE |
+			   V4L2_CAP_STREAMING;
+	cap->capabilities = cap->device_caps | V4L2_CAP_DEVICE_CAPS;
 
 	return 0;
 }
 
-/**
- * vidioc_s_std - set video standard
- * @file: descriptor of device (not used)
- * @priv: points to current videodevice
- * @std: contains standard to be set
- *
- * the video standard is set
- *
- * return value: 0, no error.
- *
- * -EIO, no input signal detected
- *
- * other, returned from video DAC.
- */
 static int vidioc_s_std(struct file *file, void *priv, v4l2_std_id *std)
 {
-	struct video_device *dev = priv;
-	struct sta2x11_vip *vip = video_get_drvdata(dev);
+	struct sta2x11_vip *vip = video_drvdata(file);
 	v4l2_std_id oldstd = vip->std, newstd;
 	int status;
 
@@ -590,110 +420,21 @@ static int vidioc_s_std(struct file *file, void *priv, v4l2_std_id *std)
 	return v4l2_subdev_call(vip->decoder, core, s_std, *std);
 }
 
-/**
- * vidioc_g_std - get video standard
- * @file: descriptor of device (not used)
- * @priv: points to current videodevice
- * @std: contains return values
- *
- * the current video standard is returned
- *
- * return value: 0, no error.
- */
 static int vidioc_g_std(struct file *file, void *priv, v4l2_std_id *std)
 {
-	struct video_device *dev = priv;
-	struct sta2x11_vip *vip = video_get_drvdata(dev);
+	struct sta2x11_vip *vip = video_drvdata(file);
 
 	*std = vip->std;
 	return 0;
 }
 
-/**
- * vidioc_querystd - get possible video standards
- * @file: descriptor of device (not used)
- * @priv: points to current videodevice
- * @std: contains return values
- *
- * all possible video standards are returned
- *
- * return value: delivered by video DAC routine.
- */
 static int vidioc_querystd(struct file *file, void *priv, v4l2_std_id *std)
 {
-	struct video_device *dev = priv;
-	struct sta2x11_vip *vip = video_get_drvdata(dev);
+	struct sta2x11_vip *vip = video_drvdata(file);
 
 	return v4l2_subdev_call(vip->decoder, video, querystd, std);
-
 }
 
-/**
- * vidioc_queryctl - get possible control settings
- * @file: descriptor of device (not used)
- * @priv: points to current videodevice
- * @ctrl: contains return values
- *
- * return possible values for a control
- * return value: delivered by video DAC routine.
- */
-static int vidioc_queryctrl(struct file *file, void *priv,
-			    struct v4l2_queryctrl *ctrl)
-{
-	struct video_device *dev = priv;
-	struct sta2x11_vip *vip = video_get_drvdata(dev);
-
-	return v4l2_subdev_call(vip->decoder, core, queryctrl, ctrl);
-}
-
-/**
- * vidioc_g_ctl - get control value
- * @file: descriptor of device (not used)
- * @priv: points to current videodevice
- * @ctrl: contains return values
- *
- * return setting for a control value
- * return value: delivered by video DAC routine.
- */
-static int vidioc_g_ctrl(struct file *file, void *priv,
-			 struct v4l2_control *ctrl)
-{
-	struct video_device *dev = priv;
-	struct sta2x11_vip *vip = video_get_drvdata(dev);
-
-	return v4l2_subdev_call(vip->decoder, core, g_ctrl, ctrl);
-}
-
-/**
- * vidioc_s_ctl - set control value
- * @file: descriptor of device (not used)
- * @priv: points to current videodevice
- * @ctrl: contains value to be set
- *
- * set value for a specific control
- * return value: delivered by video DAC routine.
- */
-static int vidioc_s_ctrl(struct file *file, void *priv,
-			 struct v4l2_control *ctrl)
-{
-	struct video_device *dev = priv;
-	struct sta2x11_vip *vip = video_get_drvdata(dev);
-
-	return v4l2_subdev_call(vip->decoder, core, s_ctrl, ctrl);
-}
-
-/**
- * vidioc_enum_input - return name of input line
- * @file: descriptor of device (not used)
- * @priv: points to current videodevice
- * @inp: contains return values
- *
- * the user friendly name of the input line is returned
- *
- * return value: 0, no error.
- *
- * -EINVAL, input line number out of range
- */
 static int vidioc_enum_input(struct file *file, void *priv,
 			     struct v4l2_input *inp)
 {
@@ -707,22 +448,9 @@ static int vidioc_enum_input(struct file *file, void *priv,
 	return 0;
 }
 
-/**
- * vidioc_s_input - set input line
- * @file: descriptor of device ( not used)
- * @priv: points to current videodevice
- * @i: new input line number
- *
- * the current active input line is set
- *
- * return value: 0, no error.
- *
- * -EINVAL, line number out of range
- */
 static int vidioc_s_input(struct file *file, void *priv, unsigned int i)
 {
-	struct video_device *dev = priv;
-	struct sta2x11_vip *vip = video_get_drvdata(dev);
+	struct sta2x11_vip *vip = video_drvdata(file);
 	int ret;
 
 	if (i > 1)
@@ -735,36 +463,14 @@ static int vidioc_s_input(struct file *file, void *priv, unsigned int i)
 	return 0;
 }
 
-/**
- * vidioc_g_input - return input line
- * @file: descriptor of device ( not used)
- * @priv: points to current videodevice
- * @i: returned input line number
- *
- * the current active input line is returned
- *
- * return value: always 0.
- */
 static int vidioc_g_input(struct file *file, void *priv, unsigned int *i)
 {
-	struct video_device *dev = priv;
-	struct sta2x11_vip *vip = video_get_drvdata(dev);
+	struct sta2x11_vip *vip = video_drvdata(file);
 
 	*i = vip->input;
 	return 0;
 }
 
-/**
- * vidioc_enum_fmt_vid_cap - return video capture format
- * @file: descriptor of device ( not used)
- * @priv: points to current videodevice
- * @f: returned format information
- *
- * returns name and format of video capture
- * Only UYVY is supported by hardware.
- *
- * return value: always 0.
- */
 static int vidioc_enum_fmt_vid_cap(struct file *file, void *priv,
 				   struct v4l2_fmtdesc *f)
 {
@@ -778,38 +484,19 @@ static int vidioc_enum_fmt_vid_cap(struct file *file, void *priv,
 	return 0;
 }
 
-/**
- * vidioc_try_fmt_vid_cap - set video capture format
- * @file: descriptor of device ( not used)
- * @priv: points to current videodevice
- * @f: new format
- *
- * new video format is set which includes width and
- * field type. width is fixed to 720, no scaling.
- * Only UYVY is supported by this hardware.
- * the minimum height is 200, the maximum is 576 (PAL)
- *
- * return value: 0, no error
- *
- * -EINVAL, pixel or field format not supported
- *
- */
 static int vidioc_try_fmt_vid_cap(struct file *file, void *priv,
 				  struct v4l2_format *f)
 {
-	struct video_device *dev = priv;
-	struct sta2x11_vip *vip = video_get_drvdata(dev);
+	struct sta2x11_vip *vip = video_drvdata(file);
 	int interlace_lim;
 
-	if (V4L2_PIX_FMT_UYVY != f->fmt.pix.pixelformat)
-		return -EINVAL;
-
 	if (V4L2_STD_525_60 & vip->std)
 		interlace_lim = 240;
 	else
 		interlace_lim = 288;
 
 	switch (f->fmt.pix.field) {
+	default:
 	case V4L2_FIELD_ANY:
 		if (interlace_lim < f->fmt.pix.height)
 			f->fmt.pix.field = V4L2_FIELD_INTERLACED;
@@ -823,10 +510,10 @@ static int vidioc_try_fmt_vid_cap(struct file *file, void *priv,
 		break;
 	case V4L2_FIELD_INTERLACED:
 		break;
-	default:
-		return -EINVAL;
 	}
 
+	/* It is the only supported format */
+	f->fmt.pix.pixelformat = V4L2_PIX_FMT_UYVY;
 	f->fmt.pix.height &= ~1;
 	if (2 * interlace_lim < f->fmt.pix.height)
 		f->fmt.pix.height = 2 * interlace_lim;
@@ -840,304 +527,221 @@ static int vidioc_try_fmt_vid_cap(struct file *file, void *priv,
 	return 0;
 }
 
-/**
- * vidioc_s_fmt_vid_cap - set current video format parameters
- * @file: descriptor of device ( not used)
- * @priv: points to current videodevice
- * @f: returned format information
- *
- * set new capture format
- * return value: 0, no error
- *
- * other, delivered by video DAC routine.
- */
 static int vidioc_s_fmt_vid_cap(struct file *file, void *priv,
 				struct v4l2_format *f)
 {
-	struct video_device *dev = priv;
-	struct sta2x11_vip *vip = video_get_drvdata(dev);
+	struct sta2x11_vip *vip = video_drvdata(file);
+	unsigned int t_stop, b_stop, pitch;
 	int ret;
 
 	ret = vidioc_try_fmt_vid_cap(file, priv, f);
 	if (ret)
 		return ret;
 
-	memcpy(&vip->format, &f->fmt.pix, sizeof(struct v4l2_pix_format));
-	return 0;
-}
+	if (vb2_is_busy(&vip->vb_vidq)) {
+		/* Can't change format during acquisition */
+		v4l2_err(&vip->v4l2_dev, "device busy\n");
+		return -EBUSY;
+	}
+	vip->format = f->fmt.pix;
+	switch (vip->format.field) {
+	case V4L2_FIELD_INTERLACED:
+		t_stop = ((vip->format.height / 2 - 1) << 16) |
+			 (2 * vip->format.width - 1);
+		b_stop = t_stop;
+		pitch = 4 * vip->format.width;
+		break;
+	case V4L2_FIELD_TOP:
+		t_stop = ((vip->format.height - 1) << 16) |
+			 (2 * vip->format.width - 1);
+		b_stop = (0 << 16) | (2 * vip->format.width - 1);
+		pitch = 2 * vip->format.width;
+		break;
+	case V4L2_FIELD_BOTTOM:
+		t_stop = (0 << 16) | (2 * vip->format.width - 1);
+		b_stop = (vip->format.height << 16) |
+			 (2 * vip->format.width - 1);
+		pitch = 2 * vip->format.width;
+		break;
+	default:
+		v4l2_err(&vip->v4l2_dev, "unknown field format\n");
+		return -EINVAL;
+	}
 
-/**
- * vidioc_g_fmt_vid_cap - get current video format parameters
- * @file: descriptor of device ( not used)
- * @priv: points to current videodevice
- * @f: contains format information
- *
- * returns current video format parameters
- *
- * return value: 0, always successful
- */
-static int vidioc_g_fmt_vid_cap(struct file *file, void *priv,
-				struct v4l2_format *f)
-{
-	struct video_device *dev = priv;
-	struct sta2x11_vip *vip = video_get_drvdata(dev);
+	spin_lock_irq(&vip->slock);
+	/* Y-X Top Field Offset */
+	reg_write(vip, DVP_TFO, 0);
+	/* Y-X Bottom Field Offset */
+	reg_write(vip, DVP_BFO, 0);
+	/* Y-X Top Field Stop*/
+	reg_write(vip, DVP_TFS, t_stop);
+	/* Y-X Bottom Field Stop */
+	reg_write(vip, DVP_BFS, b_stop);
+	/* Video Memory Pitch */
+	reg_write(vip, DVP_VMP, pitch);
+	spin_unlock_irq(&vip->slock);
 
-	memcpy(&f->fmt.pix, &vip->format, sizeof(struct v4l2_pix_format));
 	return 0;
 }
 
-/**
- * vidioc_reqfs - request buffer
- * @file: descriptor of device ( not used)
- * @priv: points to current videodevice
- * @p: video buffer
- *
- * Handling is done in generic videobuf layer.
- */
-static int vidioc_reqbufs(struct file *file, void *priv,
-			  struct v4l2_requestbuffers *p)
-{
-	struct video_device *dev = priv;
-	struct sta2x11_vip *vip = video_get_drvdata(dev);
-
-	return videobuf_reqbufs(&vip->vb_vidq, p);
-}
-
-/**
- * vidioc_querybuf - query buffer
- * @file: descriptor of device ( not used)
- * @priv: points to current videodevice
- * @p: video buffer
- *
- * query buffer state.
- * Handling is done in generic videobuf layer.
- */
-static int vidioc_querybuf(struct file *file, void *priv, struct v4l2_buffer *p)
-{
-	struct video_device *dev = priv;
-	struct sta2x11_vip *vip = video_get_drvdata(dev);
-
-	return videobuf_querybuf(&vip->vb_vidq, p);
-}
-
-/**
- * vidioc_qbuf - queue a buffer
- * @file: descriptor of device ( not used)
- * @priv: points to current videodevice
- * @p: video buffer
- *
- * Handling is done in generic videobuf layer.
- */
-static int vidioc_qbuf(struct file *file, void *priv, struct v4l2_buffer *p)
-{
-	struct video_device *dev = priv;
-	struct sta2x11_vip *vip = video_get_drvdata(dev);
-
-	return videobuf_qbuf(&vip->vb_vidq, p);
-}
-
-/**
- * vidioc_dqbuf - dequeue a buffer
- * @file: descriptor of device ( not used)
- * @priv: points to current videodevice
- * @p: video buffer
- *
- * Handling is done in generic videobuf layer.
- */
-static int vidioc_dqbuf(struct file *file, void *priv, struct v4l2_buffer *p)
-{
-	struct video_device *dev = priv;
-	struct sta2x11_vip *vip = video_get_drvdata(dev);
-
-	return videobuf_dqbuf(&vip->vb_vidq, p, file->f_flags & O_NONBLOCK);
-}
-
-/**
- * vidioc_streamon - turn on streaming
- * @file: descriptor of device ( not used)
- * @priv: points to current videodevice
- * @type: type of capture
- *
- * turn on streaming.
- * Handling is done in generic videobuf layer.
- */
-static int vidioc_streamon(struct file *file, void *priv,
-			   enum v4l2_buf_type type)
+static int vidioc_g_fmt_vid_cap(struct file *file, void *priv,
+				struct v4l2_format *f)
 {
-	struct video_device *dev = priv;
-	struct sta2x11_vip *vip = video_get_drvdata(dev);
+	struct sta2x11_vip *vip = video_drvdata(file);
 
-	return videobuf_streamon(&vip->vb_vidq);
-}
+	f->fmt.pix = vip->format;
 
-/**
- * vidioc_streamoff - turn off streaming
- * @file: descriptor of device ( not used)
- * @priv: points to current videodevice
- * @type: type of capture
- *
- * turn off streaming.
- * Handling is done in generic videobuf layer.
- */
-static int vidioc_streamoff(struct file *file, void *priv,
-			    enum v4l2_buf_type type)
-{
-	struct video_device *dev = priv;
-	struct sta2x11_vip *vip = video_get_drvdata(dev);
-
-	return videobuf_streamoff(&vip->vb_vidq);
+	return 0;
 }
 
-static const struct v4l2_file_operations vip_fops = {
-	.owner = THIS_MODULE,
-	.open = vip_open,
-	.release = vip_close,
-	.ioctl = video_ioctl2,
-	.read = vip_read,
-	.mmap = vip_mmap,
-	.poll = vip_poll
-};
-
 static const struct v4l2_ioctl_ops vip_ioctl_ops = {
 	.vidioc_querycap = vidioc_querycap,
-	.vidioc_s_std = vidioc_s_std,
+	/* FMT handling */
+	.vidioc_enum_fmt_vid_cap = vidioc_enum_fmt_vid_cap,
+	.vidioc_g_fmt_vid_cap = vidioc_g_fmt_vid_cap,
+	.vidioc_s_fmt_vid_cap = vidioc_s_fmt_vid_cap,
+	.vidioc_try_fmt_vid_cap = vidioc_try_fmt_vid_cap,
+	/* Buffer handlers */
+	.vidioc_reqbufs = vb2_ioctl_reqbufs,
+	.vidioc_querybuf = vb2_ioctl_querybuf,
+	.vidioc_qbuf = vb2_ioctl_qbuf,
+	.vidioc_dqbuf = vb2_ioctl_dqbuf,
+	/* Stream on/off */
+	.vidioc_streamon = vb2_ioctl_streamon,
+	.vidioc_streamoff = vb2_ioctl_streamoff,
+	/* Standard handling */
 	.vidioc_g_std = vidioc_g_std,
+	.vidioc_s_std = vidioc_s_std,
 	.vidioc_querystd = vidioc_querystd,
-	.vidioc_queryctrl = vidioc_queryctrl,
-	.vidioc_g_ctrl = vidioc_g_ctrl,
-	.vidioc_s_ctrl = vidioc_s_ctrl,
+	/* Input handling */
 	.vidioc_enum_input = vidioc_enum_input,
-	.vidioc_try_fmt_vid_cap = vidioc_try_fmt_vid_cap,
-	.vidioc_s_input = vidioc_s_input,
 	.vidioc_g_input = vidioc_g_input,
-	.vidioc_enum_fmt_vid_cap = vidioc_enum_fmt_vid_cap,
-	.vidioc_s_fmt_vid_cap = vidioc_s_fmt_vid_cap,
-	.vidioc_g_fmt_vid_cap = vidioc_g_fmt_vid_cap,
-	.vidioc_reqbufs = vidioc_reqbufs,
-	.vidioc_querybuf = vidioc_querybuf,
-	.vidioc_qbuf = vidioc_qbuf,
-	.vidioc_dqbuf = vidioc_dqbuf,
-	.vidioc_streamon = vidioc_streamon,
-	.vidioc_streamoff = vidioc_streamoff,
+	.vidioc_s_input = vidioc_s_input,
+	/* Log status ioctl */
+	.vidioc_log_status = v4l2_ctrl_log_status,
+	/* Event handling */
+	.vidioc_subscribe_event = v4l2_ctrl_subscribe_event,
+	.vidioc_unsubscribe_event = v4l2_event_unsubscribe,
 };
 
 static struct video_device video_dev_template = {
-	.name = DRV_NAME,
+	.name = KBUILD_MODNAME,
 	.release = video_device_release,
 	.fops = &vip_fops,
 	.ioctl_ops = &vip_ioctl_ops,
 	.tvnorms = V4L2_STD_ALL,
 };
 
-/**
- * vip_irq - interrupt routine
- * @irq: Number of interrupt ( not used, correct number is assumed )
- * @vip: local data structure containing all information
- *
- * check for both frame interrupts set ( top and bottom ).
- * check FIFO overflow, but limit number of log messages after open.
- * signal a complete buffer if done.
- * dequeue a new buffer if available.
- * disable VIP if no buffer available.
- *
- * return value: IRQ_NONE, interrupt was not generated by VIP
- *
- * IRQ_HANDLED, interrupt done.
- */
+
 static irqreturn_t vip_irq(int irq, struct sta2x11_vip *vip)
 {
-	u32 status, dma;
-	unsigned long flags;
-	struct videobuf_buffer *vb;
+	unsigned int status;
 
-	status = REG_READ(vip, DVP_ITS);
+	status = reg_read(vip, DVP_ITS);
 
-	if (!status) {
-		pr_debug("VIP: irq ignored\n");
+	if (!status)		/* No interrupt to handle */
 		return IRQ_NONE;
-	}
-
-	if (!vip->started)
-		return IRQ_HANDLED;
-
-	if (status & DVP_IT_VSB)
-		vip->bcount++;
 
-	if (status & DVP_IT_VST)
-		vip->tcount++;
+	if (status & DVP_IT_FIFO)
+		if (vip->overflow++ > 5)
+			pr_info("VIP: fifo overflow\n");
 
-	if ((DVP_IT_VSB | DVP_IT_VST) == (status & (DVP_IT_VST | DVP_IT_VSB))) {
+	if ((status & DVP_IT_VST) && (status & DVP_IT_VSB)) {
 		/* this is bad, we are too slow, hope the condition is gone
 		 * on the next frame */
-		pr_info("VIP: both irqs\n");
 		return IRQ_HANDLED;
 	}
 
-	if (status & DVP_IT_FIFO) {
-		if (5 > vip->overflow++)
-			pr_info("VIP: fifo overflow\n");
-	}
-
-	if (2 > vip->tcount)
+	if (status & DVP_IT_VST)
+		if ((++vip->tcount) < 2)
+			return IRQ_HANDLED;
+	if (status & DVP_IT_VSB) {
+		vip->bcount++;
 		return IRQ_HANDLED;
+	}
 
-	if (status & DVP_IT_VSB)
-		return IRQ_HANDLED;
+	if (vip->active) { /* Acquisition is over on this buffer */
+		/* Disable acquisition */
+		reg_write(vip, DVP_CTL, reg_read(vip, DVP_CTL) & ~DVP_CTL_ENA);
+		/* Remove the active buffer from the list */
+		do_gettimeofday(&vip->active->vb.v4l2_buf.timestamp);
+		vip->active->vb.v4l2_buf.sequence = vip->sequence++;
+		vb2_buffer_done(&vip->active->vb, VB2_BUF_STATE_DONE);
+	}
 
-	spin_lock_irqsave(&vip->slock, flags);
+	return IRQ_HANDLED;
+}
 
-	REG_WRITE(vip, DVP_CTL, REG_READ(vip, DVP_CTL) & ~DVP_CTL_ENA);
-	if (vip->active) {
-		do_gettimeofday(&vip->active->ts);
-		vip->active->field_count++;
-		vip->active->state = VIDEOBUF_DONE;
-		wake_up(&vip->active->done);
-		vip->active = NULL;
+static void sta2x11_vip_init_register(struct sta2x11_vip *vip)
+{
+	/* Register initialization */
+	spin_lock_irq(&vip->slock);
+	/* Clean interrupt */
+	reg_read(vip, DVP_ITS);
+	/* Enable Half Line per vertical */
+	reg_write(vip, DVP_HLFLN, DVP_HLFLN_SD);
+	/* Reset VIP control */
+	reg_write(vip, DVP_CTL, DVP_CTL_RST);
+	/* Clear VIP control */
+	reg_write(vip, DVP_CTL, 0);
+	spin_unlock_irq(&vip->slock);
+}
+static void sta2x11_vip_clear_register(struct sta2x11_vip *vip)
+{
+	spin_lock_irq(&vip->slock);
+	/* Disable interrupt */
+	reg_write(vip, DVP_ITM, 0);
+	/* Reset VIP Control */
+	reg_write(vip, DVP_CTL, DVP_CTL_RST);
+	/* Clear VIP Control */
+	reg_write(vip, DVP_CTL, 0);
+	/* Clean VIP Interrupt */
+	reg_read(vip, DVP_ITS);
+	spin_unlock_irq(&vip->slock);
+}
+static int sta2x11_vip_init_buffer(struct sta2x11_vip *vip)
+{
+	memset(&vip->vb_vidq, 0, sizeof(struct vb2_queue));
+	vip->vb_vidq.type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
+	vip->vb_vidq.io_modes = VB2_MMAP | VB2_READ;
+	vip->vb_vidq.drv_priv = vip;
+	vip->vb_vidq.buf_struct_size = sizeof(struct vip_buffer);
+	vip->vb_vidq.ops = &vip_video_qops;
+	vip->vb_vidq.mem_ops = &vb2_dma_streaming_memops;
+	vb2_queue_init(&vip->vb_vidq);
+	INIT_LIST_HEAD(&vip->buffer_list);
+	spin_lock_init(&vip->lock);
+
+	vip->alloc_ctx = vb2_dma_streaming_init_ctx(&vip->pdev->dev);
+	if (IS_ERR(vip->alloc_ctx)) {
+		v4l2_err(&vip->v4l2_dev, "Can't allocate buffer context");
+		return PTR_ERR(vip->alloc_ctx);
 	}
-	if (!vip->closing) {
-		if (list_empty(&vip->capture))
-			goto done;
-
-		vb = list_first_entry(&vip->capture, struct videobuf_buffer,
-				      queue);
-		if (NULL == vb) {
-			pr_info("VIP: no buffer\n");
-			goto done;
-		}
-		vb->state = VIDEOBUF_ACTIVE;
-		list_del(&vb->queue);
-		vip->active = vb;
-		dma = videobuf_to_dma_contig(vb);
-		switch (vip->format.field) {
-		case V4L2_FIELD_INTERLACED:
-			REG_WRITE(vip, DVP_VTP, dma);
-			REG_WRITE(vip, DVP_VBP, dma + vip->format.width * 2);
-			break;
-		case V4L2_FIELD_TOP:
-		case V4L2_FIELD_BOTTOM:
-			REG_WRITE(vip, DVP_VTP, dma);
-			REG_WRITE(vip, DVP_VBP, dma);
-			break;
-		default:
-			pr_warning("VIP: unknown field format\n");
-			goto done;
-			break;
-		}
-		REG_WRITE(vip, DVP_CTL, REG_READ(vip, DVP_CTL) | DVP_CTL_ENA);
+	return 0;
+}
+static void sta2x11_vip_release_buffer(struct sta2x11_vip *vip)
+{
+	vb2_dma_streaming_cleanup_ctx(vip->alloc_ctx);
+}
+static int sta2x11_vip_init_controls(struct sta2x11_vip *vip)
+{
+	/*
+	 * Inititialize an empty control so VIP can inerithing controls
+	 * from ADV7180
+	 */
+	v4l2_ctrl_handler_init(&vip->ctrl_hdl, 0);
+
+	vip->v4l2_dev.ctrl_handler = &vip->ctrl_hdl;
+	if (vip->ctrl_hdl.error) {
+		int err = vip->ctrl_hdl.error;
+
+		v4l2_ctrl_handler_free(&vip->ctrl_hdl);
+		return err;
 	}
-done:
-	spin_unlock_irqrestore(&vip->slock, flags);
-	return IRQ_HANDLED;
+
+	return 0;
 }
 
-/**
- * vip_gpio_reserve - reserve gpio pin
- * @dev: device
- * @pin: GPIO pin number
- * @dir: direction, input or output
- * @name: GPIO pin name
- *
- */
 static int vip_gpio_reserve(struct device *dev, int pin, int dir,
 			    const char *name)
 {
@@ -1170,13 +774,6 @@ static int vip_gpio_reserve(struct device *dev, int pin, int dir,
 	return 0;
 }
 
-/**
- * vip_gpio_release - release gpio pin
- * @dev: device
- * @pin: GPIO pin number
- * @name: GPIO pin name
- *
- */
 static void vip_gpio_release(struct device *dev, int pin, const char *name)
 {
 	if (pin != -1) {
@@ -1186,25 +783,6 @@ static void vip_gpio_release(struct device *dev, int pin, const char *name)
 	}
 }
 
-/**
- * sta2x11_vip_init_one - init one instance of video device
- * @pdev: PCI device
- * @ent: (not used)
- *
- * allocate reset pins for DAC.
- * Reset video DAC, this is done via reset line.
- * allocate memory for managing device
- * request interrupt
- * map IO region
- * register device
- * find and initialize video DAC
- *
- * return value: 0, no error
- *
- * -ENOMEM, no memory
- *
- * -ENODEV, device could not be detected or registered
- */
 static int __devinit sta2x11_vip_init_one(struct pci_dev *pdev,
 					  const struct pci_device_id *ent)
 {
@@ -1212,10 +790,17 @@ static int __devinit sta2x11_vip_init_one(struct pci_dev *pdev,
 	struct sta2x11_vip *vip;
 	struct vip_config *config;
 
+	/* Check if hardware support 26-bit DMA */
+	if (dma_set_mask(&pdev->dev, DMA_BIT_MASK(26))) {
+		dev_err(&pdev->dev, "26-bit DMA addressing not available\n");
+		return -EINVAL;
+	}
+	/* Enable PCI */
 	ret = pci_enable_device(pdev);
 	if (ret)
 		return ret;
 
+	/* Get VIP platform data */
 	config = dev_get_platdata(&pdev->dev);
 	if (!config) {
 		dev_info(&pdev->dev, "VIP slot disabled\n");
@@ -1223,6 +808,7 @@ static int __devinit sta2x11_vip_init_one(struct pci_dev *pdev,
 		goto disable;
 	}
 
+	/* Power configuration */
 	ret = vip_gpio_reserve(&pdev->dev, config->pwr_pin, 0,
 			       config->pwr_name);
 	if (ret)
@@ -1237,7 +823,6 @@ static int __devinit sta2x11_vip_init_one(struct pci_dev *pdev,
 			goto disable;
 		}
 	}
-
 	if (config->pwr_pin != -1) {
 		/* Datasheet says 5ms between PWR and RST */
 		usleep_range(5000, 25000);
@@ -1251,17 +836,20 @@ static int __devinit sta2x11_vip_init_one(struct pci_dev *pdev,
 	}
 	usleep_range(5000, 25000);
 
+	/* Allocate a new VIP instance */
 	vip = kzalloc(sizeof(struct sta2x11_vip), GFP_KERNEL);
 	if (!vip) {
 		ret = -ENOMEM;
 		goto release_gpios;
 	}
-
 	vip->pdev = pdev;
 	vip->std = V4L2_STD_PAL;
 	vip->format = formats_50[0];
 	vip->config = config;
 
+	ret = sta2x11_vip_init_controls(vip);
+	if (ret)
+		goto free_mem;
 	if (v4l2_device_register(&pdev->dev, &vip->v4l2_dev))
 		goto free_mem;
 
@@ -1271,46 +859,52 @@ static int __devinit sta2x11_vip_init_one(struct pci_dev *pdev,
 
 	pci_set_master(pdev);
 
-	ret = pci_request_regions(pdev, DRV_NAME);
+	ret = pci_request_regions(pdev, KBUILD_MODNAME);
 	if (ret)
 		goto unreg;
 
 	vip->iomem = pci_iomap(pdev, 0, 0x100);
 	if (!vip->iomem) {
-		ret = -ENOMEM; /* FIXME */
+		ret = -ENOMEM;
 		goto release;
 	}
 
 	pci_enable_msi(pdev);
 
-	INIT_LIST_HEAD(&vip->capture);
+	/* Initialize buffer */
+	ret = sta2x11_vip_init_buffer(vip);
+	if (ret)
+		goto unmap;
+
 	spin_lock_init(&vip->slock);
-	mutex_init(&vip->mutex);
-	vip->started = 0;
-	vip->disabled = 0;
 
 	ret = request_irq(pdev->irq,
 			  (irq_handler_t) vip_irq,
-			  IRQF_SHARED, DRV_NAME, vip);
+			  IRQF_SHARED, KBUILD_MODNAME, vip);
 	if (ret) {
 		dev_err(&pdev->dev, "request_irq failed\n");
 		ret = -ENODEV;
-		goto unmap;
+		goto release_buf;
 	}
 
+	/* Alloc, initialize and register video device */
 	vip->video_dev = video_device_alloc();
 	if (!vip->video_dev) {
 		ret = -ENOMEM;
 		goto release_irq;
 	}
 
-	*(vip->video_dev) = video_dev_template;
+	vip->video_dev = &video_dev_template;
+	vip->video_dev->v4l2_dev = &vip->v4l2_dev;
+	vip->video_dev->queue = &vip->vb_vidq;
+	set_bit(V4L2_FL_USE_FH_PRIO, &vip->video_dev->flags);
 	video_set_drvdata(vip->video_dev, vip);
 
 	ret = video_register_device(vip->video_dev, VFL_TYPE_GRABBER, -1);
 	if (ret)
 		goto vrelease;
 
+	/* Get ADV7180 subdevice */
 	vip->adapter = i2c_get_adapter(vip->config->i2c_id);
 	if (!vip->adapter) {
 		ret = -ENODEV;
@@ -1328,10 +922,11 @@ static int __devinit sta2x11_vip_init_one(struct pci_dev *pdev,
 	}
 
 	i2c_put_adapter(vip->adapter);
-
 	v4l2_subdev_call(vip->decoder, core, init, 0);
 
-	pr_info("STA2X11 Video Input Port (VIP) loaded\n");
+	sta2x11_vip_init_register(vip);
+
+	dev_info(&pdev->dev, "STA2X11 Video Input Port (VIP) loaded\n");
 	return 0;
 
 vunreg:
@@ -1343,10 +938,12 @@ vrelease:
 		video_device_release(vip->video_dev);
 release_irq:
 	free_irq(pdev->irq, vip);
+release_buf:
+	sta2x11_vip_release_buffer(vip);
 	pci_disable_msi(pdev);
 unmap:
+	vb2_queue_release(&vip->vb_vidq);
 	pci_iounmap(pdev, vip->iomem);
-	mutex_destroy(&vip->mutex);
 release:
 	pci_release_regions(pdev);
 unreg:
@@ -1364,34 +961,24 @@ disable:
 	return ret;
 }
 
-/**
- * sta2x11_vip_remove_one - release device
- * @pdev: PCI device
- *
- * Undo everything done in .._init_one
- *
- * unregister video device
- * free interrupt
- * unmap ioadresses
- * free memory
- * free GPIO pins
- */
 static void __devexit sta2x11_vip_remove_one(struct pci_dev *pdev)
 {
 	struct v4l2_device *v4l2_dev = pci_get_drvdata(pdev);
 	struct sta2x11_vip *vip =
 	    container_of(v4l2_dev, struct sta2x11_vip, v4l2_dev);
 
+	sta2x11_vip_clear_register(vip);
+
 	video_set_drvdata(vip->video_dev, NULL);
 	video_unregister_device(vip->video_dev);
 	/*do not call video_device_release() here, is already done */
 	free_irq(pdev->irq, vip);
 	pci_disable_msi(pdev);
+	vb2_queue_release(&vip->vb_vidq);
 	pci_iounmap(pdev, vip->iomem);
 	pci_release_regions(pdev);
 
 	v4l2_device_unregister(&vip->v4l2_dev);
-	mutex_destroy(&vip->mutex);
 
 	vip_gpio_release(&pdev->dev, vip->config->pwr_pin,
 			 vip->config->pwr_name);
@@ -1407,18 +994,12 @@ static void __devexit sta2x11_vip_remove_one(struct pci_dev *pdev)
 
 #ifdef CONFIG_PM
 
-/**
- * sta2x11_vip_suspend - set device into power save mode
- * @pdev: PCI device
- * @state: new state of device
+/*
  *
  * all relevant registers are saved and an attempt to set a new state is made.
  *
  * return value: 0 always indicate success,
  * even if device could not be disabled. (workaround for hardware problem)
- *
- * reurn value : 0, always succesful, even if hardware does not not support
- * power down mode.
  */
 static int sta2x11_vip_suspend(struct pci_dev *pdev, pm_message_t state)
 {
@@ -1429,15 +1010,15 @@ static int sta2x11_vip_suspend(struct pci_dev *pdev, pm_message_t state)
 	int i;
 
 	spin_lock_irqsave(&vip->slock, flags);
-	vip->register_save_area[0] = REG_READ(vip, DVP_CTL);
-	REG_WRITE(vip, DVP_CTL, vip->register_save_area[0] & DVP_CTL_DIS);
-	vip->register_save_area[SAVE_COUNT] = REG_READ(vip, DVP_ITM);
-	REG_WRITE(vip, DVP_ITM, 0);
+	vip->register_save_area[0] = reg_read(vip, DVP_CTL);
+	reg_write(vip, DVP_CTL, vip->register_save_area[0] & DVP_CTL_DIS);
+	vip->register_save_area[SAVE_COUNT] = reg_read(vip, DVP_ITM);
+	reg_write(vip, DVP_ITM, 0);
 	for (i = 1; i < SAVE_COUNT; i++)
-		vip->register_save_area[i] = REG_READ(vip, 4 * i);
+		vip->register_save_area[i] = reg_read(vip, 4 * i);
 	for (i = 0; i < AUX_COUNT; i++)
 		vip->register_save_area[SAVE_COUNT + IRQ_COUNT + i] =
-		    REG_READ(vip, registers_to_save[i]);
+		    reg_read(vip, registers_to_save[i]);
 	spin_unlock_irqrestore(&vip->slock, flags);
 	/* save pci state */
 	pci_save_state(pdev);
@@ -1453,16 +1034,9 @@ static int sta2x11_vip_suspend(struct pci_dev *pdev, pm_message_t state)
 	return 0;
 }
 
-/**
- * sta2x11_vip_resume - resume device operation
- * @pdev : PCI device
- *
+/*
  * re-enable device, set PCI state to powered and restore registers.
  * resume normal device operation afterwards.
- *
- * return value: 0, no error.
- *
- * other, could not set device to power on state.
  */
 static int sta2x11_vip_resume(struct pci_dev *pdev)
 {
@@ -1477,7 +1051,7 @@ static int sta2x11_vip_resume(struct pci_dev *pdev)
 	if (vip->disabled) {
 		ret = pci_enable_device(pdev);
 		if (ret) {
-			pr_warning("VIP: Can't enable device.\n");
+			pr_warn("VIP: Can't enable device.\n");
 			return ret;
 		}
 		vip->disabled = 0;
@@ -1488,7 +1062,7 @@ static int sta2x11_vip_resume(struct pci_dev *pdev)
 		 * do not call pci_disable_device on sta2x11 because it
 		 * break all other Bus masters on this EP
 		 */
-		pr_warning("VIP: Can't enable device.\n");
+		pr_warn("VIP: Can't enable device.\n");
 		vip->disabled = 1;
 		return ret;
 	}
@@ -1497,12 +1071,12 @@ static int sta2x11_vip_resume(struct pci_dev *pdev)
 
 	spin_lock_irqsave(&vip->slock, flags);
 	for (i = 1; i < SAVE_COUNT; i++)
-		REG_WRITE(vip, 4 * i, vip->register_save_area[i]);
+		reg_write(vip, 4 * i, vip->register_save_area[i]);
 	for (i = 0; i < AUX_COUNT; i++)
-		REG_WRITE(vip, registers_to_save[i],
+		reg_write(vip, registers_to_save[i],
 			  vip->register_save_area[SAVE_COUNT + IRQ_COUNT + i]);
-	REG_WRITE(vip, DVP_CTL, vip->register_save_area[0]);
-	REG_WRITE(vip, DVP_ITM, vip->register_save_area[SAVE_COUNT]);
+	reg_write(vip, DVP_CTL, vip->register_save_area[0]);
+	reg_write(vip, DVP_ITM, vip->register_save_area[SAVE_COUNT]);
 	spin_unlock_irqrestore(&vip->slock, flags);
 	return 0;
 }
@@ -1515,7 +1089,7 @@ static DEFINE_PCI_DEVICE_TABLE(sta2x11_vip_pci_tbl) = {
 };
 
 static struct pci_driver sta2x11_vip_driver = {
-	.name = DRV_NAME,
+	.name = KBUILD_MODNAME,
 	.probe = sta2x11_vip_init_one,
 	.remove = __devexit_p(sta2x11_vip_remove_one),
 	.id_table = sta2x11_vip_pci_tbl,
@@ -1543,7 +1117,7 @@ late_initcall_sync(sta2x11_vip_init_module);
 #endif
 
 MODULE_DESCRIPTION("STA2X11 Video Input Port driver");
-MODULE_AUTHOR("Wind River");
+MODULE_AUTHOR("Federico Vaga <federico.vaga@gmail.com>");
 MODULE_LICENSE("GPL v2");
 MODULE_SUPPORTED_DEVICE("sta2x11 video input");
 MODULE_VERSION(DRV_VERSION);
-- 
1.7.11.4


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

* [PATCH v3 4/4] adv7180: remove {query/g_/s_}ctrl
  2012-09-24 10:58 [PATCH v3 1/4] v4l: vb2: add prepare/finish callbacks to allocators Federico Vaga
  2012-09-24 10:58 ` [PATCH v3 2/4] videobuf2-dma-streaming: new videobuf2 memory allocator Federico Vaga
  2012-09-24 10:58 ` [PATCH v3 3/4] sta2x11_vip: convert to videobuf2 and control framework Federico Vaga
@ 2012-09-24 10:58 ` Federico Vaga
  2012-09-24 12:46 ` [PATCH v3 1/4] v4l: vb2: add prepare/finish callbacks to allocators Marek Szyprowski
  2012-09-25 15:04 ` Federico Vaga
  4 siblings, 0 replies; 32+ messages in thread
From: Federico Vaga @ 2012-09-24 10:58 UTC (permalink / raw)
  To: Mauro Carvalho Chehab, Pawel Osciak, Marek Szyprowski, Hans Verkuil
  Cc: Giancarlo Asnaghi, linux-media, linux-kernel, Jonathan Corbet,
	Federico Vaga

All drivers which use this subdevice use also the control framework.
The v4l2_subdev_core_ops operations {query/g_/s_}ctrl are useless because
device drivers will inherit controls from this subdevice.

Signed-off-by: Federico Vaga <federico.vaga@gmail.com>
---
 drivers/media/i2c/adv7180.c | 3 ---
 1 file modificato, 3 rimozioni(-)

diff --git a/drivers/media/i2c/adv7180.c b/drivers/media/i2c/adv7180.c
index 45ecf8d..43bc2b9 100644
--- a/drivers/media/i2c/adv7180.c
+++ b/drivers/media/i2c/adv7180.c
@@ -402,9 +402,6 @@ static const struct v4l2_subdev_video_ops adv7180_video_ops = {
 static const struct v4l2_subdev_core_ops adv7180_core_ops = {
 	.g_chip_ident = adv7180_g_chip_ident,
 	.s_std = adv7180_s_std,
-	.queryctrl = v4l2_subdev_queryctrl,
-	.g_ctrl = v4l2_subdev_g_ctrl,
-	.s_ctrl = v4l2_subdev_s_ctrl,
 };
 
 static const struct v4l2_subdev_ops adv7180_ops = {
-- 
1.7.11.4


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

* RE: [PATCH v3 2/4] videobuf2-dma-streaming: new videobuf2 memory allocator
  2012-09-24 10:58 ` [PATCH v3 2/4] videobuf2-dma-streaming: new videobuf2 memory allocator Federico Vaga
@ 2012-09-24 12:44   ` Marek Szyprowski
  2012-12-04 16:04     ` Mauro Carvalho Chehab
  0 siblings, 1 reply; 32+ messages in thread
From: Marek Szyprowski @ 2012-09-24 12:44 UTC (permalink / raw)
  To: 'Federico Vaga', 'Mauro Carvalho Chehab',
	'Pawel Osciak', 'Hans Verkuil'
  Cc: 'Giancarlo Asnaghi',
	linux-media, linux-kernel, 'Jonathan Corbet'

Hello,

On Monday, September 24, 2012 12:59 PM Federico Vaga wrote:

> The DMA streaming allocator is similar to the DMA contig but it use the
> DMA streaming interface (dma_map_single, dma_unmap_single). The
> allocator allocates buffers and immediately map the memory for DMA
> transfer. For each buffer prepare/finish it does a DMA synchronization.
> 
> Signed-off-by: Federico Vaga <federico.vaga@gmail.com>

Acked-by: Marek Szyprowski <m.szyprowski@samsung.com>

> ---
>  drivers/media/v4l2-core/Kconfig                   |   5 +
>  drivers/media/v4l2-core/Makefile                  |   1 +
>  drivers/media/v4l2-core/videobuf2-dma-streaming.c | 205 ++++++++++++++++++++++
>  include/media/videobuf2-dma-streaming.h           |  32 ++++
>  4 file modificati, 243 inserzioni(+)
>  create mode 100644 drivers/media/v4l2-core/videobuf2-dma-streaming.c
>  create mode 100644 include/media/videobuf2-dma-streaming.h
> 
> diff --git a/drivers/media/v4l2-core/Kconfig b/drivers/media/v4l2-core/Kconfig
> index 0c54e19..60548a7 100644
> --- a/drivers/media/v4l2-core/Kconfig
> +++ b/drivers/media/v4l2-core/Kconfig
> @@ -79,3 +79,8 @@ config VIDEOBUF2_DMA_SG
>  	#depends on HAS_DMA
>  	select VIDEOBUF2_CORE
>  	select VIDEOBUF2_MEMOPS
> +
> +config VIDEOBUF2_DMA_STREAMING
> +	select VIDEOBUF2_CORE
> +	select VIDEOBUF2_MEMOPS
> +	tristate
> diff --git a/drivers/media/v4l2-core/Makefile b/drivers/media/v4l2-core/Makefile
> index c2d61d4..0b2756f 100644
> --- a/drivers/media/v4l2-core/Makefile
> +++ b/drivers/media/v4l2-core/Makefile
> @@ -28,6 +28,7 @@ obj-$(CONFIG_VIDEOBUF2_MEMOPS) += videobuf2-memops.o
>  obj-$(CONFIG_VIDEOBUF2_VMALLOC) += videobuf2-vmalloc.o
>  obj-$(CONFIG_VIDEOBUF2_DMA_CONTIG) += videobuf2-dma-contig.o
>  obj-$(CONFIG_VIDEOBUF2_DMA_SG) += videobuf2-dma-sg.o
> +obj-$(CONFIG_VIDEOBUF2_DMA_STREAMING) += videobuf2-dma-streaming.o
> 
>  ccflags-y += -I$(srctree)/drivers/media/dvb-core
>  ccflags-y += -I$(srctree)/drivers/media/dvb-frontends
> diff --git a/drivers/media/v4l2-core/videobuf2-dma-streaming.c b/drivers/media/v4l2-
> core/videobuf2-dma-streaming.c
> new file mode 100644
> index 0000000..c839e05
> --- /dev/null
> +++ b/drivers/media/v4l2-core/videobuf2-dma-streaming.c
> @@ -0,0 +1,205 @@
> +/*
> + * videobuf2-dma-streaming.c - DMA streaming memory allocator for videobuf2
> + *
> + * Copyright (C) 2012 Federico Vaga <federico.vaga@gmail.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#include <linux/module.h>
> +#include <linux/slab.h>
> +#include <linux/pagemap.h>
> +#include <linux/dma-mapping.h>
> +
> +#include <media/videobuf2-core.h>
> +#include <media/videobuf2-dma-streaming.h>
> +#include <media/videobuf2-memops.h>
> +
> +struct vb2_streaming_conf {
> +	struct device			*dev;
> +};
> +struct vb2_streaming_buf {
> +	struct vb2_streaming_conf	*conf;
> +	void				*vaddr;
> +
> +	dma_addr_t			dma_handle;
> +
> +	unsigned long			size;
> +	struct vm_area_struct		*vma;
> +
> +	atomic_t			refcount;
> +	struct vb2_vmarea_handler	handler;
> +};
> +
> +static void vb2_dma_streaming_put(void *buf_priv)
> +{
> +	struct vb2_streaming_buf *buf = buf_priv;
> +
> +	if (atomic_dec_and_test(&buf->refcount)) {
> +		dma_unmap_single(buf->conf->dev, buf->dma_handle, buf->size,
> +				 DMA_FROM_DEVICE);
> +		free_pages_exact(buf->vaddr, buf->size);
> +		kfree(buf);
> +	}
> +
> +}
> +
> +static void *vb2_dma_streaming_alloc(void *alloc_ctx, unsigned long size)
> +{
> +	struct vb2_streaming_conf *conf = alloc_ctx;
> +	struct vb2_streaming_buf *buf;
> +	int err;
> +
> +	buf = kzalloc(sizeof(struct vb2_streaming_buf), GFP_KERNEL);
> +	if (!buf)
> +		return ERR_PTR(-ENOMEM);
> +	buf->vaddr = alloc_pages_exact(size, GFP_KERNEL | GFP_DMA);
> +	if (!buf->vaddr) {
> +		err = -ENOMEM;
> +		goto out;
> +	}
> +	buf->dma_handle = dma_map_single(conf->dev, buf->vaddr, size,
> +					 DMA_FROM_DEVICE);
> +	err = dma_mapping_error(conf->dev, buf->dma_handle);
> +	if (err) {
> +		dev_err(conf->dev, "dma_map_single failed\n");
> +
> +		free_pages_exact(buf->vaddr, size);
> +		buf->vaddr = NULL;
> +		goto out_pages;
> +	}
> +	buf->conf = conf;
> +	buf->size = size;
> +	buf->handler.refcount = &buf->refcount;
> +	buf->handler.put = vb2_dma_streaming_put;
> +	buf->handler.arg = buf;
> +
> +	atomic_inc(&buf->refcount);
> +	return buf;
> +
> +out_pages:
> +	free_pages_exact(buf->vaddr, buf->size);
> +out:
> +	kfree(buf);
> +	return ERR_PTR(err);
> +}
> +
> +static void *vb2_dma_streaming_cookie(void *buf_priv)
> +{
> +	struct vb2_streaming_buf *buf = buf_priv;
> +
> +	return &buf->dma_handle;
> +}
> +
> +static void *vb2_dma_streaming_vaddr(void *buf_priv)
> +{
> +	struct vb2_streaming_buf *buf = buf_priv;
> +
> +	if (!buf)
> +		return NULL;
> +	return buf->vaddr;
> +}
> +
> +static unsigned int vb2_dma_streaming_num_users(void *buf_priv)
> +{
> +	struct vb2_streaming_buf *buf = buf_priv;
> +
> +	return atomic_read(&buf->refcount);
> +}
> +
> +static int vb2_dma_streaming_mmap(void *buf_priv, struct vm_area_struct *vma)
> +{
> +	struct vb2_streaming_buf *buf = buf_priv;
> +	unsigned long pos, start = vma->vm_start;
> +	unsigned long size;
> +	struct page *page;
> +	int err;
> +
> +	/* Try to remap memory */
> +	size = vma->vm_end - vma->vm_start;
> +	size = (size < buf->size) ? size : buf->size;
> +	pos = (unsigned long)buf->vaddr;
> +
> +	while (size > 0) {
> +		page = virt_to_page((void *)pos);
> +		if (!page) {
> +			dev_err(buf->conf->dev, "mmap: virt_to_page failed\n");
> +			return -ENOMEM;
> +		}
> +		err = vm_insert_page(vma, start, page);
> +		if (err) {
> +			dev_err(buf->conf->dev, "mmap: insert failed %d\n", err);
> +			return -ENOMEM;
> +		}
> +		start += PAGE_SIZE;
> +		pos += PAGE_SIZE;
> +
> +		if (size > PAGE_SIZE)
> +			size -= PAGE_SIZE;
> +		else
> +			size = 0;
> +	}
> +
> +
> +	vma->vm_ops = &vb2_common_vm_ops;
> +	vma->vm_flags |= VM_DONTEXPAND;
> +	vma->vm_private_data = &buf->handler;
> +
> +	vma->vm_ops->open(vma);
> +
> +	return 0;
> +}
> +
> +static void vb2_dma_streaming_prepare(void *buf_priv)
> +{
> +	struct vb2_streaming_buf *buf = buf_priv;
> +
> +	dma_sync_single_for_device(buf->conf->dev, buf->dma_handle,
> +				   buf->size, DMA_FROM_DEVICE);
> +}
> +
> +static void vb2_dma_streaming_finish(void *buf_priv)
> +{
> +	struct vb2_streaming_buf *buf = buf_priv;
> +
> +	dma_sync_single_for_cpu(buf->conf->dev, buf->dma_handle,
> +				buf->size, DMA_FROM_DEVICE);
> +}
> +
> +const struct vb2_mem_ops vb2_dma_streaming_memops = {
> +	.alloc		= vb2_dma_streaming_alloc,
> +	.put		= vb2_dma_streaming_put,
> +	.cookie		= vb2_dma_streaming_cookie,
> +	.vaddr		= vb2_dma_streaming_vaddr,
> +	.mmap		= vb2_dma_streaming_mmap,
> +	.num_users	= vb2_dma_streaming_num_users,
> +	.prepare	= vb2_dma_streaming_prepare,
> +	.finish		= vb2_dma_streaming_finish,
> +};
> +EXPORT_SYMBOL_GPL(vb2_dma_streaming_memops);
> +
> +void *vb2_dma_streaming_init_ctx(struct device *dev)
> +{
> +	struct vb2_streaming_conf *conf;
> +
> +	conf = kmalloc(sizeof(struct vb2_streaming_conf), GFP_KERNEL);
> +	if (!conf)
> +		return ERR_PTR(-ENOMEM);
> +
> +	conf->dev = dev;
> +
> +	return conf;
> +}
> +EXPORT_SYMBOL_GPL(vb2_dma_streaming_init_ctx);
> +
> +void vb2_dma_streaming_cleanup_ctx(void *alloc_ctx)
> +{
> +	kfree(alloc_ctx);
> +}
> +EXPORT_SYMBOL_GPL(vb2_dma_streaming_cleanup_ctx);
> +
> +MODULE_DESCRIPTION("DMA-streaming memory allocator for videobuf2");
> +MODULE_AUTHOR("Federico Vaga <federico.vaga@gmail.com>");
> +MODULE_LICENSE("GPL v2");
> diff --git a/include/media/videobuf2-dma-streaming.h b/include/media/videobuf2-dma-streaming.h
> new file mode 100644
> index 0000000..2a62d93
> --- /dev/null
> +++ b/include/media/videobuf2-dma-streaming.h
> @@ -0,0 +1,32 @@
> +/*
> + * videobuf2-dma-streaming.h - DMA streaming memory allocator for videobuf2
> + *
> + * Copyright (C) 2012 Federico Vaga
> + *
> + * Author: Federico Vaga <federico.vaga@gmail.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation.
> + */
> +
> +#ifndef _MEDIA_VIDEOBUF2_DMA_STREAMING_H
> +#define _MEDIA_VIDEOBUF2_DMA_STREAMING_H
> +
> +#include <media/videobuf2-core.h>
> +#include <linux/dma-mapping.h>
> +
> +void *vb2_dma_streaming_init_ctx(struct device *dev);
> +void vb2_dma_streaming_cleanup_ctx(void *alloc_ctx);
> +
> +extern const struct vb2_mem_ops vb2_dma_streaming_memops;
> +
> +static inline dma_addr_t
> +vb2_dma_streaming_plane_paddr(struct vb2_buffer *vb, unsigned int plane_no)
> +{
> +	dma_addr_t *dma_addr = vb2_plane_cookie(vb, plane_no);
> +
> +	return *dma_addr;
> +}
> +
> +#endif
> --
> 1.7.11.4

Best regards
-- 
Marek Szyprowski
Samsung Poland R&D Center



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

* RE: [PATCH v3 1/4] v4l: vb2: add prepare/finish callbacks to allocators
  2012-09-24 10:58 [PATCH v3 1/4] v4l: vb2: add prepare/finish callbacks to allocators Federico Vaga
                   ` (2 preceding siblings ...)
  2012-09-24 10:58 ` [PATCH v3 4/4] adv7180: remove {query/g_/s_}ctrl Federico Vaga
@ 2012-09-24 12:46 ` Marek Szyprowski
  2012-09-25 15:04 ` Federico Vaga
  4 siblings, 0 replies; 32+ messages in thread
From: Marek Szyprowski @ 2012-09-24 12:46 UTC (permalink / raw)
  To: 'Federico Vaga', 'Mauro Carvalho Chehab',
	'Pawel Osciak', 'Hans Verkuil'
  Cc: 'Giancarlo Asnaghi',
	linux-media, linux-kernel, 'Jonathan Corbet'

Hello,

It would be great if you could keep the correct authorship of the patch by adding the following
line on top of the patch (git will handle it automatically after applying):

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

On Monday, September 24, 2012 12:59 PM Federico Vaga wrote:

> This patch adds support for prepare/finish callbacks in VB2 allocators.
> These callback are used for buffer flushing.
> 
> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
> Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Acked-by: Federico Vaga <federico.vaga@gmail.com>
> ---
>  drivers/media/v4l2-core/videobuf2-core.c | 11 +++++++++++
>  include/media/videobuf2-core.h           |  7 +++++++
>  2 file modificati, 18 inserzioni(+)
> 
> diff --git a/drivers/media/v4l2-core/videobuf2-core.c b/drivers/media/v4l2-core/videobuf2-
> core.c
> index 4da3df6..079fa79 100644
> --- a/drivers/media/v4l2-core/videobuf2-core.c
> +++ b/drivers/media/v4l2-core/videobuf2-core.c
> @@ -790,6 +790,7 @@ void vb2_buffer_done(struct vb2_buffer *vb, enum vb2_buffer_state state)
>  {
>  	struct vb2_queue *q = vb->vb2_queue;
>  	unsigned long flags;
> +	unsigned int plane;
> 
>  	if (vb->state != VB2_BUF_STATE_ACTIVE)
>  		return;
> @@ -800,6 +801,10 @@ void vb2_buffer_done(struct vb2_buffer *vb, enum vb2_buffer_state state)
>  	dprintk(4, "Done processing on buffer %d, state: %d\n",
>  			vb->v4l2_buf.index, vb->state);
> 
> +	/* sync buffers */
> +	for (plane = 0; plane < vb->num_planes; ++plane)
> +		call_memop(q, finish, vb->planes[plane].mem_priv);
> +
>  	/* Add the buffer to the done buffers list */
>  	spin_lock_irqsave(&q->done_lock, flags);
>  	vb->state = state;
> @@ -975,9 +980,15 @@ static int __qbuf_mmap(struct vb2_buffer *vb, const struct v4l2_buffer
> *b)
>  static void __enqueue_in_driver(struct vb2_buffer *vb)
>  {
>  	struct vb2_queue *q = vb->vb2_queue;
> +	unsigned int plane;
> 
>  	vb->state = VB2_BUF_STATE_ACTIVE;
>  	atomic_inc(&q->queued_count);
> +
> +	/* sync buffers */
> +	for (plane = 0; plane < vb->num_planes; ++plane)
> +		call_memop(q, prepare, vb->planes[plane].mem_priv);
> +
>  	q->ops->buf_queue(vb);
>  }
> 
> diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h
> index 8dd9b6c..2508609 100644
> --- a/include/media/videobuf2-core.h
> +++ b/include/media/videobuf2-core.h
> @@ -41,6 +41,10 @@ struct vb2_fileio_data;
>   *		 argument to other ops in this structure
>   * @put_userptr: inform the allocator that a USERPTR buffer will no longer
>   *		 be used
> + * @prepare:	called every time the buffer is passed from userspace to the
> + *		driver, usefull for cache synchronisation, optional
> + * @finish:	called every time the buffer is passed back from the driver
> + *		to the userspace, also optional
>   * @vaddr:	return a kernel virtual address to a given memory buffer
>   *		associated with the passed private structure or NULL if no
>   *		such mapping exists
> @@ -65,6 +69,9 @@ struct vb2_mem_ops {
>  					unsigned long size, int write);
>  	void		(*put_userptr)(void *buf_priv);
> 
> +	void		(*prepare)(void *buf_priv);
> +	void		(*finish)(void *buf_priv);
> +
>  	void		*(*vaddr)(void *buf_priv);
>  	void		*(*cookie)(void *buf_priv);
> 
> --
> 1.7.11.4

Best regards
-- 
Marek Szyprowski
Samsung Poland R&D Center



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

* [PATCH v3 1/4] v4l: vb2: add prepare/finish callbacks to allocators
  2012-09-24 10:58 [PATCH v3 1/4] v4l: vb2: add prepare/finish callbacks to allocators Federico Vaga
                   ` (3 preceding siblings ...)
  2012-09-24 12:46 ` [PATCH v3 1/4] v4l: vb2: add prepare/finish callbacks to allocators Marek Szyprowski
@ 2012-09-25 15:04 ` Federico Vaga
  4 siblings, 0 replies; 32+ messages in thread
From: Federico Vaga @ 2012-09-25 15:04 UTC (permalink / raw)
  To: Mauro Carvalho Chehab, Pawel Osciak, Marek Szyprowski, Hans Verkuil
  Cc: Giancarlo Asnaghi, linux-media, linux-kernel, Jonathan Corbet,
	Federico Vaga

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

This patch adds support for prepare/finish callbacks in VB2 allocators.
These callback are used for buffer flushing.

Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Acked-by: Federico Vaga <federico.vaga@gmail.com>
---
 drivers/media/v4l2-core/videobuf2-core.c | 11 +++++++++++
 include/media/videobuf2-core.h           |  7 +++++++
 2 file modificati, 18 inserzioni(+)

diff --git a/drivers/media/v4l2-core/videobuf2-core.c b/drivers/media/v4l2-core/videobuf2-core.c
index 4da3df6..079fa79 100644
--- a/drivers/media/v4l2-core/videobuf2-core.c
+++ b/drivers/media/v4l2-core/videobuf2-core.c
@@ -790,6 +790,7 @@ void vb2_buffer_done(struct vb2_buffer *vb, enum vb2_buffer_state state)
 {
 	struct vb2_queue *q = vb->vb2_queue;
 	unsigned long flags;
+	unsigned int plane;
 
 	if (vb->state != VB2_BUF_STATE_ACTIVE)
 		return;
@@ -800,6 +801,10 @@ void vb2_buffer_done(struct vb2_buffer *vb, enum vb2_buffer_state state)
 	dprintk(4, "Done processing on buffer %d, state: %d\n",
 			vb->v4l2_buf.index, vb->state);
 
+	/* sync buffers */
+	for (plane = 0; plane < vb->num_planes; ++plane)
+		call_memop(q, finish, vb->planes[plane].mem_priv);
+
 	/* Add the buffer to the done buffers list */
 	spin_lock_irqsave(&q->done_lock, flags);
 	vb->state = state;
@@ -975,9 +980,15 @@ static int __qbuf_mmap(struct vb2_buffer *vb, const struct v4l2_buffer *b)
 static void __enqueue_in_driver(struct vb2_buffer *vb)
 {
 	struct vb2_queue *q = vb->vb2_queue;
+	unsigned int plane;
 
 	vb->state = VB2_BUF_STATE_ACTIVE;
 	atomic_inc(&q->queued_count);
+
+	/* sync buffers */
+	for (plane = 0; plane < vb->num_planes; ++plane)
+		call_memop(q, prepare, vb->planes[plane].mem_priv);
+
 	q->ops->buf_queue(vb);
 }
 
diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h
index 8dd9b6c..2508609 100644
--- a/include/media/videobuf2-core.h
+++ b/include/media/videobuf2-core.h
@@ -41,6 +41,10 @@ struct vb2_fileio_data;
  *		 argument to other ops in this structure
  * @put_userptr: inform the allocator that a USERPTR buffer will no longer
  *		 be used
+ * @prepare:	called every time the buffer is passed from userspace to the
+ *		driver, usefull for cache synchronisation, optional
+ * @finish:	called every time the buffer is passed back from the driver
+ *		to the userspace, also optional
  * @vaddr:	return a kernel virtual address to a given memory buffer
  *		associated with the passed private structure or NULL if no
  *		such mapping exists
@@ -65,6 +69,9 @@ struct vb2_mem_ops {
 					unsigned long size, int write);
 	void		(*put_userptr)(void *buf_priv);
 
+	void		(*prepare)(void *buf_priv);
+	void		(*finish)(void *buf_priv);
+
 	void		*(*vaddr)(void *buf_priv);
 	void		*(*cookie)(void *buf_priv);
 
-- 
1.7.11.4


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

* Re: [PATCH v3 2/4] videobuf2-dma-streaming: new videobuf2 memory allocator
  2012-09-24 12:44   ` Marek Szyprowski
@ 2012-12-04 16:04     ` Mauro Carvalho Chehab
  2012-12-05 12:50       ` Federico Vaga
  0 siblings, 1 reply; 32+ messages in thread
From: Mauro Carvalho Chehab @ 2012-12-04 16:04 UTC (permalink / raw)
  To: Marek Szyprowski
  Cc: 'Federico Vaga', 'Mauro Carvalho Chehab',
	'Pawel Osciak', 'Hans Verkuil',
	'Giancarlo Asnaghi',
	linux-media, linux-kernel, 'Jonathan Corbet'

Em 24-09-2012 09:44, Marek Szyprowski escreveu:
> Hello,
>
> On Monday, September 24, 2012 12:59 PM Federico Vaga wrote:
>
>> The DMA streaming allocator is similar to the DMA contig but it use the
>> DMA streaming interface (dma_map_single, dma_unmap_single). The
>> allocator allocates buffers and immediately map the memory for DMA
>> transfer. For each buffer prepare/finish it does a DMA synchronization.

Hmm.. the explanation didn't convince me, e. g.:
	1) why is it needed;
	2) why vb2-dma-config can't be patched to use dma_map_single
(eventually using a different vb2_io_modes bit?);
	3) what are the usecases for it.

Could you please detail it? Without that, one that would be needing to
write a driver will have serious doubts about what would be the right
driver for its usage. Also, please document it at the driver itself.

Thanks!
Mauro

>>
>> Signed-off-by: Federico Vaga <federico.vaga@gmail.com>
>
> Acked-by: Marek Szyprowski <m.szyprowski@samsung.com>
>
>> ---
>>   drivers/media/v4l2-core/Kconfig                   |   5 +
>>   drivers/media/v4l2-core/Makefile                  |   1 +
>>   drivers/media/v4l2-core/videobuf2-dma-streaming.c | 205 ++++++++++++++++++++++
>>   include/media/videobuf2-dma-streaming.h           |  32 ++++
>>   4 file modificati, 243 inserzioni(+)
>>   create mode 100644 drivers/media/v4l2-core/videobuf2-dma-streaming.c
>>   create mode 100644 include/media/videobuf2-dma-streaming.h
>>
>> diff --git a/drivers/media/v4l2-core/Kconfig b/drivers/media/v4l2-core/Kconfig
>> index 0c54e19..60548a7 100644
>> --- a/drivers/media/v4l2-core/Kconfig
>> +++ b/drivers/media/v4l2-core/Kconfig
>> @@ -79,3 +79,8 @@ config VIDEOBUF2_DMA_SG
>>   	#depends on HAS_DMA
>>   	select VIDEOBUF2_CORE
>>   	select VIDEOBUF2_MEMOPS
>> +
>> +config VIDEOBUF2_DMA_STREAMING
>> +	select VIDEOBUF2_CORE
>> +	select VIDEOBUF2_MEMOPS
>> +	tristate
>> diff --git a/drivers/media/v4l2-core/Makefile b/drivers/media/v4l2-core/Makefile
>> index c2d61d4..0b2756f 100644
>> --- a/drivers/media/v4l2-core/Makefile
>> +++ b/drivers/media/v4l2-core/Makefile
>> @@ -28,6 +28,7 @@ obj-$(CONFIG_VIDEOBUF2_MEMOPS) += videobuf2-memops.o
>>   obj-$(CONFIG_VIDEOBUF2_VMALLOC) += videobuf2-vmalloc.o
>>   obj-$(CONFIG_VIDEOBUF2_DMA_CONTIG) += videobuf2-dma-contig.o
>>   obj-$(CONFIG_VIDEOBUF2_DMA_SG) += videobuf2-dma-sg.o
>> +obj-$(CONFIG_VIDEOBUF2_DMA_STREAMING) += videobuf2-dma-streaming.o
>>
>>   ccflags-y += -I$(srctree)/drivers/media/dvb-core
>>   ccflags-y += -I$(srctree)/drivers/media/dvb-frontends
>> diff --git a/drivers/media/v4l2-core/videobuf2-dma-streaming.c b/drivers/media/v4l2-
>> core/videobuf2-dma-streaming.c
>> new file mode 100644
>> index 0000000..c839e05
>> --- /dev/null
>> +++ b/drivers/media/v4l2-core/videobuf2-dma-streaming.c
>> @@ -0,0 +1,205 @@
>> +/*
>> + * videobuf2-dma-streaming.c - DMA streaming memory allocator for videobuf2
>> + *
>> + * Copyright (C) 2012 Federico Vaga <federico.vaga@gmail.com>
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2 as
>> + * published by the Free Software Foundation.
>> + */
>> +
>> +#include <linux/module.h>
>> +#include <linux/slab.h>
>> +#include <linux/pagemap.h>
>> +#include <linux/dma-mapping.h>
>> +
>> +#include <media/videobuf2-core.h>
>> +#include <media/videobuf2-dma-streaming.h>
>> +#include <media/videobuf2-memops.h>
>> +
>> +struct vb2_streaming_conf {
>> +	struct device			*dev;
>> +};
>> +struct vb2_streaming_buf {
>> +	struct vb2_streaming_conf	*conf;
>> +	void				*vaddr;
>> +
>> +	dma_addr_t			dma_handle;
>> +
>> +	unsigned long			size;
>> +	struct vm_area_struct		*vma;
>> +
>> +	atomic_t			refcount;
>> +	struct vb2_vmarea_handler	handler;
>> +};
>> +
>> +static void vb2_dma_streaming_put(void *buf_priv)
>> +{
>> +	struct vb2_streaming_buf *buf = buf_priv;
>> +
>> +	if (atomic_dec_and_test(&buf->refcount)) {
>> +		dma_unmap_single(buf->conf->dev, buf->dma_handle, buf->size,
>> +				 DMA_FROM_DEVICE);
>> +		free_pages_exact(buf->vaddr, buf->size);
>> +		kfree(buf);
>> +	}
>> +
>> +}
>> +
>> +static void *vb2_dma_streaming_alloc(void *alloc_ctx, unsigned long size)
>> +{
>> +	struct vb2_streaming_conf *conf = alloc_ctx;
>> +	struct vb2_streaming_buf *buf;
>> +	int err;
>> +
>> +	buf = kzalloc(sizeof(struct vb2_streaming_buf), GFP_KERNEL);
>> +	if (!buf)
>> +		return ERR_PTR(-ENOMEM);
>> +	buf->vaddr = alloc_pages_exact(size, GFP_KERNEL | GFP_DMA);
>> +	if (!buf->vaddr) {
>> +		err = -ENOMEM;
>> +		goto out;
>> +	}
>> +	buf->dma_handle = dma_map_single(conf->dev, buf->vaddr, size,
>> +					 DMA_FROM_DEVICE);
>> +	err = dma_mapping_error(conf->dev, buf->dma_handle);
>> +	if (err) {
>> +		dev_err(conf->dev, "dma_map_single failed\n");
>> +
>> +		free_pages_exact(buf->vaddr, size);
>> +		buf->vaddr = NULL;
>> +		goto out_pages;
>> +	}
>> +	buf->conf = conf;
>> +	buf->size = size;
>> +	buf->handler.refcount = &buf->refcount;
>> +	buf->handler.put = vb2_dma_streaming_put;
>> +	buf->handler.arg = buf;
>> +
>> +	atomic_inc(&buf->refcount);
>> +	return buf;
>> +
>> +out_pages:
>> +	free_pages_exact(buf->vaddr, buf->size);
>> +out:
>> +	kfree(buf);
>> +	return ERR_PTR(err);
>> +}
>> +
>> +static void *vb2_dma_streaming_cookie(void *buf_priv)
>> +{
>> +	struct vb2_streaming_buf *buf = buf_priv;
>> +
>> +	return &buf->dma_handle;
>> +}
>> +
>> +static void *vb2_dma_streaming_vaddr(void *buf_priv)
>> +{
>> +	struct vb2_streaming_buf *buf = buf_priv;
>> +
>> +	if (!buf)
>> +		return NULL;
>> +	return buf->vaddr;
>> +}
>> +
>> +static unsigned int vb2_dma_streaming_num_users(void *buf_priv)
>> +{
>> +	struct vb2_streaming_buf *buf = buf_priv;
>> +
>> +	return atomic_read(&buf->refcount);
>> +}
>> +
>> +static int vb2_dma_streaming_mmap(void *buf_priv, struct vm_area_struct *vma)
>> +{
>> +	struct vb2_streaming_buf *buf = buf_priv;
>> +	unsigned long pos, start = vma->vm_start;
>> +	unsigned long size;
>> +	struct page *page;
>> +	int err;
>> +
>> +	/* Try to remap memory */
>> +	size = vma->vm_end - vma->vm_start;
>> +	size = (size < buf->size) ? size : buf->size;
>> +	pos = (unsigned long)buf->vaddr;
>> +
>> +	while (size > 0) {
>> +		page = virt_to_page((void *)pos);
>> +		if (!page) {
>> +			dev_err(buf->conf->dev, "mmap: virt_to_page failed\n");
>> +			return -ENOMEM;
>> +		}
>> +		err = vm_insert_page(vma, start, page);
>> +		if (err) {
>> +			dev_err(buf->conf->dev, "mmap: insert failed %d\n", err);
>> +			return -ENOMEM;
>> +		}
>> +		start += PAGE_SIZE;
>> +		pos += PAGE_SIZE;
>> +
>> +		if (size > PAGE_SIZE)
>> +			size -= PAGE_SIZE;
>> +		else
>> +			size = 0;
>> +	}
>> +
>> +
>> +	vma->vm_ops = &vb2_common_vm_ops;
>> +	vma->vm_flags |= VM_DONTEXPAND;
>> +	vma->vm_private_data = &buf->handler;
>> +
>> +	vma->vm_ops->open(vma);
>> +
>> +	return 0;
>> +}
>> +
>> +static void vb2_dma_streaming_prepare(void *buf_priv)
>> +{
>> +	struct vb2_streaming_buf *buf = buf_priv;
>> +
>> +	dma_sync_single_for_device(buf->conf->dev, buf->dma_handle,
>> +				   buf->size, DMA_FROM_DEVICE);
>> +}
>> +
>> +static void vb2_dma_streaming_finish(void *buf_priv)
>> +{
>> +	struct vb2_streaming_buf *buf = buf_priv;
>> +
>> +	dma_sync_single_for_cpu(buf->conf->dev, buf->dma_handle,
>> +				buf->size, DMA_FROM_DEVICE);
>> +}
>> +
>> +const struct vb2_mem_ops vb2_dma_streaming_memops = {
>> +	.alloc		= vb2_dma_streaming_alloc,
>> +	.put		= vb2_dma_streaming_put,
>> +	.cookie		= vb2_dma_streaming_cookie,
>> +	.vaddr		= vb2_dma_streaming_vaddr,
>> +	.mmap		= vb2_dma_streaming_mmap,
>> +	.num_users	= vb2_dma_streaming_num_users,
>> +	.prepare	= vb2_dma_streaming_prepare,
>> +	.finish		= vb2_dma_streaming_finish,
>> +};
>> +EXPORT_SYMBOL_GPL(vb2_dma_streaming_memops);
>> +
>> +void *vb2_dma_streaming_init_ctx(struct device *dev)
>> +{
>> +	struct vb2_streaming_conf *conf;
>> +
>> +	conf = kmalloc(sizeof(struct vb2_streaming_conf), GFP_KERNEL);
>> +	if (!conf)
>> +		return ERR_PTR(-ENOMEM);
>> +
>> +	conf->dev = dev;
>> +
>> +	return conf;
>> +}
>> +EXPORT_SYMBOL_GPL(vb2_dma_streaming_init_ctx);
>> +
>> +void vb2_dma_streaming_cleanup_ctx(void *alloc_ctx)
>> +{
>> +	kfree(alloc_ctx);
>> +}
>> +EXPORT_SYMBOL_GPL(vb2_dma_streaming_cleanup_ctx);
>> +
>> +MODULE_DESCRIPTION("DMA-streaming memory allocator for videobuf2");
>> +MODULE_AUTHOR("Federico Vaga <federico.vaga@gmail.com>");
>> +MODULE_LICENSE("GPL v2");
>> diff --git a/include/media/videobuf2-dma-streaming.h b/include/media/videobuf2-dma-streaming.h
>> new file mode 100644
>> index 0000000..2a62d93
>> --- /dev/null
>> +++ b/include/media/videobuf2-dma-streaming.h
>> @@ -0,0 +1,32 @@
>> +/*
>> + * videobuf2-dma-streaming.h - DMA streaming memory allocator for videobuf2
>> + *
>> + * Copyright (C) 2012 Federico Vaga
>> + *
>> + * Author: Federico Vaga <federico.vaga@gmail.com>
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License as published by
>> + * the Free Software Foundation.
>> + */
>> +
>> +#ifndef _MEDIA_VIDEOBUF2_DMA_STREAMING_H
>> +#define _MEDIA_VIDEOBUF2_DMA_STREAMING_H
>> +
>> +#include <media/videobuf2-core.h>
>> +#include <linux/dma-mapping.h>
>> +
>> +void *vb2_dma_streaming_init_ctx(struct device *dev);
>> +void vb2_dma_streaming_cleanup_ctx(void *alloc_ctx);
>> +
>> +extern const struct vb2_mem_ops vb2_dma_streaming_memops;
>> +
>> +static inline dma_addr_t
>> +vb2_dma_streaming_plane_paddr(struct vb2_buffer *vb, unsigned int plane_no)
>> +{
>> +	dma_addr_t *dma_addr = vb2_plane_cookie(vb, plane_no);
>> +
>> +	return *dma_addr;
>> +}
>> +
>> +#endif
>> --
>> 1.7.11.4
>
> Best regards
>


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

* Re: [PATCH v3 3/4] sta2x11_vip: convert to videobuf2 and control framework
  2012-09-24 10:58 ` [PATCH v3 3/4] sta2x11_vip: convert to videobuf2 and control framework Federico Vaga
@ 2012-12-04 16:15   ` Mauro Carvalho Chehab
  2012-12-05  1:12     ` Federico Vaga
  0 siblings, 1 reply; 32+ messages in thread
From: Mauro Carvalho Chehab @ 2012-12-04 16:15 UTC (permalink / raw)
  To: Federico Vaga
  Cc: Mauro Carvalho Chehab, Pawel Osciak, Marek Szyprowski,
	Hans Verkuil, Giancarlo Asnaghi, linux-media, linux-kernel,
	Jonathan Corbet

Em 24-09-2012 07:58, Federico Vaga escreveu:
> This patch re-write the driver and use the videobuf2
> interface instead of the old videobuf. Moreover, it uses also
> the control framework which allows the driver to inherit
> controls from its subdevice (ADV7180)
>
> Signed-off-by: Federico Vaga <federico.vaga@gmail.com>
> Acked-by: Giancarlo Asnaghi <giancarlo.asnaghi@st.com>
> ---
>   drivers/media/pci/sta2x11/Kconfig       |    2 +-
>   drivers/media/pci/sta2x11/sta2x11_vip.c | 1238 ++++++++++---------------------
>   2 file modificati, 407 inserzioni(+), 833 rimozioni(-)
>
> diff --git a/drivers/media/pci/sta2x11/Kconfig b/drivers/media/pci/sta2x11/Kconfig
> index 6749f67..654339f 100644
> --- a/drivers/media/pci/sta2x11/Kconfig
> +++ b/drivers/media/pci/sta2x11/Kconfig
> @@ -2,7 +2,7 @@ config STA2X11_VIP
>   	tristate "STA2X11 VIP Video For Linux"
>   	depends on STA2X11
>   	select VIDEO_ADV7180 if MEDIA_SUBDRV_AUTOSELECT
> -	select VIDEOBUF_DMA_CONTIG
> +	select VIDEOBUF2_DMA_STREAMING
>   	depends on PCI && VIDEO_V4L2 && VIRT_TO_BUS
>   	help
>   	  Say Y for support for STA2X11 VIP (Video Input Port) capture
> diff --git a/drivers/media/pci/sta2x11/sta2x11_vip.c b/drivers/media/pci/sta2x11/sta2x11_vip.c
> index 4c10205..b9ff926 100644
> --- a/drivers/media/pci/sta2x11/sta2x11_vip.c
> +++ b/drivers/media/pci/sta2x11/sta2x11_vip.c
> @@ -1,6 +1,7 @@
>   /*
>    * This is the driver for the STA2x11 Video Input Port.
>    *
> + * Copyright (C) 2012       ST Microelectronics
>    * Copyright (C) 2010       WindRiver Systems, Inc.
>    *
>    * This program is free software; you can redistribute it and/or modify it
> @@ -19,36 +20,30 @@
>    * The full GNU General Public License is included in this distribution in
>    * the file called "COPYING".
>    *
> - * Author: Andreas Kies <andreas.kies@windriver.com>
> - *		Vlad Lungu <vlad.lungu@windriver.com>


Why are you dropping those authorship data?

Ok, it is clear to me that most of the code there got rewritten, and,
while IANAL, I think they still have some copyrights on it.

So, if you're willing to do that, you need to get authors ack
on such patch.

...

>
>   MODULE_DESCRIPTION("STA2X11 Video Input Port driver");
> -MODULE_AUTHOR("Wind River");

Same note applies here: we need Wind River's ack on that to drop it.

> +MODULE_AUTHOR("Federico Vaga <federico.vaga@gmail.com>");
>   MODULE_LICENSE("GPL v2");
>   MODULE_SUPPORTED_DEVICE("sta2x11 video input");
>   MODULE_VERSION(DRV_VERSION);
>


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

* Re: [PATCH v3 3/4] sta2x11_vip: convert to videobuf2 and control framework
  2012-12-04 16:15   ` Mauro Carvalho Chehab
@ 2012-12-05  1:12     ` Federico Vaga
  2012-12-05 11:34       ` Mauro Carvalho Chehab
  0 siblings, 1 reply; 32+ messages in thread
From: Federico Vaga @ 2012-12-05  1:12 UTC (permalink / raw)
  To: Mauro Carvalho Chehab
  Cc: Mauro Carvalho Chehab, Pawel Osciak, Marek Szyprowski,
	Hans Verkuil, Giancarlo Asnaghi, linux-media, linux-kernel,
	Jonathan Corbet

On Tuesday 04 December 2012 14:15:15 Mauro Carvalho Chehab wrote:
> Em 24-09-2012 07:58, Federico Vaga escreveu:
> > This patch re-write the driver and use the videobuf2
> > interface instead of the old videobuf. Moreover, it uses also
> > the control framework which allows the driver to inherit
> > controls from its subdevice (ADV7180)
> > 
> > Signed-off-by: Federico Vaga <federico.vaga@gmail.com>
> > Acked-by: Giancarlo Asnaghi <giancarlo.asnaghi@st.com>
> >
> > [..........]
> > 
> >   /*
> >   
> >    * This is the driver for the STA2x11 Video Input Port.
> >    *
> > 
> > + * Copyright (C) 2012       ST Microelectronics
> > 
> >    * Copyright (C) 2010       WindRiver Systems, Inc.
> >    *
> >    * This program is free software; you can redistribute it and/or modify
> >    it
> > 
> > @@ -19,36 +20,30 @@
> > 
> >    * The full GNU General Public License is included in this distribution
> >    in
> >    * the file called "COPYING".
> >    *
> > 
> > - * Author: Andreas Kies <andreas.kies@windriver.com>
> > - *		Vlad Lungu <vlad.lungu@windriver.com>
> 
> Why are you dropping those authorship data?
> 
> Ok, it is clear to me that most of the code there got rewritten, and,
> while IANAL, I think they still have some copyrights on it.
> 
> So, if you're willing to do that, you need to get authors ack
> on such patch.

I re-write the driver, and also the first version of the driver has many 
modification made by me, many bug fix, style review, remove useless code.
The first time I didn't add myself as author because the logic of the driver 
did not change. This time, plus the old change I think there is nothing of the 
original driver because I rewrite it from the hardware manual. Practically, It 
is a new driver for the same device.

Anyway I will try to contact the original authors for the acked-by.

> >   MODULE_DESCRIPTION("STA2X11 Video Input Port driver");
> > 
> > -MODULE_AUTHOR("Wind River");
> 
> Same note applies here: we need Wind River's ack on that to drop it.

I will try also for this. But I think that this is not a windriver driver 
because I re-wrote it from the hardware manual. I used the old driver because 
I thought that it was better than propose a patch that remove the old driver 
and add my driver.
I did not remove the 2010 Copyright from windriver, because they did the job, 
but this work was paid by ST (copyright 2012) and made completely by me.

Is my thinking wrong?

Just a question for the future so I avoid to redo the same error. If I re-
wrote most of a driver I cannot change the authorship automatically without 
the acked-by of the previous author. If I ask to the previous author and he 
does not give me the acked-by (or he is unreachable, he change email address), 
then the driver is written by me but the author is someone else? Right? So, it 
is better if I propose a patch which remove a driver and a patch which add my 
driver?

Thank you

-- 
Federico Vaga

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

* Re: [PATCH v3 3/4] sta2x11_vip: convert to videobuf2 and control framework
  2012-12-05  1:12     ` Federico Vaga
@ 2012-12-05 11:34       ` Mauro Carvalho Chehab
  2012-12-05 12:24         ` Federico Vaga
  0 siblings, 1 reply; 32+ messages in thread
From: Mauro Carvalho Chehab @ 2012-12-05 11:34 UTC (permalink / raw)
  To: Federico Vaga
  Cc: Mauro Carvalho Chehab, Pawel Osciak, Marek Szyprowski,
	Hans Verkuil, Giancarlo Asnaghi, linux-media, linux-kernel,
	Jonathan Corbet

Hi Federico,

Em 04-12-2012 23:12, Federico Vaga escreveu:
> On Tuesday 04 December 2012 14:15:15 Mauro Carvalho Chehab wrote:
>> Em 24-09-2012 07:58, Federico Vaga escreveu:
>>> This patch re-write the driver and use the videobuf2
>>> interface instead of the old videobuf. Moreover, it uses also
>>> the control framework which allows the driver to inherit
>>> controls from its subdevice (ADV7180)
>>>
>>> Signed-off-by: Federico Vaga <federico.vaga@gmail.com>
>>> Acked-by: Giancarlo Asnaghi <giancarlo.asnaghi@st.com>
>>>
>>> [..........]
>>>
>>>    /*
>>>
>>>     * This is the driver for the STA2x11 Video Input Port.
>>>     *
>>>
>>> + * Copyright (C) 2012       ST Microelectronics
>>>
>>>     * Copyright (C) 2010       WindRiver Systems, Inc.
>>>     *
>>>     * This program is free software; you can redistribute it and/or modify
>>>     it
>>>
>>> @@ -19,36 +20,30 @@
>>>
>>>     * The full GNU General Public License is included in this distribution
>>>     in
>>>     * the file called "COPYING".
>>>     *
>>>
>>> - * Author: Andreas Kies <andreas.kies@windriver.com>
>>> - *		Vlad Lungu <vlad.lungu@windriver.com>
>>
>> Why are you dropping those authorship data?
>>
>> Ok, it is clear to me that most of the code there got rewritten, and,
>> while IANAL, I think they still have some copyrights on it.
>>
>> So, if you're willing to do that, you need to get authors ack
>> on such patch.
>
> I re-write the driver, and also the first version of the driver has many
> modification made by me, many bug fix, style review, remove useless code.
> The first time I didn't add myself as author because the logic of the driver
> did not change. This time, plus the old change I think there is nothing of the
> original driver because I rewrite it from the hardware manual. Practically, It
> is a new driver for the same device.

Yeah, there are many changes there that justifies adding you at its
authorship, and that's ok. Also, anyone saying the size of your patch
will recognize your and ST efforts to improve the driver.

However, as some parts of the code were preserved, dropping the old
authors doesn't sound right (and can even be illegal, in the light
of the GPL license). It would be ok, though, if you would be
changing it to something like:

	Copyright (c) 2010 by ...
or
	Original driver from ...

or some other similar wording that would preserve their names
there. We do that even when something is rewritten, like, for
example, drivers/media/v4l2-core/videobuf-core.c:

  * (c) 2007 Mauro Carvalho Chehab, <mchehab@infradead.org>
  *
  * Highly based on video-buf written originally by:
  * (c) 2001,02 Gerd Knorr <kraxel@bytesex.org>
  * (c) 2006 Mauro Carvalho Chehab, <mchehab@infradead.org>
  * (c) 2006 Ted Walther and John Sokol

>
> Anyway I will try to contact the original authors for the acked-by.
>
>>>    MODULE_DESCRIPTION("STA2X11 Video Input Port driver");
>>>
>>> -MODULE_AUTHOR("Wind River");
>>
>> Same note applies here: we need Wind River's ack on that to drop it.
>
> I will try also for this. But I think that this is not a windriver driver
> because I re-wrote it from the hardware manual. I used the old driver because
> I thought that it was better than propose a patch that remove the old driver
> and add my driver.
> I did not remove the 2010 Copyright from windriver, because they did the job,
> but this work was paid by ST (copyright 2012) and made completely by me.

The same reason why you didn't drop windriver's copyright also applies here:
while most of the code changed, there are still a few things left from the
original driver.

> Is my thinking wrong?
>
> Just a question for the future so I avoid to redo the same error. If I re-
> wrote most of a driver I cannot change the authorship automatically without
> the acked-by of the previous author. If I ask to the previous author and he
> does not give me the acked-by (or he is unreachable, he change email address),
> then the driver is written by me but the author is someone else? Right? So, it
> is better if I propose a patch which remove a driver and a patch which add my
> driver?

The only way of not preserving the original authors here were if you
start from scratch, without looking at the original code (and you can
somehow, be able to proof it), otherwise, the code will be fit as a
"derivative work", in the light of GPL, and should be preserving the
original authorship.

Something started from scratch like that will hardly be accepted upstream,
as regressions will likely be introduced, and previously supported
hardware/features may be lost in the process.

Of course the original author can abdicate to his rights of keeping his
name on it. Yet, even if he opt/accept to not keep his name explicitly
there, his copyrights are preserved, with the help of the git history.

That's said, no single kernel developer/company has full copyrights on
any part of the Kernel, as their code are based on someone else's work.
For example, all Kernel drivers depend on drivers/base, with in turn,
depends on memory management, generic helper functions, arch code, etc.

So, IMHO, there's not much point on dropping authorship messages.

Regards,
Mauro

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

* Re: [PATCH v3 3/4] sta2x11_vip: convert to videobuf2 and control framework
  2012-12-05 11:34       ` Mauro Carvalho Chehab
@ 2012-12-05 12:24         ` Federico Vaga
  2012-12-05 13:10           ` Mauro Carvalho Chehab
  0 siblings, 1 reply; 32+ messages in thread
From: Federico Vaga @ 2012-12-05 12:24 UTC (permalink / raw)
  To: Mauro Carvalho Chehab
  Cc: Mauro Carvalho Chehab, Pawel Osciak, Marek Szyprowski,
	Hans Verkuil, Giancarlo Asnaghi, linux-media, linux-kernel,
	Jonathan Corbet

Thank you Mauro for the good explanation

> Yeah, there are many changes there that justifies adding you at its
> authorship, and that's ok. Also, anyone saying the size of your patch
> will recognize your and ST efforts to improve the driver.
> 
> However, as some parts of the code were preserved, dropping the old
> authors doesn't sound right (and can even be illegal, in the light
> of the GPL license). It would be ok, though, if you would be
> changing it to something like:
> 
> 	Copyright (c) 2010 by ...
> or
> 	Original driver from ...

Ok, I understand. I will write something like this.
 * Copyright (C) 2012       ST Microelectronics
 *      author: Federico Vaga <federico.vaga@gmail.com>
 * Copyright (C) 2010       WindRiver Systems, Inc.
 *      authors: Andreas Kies <andreas.kies@windriver.com>
 *               Vlad Lungu <vlad.lungu@windriver.com>


> The only way of not preserving the original authors here were if you
> start from scratch, without looking at the original code (and you can
> somehow, be able to proof it), otherwise, the code will be fit as a
> "derivative work", in the light of GPL, and should be preserving the
> original authorship.
> 
> Something started from scratch like that will hardly be accepted upstream,
> as regressions will likely be introduced, and previously supported
> hardware/features may be lost in the process.

I understand
 
> Of course the original author can abdicate to his rights of keeping his
> name on it. Yet, even if he opt/accept to not keep his name explicitly
> there, his copyrights are preserved, with the help of the git history.
> 
> That's said, no single kernel developer/company has full copyrights on
> any part of the Kernel, as their code are based on someone else's work.
> For example, all Kernel drivers depend on drivers/base, with in turn,
> depends on memory management, generic helper functions, arch code, etc.

yeah I know :)

> So, IMHO, there's not much point on dropping authorship messages.

So the MODULE_AUTHOR will be Windriver forever until they drop authorship? 
Also if in the next future 0 windriver lines will be in the code?

(general talking, not about this driver)
If I do git blame on a driver with MODULE_AUTHOR("Mr. X"); but only the 
MODULE_AUTHOR line is from Mr X; there is not an automatically system which 
remove the MODULE_AUTHOR? Ok, probably it was the original author of the code 
and the comment header with the copyright history gives to Mr X all the 
honours, but there is nothing from him in the code. It is not better to remove 
MODULE_AUTHOR or replace it with who wrotes most of the code?
I know that this is not the best place to talk about this, just a little 
curiosity

-- 
Federico Vaga

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

* Re: [PATCH v3 2/4] videobuf2-dma-streaming: new videobuf2 memory allocator
  2012-12-04 16:04     ` Mauro Carvalho Chehab
@ 2012-12-05 12:50       ` Federico Vaga
  2012-12-05 14:25         ` Mauro Carvalho Chehab
  0 siblings, 1 reply; 32+ messages in thread
From: Federico Vaga @ 2012-12-05 12:50 UTC (permalink / raw)
  To: Mauro Carvalho Chehab
  Cc: Marek Szyprowski, 'Mauro Carvalho Chehab',
	'Pawel Osciak', 'Hans Verkuil',
	'Giancarlo Asnaghi',
	linux-media, linux-kernel, 'Jonathan Corbet'

On Tuesday 04 December 2012 14:04:22 Mauro Carvalho Chehab wrote:
> Em 24-09-2012 09:44, Marek Szyprowski escreveu:
> > Hello,
> > 
> > On Monday, September 24, 2012 12:59 PM Federico Vaga wrote:
> >> The DMA streaming allocator is similar to the DMA contig but it use the
> >> DMA streaming interface (dma_map_single, dma_unmap_single). The
> >> allocator allocates buffers and immediately map the memory for DMA
> >> transfer. For each buffer prepare/finish it does a DMA synchronization.
> 
> Hmm.. the explanation didn't convince me, e. g.:
> 	1) why is it needed;

This allocator is needed because some device (like STA2X11 VIP) cannot work 
with DMA sg or DMA coherent. Some other device (like the one used by Jonathan 
when he proposes vb2-dma-nc allocator) can obtain much better performance with 
DMA streaming than coherent.

> 	2) why vb2-dma-config can't be patched to use dma_map_single
> (eventually using a different vb2_io_modes bit?);

I did not modify vb2-dma-contig because I was thinking that each DMA memory 
allocator should reflect a DMA API.

> 	3) what are the usecases for it.
> 
> Could you please detail it? Without that, one that would be needing to
> write a driver will have serious doubts about what would be the right
> driver for its usage. Also, please document it at the driver itself.

I did not write all this details because the reasons to use vb2-dma-contig, 
vb2-dma-sg or vb2-dma-streaming are the same reasons because someone choose 
SG, coherent or streaming API. This is already documented in the DMA-*.txt 
files, so I did not rewrite it to avoid duplication.

-- 
Federico Vaga

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

* Re: [PATCH v3 3/4] sta2x11_vip: convert to videobuf2 and control framework
  2012-12-05 12:24         ` Federico Vaga
@ 2012-12-05 13:10           ` Mauro Carvalho Chehab
  2012-12-05 13:27             ` Federico Vaga
  2012-12-06 18:59             ` Federico Vaga
  0 siblings, 2 replies; 32+ messages in thread
From: Mauro Carvalho Chehab @ 2012-12-05 13:10 UTC (permalink / raw)
  To: Federico Vaga
  Cc: Mauro Carvalho Chehab, Pawel Osciak, Marek Szyprowski,
	Hans Verkuil, Giancarlo Asnaghi, linux-media, linux-kernel,
	Jonathan Corbet

Em 05-12-2012 10:24, Federico Vaga escreveu:
> Thank you Mauro for the good explanation
>
>> Yeah, there are many changes there that justifies adding you at its
>> authorship, and that's ok. Also, anyone saying the size of your patch
>> will recognize your and ST efforts to improve the driver.
>>
>> However, as some parts of the code were preserved, dropping the old
>> authors doesn't sound right (and can even be illegal, in the light
>> of the GPL license). It would be ok, though, if you would be
>> changing it to something like:
>>
>> 	Copyright (c) 2010 by ...
>> or
>> 	Original driver from ...
>
> Ok, I understand. I will write something like this.
>   * Copyright (C) 2012       ST Microelectronics
>   *      author: Federico Vaga <federico.vaga@gmail.com>
>   * Copyright (C) 2010       WindRiver Systems, Inc.
>   *      authors: Andreas Kies <andreas.kies@windriver.com>
>   *               Vlad Lungu <vlad.lungu@windriver.com>

Sounds perfect to me.

>> So, IMHO, there's not much point on dropping authorship messages.
>
> So the MODULE_AUTHOR will be Windriver forever until they drop authorship?
> Also if in the next future 0 windriver lines will be in the code?
>
> (general talking, not about this driver)
> If I do git blame on a driver with MODULE_AUTHOR("Mr. X"); but only the
> MODULE_AUTHOR line is from Mr X; there is not an automatically system which
> remove the MODULE_AUTHOR? Ok, probably it was the original author of the code
> and the comment header with the copyright history gives to Mr X all the
> honours, but there is nothing from him in the code. It is not better to remove
> MODULE_AUTHOR or replace it with who wrotes most of the code?
> I know that this is not the best place to talk about this, just a little
> curiosity

As you said, the best place to discuss about it is likely at LKML.
It could make sense to have some policy with regards to MODULE_AUTHOR(),
stating when it could be modified, preferably reviewed by some experienced
open source lawyers. I'm not aware of any.

In any case, I don't think anyone should rely on "git blame" for it.
The tool that helps to get code authorship is "git log -M", but
to get the real authorship requires a lot more of just calling it.

A trivial case where git blame will likely be giving wrong "credits"
is if a driver has all functions reordered. It may happen that git blame
will give credits to the one that reordered the functions, and not to
the original code's author.

There are more subtle cases, though. For example, please assume that
the original driver has to call a certain function to do some needed
work to initialize the device:

	call_function_foo();

And a reviewer latter add a logic to deal with errors, like:

	ret = call_function_foo();
	if (ret)
		return ret;

The reviewer wrote 2 additional lines, and modified 1 original line.
Yet, his contribution didn't make the code to work. The one that did it
was the single-line patch that added call_function_foo(). So, in this
case, a single line change was more significant for the driver than a
3 line addition[1]. Also, again, git blame will give the "credits"
for the line that calls call_function_foo() to the wrong guy.

Btw, this is why it is called "git blame", and not "git authorship":
it is a tool to identify who was the last one that modified the code.
Its main usage is to identify who might have introduced a bug on the
code.

Let's assume that call_function_foo() could be returning some non-error
positive values, under certain circumstances. In that case, the above
code would actually be introducing a bug by using:
	if (ret)
instead of
	if (ret < 0)

So, the 3 line patch is not only be less relevant for the driver to
work than the original author's code, but it is actually wrong
and introduced a serious driver regression, preventing it to work.
Git blame was made to address such cases: once a bug got bisected, it
helps to get the changelog that made the change, pointing to who made
the changes and why. So, a proper fix for the bug can be better
prepared, to not only fix the bug, but to also address the issue
pointed by the blamed patchset.

Regards,
Mauro

[1] Please, don't get be wrong on that: we do want patches adding
proper error handling. They are important to improve the driver's
reliability.

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

* Re: [PATCH v3 3/4] sta2x11_vip: convert to videobuf2 and control framework
  2012-12-05 13:10           ` Mauro Carvalho Chehab
@ 2012-12-05 13:27             ` Federico Vaga
  2012-12-05 13:37               ` Mauro Carvalho Chehab
  2012-12-06 18:59             ` Federico Vaga
  1 sibling, 1 reply; 32+ messages in thread
From: Federico Vaga @ 2012-12-05 13:27 UTC (permalink / raw)
  To: Mauro Carvalho Chehab
  Cc: Mauro Carvalho Chehab, Pawel Osciak, Marek Szyprowski,
	Hans Verkuil, Giancarlo Asnaghi, linux-media, linux-kernel,
	Jonathan Corbet

> > Ok, I understand. I will write something like this.
> > 
> >   * Copyright (C) 2012       ST Microelectronics
> >   *      author: Federico Vaga <federico.vaga@gmail.com>
> >   * Copyright (C) 2010       WindRiver Systems, Inc.
> >   *      authors: Andreas Kies <andreas.kies@windriver.com>
> >   *               Vlad Lungu <vlad.lungu@windriver.com>
> 
> Sounds perfect to me.

I will answer to this with a patch

> As you said, the best place to discuss about it is likely at LKML.
> [...]
> Btw, this is why it is called "git blame", and not "git authorship":
> it is a tool to identify who was the last one that modified the code.
> Its main usage is to identify who might have introduced a bug on the
> code.

I know I know, it was just a stupid example to expose the problem that I have 
in my mind. I know that it is very difficult (impossible?) to assign the 
authorship of a single line, and git blame it is not the tool to do this :)

I think you understand what I mean despite the stupid example

-- 
Federico Vaga

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

* Re: [PATCH v3 3/4] sta2x11_vip: convert to videobuf2 and control framework
  2012-12-05 13:27             ` Federico Vaga
@ 2012-12-05 13:37               ` Mauro Carvalho Chehab
  2012-12-05 13:45                 ` Federico Vaga
  0 siblings, 1 reply; 32+ messages in thread
From: Mauro Carvalho Chehab @ 2012-12-05 13:37 UTC (permalink / raw)
  To: Federico Vaga
  Cc: Mauro Carvalho Chehab, Pawel Osciak, Marek Szyprowski,
	Hans Verkuil, Giancarlo Asnaghi, linux-media, linux-kernel,
	Jonathan Corbet

Em 05-12-2012 11:27, Federico Vaga escreveu:
>>> Ok, I understand. I will write something like this.
>>>
>>>    * Copyright (C) 2012       ST Microelectronics
>>>    *      author: Federico Vaga <federico.vaga@gmail.com>
>>>    * Copyright (C) 2010       WindRiver Systems, Inc.
>>>    *      authors: Andreas Kies <andreas.kies@windriver.com>
>>>    *               Vlad Lungu <vlad.lungu@windriver.com>
>>
>> Sounds perfect to me.
>
> I will answer to this with a patch

Thanks!

>> As you said, the best place to discuss about it is likely at LKML.
>> [...]
>> Btw, this is why it is called "git blame", and not "git authorship":
>> it is a tool to identify who was the last one that modified the code.
>> Its main usage is to identify who might have introduced a bug on the
>> code.
>
> I know I know, it was just a stupid example to expose the problem that I have
> in my mind. I know that it is very difficult (impossible?) to assign the
> authorship of a single line, and git blame it is not the tool to do this :)
>
> I think you understand what I mean despite the stupid example

Yeah, I hear you.

Not sure if you got my point: the main point of removing MODULE_AUTHOR
and other copyright stuff is that such patch may easily be doing something
that could be considered a copyright violation, being bad not only to
the affected driver, but to the entire Kernel.

So, we need to handle it with due care. Getting other authors's
acks on such patch seems to be the only safe way of doing that.

Regards,
Mauro


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

* Re: [PATCH v3 3/4] sta2x11_vip: convert to videobuf2 and control framework
  2012-12-05 13:37               ` Mauro Carvalho Chehab
@ 2012-12-05 13:45                 ` Federico Vaga
  0 siblings, 0 replies; 32+ messages in thread
From: Federico Vaga @ 2012-12-05 13:45 UTC (permalink / raw)
  To: Mauro Carvalho Chehab
  Cc: Mauro Carvalho Chehab, Pawel Osciak, Marek Szyprowski,
	Hans Verkuil, Giancarlo Asnaghi, linux-media, linux-kernel,
	Jonathan Corbet

> Not sure if you got my point: the main point of removing MODULE_AUTHOR
> and other copyright stuff is that such patch may easily be doing something
> that could be considered a copyright violation, being bad not only to
> the affected driver, but to the entire Kernel.
> 
> So, we need to handle it with due care. Getting other authors's
> acks on such patch seems to be the only safe way of doing that.

Yes I got the point.

Thank you

-- 
Federico Vaga

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

* Re: [PATCH v3 2/4] videobuf2-dma-streaming: new videobuf2 memory allocator
  2012-12-05 12:50       ` Federico Vaga
@ 2012-12-05 14:25         ` Mauro Carvalho Chehab
  2012-12-11 13:54           ` Federico Vaga
  0 siblings, 1 reply; 32+ messages in thread
From: Mauro Carvalho Chehab @ 2012-12-05 14:25 UTC (permalink / raw)
  To: Federico Vaga
  Cc: Marek Szyprowski, 'Mauro Carvalho Chehab',
	'Pawel Osciak', 'Hans Verkuil',
	'Giancarlo Asnaghi',
	linux-media, linux-kernel, 'Jonathan Corbet',
	sylwester Nawrocki

Em 05-12-2012 10:50, Federico Vaga escreveu:
> On Tuesday 04 December 2012 14:04:22 Mauro Carvalho Chehab wrote:
>> Em 24-09-2012 09:44, Marek Szyprowski escreveu:
>>> Hello,
>>>
>>> On Monday, September 24, 2012 12:59 PM Federico Vaga wrote:
>>>> The DMA streaming allocator is similar to the DMA contig but it use the
>>>> DMA streaming interface (dma_map_single, dma_unmap_single). The
>>>> allocator allocates buffers and immediately map the memory for DMA
>>>> transfer. For each buffer prepare/finish it does a DMA synchronization.
>>
>> Hmm.. the explanation didn't convince me, e. g.:
>> 	1) why is it needed;
>
> This allocator is needed because some device (like STA2X11 VIP) cannot work
> with DMA sg or DMA coherent. Some other device (like the one used by Jonathan
> when he proposes vb2-dma-nc allocator) can obtain much better performance with
> DMA streaming than coherent.

Ok, please add such explanations at the patch's descriptions, as it is
important not only for me, but to others that may need to use it..

>
>> 	2) why vb2-dma-config can't be patched to use dma_map_single
>> (eventually using a different vb2_io_modes bit?);
>
> I did not modify vb2-dma-contig because I was thinking that each DMA memory
> allocator should reflect a DMA API.

The basic reason for having more than one VB low-level handling (vb2 was
inspired on this concept) is that some DMA APIs are very different than
the other ones (see vmalloc x DMA S/G for example).

I didn't make a diff between videobuf2-dma-streaming and videobuf2-dma-contig,
so I can't tell if it makes sense to merge them or not, but the above
argument seems too weak. I was expecting for a technical reason why
it wouldn't make sense for merging them.

>
>> 	3) what are the usecases for it.
>>
>> Could you please detail it? Without that, one that would be needing to
>> write a driver will have serious doubts about what would be the right
>> driver for its usage. Also, please document it at the driver itself.
>
> I did not write all this details because the reasons to use vb2-dma-contig,
> vb2-dma-sg or vb2-dma-streaming are the same reasons because someone choose
> SG, coherent or streaming API. This is already documented in the DMA-*.txt
> files, so I did not rewrite it to avoid duplication.

I see. It doesn't hurt to add a short explanation then at the patch description,
pointing to Documentation/DMA-API-HOWTO.txt, describing when using it instead
of vb2-dma-config (or vb2-dma-sg) would likely give better performance results,
and when the reverse is true.

Btw, from Documentation/DMA-API-HOWTO.txt:

   "Good examples of what to use streaming mappings for are:

	- Networking buffers transmitted/received by a device.
	- Filesystem buffers written/read by a SCSI device.

    The interfaces for using this type of mapping were designed in
    such a way that an implementation can make whatever performance
    optimizations the hardware allows.  To this end, when using
    such mappings you must be explicit about what you want to happen."

I'm not a DMA performance expert. As such, from that comment, it sounded to me
that replacing dma-config/dma-sg by dma streaming will always give "performance
optimizations the hardware allow".

If this is always true, why to preserve the old vb2-dma-config/vb2-dma-sg?

In other words, I suspect that the above is just half of the history ;)

On a separate but related issue, while doing DMABUF tests with an Exynos4
hardware, using a s5p sensor, sending data to s5p-tv, I noticed a CPU
consumption of about 42%, which seems too high. Could it be related to
not using the DMA streaming API?

(c/c Sylwester, due to this last comment)

Regards,
Mauro




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

* [PATCH v3 3/4] sta2x11_vip: convert to videobuf2 and control framework
  2012-12-05 13:10           ` Mauro Carvalho Chehab
  2012-12-05 13:27             ` Federico Vaga
@ 2012-12-06 18:59             ` Federico Vaga
  1 sibling, 0 replies; 32+ messages in thread
From: Federico Vaga @ 2012-12-06 18:59 UTC (permalink / raw)
  To: Mauro Carvalho Chehab, Pawel Osciak, Marek Szyprowski, Hans Verkuil
  Cc: Mauro Carvalho Chehab, Giancarlo Asnaghi, linux-media,
	linux-kernel, Jonathan Corbet, Federico Vaga

This patch re-write the driver and use the videobuf2
interface instead of the old videobuf. Moreover, it uses also
the control framework which allows the driver to inherit
controls from its subdevice (ADV7180)

Signed-off-by: Federico Vaga <federico.vaga@gmail.com>
Acked-by: Giancarlo Asnaghi <giancarlo.asnaghi@st.com>
---
 drivers/media/pci/sta2x11/Kconfig       |    2 +-
 drivers/media/pci/sta2x11/sta2x11_vip.c | 1239 ++++++++++---------------------
 2 file modificati, 409 inserzioni(+), 832 rimozioni(-)

diff --git a/drivers/media/pci/sta2x11/Kconfig b/drivers/media/pci/sta2x11/Kconfig
index 6749f67..654339f 100644
--- a/drivers/media/pci/sta2x11/Kconfig
+++ b/drivers/media/pci/sta2x11/Kconfig
@@ -2,7 +2,7 @@ config STA2X11_VIP
 	tristate "STA2X11 VIP Video For Linux"
 	depends on STA2X11
 	select VIDEO_ADV7180 if MEDIA_SUBDRV_AUTOSELECT
-	select VIDEOBUF_DMA_CONTIG
+	select VIDEOBUF2_DMA_STREAMING
 	depends on PCI && VIDEO_V4L2 && VIRT_TO_BUS
 	help
 	  Say Y for support for STA2X11 VIP (Video Input Port) capture
diff --git a/drivers/media/pci/sta2x11/sta2x11_vip.c b/drivers/media/pci/sta2x11/sta2x11_vip.c
index 4c10205..ee61acc 100644
--- a/drivers/media/pci/sta2x11/sta2x11_vip.c
+++ b/drivers/media/pci/sta2x11/sta2x11_vip.c
@@ -1,7 +1,11 @@
 /*
  * This is the driver for the STA2x11 Video Input Port.
  *
+ * Copyright (C) 2012       ST Microelectronics
+ *     author: Federico Vaga <federico.vaga@gmail.com>
  * Copyright (C) 2010       WindRiver Systems, Inc.
+ *     authors: Andreas Kies <andreas.kies@windriver.com>
+ *              Vlad Lungu   <vlad.lungu@windriver.com>
  *
  * This program is free software; you can redistribute it and/or modify it
  * under the terms and conditions of the GNU General Public License,
@@ -19,36 +23,30 @@
  * The full GNU General Public License is included in this distribution in
  * the file called "COPYING".
  *
- * Author: Andreas Kies <andreas.kies@windriver.com>
- *		Vlad Lungu <vlad.lungu@windriver.com>
- *
  */
 
 #include <linux/types.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/init.h>
-#include <linux/vmalloc.h>
-
 #include <linux/videodev2.h>
-
 #include <linux/kmod.h>
-
 #include <linux/pci.h>
 #include <linux/interrupt.h>
-#include <linux/mutex.h>
 #include <linux/io.h>
 #include <linux/gpio.h>
 #include <linux/i2c.h>
 #include <linux/delay.h>
 #include <media/v4l2-common.h>
 #include <media/v4l2-device.h>
+#include <media/v4l2-ctrls.h>
 #include <media/v4l2-ioctl.h>
-#include <media/videobuf-dma-contig.h>
+#include <media/v4l2-fh.h>
+#include <media/v4l2-event.h>
+#include <media/videobuf2-dma-streaming.h>
 
 #include "sta2x11_vip.h"
 
-#define DRV_NAME "sta2x11_vip"
 #define DRV_VERSION "1.3"
 
 #ifndef PCI_DEVICE_ID_STMICRO_VIP
@@ -63,8 +61,8 @@
 #define DVP_TFS		0x08
 #define DVP_BFO		0x0C
 #define DVP_BFS		0x10
-#define DVP_VTP         0x14
-#define DVP_VBP         0x18
+#define DVP_VTP		0x14
+#define DVP_VBP		0x18
 #define DVP_VMP		0x1C
 #define DVP_ITM		0x98
 #define DVP_ITS		0x9C
@@ -84,43 +82,20 @@
 
 #define DVP_HLFLN_SD	0x00000001
 
-#define REG_WRITE(vip, reg, value) iowrite32((value), (vip->iomem)+(reg))
-#define REG_READ(vip, reg) ioread32((vip->iomem)+(reg))
-
 #define SAVE_COUNT 8
 #define AUX_COUNT 3
 #define IRQ_COUNT 1
 
-/**
- * struct sta2x11_vip - All internal data for one instance of device
- * @v4l2_dev: device registered in v4l layer
- * @video_dev: properties of our device
- * @pdev: PCI device
- * @adapter: contains I2C adapter information
- * @register_save_area: All relevant register are saved here during suspend
- * @decoder: contains information about video DAC
- * @format: pixel format, fixed UYVY
- * @std: video standard (e.g. PAL/NTSC)
- * @input: input line for video signal ( 0 or 1 )
- * @users: Number of open of device ( max. 1 )
- * @disabled: Device is in power down state
- * @mutex: ensures exclusive opening of device
- * @slock: for excluse acces of registers
- * @vb_vidq: queue maintained by videobuf layer
- * @capture: linked list of capture buffer
- * @active: struct videobuf_buffer currently beingg filled
- * @started: device is ready to capture frame
- * @closing: device will be shut down
- * @tcount: Number of top frames
- * @bcount: Number of bottom frames
- * @overflow: Number of FIFO overflows
- * @mem_spare: small buffer of unused frame
- * @dma_spare: dma addres of mem_spare
- * @iomem: hardware base address
- * @config: I2C and gpio config from platform
- *
- * All non-local data is accessed via this structure.
- */
+
+struct vip_buffer {
+	struct vb2_buffer	vb;
+	struct list_head	list;
+	dma_addr_t		dma;
+};
+static inline struct vip_buffer *to_vip_buffer(struct vb2_buffer *vb2)
+{
+	return container_of(vb2, struct vip_buffer, vb);
+}
 
 struct sta2x11_vip {
 	struct v4l2_device v4l2_dev;
@@ -129,21 +104,27 @@ struct sta2x11_vip {
 	struct i2c_adapter *adapter;
 	unsigned int register_save_area[IRQ_COUNT + SAVE_COUNT + AUX_COUNT];
 	struct v4l2_subdev *decoder;
-	struct v4l2_pix_format format;
-	v4l2_std_id std;
-	unsigned int input;
-	int users;
-	int disabled;
-	struct mutex mutex;	/* exclusive access during open */
-	spinlock_t slock;	/* spin lock for hardware and queue access */
-	struct videobuf_queue vb_vidq;
-	struct list_head capture;
-	struct videobuf_buffer *active;
-	int started, closing, tcount, bcount;
+	struct v4l2_ctrl_handler ctrl_hdl;
+
+
+	struct v4l2_pix_format format;	/* pixel format, fixed UYVY */
+	v4l2_std_id std;	/* Video standard (PAL/NTSC)*/
+	unsigned int input;	/* Input line (0 or 1) */
+	int disabled; /* 1 disabled 0 enabled */
+	spinlock_t slock; /* spin lock for hardware */
+
+	struct vb2_alloc_ctx *alloc_ctx;
+	struct vb2_queue vb_vidq; /* queue maintaned by videobuf2 */
+	struct list_head buffer_list; /* list of buffers */
+	unsigned int sequence;
+	struct vip_buffer *active; /* current active buffer */
+	spinlock_t lock; /* Used in videobuf2 callback */
+
+	/* Interrupt counters */
+	int tcount, bcount;
 	int overflow;
-	void *mem_spare;
-	dma_addr_t dma_spare;
-	void *iomem;
+
+	void *iomem;	/* I/O Memory */
 	struct vip_config *config;
 };
 
@@ -206,360 +187,212 @@ static struct v4l2_pix_format formats_60[] = {
 	 .colorspace = V4L2_COLORSPACE_SMPTE170M},
 };
 
-/**
- * buf_setup - Get size and number of video buffer
- * @vq: queue in videobuf
- * @count: Number of buffers (1..MAX_FRAMES).
- *		0 use default value.
- * @size:  size of buffer in bytes
- *
- * returns size and number of buffers
- * a preset value of 0 returns the default number.
- * return value: 0, always succesfull.
- */
-static int buf_setup(struct videobuf_queue *vq, unsigned int *count,
-		     unsigned int *size)
+/* Write VIP register */
+static inline void reg_write(struct sta2x11_vip *vip, unsigned int reg, u32 val)
 {
-	struct sta2x11_vip *vip = vq->priv_data;
-
-	*size = vip->format.width * vip->format.height * 2;
-	if (0 == *count || MAX_FRAMES < *count)
-		*count = MAX_FRAMES;
-	return 0;
-};
-
-/**
- * buf_prepare - prepare buffer for usage
- * @vq: queue in videobuf layer
- * @vb: buffer to be prepared
- * @field: type of video data (interlaced/non-interlaced)
- *
- * Allocate or realloc buffer
- * return value: 0, successful.
- *
- * -EINVAL, supplied buffer is too small.
- *
- *  other, buffer could not be locked.
- */
-static int buf_prepare(struct videobuf_queue *vq, struct videobuf_buffer *vb,
-		       enum v4l2_field field)
+	iowrite32((val), (vip->iomem)+(reg));
+}
+/* Read VIP register */
+static inline u32 reg_read(struct sta2x11_vip *vip, unsigned int reg)
 {
-	struct sta2x11_vip *vip = vq->priv_data;
-	int ret;
-
-	vb->size = vip->format.width * vip->format.height * 2;
-	if ((0 != vb->baddr) && (vb->bsize < vb->size))
-		return -EINVAL;
-	vb->width = vip->format.width;
-	vb->height = vip->format.height;
-	vb->field = field;
-
-	if (VIDEOBUF_NEEDS_INIT == vb->state) {
-		ret = videobuf_iolock(vq, vb, NULL);
-		if (ret)
-			goto fail;
-	}
-	vb->state = VIDEOBUF_PREPARED;
-	return 0;
-fail:
-	videobuf_dma_contig_free(vq, vb);
-	vb->state = VIDEOBUF_NEEDS_INIT;
-	return ret;
+	return  ioread32((vip->iomem)+(reg));
 }
-
-/**
- * buf_queu - queue buffer for filling
- * @vq: queue in videobuf layer
- * @vb: buffer to be queued
- *
- * if capturing is already running, the buffer will be queued. Otherwise
- * capture is started and the buffer is used directly.
- */
-static void buf_queue(struct videobuf_queue *vq, struct videobuf_buffer *vb)
+/* Start DMA acquisition */
+static void start_dma(struct sta2x11_vip *vip, struct vip_buffer *vip_buf)
 {
-	struct sta2x11_vip *vip = vq->priv_data;
-	u32 dma;
+	unsigned long offset = 0;
+
+	if (vip->format.field == V4L2_FIELD_INTERLACED)
+		offset = vip->format.width * 2;
 
-	vb->state = VIDEOBUF_QUEUED;
+	spin_lock_irq(&vip->slock);
+	/* Enable acquisition */
+	reg_write(vip, DVP_CTL, reg_read(vip, DVP_CTL) | DVP_CTL_ENA);
+	/* Set Top and Bottom Field memory address */
+	reg_write(vip, DVP_VTP, (u32)vip_buf->dma);
+	reg_write(vip, DVP_VBP, (u32)vip_buf->dma + offset);
+	spin_unlock_irq(&vip->slock);
+}
 
-	if (vip->active) {
-		list_add_tail(&vb->queue, &vip->capture);
+/* Fetch the next buffer to activate */
+static void vip_active_buf_next(struct sta2x11_vip *vip)
+{
+	/* Get the next buffer */
+	spin_lock(&vip->lock);
+	if (list_empty(&vip->buffer_list)) {/* No available buffer */
+		spin_unlock(&vip->lock);
 		return;
 	}
-
-	vip->started = 1;
+	vip->active = list_first_entry(&vip->buffer_list,
+				       struct vip_buffer,
+				       list);
+	/* Reset Top and Bottom counter */
 	vip->tcount = 0;
 	vip->bcount = 0;
-	vip->active = vb;
-	vb->state = VIDEOBUF_ACTIVE;
+	spin_unlock(&vip->lock);
+	if (vb2_is_streaming(&vip->vb_vidq)) {	/* streaming is on */
+		start_dma(vip, vip->active);	/* start dma capture */
+	}
+}
 
-	dma = videobuf_to_dma_contig(vb);
 
-	REG_WRITE(vip, DVP_TFO, (0 << 16) | (0));
-	/* despite of interlace mode, upper and lower frames start at zero */
-	REG_WRITE(vip, DVP_BFO, (0 << 16) | (0));
+/* Videobuf2 Operations */
+static int queue_setup(struct vb2_queue *vq, const struct v4l2_format *fmt,
+		       unsigned int *nbuffers, unsigned int *nplanes,
+		       unsigned int sizes[], void *alloc_ctxs[])
+{
+	struct sta2x11_vip *vip = vb2_get_drv_priv(vq);
 
-	switch (vip->format.field) {
-	case V4L2_FIELD_INTERLACED:
-		REG_WRITE(vip, DVP_TFS,
-			  ((vip->format.height / 2 - 1) << 16) |
-			  (2 * vip->format.width - 1));
-		REG_WRITE(vip, DVP_BFS, ((vip->format.height / 2 - 1) << 16) |
-			  (2 * vip->format.width - 1));
-		REG_WRITE(vip, DVP_VTP, dma);
-		REG_WRITE(vip, DVP_VBP, dma + vip->format.width * 2);
-		REG_WRITE(vip, DVP_VMP, 4 * vip->format.width);
-		break;
-	case V4L2_FIELD_TOP:
-		REG_WRITE(vip, DVP_TFS,
-			  ((vip->format.height - 1) << 16) |
-			  (2 * vip->format.width - 1));
-		REG_WRITE(vip, DVP_BFS, ((0) << 16) |
-			  (2 * vip->format.width - 1));
-		REG_WRITE(vip, DVP_VTP, dma);
-		REG_WRITE(vip, DVP_VBP, dma);
-		REG_WRITE(vip, DVP_VMP, 2 * vip->format.width);
-		break;
-	case V4L2_FIELD_BOTTOM:
-		REG_WRITE(vip, DVP_TFS, ((0) << 16) |
-			  (2 * vip->format.width - 1));
-		REG_WRITE(vip, DVP_BFS,
-			  ((vip->format.height) << 16) |
-			  (2 * vip->format.width - 1));
-		REG_WRITE(vip, DVP_VTP, dma);
-		REG_WRITE(vip, DVP_VBP, dma);
-		REG_WRITE(vip, DVP_VMP, 2 * vip->format.width);
-		break;
+	if (!(*nbuffers) || *nbuffers < MAX_FRAMES)
+		*nbuffers = MAX_FRAMES;
 
-	default:
-		pr_warning("VIP: unknown field format\n");
-		return;
-	}
+	*nplanes = 1;
+	sizes[0] = vip->format.sizeimage;
+	alloc_ctxs[0] = vip->alloc_ctx;
 
-	REG_WRITE(vip, DVP_CTL, DVP_CTL_ENA);
-}
+	vip->sequence = 0;
+	vip->active = NULL;
+	vip->tcount = 0;
+	vip->bcount = 0;
 
-/**
- * buff_release - release buffer
- * @vq: queue in videobuf layer
- * @vb: buffer to be released
- *
- * release buffer in videobuf layer
- */
-static void buf_release(struct videobuf_queue *vq, struct videobuf_buffer *vb)
+	return 0;
+};
+static int buffer_init(struct vb2_buffer *vb)
 {
+	struct vip_buffer *vip_buf = to_vip_buffer(vb);
 
-	videobuf_dma_contig_free(vq, vb);
-	vb->state = VIDEOBUF_NEEDS_INIT;
+	vip_buf->dma = vb2_dma_streaming_plane_paddr(vb, 0);
+	INIT_LIST_HEAD(&vip_buf->list);
+	return 0;
 }
 
-static struct videobuf_queue_ops vip_qops = {
-	.buf_setup = buf_setup,
-	.buf_prepare = buf_prepare,
-	.buf_queue = buf_queue,
-	.buf_release = buf_release,
-};
-
-/**
- * vip_open - open video device
- * @file: descriptor of device
- *
- * open device, make sure it is only opened once.
- * return value: 0, no error.
- *
- * -EBUSY, device is already opened
- *
- * -ENOMEM, no memory for auxiliary DMA buffer
- */
-static int vip_open(struct file *file)
+static int buffer_prepare(struct vb2_buffer *vb)
 {
-	struct video_device *dev = video_devdata(file);
-	struct sta2x11_vip *vip = video_get_drvdata(dev);
+	struct sta2x11_vip *vip = vb2_get_drv_priv(vb->vb2_queue);
+	struct vip_buffer *vip_buf = to_vip_buffer(vb);
+	unsigned long size;
+
+	size = vip->format.sizeimage;
+	if (vb2_plane_size(vb, 0) < size) {
+		v4l2_err(&vip->v4l2_dev, "buffer too small (%lu < %lu)\n",
+			 vb2_plane_size(vb, 0), size);
+		return -EINVAL;
+	}
 
-	mutex_lock(&vip->mutex);
-	vip->users++;
+	vb2_set_plane_payload(&vip_buf->vb, 0, size);
 
-	if (vip->users > 1) {
-		vip->users--;
-		mutex_unlock(&vip->mutex);
-		return -EBUSY;
+	return 0;
+}
+static void buffer_queue(struct vb2_buffer *vb)
+{
+	struct sta2x11_vip *vip = vb2_get_drv_priv(vb->vb2_queue);
+	struct vip_buffer *vip_buf = to_vip_buffer(vb);
+
+	spin_lock(&vip->lock);
+	list_add_tail(&vip_buf->list, &vip->buffer_list);
+	if (!vip->active) {	/* No active buffer, active the first one */
+		vip->active = list_first_entry(&vip->buffer_list,
+					       struct vip_buffer,
+					       list);
+		if (vb2_is_streaming(&vip->vb_vidq))	/* streaming is on */
+			start_dma(vip, vip_buf);	/* start dma capture */
 	}
+	spin_unlock(&vip->lock);
+}
+static int buffer_finish(struct vb2_buffer *vb)
+{
+	struct sta2x11_vip *vip = vb2_get_drv_priv(vb->vb2_queue);
+	struct vip_buffer *vip_buf = to_vip_buffer(vb);
 
-	file->private_data = dev;
-	vip->overflow = 0;
-	vip->started = 0;
-	vip->closing = 0;
-	vip->active = NULL;
+	/* Buffer handled, remove it from the list */
+	spin_lock(&vip->lock);
+	list_del_init(&vip_buf->list);
+	spin_unlock(&vip->lock);
 
-	INIT_LIST_HEAD(&vip->capture);
-	vip->mem_spare = dma_alloc_coherent(&vip->pdev->dev, 64,
-					    &vip->dma_spare, GFP_KERNEL);
-	if (!vip->mem_spare) {
-		vip->users--;
-		mutex_unlock(&vip->mutex);
-		return -ENOMEM;
-	}
+	vip_active_buf_next(vip);
 
-	mutex_unlock(&vip->mutex);
-	videobuf_queue_dma_contig_init_cached(&vip->vb_vidq,
-					      &vip_qops,
-					      &vip->pdev->dev,
-					      &vip->slock,
-					      V4L2_BUF_TYPE_VIDEO_CAPTURE,
-					      V4L2_FIELD_INTERLACED,
-					      sizeof(struct videobuf_buffer),
-					      vip, NULL);
-	REG_READ(vip, DVP_ITS);
-	REG_WRITE(vip, DVP_HLFLN, DVP_HLFLN_SD);
-	REG_WRITE(vip, DVP_ITM, DVP_IT_VSB | DVP_IT_VST);
-	REG_WRITE(vip, DVP_CTL, DVP_CTL_RST);
-	REG_WRITE(vip, DVP_CTL, 0);
-	REG_READ(vip, DVP_ITS);
 	return 0;
 }
 
-/**
- * vip_close - close video device
- * @file: descriptor of device
- *
- * close video device, wait until all pending operations are finished
- * ( maximum FRAME_MAX buffers pending )
- * Turn off interrupts.
- *
- * return value: 0, always succesful.
- */
-static int vip_close(struct file *file)
+static int start_streaming(struct vb2_queue *vq, unsigned int count)
 {
-	struct video_device *dev = video_devdata(file);
-	struct sta2x11_vip *vip = video_get_drvdata(dev);
+	struct sta2x11_vip *vip = vb2_get_drv_priv(vq);
 
-	vip->closing = 1;
-	if (vip->active)
-		videobuf_waiton(&vip->vb_vidq, vip->active, 0, 0);
 	spin_lock_irq(&vip->slock);
-
-	REG_WRITE(vip, DVP_ITM, 0);
-	REG_WRITE(vip, DVP_CTL, DVP_CTL_RST);
-	REG_WRITE(vip, DVP_CTL, 0);
-	REG_READ(vip, DVP_ITS);
-
-	vip->started = 0;
-	vip->active = NULL;
-
+	/* Enable interrupt VSYNC Top and Bottom*/
+	reg_write(vip, DVP_ITM, DVP_IT_VSB | DVP_IT_VST);
 	spin_unlock_irq(&vip->slock);
 
-	videobuf_stop(&vip->vb_vidq);
-	videobuf_mmap_free(&vip->vb_vidq);
+	if (count)
+		start_dma(vip, vip->active);
 
-	dma_free_coherent(&vip->pdev->dev, 64, vip->mem_spare, vip->dma_spare);
-	file->private_data = NULL;
-	mutex_lock(&vip->mutex);
-	vip->users--;
-	mutex_unlock(&vip->mutex);
 	return 0;
 }
 
-/**
- * vip_read - read from video input
- * @file: descriptor of device
- * @data: user buffer
- * @count: number of bytes to be read
- * @ppos: position within stream
- *
- * read video data from video device.
- * handling is done in generic videobuf layer
- * return value: provided by videobuf layer
- */
-static ssize_t vip_read(struct file *file, char __user *data,
-			size_t count, loff_t *ppos)
+/* abort streaming and wait for last buffer */
+static int stop_streaming(struct vb2_queue *vq)
 {
-	struct video_device *dev = file->private_data;
-	struct sta2x11_vip *vip = video_get_drvdata(dev);
-
-	return videobuf_read_stream(&vip->vb_vidq, data, count, ppos, 0,
-				    file->f_flags & O_NONBLOCK);
+	struct sta2x11_vip *vip = vb2_get_drv_priv(vq);
+	struct vip_buffer *vip_buf, *node;
+
+	/* Disable acquisition */
+	reg_write(vip, DVP_CTL, reg_read(vip, DVP_CTL) & ~DVP_CTL_ENA);
+	/* Disable all interrupts */
+	reg_write(vip, DVP_ITM, 0);
+
+	/* Release all active buffers */
+	spin_lock(&vip->lock);
+	list_for_each_entry_safe(vip_buf, node, &vip->buffer_list, list) {
+		vb2_buffer_done(&vip_buf->vb, VB2_BUF_STATE_ERROR);
+		list_del(&vip_buf->list);
+	}
+	spin_unlock(&vip->lock);
+	return 0;
 }
 
-/**
- * vip_mmap - map user buffer
- * @file: descriptor of device
- * @vma: user buffer
- *
- * map user space buffer into kernel mode, including DMA address.
- * handling is done in generic videobuf layer.
- * return value: provided by videobuf layer
- */
-static int vip_mmap(struct file *file, struct vm_area_struct *vma)
-{
-	struct video_device *dev = file->private_data;
-	struct sta2x11_vip *vip = video_get_drvdata(dev);
+static struct vb2_ops vip_video_qops = {
+	.queue_setup		= queue_setup,
+	.buf_init		= buffer_init,
+	.buf_prepare		= buffer_prepare,
+	.buf_finish		= buffer_finish,
+	.buf_queue		= buffer_queue,
+	.start_streaming	= start_streaming,
+	.stop_streaming		= stop_streaming,
+};
 
-	return videobuf_mmap_mapper(&vip->vb_vidq, vma);
-}
 
-/**
- * vip_poll - poll for event
- * @file: descriptor of device
- * @wait: contains events to be waited for
- *
- * wait for event related to video device.
- * handling is done in generic videobuf layer.
- * return value: provided by videobuf layer
- */
-static unsigned int vip_poll(struct file *file, struct poll_table_struct *wait)
-{
-	struct video_device *dev = file->private_data;
-	struct sta2x11_vip *vip = video_get_drvdata(dev);
+/* File Operations */
+static const struct v4l2_file_operations vip_fops = {
+	.owner = THIS_MODULE,
+	.open = v4l2_fh_open,
+	.release = vb2_fop_release,
+	.unlocked_ioctl = video_ioctl2,
+	.read = vb2_fop_read,
+	.mmap = vb2_fop_mmap,
+	.poll = vb2_fop_poll
+};
 
-	return videobuf_poll_stream(file, &vip->vb_vidq, wait);
-}
 
-/**
- * vidioc_querycap - return capabilities of device
- * @file: descriptor of device (not used)
- * @priv: points to current videodevice
- * @cap: contains return values
- *
- * the capabilities of the device are returned
- *
- * return value: 0, no error.
- */
+/* V4L2 ioctl Operations */
 static int vidioc_querycap(struct file *file, void *priv,
 			   struct v4l2_capability *cap)
 {
-	struct video_device *dev = priv;
-	struct sta2x11_vip *vip = video_get_drvdata(dev);
+	struct sta2x11_vip *vip = video_drvdata(file);
 
-	memset(cap, 0, sizeof(struct v4l2_capability));
-	strcpy(cap->driver, DRV_NAME);
-	strcpy(cap->card, DRV_NAME);
-	cap->version = 0;
+	strcpy(cap->driver, KBUILD_MODNAME);
+	strcpy(cap->card, KBUILD_MODNAME);
 	snprintf(cap->bus_info, sizeof(cap->bus_info), "PCI:%s",
 		 pci_name(vip->pdev));
-	cap->capabilities = V4L2_CAP_VIDEO_CAPTURE | V4L2_CAP_READWRITE |
-	    V4L2_CAP_STREAMING;
+	cap->device_caps = V4L2_CAP_VIDEO_CAPTURE | V4L2_CAP_READWRITE |
+			   V4L2_CAP_STREAMING;
+	cap->capabilities = cap->device_caps | V4L2_CAP_DEVICE_CAPS;
 
 	return 0;
 }
 
-/**
- * vidioc_s_std - set video standard
- * @file: descriptor of device (not used)
- * @priv: points to current videodevice
- * @std: contains standard to be set
- *
- * the video standard is set
- *
- * return value: 0, no error.
- *
- * -EIO, no input signal detected
- *
- * other, returned from video DAC.
- */
 static int vidioc_s_std(struct file *file, void *priv, v4l2_std_id *std)
 {
-	struct video_device *dev = priv;
-	struct sta2x11_vip *vip = video_get_drvdata(dev);
+	struct sta2x11_vip *vip = video_drvdata(file);
 	v4l2_std_id oldstd = vip->std, newstd;
 	int status;
 
@@ -590,110 +423,21 @@ static int vidioc_s_std(struct file *file, void *priv, v4l2_std_id *std)
 	return v4l2_subdev_call(vip->decoder, core, s_std, *std);
 }
 
-/**
- * vidioc_g_std - get video standard
- * @file: descriptor of device (not used)
- * @priv: points to current videodevice
- * @std: contains return values
- *
- * the current video standard is returned
- *
- * return value: 0, no error.
- */
 static int vidioc_g_std(struct file *file, void *priv, v4l2_std_id *std)
 {
-	struct video_device *dev = priv;
-	struct sta2x11_vip *vip = video_get_drvdata(dev);
+	struct sta2x11_vip *vip = video_drvdata(file);
 
 	*std = vip->std;
 	return 0;
 }
 
-/**
- * vidioc_querystd - get possible video standards
- * @file: descriptor of device (not used)
- * @priv: points to current videodevice
- * @std: contains return values
- *
- * all possible video standards are returned
- *
- * return value: delivered by video DAC routine.
- */
 static int vidioc_querystd(struct file *file, void *priv, v4l2_std_id *std)
 {
-	struct video_device *dev = priv;
-	struct sta2x11_vip *vip = video_get_drvdata(dev);
+	struct sta2x11_vip *vip = video_drvdata(file);
 
 	return v4l2_subdev_call(vip->decoder, video, querystd, std);
-
 }
 
-/**
- * vidioc_queryctl - get possible control settings
- * @file: descriptor of device (not used)
- * @priv: points to current videodevice
- * @ctrl: contains return values
- *
- * return possible values for a control
- * return value: delivered by video DAC routine.
- */
-static int vidioc_queryctrl(struct file *file, void *priv,
-			    struct v4l2_queryctrl *ctrl)
-{
-	struct video_device *dev = priv;
-	struct sta2x11_vip *vip = video_get_drvdata(dev);
-
-	return v4l2_subdev_call(vip->decoder, core, queryctrl, ctrl);
-}
-
-/**
- * vidioc_g_ctl - get control value
- * @file: descriptor of device (not used)
- * @priv: points to current videodevice
- * @ctrl: contains return values
- *
- * return setting for a control value
- * return value: delivered by video DAC routine.
- */
-static int vidioc_g_ctrl(struct file *file, void *priv,
-			 struct v4l2_control *ctrl)
-{
-	struct video_device *dev = priv;
-	struct sta2x11_vip *vip = video_get_drvdata(dev);
-
-	return v4l2_subdev_call(vip->decoder, core, g_ctrl, ctrl);
-}
-
-/**
- * vidioc_s_ctl - set control value
- * @file: descriptor of device (not used)
- * @priv: points to current videodevice
- * @ctrl: contains value to be set
- *
- * set value for a specific control
- * return value: delivered by video DAC routine.
- */
-static int vidioc_s_ctrl(struct file *file, void *priv,
-			 struct v4l2_control *ctrl)
-{
-	struct video_device *dev = priv;
-	struct sta2x11_vip *vip = video_get_drvdata(dev);
-
-	return v4l2_subdev_call(vip->decoder, core, s_ctrl, ctrl);
-}
-
-/**
- * vidioc_enum_input - return name of input line
- * @file: descriptor of device (not used)
- * @priv: points to current videodevice
- * @inp: contains return values
- *
- * the user friendly name of the input line is returned
- *
- * return value: 0, no error.
- *
- * -EINVAL, input line number out of range
- */
 static int vidioc_enum_input(struct file *file, void *priv,
 			     struct v4l2_input *inp)
 {
@@ -707,22 +451,9 @@ static int vidioc_enum_input(struct file *file, void *priv,
 	return 0;
 }
 
-/**
- * vidioc_s_input - set input line
- * @file: descriptor of device ( not used)
- * @priv: points to current videodevice
- * @i: new input line number
- *
- * the current active input line is set
- *
- * return value: 0, no error.
- *
- * -EINVAL, line number out of range
- */
 static int vidioc_s_input(struct file *file, void *priv, unsigned int i)
 {
-	struct video_device *dev = priv;
-	struct sta2x11_vip *vip = video_get_drvdata(dev);
+	struct sta2x11_vip *vip = video_drvdata(file);
 	int ret;
 
 	if (i > 1)
@@ -735,36 +466,14 @@ static int vidioc_s_input(struct file *file, void *priv, unsigned int i)
 	return 0;
 }
 
-/**
- * vidioc_g_input - return input line
- * @file: descriptor of device ( not used)
- * @priv: points to current videodevice
- * @i: returned input line number
- *
- * the current active input line is returned
- *
- * return value: always 0.
- */
 static int vidioc_g_input(struct file *file, void *priv, unsigned int *i)
 {
-	struct video_device *dev = priv;
-	struct sta2x11_vip *vip = video_get_drvdata(dev);
+	struct sta2x11_vip *vip = video_drvdata(file);
 
 	*i = vip->input;
 	return 0;
 }
 
-/**
- * vidioc_enum_fmt_vid_cap - return video capture format
- * @file: descriptor of device ( not used)
- * @priv: points to current videodevice
- * @f: returned format information
- *
- * returns name and format of video capture
- * Only UYVY is supported by hardware.
- *
- * return value: always 0.
- */
 static int vidioc_enum_fmt_vid_cap(struct file *file, void *priv,
 				   struct v4l2_fmtdesc *f)
 {
@@ -778,38 +487,19 @@ static int vidioc_enum_fmt_vid_cap(struct file *file, void *priv,
 	return 0;
 }
 
-/**
- * vidioc_try_fmt_vid_cap - set video capture format
- * @file: descriptor of device ( not used)
- * @priv: points to current videodevice
- * @f: new format
- *
- * new video format is set which includes width and
- * field type. width is fixed to 720, no scaling.
- * Only UYVY is supported by this hardware.
- * the minimum height is 200, the maximum is 576 (PAL)
- *
- * return value: 0, no error
- *
- * -EINVAL, pixel or field format not supported
- *
- */
 static int vidioc_try_fmt_vid_cap(struct file *file, void *priv,
 				  struct v4l2_format *f)
 {
-	struct video_device *dev = priv;
-	struct sta2x11_vip *vip = video_get_drvdata(dev);
+	struct sta2x11_vip *vip = video_drvdata(file);
 	int interlace_lim;
 
-	if (V4L2_PIX_FMT_UYVY != f->fmt.pix.pixelformat)
-		return -EINVAL;
-
 	if (V4L2_STD_525_60 & vip->std)
 		interlace_lim = 240;
 	else
 		interlace_lim = 288;
 
 	switch (f->fmt.pix.field) {
+	default:
 	case V4L2_FIELD_ANY:
 		if (interlace_lim < f->fmt.pix.height)
 			f->fmt.pix.field = V4L2_FIELD_INTERLACED;
@@ -823,10 +513,10 @@ static int vidioc_try_fmt_vid_cap(struct file *file, void *priv,
 		break;
 	case V4L2_FIELD_INTERLACED:
 		break;
-	default:
-		return -EINVAL;
 	}
 
+	/* It is the only supported format */
+	f->fmt.pix.pixelformat = V4L2_PIX_FMT_UYVY;
 	f->fmt.pix.height &= ~1;
 	if (2 * interlace_lim < f->fmt.pix.height)
 		f->fmt.pix.height = 2 * interlace_lim;
@@ -840,304 +530,221 @@ static int vidioc_try_fmt_vid_cap(struct file *file, void *priv,
 	return 0;
 }
 
-/**
- * vidioc_s_fmt_vid_cap - set current video format parameters
- * @file: descriptor of device ( not used)
- * @priv: points to current videodevice
- * @f: returned format information
- *
- * set new capture format
- * return value: 0, no error
- *
- * other, delivered by video DAC routine.
- */
 static int vidioc_s_fmt_vid_cap(struct file *file, void *priv,
 				struct v4l2_format *f)
 {
-	struct video_device *dev = priv;
-	struct sta2x11_vip *vip = video_get_drvdata(dev);
+	struct sta2x11_vip *vip = video_drvdata(file);
+	unsigned int t_stop, b_stop, pitch;
 	int ret;
 
 	ret = vidioc_try_fmt_vid_cap(file, priv, f);
 	if (ret)
 		return ret;
 
-	memcpy(&vip->format, &f->fmt.pix, sizeof(struct v4l2_pix_format));
-	return 0;
-}
+	if (vb2_is_busy(&vip->vb_vidq)) {
+		/* Can't change format during acquisition */
+		v4l2_err(&vip->v4l2_dev, "device busy\n");
+		return -EBUSY;
+	}
+	vip->format = f->fmt.pix;
+	switch (vip->format.field) {
+	case V4L2_FIELD_INTERLACED:
+		t_stop = ((vip->format.height / 2 - 1) << 16) |
+			 (2 * vip->format.width - 1);
+		b_stop = t_stop;
+		pitch = 4 * vip->format.width;
+		break;
+	case V4L2_FIELD_TOP:
+		t_stop = ((vip->format.height - 1) << 16) |
+			 (2 * vip->format.width - 1);
+		b_stop = (0 << 16) | (2 * vip->format.width - 1);
+		pitch = 2 * vip->format.width;
+		break;
+	case V4L2_FIELD_BOTTOM:
+		t_stop = (0 << 16) | (2 * vip->format.width - 1);
+		b_stop = (vip->format.height << 16) |
+			 (2 * vip->format.width - 1);
+		pitch = 2 * vip->format.width;
+		break;
+	default:
+		v4l2_err(&vip->v4l2_dev, "unknown field format\n");
+		return -EINVAL;
+	}
 
-/**
- * vidioc_g_fmt_vid_cap - get current video format parameters
- * @file: descriptor of device ( not used)
- * @priv: points to current videodevice
- * @f: contains format information
- *
- * returns current video format parameters
- *
- * return value: 0, always successful
- */
-static int vidioc_g_fmt_vid_cap(struct file *file, void *priv,
-				struct v4l2_format *f)
-{
-	struct video_device *dev = priv;
-	struct sta2x11_vip *vip = video_get_drvdata(dev);
+	spin_lock_irq(&vip->slock);
+	/* Y-X Top Field Offset */
+	reg_write(vip, DVP_TFO, 0);
+	/* Y-X Bottom Field Offset */
+	reg_write(vip, DVP_BFO, 0);
+	/* Y-X Top Field Stop*/
+	reg_write(vip, DVP_TFS, t_stop);
+	/* Y-X Bottom Field Stop */
+	reg_write(vip, DVP_BFS, b_stop);
+	/* Video Memory Pitch */
+	reg_write(vip, DVP_VMP, pitch);
+	spin_unlock_irq(&vip->slock);
 
-	memcpy(&f->fmt.pix, &vip->format, sizeof(struct v4l2_pix_format));
 	return 0;
 }
 
-/**
- * vidioc_reqfs - request buffer
- * @file: descriptor of device ( not used)
- * @priv: points to current videodevice
- * @p: video buffer
- *
- * Handling is done in generic videobuf layer.
- */
-static int vidioc_reqbufs(struct file *file, void *priv,
-			  struct v4l2_requestbuffers *p)
-{
-	struct video_device *dev = priv;
-	struct sta2x11_vip *vip = video_get_drvdata(dev);
-
-	return videobuf_reqbufs(&vip->vb_vidq, p);
-}
-
-/**
- * vidioc_querybuf - query buffer
- * @file: descriptor of device ( not used)
- * @priv: points to current videodevice
- * @p: video buffer
- *
- * query buffer state.
- * Handling is done in generic videobuf layer.
- */
-static int vidioc_querybuf(struct file *file, void *priv, struct v4l2_buffer *p)
-{
-	struct video_device *dev = priv;
-	struct sta2x11_vip *vip = video_get_drvdata(dev);
-
-	return videobuf_querybuf(&vip->vb_vidq, p);
-}
-
-/**
- * vidioc_qbuf - queue a buffer
- * @file: descriptor of device ( not used)
- * @priv: points to current videodevice
- * @p: video buffer
- *
- * Handling is done in generic videobuf layer.
- */
-static int vidioc_qbuf(struct file *file, void *priv, struct v4l2_buffer *p)
-{
-	struct video_device *dev = priv;
-	struct sta2x11_vip *vip = video_get_drvdata(dev);
-
-	return videobuf_qbuf(&vip->vb_vidq, p);
-}
-
-/**
- * vidioc_dqbuf - dequeue a buffer
- * @file: descriptor of device ( not used)
- * @priv: points to current videodevice
- * @p: video buffer
- *
- * Handling is done in generic videobuf layer.
- */
-static int vidioc_dqbuf(struct file *file, void *priv, struct v4l2_buffer *p)
-{
-	struct video_device *dev = priv;
-	struct sta2x11_vip *vip = video_get_drvdata(dev);
-
-	return videobuf_dqbuf(&vip->vb_vidq, p, file->f_flags & O_NONBLOCK);
-}
-
-/**
- * vidioc_streamon - turn on streaming
- * @file: descriptor of device ( not used)
- * @priv: points to current videodevice
- * @type: type of capture
- *
- * turn on streaming.
- * Handling is done in generic videobuf layer.
- */
-static int vidioc_streamon(struct file *file, void *priv,
-			   enum v4l2_buf_type type)
+static int vidioc_g_fmt_vid_cap(struct file *file, void *priv,
+				struct v4l2_format *f)
 {
-	struct video_device *dev = priv;
-	struct sta2x11_vip *vip = video_get_drvdata(dev);
+	struct sta2x11_vip *vip = video_drvdata(file);
 
-	return videobuf_streamon(&vip->vb_vidq);
-}
+	f->fmt.pix = vip->format;
 
-/**
- * vidioc_streamoff - turn off streaming
- * @file: descriptor of device ( not used)
- * @priv: points to current videodevice
- * @type: type of capture
- *
- * turn off streaming.
- * Handling is done in generic videobuf layer.
- */
-static int vidioc_streamoff(struct file *file, void *priv,
-			    enum v4l2_buf_type type)
-{
-	struct video_device *dev = priv;
-	struct sta2x11_vip *vip = video_get_drvdata(dev);
-
-	return videobuf_streamoff(&vip->vb_vidq);
+	return 0;
 }
 
-static const struct v4l2_file_operations vip_fops = {
-	.owner = THIS_MODULE,
-	.open = vip_open,
-	.release = vip_close,
-	.ioctl = video_ioctl2,
-	.read = vip_read,
-	.mmap = vip_mmap,
-	.poll = vip_poll
-};
-
 static const struct v4l2_ioctl_ops vip_ioctl_ops = {
 	.vidioc_querycap = vidioc_querycap,
-	.vidioc_s_std = vidioc_s_std,
+	/* FMT handling */
+	.vidioc_enum_fmt_vid_cap = vidioc_enum_fmt_vid_cap,
+	.vidioc_g_fmt_vid_cap = vidioc_g_fmt_vid_cap,
+	.vidioc_s_fmt_vid_cap = vidioc_s_fmt_vid_cap,
+	.vidioc_try_fmt_vid_cap = vidioc_try_fmt_vid_cap,
+	/* Buffer handlers */
+	.vidioc_reqbufs = vb2_ioctl_reqbufs,
+	.vidioc_querybuf = vb2_ioctl_querybuf,
+	.vidioc_qbuf = vb2_ioctl_qbuf,
+	.vidioc_dqbuf = vb2_ioctl_dqbuf,
+	/* Stream on/off */
+	.vidioc_streamon = vb2_ioctl_streamon,
+	.vidioc_streamoff = vb2_ioctl_streamoff,
+	/* Standard handling */
 	.vidioc_g_std = vidioc_g_std,
+	.vidioc_s_std = vidioc_s_std,
 	.vidioc_querystd = vidioc_querystd,
-	.vidioc_queryctrl = vidioc_queryctrl,
-	.vidioc_g_ctrl = vidioc_g_ctrl,
-	.vidioc_s_ctrl = vidioc_s_ctrl,
+	/* Input handling */
 	.vidioc_enum_input = vidioc_enum_input,
-	.vidioc_try_fmt_vid_cap = vidioc_try_fmt_vid_cap,
-	.vidioc_s_input = vidioc_s_input,
 	.vidioc_g_input = vidioc_g_input,
-	.vidioc_enum_fmt_vid_cap = vidioc_enum_fmt_vid_cap,
-	.vidioc_s_fmt_vid_cap = vidioc_s_fmt_vid_cap,
-	.vidioc_g_fmt_vid_cap = vidioc_g_fmt_vid_cap,
-	.vidioc_reqbufs = vidioc_reqbufs,
-	.vidioc_querybuf = vidioc_querybuf,
-	.vidioc_qbuf = vidioc_qbuf,
-	.vidioc_dqbuf = vidioc_dqbuf,
-	.vidioc_streamon = vidioc_streamon,
-	.vidioc_streamoff = vidioc_streamoff,
+	.vidioc_s_input = vidioc_s_input,
+	/* Log status ioctl */
+	.vidioc_log_status = v4l2_ctrl_log_status,
+	/* Event handling */
+	.vidioc_subscribe_event = v4l2_ctrl_subscribe_event,
+	.vidioc_unsubscribe_event = v4l2_event_unsubscribe,
 };
 
 static struct video_device video_dev_template = {
-	.name = DRV_NAME,
+	.name = KBUILD_MODNAME,
 	.release = video_device_release,
 	.fops = &vip_fops,
 	.ioctl_ops = &vip_ioctl_ops,
 	.tvnorms = V4L2_STD_ALL,
 };
 
-/**
- * vip_irq - interrupt routine
- * @irq: Number of interrupt ( not used, correct number is assumed )
- * @vip: local data structure containing all information
- *
- * check for both frame interrupts set ( top and bottom ).
- * check FIFO overflow, but limit number of log messages after open.
- * signal a complete buffer if done.
- * dequeue a new buffer if available.
- * disable VIP if no buffer available.
- *
- * return value: IRQ_NONE, interrupt was not generated by VIP
- *
- * IRQ_HANDLED, interrupt done.
- */
+
 static irqreturn_t vip_irq(int irq, struct sta2x11_vip *vip)
 {
-	u32 status, dma;
-	unsigned long flags;
-	struct videobuf_buffer *vb;
+	unsigned int status;
 
-	status = REG_READ(vip, DVP_ITS);
+	status = reg_read(vip, DVP_ITS);
 
-	if (!status) {
-		pr_debug("VIP: irq ignored\n");
+	if (!status)		/* No interrupt to handle */
 		return IRQ_NONE;
-	}
-
-	if (!vip->started)
-		return IRQ_HANDLED;
-
-	if (status & DVP_IT_VSB)
-		vip->bcount++;
 
-	if (status & DVP_IT_VST)
-		vip->tcount++;
+	if (status & DVP_IT_FIFO)
+		if (vip->overflow++ > 5)
+			pr_info("VIP: fifo overflow\n");
 
-	if ((DVP_IT_VSB | DVP_IT_VST) == (status & (DVP_IT_VST | DVP_IT_VSB))) {
+	if ((status & DVP_IT_VST) && (status & DVP_IT_VSB)) {
 		/* this is bad, we are too slow, hope the condition is gone
 		 * on the next frame */
-		pr_info("VIP: both irqs\n");
 		return IRQ_HANDLED;
 	}
 
-	if (status & DVP_IT_FIFO) {
-		if (5 > vip->overflow++)
-			pr_info("VIP: fifo overflow\n");
-	}
-
-	if (2 > vip->tcount)
+	if (status & DVP_IT_VST)
+		if ((++vip->tcount) < 2)
+			return IRQ_HANDLED;
+	if (status & DVP_IT_VSB) {
+		vip->bcount++;
 		return IRQ_HANDLED;
+	}
 
-	if (status & DVP_IT_VSB)
-		return IRQ_HANDLED;
+	if (vip->active) { /* Acquisition is over on this buffer */
+		/* Disable acquisition */
+		reg_write(vip, DVP_CTL, reg_read(vip, DVP_CTL) & ~DVP_CTL_ENA);
+		/* Remove the active buffer from the list */
+		do_gettimeofday(&vip->active->vb.v4l2_buf.timestamp);
+		vip->active->vb.v4l2_buf.sequence = vip->sequence++;
+		vb2_buffer_done(&vip->active->vb, VB2_BUF_STATE_DONE);
+	}
 
-	spin_lock_irqsave(&vip->slock, flags);
+	return IRQ_HANDLED;
+}
 
-	REG_WRITE(vip, DVP_CTL, REG_READ(vip, DVP_CTL) & ~DVP_CTL_ENA);
-	if (vip->active) {
-		do_gettimeofday(&vip->active->ts);
-		vip->active->field_count++;
-		vip->active->state = VIDEOBUF_DONE;
-		wake_up(&vip->active->done);
-		vip->active = NULL;
+static void sta2x11_vip_init_register(struct sta2x11_vip *vip)
+{
+	/* Register initialization */
+	spin_lock_irq(&vip->slock);
+	/* Clean interrupt */
+	reg_read(vip, DVP_ITS);
+	/* Enable Half Line per vertical */
+	reg_write(vip, DVP_HLFLN, DVP_HLFLN_SD);
+	/* Reset VIP control */
+	reg_write(vip, DVP_CTL, DVP_CTL_RST);
+	/* Clear VIP control */
+	reg_write(vip, DVP_CTL, 0);
+	spin_unlock_irq(&vip->slock);
+}
+static void sta2x11_vip_clear_register(struct sta2x11_vip *vip)
+{
+	spin_lock_irq(&vip->slock);
+	/* Disable interrupt */
+	reg_write(vip, DVP_ITM, 0);
+	/* Reset VIP Control */
+	reg_write(vip, DVP_CTL, DVP_CTL_RST);
+	/* Clear VIP Control */
+	reg_write(vip, DVP_CTL, 0);
+	/* Clean VIP Interrupt */
+	reg_read(vip, DVP_ITS);
+	spin_unlock_irq(&vip->slock);
+}
+static int sta2x11_vip_init_buffer(struct sta2x11_vip *vip)
+{
+	memset(&vip->vb_vidq, 0, sizeof(struct vb2_queue));
+	vip->vb_vidq.type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
+	vip->vb_vidq.io_modes = VB2_MMAP | VB2_READ;
+	vip->vb_vidq.drv_priv = vip;
+	vip->vb_vidq.buf_struct_size = sizeof(struct vip_buffer);
+	vip->vb_vidq.ops = &vip_video_qops;
+	vip->vb_vidq.mem_ops = &vb2_dma_streaming_memops;
+	vb2_queue_init(&vip->vb_vidq);
+	INIT_LIST_HEAD(&vip->buffer_list);
+	spin_lock_init(&vip->lock);
+
+	vip->alloc_ctx = vb2_dma_streaming_init_ctx(&vip->pdev->dev);
+	if (IS_ERR(vip->alloc_ctx)) {
+		v4l2_err(&vip->v4l2_dev, "Can't allocate buffer context");
+		return PTR_ERR(vip->alloc_ctx);
 	}
-	if (!vip->closing) {
-		if (list_empty(&vip->capture))
-			goto done;
-
-		vb = list_first_entry(&vip->capture, struct videobuf_buffer,
-				      queue);
-		if (NULL == vb) {
-			pr_info("VIP: no buffer\n");
-			goto done;
-		}
-		vb->state = VIDEOBUF_ACTIVE;
-		list_del(&vb->queue);
-		vip->active = vb;
-		dma = videobuf_to_dma_contig(vb);
-		switch (vip->format.field) {
-		case V4L2_FIELD_INTERLACED:
-			REG_WRITE(vip, DVP_VTP, dma);
-			REG_WRITE(vip, DVP_VBP, dma + vip->format.width * 2);
-			break;
-		case V4L2_FIELD_TOP:
-		case V4L2_FIELD_BOTTOM:
-			REG_WRITE(vip, DVP_VTP, dma);
-			REG_WRITE(vip, DVP_VBP, dma);
-			break;
-		default:
-			pr_warning("VIP: unknown field format\n");
-			goto done;
-			break;
-		}
-		REG_WRITE(vip, DVP_CTL, REG_READ(vip, DVP_CTL) | DVP_CTL_ENA);
+	return 0;
+}
+static void sta2x11_vip_release_buffer(struct sta2x11_vip *vip)
+{
+	vb2_dma_streaming_cleanup_ctx(vip->alloc_ctx);
+}
+static int sta2x11_vip_init_controls(struct sta2x11_vip *vip)
+{
+	/*
+	 * Inititialize an empty control so VIP can inerithing controls
+	 * from ADV7180
+	 */
+	v4l2_ctrl_handler_init(&vip->ctrl_hdl, 0);
+
+	vip->v4l2_dev.ctrl_handler = &vip->ctrl_hdl;
+	if (vip->ctrl_hdl.error) {
+		int err = vip->ctrl_hdl.error;
+
+		v4l2_ctrl_handler_free(&vip->ctrl_hdl);
+		return err;
 	}
-done:
-	spin_unlock_irqrestore(&vip->slock, flags);
-	return IRQ_HANDLED;
+
+	return 0;
 }
 
-/**
- * vip_gpio_reserve - reserve gpio pin
- * @dev: device
- * @pin: GPIO pin number
- * @dir: direction, input or output
- * @name: GPIO pin name
- *
- */
 static int vip_gpio_reserve(struct device *dev, int pin, int dir,
 			    const char *name)
 {
@@ -1170,13 +777,6 @@ static int vip_gpio_reserve(struct device *dev, int pin, int dir,
 	return 0;
 }
 
-/**
- * vip_gpio_release - release gpio pin
- * @dev: device
- * @pin: GPIO pin number
- * @name: GPIO pin name
- *
- */
 static void vip_gpio_release(struct device *dev, int pin, const char *name)
 {
 	if (pin != -1) {
@@ -1186,25 +786,6 @@ static void vip_gpio_release(struct device *dev, int pin, const char *name)
 	}
 }
 
-/**
- * sta2x11_vip_init_one - init one instance of video device
- * @pdev: PCI device
- * @ent: (not used)
- *
- * allocate reset pins for DAC.
- * Reset video DAC, this is done via reset line.
- * allocate memory for managing device
- * request interrupt
- * map IO region
- * register device
- * find and initialize video DAC
- *
- * return value: 0, no error
- *
- * -ENOMEM, no memory
- *
- * -ENODEV, device could not be detected or registered
- */
 static int __devinit sta2x11_vip_init_one(struct pci_dev *pdev,
 					  const struct pci_device_id *ent)
 {
@@ -1212,10 +793,17 @@ static int __devinit sta2x11_vip_init_one(struct pci_dev *pdev,
 	struct sta2x11_vip *vip;
 	struct vip_config *config;
 
+	/* Check if hardware support 26-bit DMA */
+	if (dma_set_mask(&pdev->dev, DMA_BIT_MASK(26))) {
+		dev_err(&pdev->dev, "26-bit DMA addressing not available\n");
+		return -EINVAL;
+	}
+	/* Enable PCI */
 	ret = pci_enable_device(pdev);
 	if (ret)
 		return ret;
 
+	/* Get VIP platform data */
 	config = dev_get_platdata(&pdev->dev);
 	if (!config) {
 		dev_info(&pdev->dev, "VIP slot disabled\n");
@@ -1223,6 +811,7 @@ static int __devinit sta2x11_vip_init_one(struct pci_dev *pdev,
 		goto disable;
 	}
 
+	/* Power configuration */
 	ret = vip_gpio_reserve(&pdev->dev, config->pwr_pin, 0,
 			       config->pwr_name);
 	if (ret)
@@ -1237,7 +826,6 @@ static int __devinit sta2x11_vip_init_one(struct pci_dev *pdev,
 			goto disable;
 		}
 	}
-
 	if (config->pwr_pin != -1) {
 		/* Datasheet says 5ms between PWR and RST */
 		usleep_range(5000, 25000);
@@ -1251,17 +839,20 @@ static int __devinit sta2x11_vip_init_one(struct pci_dev *pdev,
 	}
 	usleep_range(5000, 25000);
 
+	/* Allocate a new VIP instance */
 	vip = kzalloc(sizeof(struct sta2x11_vip), GFP_KERNEL);
 	if (!vip) {
 		ret = -ENOMEM;
 		goto release_gpios;
 	}
-
 	vip->pdev = pdev;
 	vip->std = V4L2_STD_PAL;
 	vip->format = formats_50[0];
 	vip->config = config;
 
+	ret = sta2x11_vip_init_controls(vip);
+	if (ret)
+		goto free_mem;
 	if (v4l2_device_register(&pdev->dev, &vip->v4l2_dev))
 		goto free_mem;
 
@@ -1271,46 +862,52 @@ static int __devinit sta2x11_vip_init_one(struct pci_dev *pdev,
 
 	pci_set_master(pdev);
 
-	ret = pci_request_regions(pdev, DRV_NAME);
+	ret = pci_request_regions(pdev, KBUILD_MODNAME);
 	if (ret)
 		goto unreg;
 
 	vip->iomem = pci_iomap(pdev, 0, 0x100);
 	if (!vip->iomem) {
-		ret = -ENOMEM; /* FIXME */
+		ret = -ENOMEM;
 		goto release;
 	}
 
 	pci_enable_msi(pdev);
 
-	INIT_LIST_HEAD(&vip->capture);
+	/* Initialize buffer */
+	ret = sta2x11_vip_init_buffer(vip);
+	if (ret)
+		goto unmap;
+
 	spin_lock_init(&vip->slock);
-	mutex_init(&vip->mutex);
-	vip->started = 0;
-	vip->disabled = 0;
 
 	ret = request_irq(pdev->irq,
 			  (irq_handler_t) vip_irq,
-			  IRQF_SHARED, DRV_NAME, vip);
+			  IRQF_SHARED, KBUILD_MODNAME, vip);
 	if (ret) {
 		dev_err(&pdev->dev, "request_irq failed\n");
 		ret = -ENODEV;
-		goto unmap;
+		goto release_buf;
 	}
 
+	/* Alloc, initialize and register video device */
 	vip->video_dev = video_device_alloc();
 	if (!vip->video_dev) {
 		ret = -ENOMEM;
 		goto release_irq;
 	}
 
-	*(vip->video_dev) = video_dev_template;
+	vip->video_dev = &video_dev_template;
+	vip->video_dev->v4l2_dev = &vip->v4l2_dev;
+	vip->video_dev->queue = &vip->vb_vidq;
+	set_bit(V4L2_FL_USE_FH_PRIO, &vip->video_dev->flags);
 	video_set_drvdata(vip->video_dev, vip);
 
 	ret = video_register_device(vip->video_dev, VFL_TYPE_GRABBER, -1);
 	if (ret)
 		goto vrelease;
 
+	/* Get ADV7180 subdevice */
 	vip->adapter = i2c_get_adapter(vip->config->i2c_id);
 	if (!vip->adapter) {
 		ret = -ENODEV;
@@ -1328,10 +925,11 @@ static int __devinit sta2x11_vip_init_one(struct pci_dev *pdev,
 	}
 
 	i2c_put_adapter(vip->adapter);
-
 	v4l2_subdev_call(vip->decoder, core, init, 0);
 
-	pr_info("STA2X11 Video Input Port (VIP) loaded\n");
+	sta2x11_vip_init_register(vip);
+
+	dev_info(&pdev->dev, "STA2X11 Video Input Port (VIP) loaded\n");
 	return 0;
 
 vunreg:
@@ -1343,10 +941,12 @@ vrelease:
 		video_device_release(vip->video_dev);
 release_irq:
 	free_irq(pdev->irq, vip);
+release_buf:
+	sta2x11_vip_release_buffer(vip);
 	pci_disable_msi(pdev);
 unmap:
+	vb2_queue_release(&vip->vb_vidq);
 	pci_iounmap(pdev, vip->iomem);
-	mutex_destroy(&vip->mutex);
 release:
 	pci_release_regions(pdev);
 unreg:
@@ -1364,34 +964,24 @@ disable:
 	return ret;
 }
 
-/**
- * sta2x11_vip_remove_one - release device
- * @pdev: PCI device
- *
- * Undo everything done in .._init_one
- *
- * unregister video device
- * free interrupt
- * unmap ioadresses
- * free memory
- * free GPIO pins
- */
 static void __devexit sta2x11_vip_remove_one(struct pci_dev *pdev)
 {
 	struct v4l2_device *v4l2_dev = pci_get_drvdata(pdev);
 	struct sta2x11_vip *vip =
 	    container_of(v4l2_dev, struct sta2x11_vip, v4l2_dev);
 
+	sta2x11_vip_clear_register(vip);
+
 	video_set_drvdata(vip->video_dev, NULL);
 	video_unregister_device(vip->video_dev);
 	/*do not call video_device_release() here, is already done */
 	free_irq(pdev->irq, vip);
 	pci_disable_msi(pdev);
+	vb2_queue_release(&vip->vb_vidq);
 	pci_iounmap(pdev, vip->iomem);
 	pci_release_regions(pdev);
 
 	v4l2_device_unregister(&vip->v4l2_dev);
-	mutex_destroy(&vip->mutex);
 
 	vip_gpio_release(&pdev->dev, vip->config->pwr_pin,
 			 vip->config->pwr_name);
@@ -1407,18 +997,12 @@ static void __devexit sta2x11_vip_remove_one(struct pci_dev *pdev)
 
 #ifdef CONFIG_PM
 
-/**
- * sta2x11_vip_suspend - set device into power save mode
- * @pdev: PCI device
- * @state: new state of device
+/*
  *
  * all relevant registers are saved and an attempt to set a new state is made.
  *
  * return value: 0 always indicate success,
  * even if device could not be disabled. (workaround for hardware problem)
- *
- * reurn value : 0, always succesful, even if hardware does not not support
- * power down mode.
  */
 static int sta2x11_vip_suspend(struct pci_dev *pdev, pm_message_t state)
 {
@@ -1429,15 +1013,15 @@ static int sta2x11_vip_suspend(struct pci_dev *pdev, pm_message_t state)
 	int i;
 
 	spin_lock_irqsave(&vip->slock, flags);
-	vip->register_save_area[0] = REG_READ(vip, DVP_CTL);
-	REG_WRITE(vip, DVP_CTL, vip->register_save_area[0] & DVP_CTL_DIS);
-	vip->register_save_area[SAVE_COUNT] = REG_READ(vip, DVP_ITM);
-	REG_WRITE(vip, DVP_ITM, 0);
+	vip->register_save_area[0] = reg_read(vip, DVP_CTL);
+	reg_write(vip, DVP_CTL, vip->register_save_area[0] & DVP_CTL_DIS);
+	vip->register_save_area[SAVE_COUNT] = reg_read(vip, DVP_ITM);
+	reg_write(vip, DVP_ITM, 0);
 	for (i = 1; i < SAVE_COUNT; i++)
-		vip->register_save_area[i] = REG_READ(vip, 4 * i);
+		vip->register_save_area[i] = reg_read(vip, 4 * i);
 	for (i = 0; i < AUX_COUNT; i++)
 		vip->register_save_area[SAVE_COUNT + IRQ_COUNT + i] =
-		    REG_READ(vip, registers_to_save[i]);
+		    reg_read(vip, registers_to_save[i]);
 	spin_unlock_irqrestore(&vip->slock, flags);
 	/* save pci state */
 	pci_save_state(pdev);
@@ -1453,16 +1037,9 @@ static int sta2x11_vip_suspend(struct pci_dev *pdev, pm_message_t state)
 	return 0;
 }
 
-/**
- * sta2x11_vip_resume - resume device operation
- * @pdev : PCI device
- *
+/*
  * re-enable device, set PCI state to powered and restore registers.
  * resume normal device operation afterwards.
- *
- * return value: 0, no error.
- *
- * other, could not set device to power on state.
  */
 static int sta2x11_vip_resume(struct pci_dev *pdev)
 {
@@ -1477,7 +1054,7 @@ static int sta2x11_vip_resume(struct pci_dev *pdev)
 	if (vip->disabled) {
 		ret = pci_enable_device(pdev);
 		if (ret) {
-			pr_warning("VIP: Can't enable device.\n");
+			pr_warn("VIP: Can't enable device.\n");
 			return ret;
 		}
 		vip->disabled = 0;
@@ -1488,7 +1065,7 @@ static int sta2x11_vip_resume(struct pci_dev *pdev)
 		 * do not call pci_disable_device on sta2x11 because it
 		 * break all other Bus masters on this EP
 		 */
-		pr_warning("VIP: Can't enable device.\n");
+		pr_warn("VIP: Can't enable device.\n");
 		vip->disabled = 1;
 		return ret;
 	}
@@ -1497,12 +1074,12 @@ static int sta2x11_vip_resume(struct pci_dev *pdev)
 
 	spin_lock_irqsave(&vip->slock, flags);
 	for (i = 1; i < SAVE_COUNT; i++)
-		REG_WRITE(vip, 4 * i, vip->register_save_area[i]);
+		reg_write(vip, 4 * i, vip->register_save_area[i]);
 	for (i = 0; i < AUX_COUNT; i++)
-		REG_WRITE(vip, registers_to_save[i],
+		reg_write(vip, registers_to_save[i],
 			  vip->register_save_area[SAVE_COUNT + IRQ_COUNT + i]);
-	REG_WRITE(vip, DVP_CTL, vip->register_save_area[0]);
-	REG_WRITE(vip, DVP_ITM, vip->register_save_area[SAVE_COUNT]);
+	reg_write(vip, DVP_CTL, vip->register_save_area[0]);
+	reg_write(vip, DVP_ITM, vip->register_save_area[SAVE_COUNT]);
 	spin_unlock_irqrestore(&vip->slock, flags);
 	return 0;
 }
@@ -1515,7 +1092,7 @@ static DEFINE_PCI_DEVICE_TABLE(sta2x11_vip_pci_tbl) = {
 };
 
 static struct pci_driver sta2x11_vip_driver = {
-	.name = DRV_NAME,
+	.name = KBUILD_MODNAME,
 	.probe = sta2x11_vip_init_one,
 	.remove = __devexit_p(sta2x11_vip_remove_one),
 	.id_table = sta2x11_vip_pci_tbl,
-- 
1.7.11.7


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

* Re: [PATCH v3 2/4] videobuf2-dma-streaming: new videobuf2 memory allocator
  2012-12-05 14:25         ` Mauro Carvalho Chehab
@ 2012-12-11 13:54           ` Federico Vaga
  2012-12-18 14:41             ` Marek Szyprowski
  0 siblings, 1 reply; 32+ messages in thread
From: Federico Vaga @ 2012-12-11 13:54 UTC (permalink / raw)
  To: Mauro Carvalho Chehab
  Cc: Marek Szyprowski, 'Mauro Carvalho Chehab',
	'Pawel Osciak', 'Hans Verkuil',
	'Giancarlo Asnaghi',
	linux-media, linux-kernel, 'Jonathan Corbet',
	sylwester Nawrocki

Sorry for the late answer to this.

> > This allocator is needed because some device (like STA2X11 VIP) cannot
> > work
> > with DMA sg or DMA coherent. Some other device (like the one used by
> > Jonathan when he proposes vb2-dma-nc allocator) can obtain much better
> > performance with DMA streaming than coherent.
> 
> Ok, please add such explanations at the patch's descriptions, as it is
> important not only for me, but to others that may need to use it..

OK

> >> 	2) why vb2-dma-config can't be patched to use dma_map_single
> >> 
> >> (eventually using a different vb2_io_modes bit?);
> > 
> > I did not modify vb2-dma-contig because I was thinking that each DMA
> > memory
> > allocator should reflect a DMA API.
> 
> The basic reason for having more than one VB low-level handling (vb2 was
> inspired on this concept) is that some DMA APIs are very different than
> the other ones (see vmalloc x DMA S/G for example).
> 
> I didn't make a diff between videobuf2-dma-streaming and
> videobuf2-dma-contig, so I can't tell if it makes sense to merge them or
> not, but the above argument seems too weak. I was expecting for a technical
> reason why it wouldn't make sense for merging them.

I cannot work on this now. But I think that I can do an integration like the 
one that I pushed some month ago (a8f3c203e19b702fa5e8e83a9b6fb3c5a6d1cce4).
Wind River made that changes to videobuf-contig and I tested, fixed and 
pushed.

> >> 	3) what are the usecases for it.
> >> 
> >> Could you please detail it? Without that, one that would be needing to
> >> write a driver will have serious doubts about what would be the right
> >> driver for its usage. Also, please document it at the driver itself.

I don't have a full understand of the board so I don't know exactly why 
dma_alloc_coherent does not work. I focused my development on previous work by 
Wind River. I asked to Wind River (which did all the work on this board) for 
the technical explanation about why coherent doesn't work, but they do not 
know. That's why I made the new allocator: coherent doesn't work and HW 
doesn't support SG.

> I'm not a DMA performance expert. As such, from that comment, it sounded to
> me that replacing dma-config/dma-sg by dma streaming will always give
> "performance optimizations the hardware allow".

me too, I'm not a DMA performance expert. I'm just an user of the DMA API. On 
my hardware simply it works only with that interface, it is not a performance 
problem.

> On a separate but related issue, while doing DMABUF tests with an Exynos4
> hardware, using a s5p sensor, sending data to s5p-tv, I noticed a CPU
> consumption of about 42%, which seems too high. Could it be related to
> not using the DMA streaming API?

As I wrote above, I'm not a DMA performance expert. I skip this

-- 
Federico Vaga

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

* Re: [PATCH v3 2/4] videobuf2-dma-streaming: new videobuf2 memory allocator
  2012-12-11 13:54           ` Federico Vaga
@ 2012-12-18 14:41             ` Marek Szyprowski
  2012-12-20 15:37               ` Federico Vaga
  0 siblings, 1 reply; 32+ messages in thread
From: Marek Szyprowski @ 2012-12-18 14:41 UTC (permalink / raw)
  To: Federico Vaga
  Cc: Mauro Carvalho Chehab, 'Mauro Carvalho Chehab',
	'Pawel Osciak', 'Hans Verkuil',
	'Giancarlo Asnaghi',
	linux-media, linux-kernel, 'Jonathan Corbet',
	sylwester Nawrocki

Hello,

I'm sorry for the delay, I've been terribly busy recently.

On 12/11/2012 2:54 PM, Federico Vaga wrote:

>> > This allocator is needed because some device (like STA2X11 VIP) cannot
>> > work
>> > with DMA sg or DMA coherent. Some other device (like the one used by
>> > Jonathan when he proposes vb2-dma-nc allocator) can obtain much better
>> > performance with DMA streaming than coherent.
>>
>> Ok, please add such explanations at the patch's descriptions, as it is
>> important not only for me, but to others that may need to use it..
>
> OK
>
>> >> 	2) why vb2-dma-config can't be patched to use dma_map_single
>> >>
>> >> (eventually using a different vb2_io_modes bit?);
>> >
>> > I did not modify vb2-dma-contig because I was thinking that each DMA
>> > memory allocator should reflect a DMA API.
>>
>> The basic reason for having more than one VB low-level handling (vb2 was
>> inspired on this concept) is that some DMA APIs are very different than
>> the other ones (see vmalloc x DMA S/G for example).
>>
>> I didn't make a diff between videobuf2-dma-streaming and
>> videobuf2-dma-contig, so I can't tell if it makes sense to merge them or
>> not, but the above argument seems too weak. I was expecting for a technical
>> reason why it wouldn't make sense for merging them.
>
> I cannot work on this now. But I think that I can do an integration like the
> one that I pushed some month ago (a8f3c203e19b702fa5e8e83a9b6fb3c5a6d1cce4).
> Wind River made that changes to videobuf-contig and I tested, fixed and
> pushed.
>
>> >> 	3) what are the usecases for it.
>> >>
>> >> Could you please detail it? Without that, one that would be needing to
>> >> write a driver will have serious doubts about what would be the right
>> >> driver for its usage. Also, please document it at the driver itself.
>
> I don't have a full understand of the board so I don't know exactly why
> dma_alloc_coherent does not work. I focused my development on previous work by
> Wind River. I asked to Wind River (which did all the work on this board) for
> the technical explanation about why coherent doesn't work, but they do not
> know. That's why I made the new allocator: coherent doesn't work and HW
> doesn't support SG.

Ok, now I see the whole image. I was convinced that this so called 
streaming allocator is required for performance reasons, not because of 
the broken platform support for coherent calls.

My ultimate goal is to have support for both non-cached (coherent) and 
cached (non-coherent) buffers in the dma mapping subsystem on top of the 
common API. Then both types of buffers will be easily supported by 
dma-contig vb2 allocator. Currently support for streaming-style buffers 
requires completely different dma mapping calls, although from the 
device driver point of view the buffers behaves similarly, so 
implementing them as a separate allocator seems to be the best idea.

I can take a look at the dma coherent issues with that board, but I will 
need some help as I don't have this hardware.

>> I'm not a DMA performance expert. As such, from that comment, it sounded to
>> me that replacing dma-config/dma-sg by dma streaming will always give
>> "performance optimizations the hardware allow".
>
> me too, I'm not a DMA performance expert. I'm just an user of the DMA API. On
> my hardware simply it works only with that interface, it is not a performance
> problem.
>
>> On a separate but related issue, while doing DMABUF tests with an Exynos4
>> hardware, using a s5p sensor, sending data to s5p-tv, I noticed a CPU
>> consumption of about 42%, which seems too high. Could it be related to
>> not using the DMA streaming API?

This might be related to the excessive cpu cache flushing on dma buf 
buffers as there were some misunderstanding who is responsible of that 
(I saw some strange code in drm, but it has been changed a few times). I 
will add this issue to my todo list.

Best regards
-- 
Marek Szyprowski
Samsung Poland R&D Center


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

* Re: [PATCH v3 2/4] videobuf2-dma-streaming: new videobuf2 memory allocator
  2012-12-18 14:41             ` Marek Szyprowski
@ 2012-12-20 15:37               ` Federico Vaga
  2013-01-01 12:52                 ` Mauro Carvalho Chehab
  0 siblings, 1 reply; 32+ messages in thread
From: Federico Vaga @ 2012-12-20 15:37 UTC (permalink / raw)
  To: Marek Szyprowski
  Cc: Mauro Carvalho Chehab, 'Mauro Carvalho Chehab',
	'Pawel Osciak', 'Hans Verkuil',
	'Giancarlo Asnaghi',
	linux-media, linux-kernel, 'Jonathan Corbet',
	sylwester Nawrocki

> I can take a look at the dma coherent issues with that board, but I 
will
> need some help as I don't have this hardware.

I have the hardware, but I don't have the full knowledge of the 
boards. As I told before, I asked to windriver which develop the 
software for the whole board, but they cannot help me.

-- 
Federico Vaga

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

* Re: [PATCH v3 2/4] videobuf2-dma-streaming: new videobuf2 memory allocator
  2012-12-20 15:37               ` Federico Vaga
@ 2013-01-01 12:52                 ` Mauro Carvalho Chehab
  2013-01-03 16:13                   ` Federico Vaga
  0 siblings, 1 reply; 32+ messages in thread
From: Mauro Carvalho Chehab @ 2013-01-01 12:52 UTC (permalink / raw)
  To: Federico Vaga
  Cc: Marek Szyprowski, 'Mauro Carvalho Chehab',
	'Pawel Osciak', 'Hans Verkuil',
	'Giancarlo Asnaghi',
	linux-media, linux-kernel, 'Jonathan Corbet',
	sylwester Nawrocki

Hi Federico,

Em Thu, 20 Dec 2012 16:37:50 +0100
Federico Vaga <federico.vaga@gmail.com> escreveu:

> > I can take a look at the dma coherent issues with that board, but I 
> will
> > need some help as I don't have this hardware.
> 
> I have the hardware, but I don't have the full knowledge of the 
> boards. As I told before, I asked to windriver which develop the 
> software for the whole board, but they cannot help me.
> 

After all those discussions, I'm ok on adding this new driver, but please
add a summary of those discussions at the patch description. As I said,
the reason why this driver is needed is not obvious. So, it needs to be
very well described.

Your new "v3 3/4" patch seems OK on my eyes (I can't test it, as I don't
have the hardware). Yet, there was one merge conflict on it.

Patch 1/4 of this series doesn't apply anymore (maybe it were already
applied?).

So, could you please send us a v4, rebased on the top of staging/for_v3.9
branch of the media-tree?

Thanks!
Mauro

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

* Re: [PATCH v3 2/4] videobuf2-dma-streaming: new videobuf2 memory allocator
  2013-01-01 12:52                 ` Mauro Carvalho Chehab
@ 2013-01-03 16:13                   ` Federico Vaga
  2013-01-04 13:30                     ` Federico Vaga
  0 siblings, 1 reply; 32+ messages in thread
From: Federico Vaga @ 2013-01-03 16:13 UTC (permalink / raw)
  To: Mauro Carvalho Chehab
  Cc: Marek Szyprowski, 'Mauro Carvalho Chehab',
	'Pawel Osciak', 'Hans Verkuil',
	'Giancarlo Asnaghi',
	linux-media, linux-kernel, 'Jonathan Corbet',
	sylwester Nawrocki

> After all those discussions, I'm ok on adding this new driver, but please
> add a summary of those discussions at the patch description. As I said,
> the reason why this driver is needed is not obvious. So, it needs to be
> very well described.

ack. I will ask more information to ST about the board because the 
architecture side it is not in the kernel mainline, but it should be.

> Patch 1/4 of this series doesn't apply anymore (maybe it were already
> applied?).

Probably already applied

> So, could you please send us a v4, rebased on the top of staging/for_v3.9
> branch of the media-tree?

I will do it

-- 
Federico Vaga

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

* Re: [PATCH v3 2/4] videobuf2-dma-streaming: new videobuf2 memory allocator
  2013-01-03 16:13                   ` Federico Vaga
@ 2013-01-04 13:30                     ` Federico Vaga
  2013-01-06 17:04                       ` Federico Vaga
  2013-01-06 23:09                       ` Alessandro Rubini
  0 siblings, 2 replies; 32+ messages in thread
From: Federico Vaga @ 2013-01-04 13:30 UTC (permalink / raw)
  To: Mauro Carvalho Chehab
  Cc: Marek Szyprowski, 'Mauro Carvalho Chehab',
	'Pawel Osciak', 'Hans Verkuil',
	'Giancarlo Asnaghi',
	linux-media, linux-kernel, 'Jonathan Corbet',
	sylwester Nawrocki

On Thursday 03 January 2013 17:13:14 Federico Vaga wrote:
> > After all those discussions, I'm ok on adding this new driver, but please
> > add a summary of those discussions at the patch description. As I said,
> > the reason why this driver is needed is not obvious. So, it needs to be
> > very well described.
> 
> ack. I will ask more information to ST about the board because the
> architecture side it is not in the kernel mainline, but it should be.

I have more information about DMA on the board that I'm using; probably, I can 
make dma-contig work with my device. Unfortunately, I cannot test at the 
moment; I hope to do a test on Monday.


-- 
Federico Vaga

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

* Re: [PATCH v3 2/4] videobuf2-dma-streaming: new videobuf2 memory allocator
  2013-01-04 13:30                     ` Federico Vaga
@ 2013-01-06 17:04                       ` Federico Vaga
  2013-01-06 23:09                       ` Alessandro Rubini
  1 sibling, 0 replies; 32+ messages in thread
From: Federico Vaga @ 2013-01-06 17:04 UTC (permalink / raw)
  To: Mauro Carvalho Chehab
  Cc: Marek Szyprowski, 'Mauro Carvalho Chehab',
	'Pawel Osciak', 'Hans Verkuil',
	'Giancarlo Asnaghi',
	linux-media, linux-kernel, 'Jonathan Corbet',
	sylwester Nawrocki, Alessandro Rubini

> I have more information about DMA on the board that I'm using; probably, I
> can make dma-contig work with my device.

Ok, the driver STA2X11 now works with a patched dma-contig allocator. So, my 
streaming allocator it is not mandatory. 

I based my work on the previous work made by Windriver, but now I understand 
the DMA problem and the solution easy.
I investigated (asked to Alessandro Rubini who worked on this board) about 
this DMA issue. The problem is that on the sta2x11 architecture only the first 
512MB are available through the PCI bus, but the allocator can allocate memory 
for DMA above this limit. By using GFP_DMA flags the allocation take place 
under the 16MB so it works.

If you think that the streaming allocator can be useful for someone else (who 
has performance problem with uncached DMA like Jonathan when he did dma-nc 
allocator), I can resend the patch.
I cannot do performance test at the moment because I don't have the time, so I 
cannot personally justify the presence of a new allocator. I think that I will 
do some performance test with this driver; if I will find that dma-streaming 
works better I will propose it again.

I will propose V4 patches soon.

-- 
Federico Vaga

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

* Re: [PATCH v3 2/4] videobuf2-dma-streaming: new videobuf2 memory allocator
  2013-01-04 13:30                     ` Federico Vaga
  2013-01-06 17:04                       ` Federico Vaga
@ 2013-01-06 23:09                       ` Alessandro Rubini
  2013-01-07 19:40                         ` Jonathan Corbet
  1 sibling, 1 reply; 32+ messages in thread
From: Alessandro Rubini @ 2013-01-06 23:09 UTC (permalink / raw)
  To: federico.vaga
  Cc: mchehab, m.szyprowski, mchehab, pawel, hans.verkuil,
	giancarlo.asnaghi, linux-media, linux-kernel, corbet, s.nawrocki

> The problem is that on the sta2x11 architecture only the first 
> 512MB are available through the PCI bus, but the allocator can allocate memory 
> for DMA above this limit. By using GFP_DMA flags the allocation take place 
> under the 16MB so it works.

Still, you are not running the upstream allocator.  IIUC, you added a
"gfp_t" field in the platform data or somewhere, so the sta2x11 can
request GFP_DMA to be OR'd, while other users remain unaffected.  Will
you please submit the patch to achieve that?

> I cannot do performance test at the moment because I don't have the time, so I 
> cannot personally justify the presence of a new allocator.

I don't expect you'll see serious performance differences on the PC. I
think ARM users will have better benefits, due to the different cache
architecture.  You told me Jon measured meaningful figures on a Marvel
CPU.

> I will propose V4 patches soon.

thanks
/alessandro

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

* Re: [PATCH v3 2/4] videobuf2-dma-streaming: new videobuf2 memory allocator
  2013-01-06 23:09                       ` Alessandro Rubini
@ 2013-01-07 19:40                         ` Jonathan Corbet
  2013-01-07 20:15                           ` Mauro Carvalho Chehab
  0 siblings, 1 reply; 32+ messages in thread
From: Jonathan Corbet @ 2013-01-07 19:40 UTC (permalink / raw)
  To: Alessandro Rubini
  Cc: federico.vaga, mchehab, m.szyprowski, mchehab, pawel,
	hans.verkuil, giancarlo.asnaghi, linux-media, linux-kernel,
	s.nawrocki

On Mon, 7 Jan 2013 00:09:47 +0100
Alessandro Rubini <rubini@gnudd.com> wrote:

> I don't expect you'll see serious performance differences on the PC. I
> think ARM users will have better benefits, due to the different cache
> architecture.  You told me Jon measured meaningful figures on a Marvel
> CPU.

It made the difference between 10 frames per second with the CPU running
flat out and 30fps mostly idle.  I think that probably counts as
meaningful, yeah...:)

jon

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

* Re: [PATCH v3 2/4] videobuf2-dma-streaming: new videobuf2 memory allocator
  2013-01-07 19:40                         ` Jonathan Corbet
@ 2013-01-07 20:15                           ` Mauro Carvalho Chehab
  2013-01-08  6:50                             ` Marek Szyprowski
  0 siblings, 1 reply; 32+ messages in thread
From: Mauro Carvalho Chehab @ 2013-01-07 20:15 UTC (permalink / raw)
  To: Jonathan Corbet
  Cc: Alessandro Rubini, federico.vaga, m.szyprowski, mchehab, pawel,
	hans.verkuil, giancarlo.asnaghi, linux-media, linux-kernel,
	s.nawrocki

Em Mon, 7 Jan 2013 12:40:50 -0700
Jonathan Corbet <corbet@lwn.net> escreveu:

> On Mon, 7 Jan 2013 00:09:47 +0100
> Alessandro Rubini <rubini@gnudd.com> wrote:
> 
> > I don't expect you'll see serious performance differences on the PC. I
> > think ARM users will have better benefits, due to the different cache
> > architecture.  You told me Jon measured meaningful figures on a Marvel
> > CPU.
> 
> It made the difference between 10 frames per second with the CPU running
> flat out and 30fps mostly idle.  I think that probably counts as
> meaningful, yeah...:)

Couldn't this performance difference be due to the usage of GFP_DMA inside
the VB2 code, like Federico's new patch series is proposing?

If not, why are there a so large performance penalty?

Regards,
Mauro

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

* Re: [PATCH v3 2/4] videobuf2-dma-streaming: new videobuf2 memory allocator
  2013-01-07 20:15                           ` Mauro Carvalho Chehab
@ 2013-01-08  6:50                             ` Marek Szyprowski
  2013-01-08 14:31                               ` Jonathan Corbet
  0 siblings, 1 reply; 32+ messages in thread
From: Marek Szyprowski @ 2013-01-08  6:50 UTC (permalink / raw)
  To: Mauro Carvalho Chehab
  Cc: Jonathan Corbet, Alessandro Rubini, federico.vaga, mchehab,
	pawel, hans.verkuil, giancarlo.asnaghi, linux-media,
	linux-kernel, s.nawrocki


On 1/7/2013 9:15 PM, Mauro Carvalho Chehab wrote:
> Em Mon, 7 Jan 2013 12:40:50 -0700
> Jonathan Corbet <corbet@lwn.net> escreveu:
>
> > On Mon, 7 Jan 2013 00:09:47 +0100
> > Alessandro Rubini <rubini@gnudd.com> wrote:
> >
> > > I don't expect you'll see serious performance differences on the PC. I
> > > think ARM users will have better benefits, due to the different cache
> > > architecture.  You told me Jon measured meaningful figures on a Marvel
> > > CPU.
> >
> > It made the difference between 10 frames per second with the CPU running
> > flat out and 30fps mostly idle.  I think that probably counts as
> > meaningful, yeah...:)
>
> Couldn't this performance difference be due to the usage of GFP_DMA inside
> the VB2 code, like Federico's new patch series is proposing?
>
> If not, why are there a so large performance penalty?

Nope, this was caused rather by a very poor CPU access to non-cached (aka
'coherent') memory and the way the video data has been accessed/read 
with CPU.

Best regards
-- 
Marek Szyprowski
Samsung Poland R&D Center



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

* Re: [PATCH v3 2/4] videobuf2-dma-streaming: new videobuf2 memory allocator
  2013-01-08  6:50                             ` Marek Szyprowski
@ 2013-01-08 14:31                               ` Jonathan Corbet
  2013-01-09  7:48                                 ` Michael Olbrich
  0 siblings, 1 reply; 32+ messages in thread
From: Jonathan Corbet @ 2013-01-08 14:31 UTC (permalink / raw)
  To: Marek Szyprowski
  Cc: Mauro Carvalho Chehab, Alessandro Rubini, federico.vaga, mchehab,
	pawel, hans.verkuil, giancarlo.asnaghi, linux-media,
	linux-kernel, s.nawrocki

On Tue, 08 Jan 2013 07:50:41 +0100
Marek Szyprowski <m.szyprowski@samsung.com> wrote:

> > Couldn't this performance difference be due to the usage of GFP_DMA inside
> > the VB2 code, like Federico's new patch series is proposing?
> >
> > If not, why are there a so large performance penalty?  
> 
> Nope, this was caused rather by a very poor CPU access to non-cached (aka
> 'coherent') memory and the way the video data has been accessed/read 
> with CPU.

Exactly.  Uncached memory *hurts*, especially if you're having to touch it
all with the CPU.

jon

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

* Re: [PATCH v3 2/4] videobuf2-dma-streaming: new videobuf2 memory allocator
  2013-01-08 14:31                               ` Jonathan Corbet
@ 2013-01-09  7:48                                 ` Michael Olbrich
  0 siblings, 0 replies; 32+ messages in thread
From: Michael Olbrich @ 2013-01-09  7:48 UTC (permalink / raw)
  To: Jonathan Corbet
  Cc: Marek Szyprowski, Mauro Carvalho Chehab, Alessandro Rubini,
	federico.vaga, mchehab, pawel, hans.verkuil, giancarlo.asnaghi,
	linux-media, linux-kernel, s.nawrocki

On Tue, Jan 08, 2013 at 07:31:30AM -0700, Jonathan Corbet wrote:
> On Tue, 08 Jan 2013 07:50:41 +0100
> Marek Szyprowski <m.szyprowski@samsung.com> wrote:
> 
> > > Couldn't this performance difference be due to the usage of GFP_DMA inside
> > > the VB2 code, like Federico's new patch series is proposing?
> > >
> > > If not, why are there a so large performance penalty?  
> > 
> > Nope, this was caused rather by a very poor CPU access to non-cached (aka
> > 'coherent') memory and the way the video data has been accessed/read 
> > with CPU.
> 
> Exactly.  Uncached memory *hurts*, especially if you're having to touch it
> all with the CPU.

Even worse, on ARMv7 (at least) the cache implements or is necessary for
(I'm not an expert here) unaligned access. I've seen applications crash
on non-cached memory with a bus error because gcc assumes unaligned access
works. And there isn't even a exception handler in the kernel, probably for
the same reason.

Michael

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

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

end of thread, other threads:[~2013-01-09  7:49 UTC | newest]

Thread overview: 32+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-09-24 10:58 [PATCH v3 1/4] v4l: vb2: add prepare/finish callbacks to allocators Federico Vaga
2012-09-24 10:58 ` [PATCH v3 2/4] videobuf2-dma-streaming: new videobuf2 memory allocator Federico Vaga
2012-09-24 12:44   ` Marek Szyprowski
2012-12-04 16:04     ` Mauro Carvalho Chehab
2012-12-05 12:50       ` Federico Vaga
2012-12-05 14:25         ` Mauro Carvalho Chehab
2012-12-11 13:54           ` Federico Vaga
2012-12-18 14:41             ` Marek Szyprowski
2012-12-20 15:37               ` Federico Vaga
2013-01-01 12:52                 ` Mauro Carvalho Chehab
2013-01-03 16:13                   ` Federico Vaga
2013-01-04 13:30                     ` Federico Vaga
2013-01-06 17:04                       ` Federico Vaga
2013-01-06 23:09                       ` Alessandro Rubini
2013-01-07 19:40                         ` Jonathan Corbet
2013-01-07 20:15                           ` Mauro Carvalho Chehab
2013-01-08  6:50                             ` Marek Szyprowski
2013-01-08 14:31                               ` Jonathan Corbet
2013-01-09  7:48                                 ` Michael Olbrich
2012-09-24 10:58 ` [PATCH v3 3/4] sta2x11_vip: convert to videobuf2 and control framework Federico Vaga
2012-12-04 16:15   ` Mauro Carvalho Chehab
2012-12-05  1:12     ` Federico Vaga
2012-12-05 11:34       ` Mauro Carvalho Chehab
2012-12-05 12:24         ` Federico Vaga
2012-12-05 13:10           ` Mauro Carvalho Chehab
2012-12-05 13:27             ` Federico Vaga
2012-12-05 13:37               ` Mauro Carvalho Chehab
2012-12-05 13:45                 ` Federico Vaga
2012-12-06 18:59             ` Federico Vaga
2012-09-24 10:58 ` [PATCH v3 4/4] adv7180: remove {query/g_/s_}ctrl Federico Vaga
2012-09-24 12:46 ` [PATCH v3 1/4] v4l: vb2: add prepare/finish callbacks to allocators Marek Szyprowski
2012-09-25 15:04 ` Federico Vaga

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).