[PATCH v3 0/3] au0828 vb2 conversion

[PATCH v3 0/3] au0828 vb2 conversion

[Shuah Khan]

This patch series includes patch v3 of the au0828 vb2 conversion,
removing video and vbi buffer timeout handling, and a patch to
fix compile error from au0828_boards initialization which uses a
a define from videobuf-core.h which is no longer included with the
vb2 conversion change.

The following work is in progress and will be done as separate
patches:
- removing users and using v4l2_fh_is_singular_file() instead.
- Changing dynamic allocation of video device structs to static
  which will reduce the overhead to allocate at register time and
  deallocating at unregister.
- New v4l2-compliance tool showed warnings on try_fmt and s_fmt
  because the driver doesn't return error in all cases.

Changes from patch v2:
- Dropped already acked from the v3 series
  [PATCH v2 2/3] media: au0828 change to not zero out fmt.pix.priv
- Split compile error fix and vb2 conversion patches
- Made changes to vb2 conversion patch to address comments on v2
  patch.
- Updated commit log for remove video and vbi buffer timeout
  work-around patch
- Rebased and tested patches on 3.19-rc4

Shuah Khan (3):
  media: fix au0828 compile error from au0828_boards initialization
  media: au0828 - convert to use videobuf2
  media: au0828 remove video and vbi buffer timeout work-around

 drivers/media/usb/au0828/Kconfig        |    2 +-
 drivers/media/usb/au082e/au0828-cards.c |    2 +-
 drivers/media/usb/au0828/au0828-vbi.c   |  122 ++--
 drivers/media/usb/au0828/au0828-video.c | 1005 +++++++++++--------------------
 drivers/media/usb/au0828/au0828.h       |   64 +-
 5 files changed, 414 insertions(+), 781 deletions(-)

-- 
2.1.0

[PATCH v3 1/3] media: fix au0828 compile error from au0828_boards initialization

[Shuah Khan]

au0828 picked up UNSET from videobuf-core.h and fails to compile
if videobuf-core.h isn't included. Change it to use -1U instead
to fix the problem.

    drivers/media/usb/au0828/au0828-cards.c:47:17: error: ‘UNSET’ undeclared here (not in a function)
       .tuner_type = UNSET,

Signed-off-by: Shuah Khan <shuahkh@osg.samsung.com>
---
 drivers/media/usb/au0828/au0828-cards.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/usb/au0828/au0828-cards.c b/drivers/media/usb/au0828/au0828-cards.c
index da87f1c..edc2735 100644
--- a/drivers/media/usb/au0828/au0828-cards.c
+++ b/drivers/media/usb/au0828/au0828-cards.c
@@ -44,7 +44,7 @@ static void hvr950q_cs5340_audio(void *priv, int enable)
 struct au0828_board au0828_boards[] = {
 	[AU0828_BOARD_UNKNOWN] = {
 		.name	= "Unknown board",
-		.tuner_type = UNSET,
+		.tuner_type = -1U,
 		.tuner_addr = ADDR_UNSET,
 	},
 	[AU0828_BOARD_HAUPPAUGE_HVR850] = {
-- 
2.1.0

Re: [PATCH v3 1/3] media: fix au0828 compile error from au0828_boards initialization

[Lad, Prabhakar]

Hi Shuah,

Thanks for the patch.

On Tue, Jan 13, 2015 at 2:56 AM, Shuah Khan <shuahkh@osg.samsung.com> wrote:
> au0828 picked up UNSET from videobuf-core.h and fails to compile
> if videobuf-core.h isn't included. Change it to use -1U instead
> to fix the problem.
>
>     drivers/media/usb/au0828/au0828-cards.c:47:17: error: ‘UNSET’ undeclared here (not in a function)
>        .tuner_type = UNSET,
>
> Signed-off-by: Shuah Khan <shuahkh@osg.samsung.com>

Acked-by: Lad, Prabhakar <prabhakar.csengg@gmail.com>

Regards,
--Prabhakar Lad

> ---
>  drivers/media/usb/au0828/au0828-cards.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/media/usb/au0828/au0828-cards.c b/drivers/media/usb/au0828/au0828-cards.c
> index da87f1c..edc2735 100644
> --- a/drivers/media/usb/au0828/au0828-cards.c
> +++ b/drivers/media/usb/au0828/au0828-cards.c
> @@ -44,7 +44,7 @@ static void hvr950q_cs5340_audio(void *priv, int enable)
>  struct au0828_board au0828_boards[] = {
>         [AU0828_BOARD_UNKNOWN] = {
>                 .name   = "Unknown board",
> -               .tuner_type = UNSET,
> +               .tuner_type = -1U,
>                 .tuner_addr = ADDR_UNSET,
>         },
>         [AU0828_BOARD_HAUPPAUGE_HVR850] = {
> --
> 2.1.0
>

[PATCH v3 2/3] media: au0828 - convert to use videobuf2

[Shuah Khan]

Convert au0828 to use videobuf2. Tested with NTSC.
Tested video and vbi devices with xawtv, tvtime,
and vlc. Ran v4l2-compliance to ensure there are
no regressions. video now has no failures and vbi
has 3 fewer failures.

video before:
test VIDIOC_G_FMT: FAIL 3 failures
Total: 72, Succeeded: 69, Failed: 3, Warnings: 0

Video after:
Total: 72, Succeeded: 72, Failed: 0, Warnings: 18

vbi before:
    test VIDIOC_REQBUFS/CREATE_BUFS/QUERYBUF: FAIL
    test VIDIOC_EXPBUF: FAIL
    test USERPTR: FAIL
    Total: 72, Succeeded: 66, Failed: 6, Warnings: 0

vbi after:
    test VIDIOC_QUERYCAP: FAIL
    test MMAP: FAIL
    Total: 78, Succeeded: 75, Failed: 3, Warnings: 0

Signed-off-by: Shuah Khan <shuahkh@osg.samsung.com>
---
 drivers/media/usb/au0828/Kconfig        |   2 +-
 drivers/media/usb/au0828/au0828-vbi.c   | 122 ++--
 drivers/media/usb/au0828/au0828-video.c | 962 ++++++++++++--------------------
 drivers/media/usb/au0828/au0828.h       |  61 +-
 4 files changed, 443 insertions(+), 704 deletions(-)

diff --git a/drivers/media/usb/au0828/Kconfig b/drivers/media/usb/au0828/Kconfig
index 1d410ac..78b797e 100644
--- a/drivers/media/usb/au0828/Kconfig
+++ b/drivers/media/usb/au0828/Kconfig
@@ -4,7 +4,7 @@ config VIDEO_AU0828
 	depends on I2C && INPUT && DVB_CORE && USB
 	select I2C_ALGOBIT
 	select VIDEO_TVEEPROM
-	select VIDEOBUF_VMALLOC
+	select VIDEOBUF2_VMALLOC
 	select DVB_AU8522_DTV if MEDIA_SUBDRV_AUTOSELECT
 	select MEDIA_TUNER_XC5000 if MEDIA_SUBDRV_AUTOSELECT
 	select MEDIA_TUNER_MXL5007T if MEDIA_SUBDRV_AUTOSELECT
diff --git a/drivers/media/usb/au0828/au0828-vbi.c b/drivers/media/usb/au0828/au0828-vbi.c
index 932d24f..f67247c 100644
--- a/drivers/media/usb/au0828/au0828-vbi.c
+++ b/drivers/media/usb/au0828/au0828-vbi.c
@@ -28,111 +28,67 @@
 #include <linux/init.h>
 #include <linux/slab.h>
 
-static unsigned int vbibufs = 5;
-module_param(vbibufs, int, 0644);
-MODULE_PARM_DESC(vbibufs, "number of vbi buffers, range 2-32");
-
 /* ------------------------------------------------------------------ */
 
-static void
-free_buffer(struct videobuf_queue *vq, struct au0828_buffer *buf)
+static int vbi_queue_setup(struct vb2_queue *vq, const struct v4l2_format *fmt,
+			   unsigned int *nbuffers, unsigned int *nplanes,
+			   unsigned int sizes[], void *alloc_ctxs[])
 {
-	struct au0828_fh     *fh  = vq->priv_data;
-	struct au0828_dev    *dev = fh->dev;
-	unsigned long flags = 0;
-	if (in_interrupt())
-		BUG();
-
-	/* We used to wait for the buffer to finish here, but this didn't work
-	   because, as we were keeping the state as VIDEOBUF_QUEUED,
-	   videobuf_queue_cancel marked it as finished for us.
-	   (Also, it could wedge forever if the hardware was misconfigured.)
-
-	   This should be safe; by the time we get here, the buffer isn't
-	   queued anymore. If we ever start marking the buffers as
-	   VIDEOBUF_ACTIVE, it won't be, though.
-	*/
-	spin_lock_irqsave(&dev->slock, flags);
-	if (dev->isoc_ctl.vbi_buf == buf)
-		dev->isoc_ctl.vbi_buf = NULL;
-	spin_unlock_irqrestore(&dev->slock, flags);
+	struct au0828_dev *dev = vb2_get_drv_priv(vq);
+	unsigned long img_size = dev->vbi_width * dev->vbi_height * 2;
+	unsigned long size;
 
-	videobuf_vmalloc_free(&buf->vb);
-	buf->vb.state = VIDEOBUF_NEEDS_INIT;
-}
-
-static int
-vbi_setup(struct videobuf_queue *q, unsigned int *count, unsigned int *size)
-{
-	struct au0828_fh     *fh  = q->priv_data;
-	struct au0828_dev    *dev = fh->dev;
+	size = fmt ? (fmt->fmt.vbi.samples_per_line *
+		(fmt->fmt.vbi.count[0] + fmt->fmt.vbi.count[1])) : img_size;
+	if (size < img_size)
+		return -EINVAL;
 
-	*size = dev->vbi_width * dev->vbi_height * 2;
+	*nplanes = 1;
+	sizes[0] = size;
 
-	if (0 == *count)
-		*count = vbibufs;
-	if (*count < 2)
-		*count = 2;
-	if (*count > 32)
-		*count = 32;
 	return 0;
 }
 
-static int
-vbi_prepare(struct videobuf_queue *q, struct videobuf_buffer *vb,
-	    enum v4l2_field field)
+static int vbi_buffer_prepare(struct vb2_buffer *vb)
 {
-	struct au0828_fh     *fh  = q->priv_data;
-	struct au0828_dev    *dev = fh->dev;
+	struct au0828_dev *dev = vb2_get_drv_priv(vb->vb2_queue);
 	struct au0828_buffer *buf = container_of(vb, struct au0828_buffer, vb);
-	int                  rc = 0;
+	unsigned long size;
 
-	buf->vb.size = dev->vbi_width * dev->vbi_height * 2;
+	size = dev->vbi_width * dev->vbi_height * 2;
 
-	if (0 != buf->vb.baddr  &&  buf->vb.bsize < buf->vb.size)
+	if (vb2_plane_size(vb, 0) < size) {
+		pr_err("%s data will not fit into plane (%lu < %lu)\n",
+			__func__, vb2_plane_size(vb, 0), size);
 		return -EINVAL;
-
-	buf->vb.width  = dev->vbi_width;
-	buf->vb.height = dev->vbi_height;
-	buf->vb.field  = field;
-
-	if (VIDEOBUF_NEEDS_INIT == buf->vb.state) {
-		rc = videobuf_iolock(q, &buf->vb, NULL);
-		if (rc < 0)
-			goto fail;
 	}
+	vb2_set_plane_payload(&buf->vb, 0, size);
 
-	buf->vb.state = VIDEOBUF_PREPARED;
 	return 0;
-
-fail:
-	free_buffer(q, buf);
-	return rc;
 }
 
 static void
-vbi_queue(struct videobuf_queue *vq, struct videobuf_buffer *vb)
-{
-	struct au0828_buffer    *buf     = container_of(vb,
-							struct au0828_buffer,
-							vb);
-	struct au0828_fh        *fh      = vq->priv_data;
-	struct au0828_dev       *dev     = fh->dev;
-	struct au0828_dmaqueue  *vbiq    = &dev->vbiq;
-
-	buf->vb.state = VIDEOBUF_QUEUED;
-	list_add_tail(&buf->vb.queue, &vbiq->active);
-}
-
-static void vbi_release(struct videobuf_queue *q, struct videobuf_buffer *vb)
+vbi_buffer_queue(struct vb2_buffer *vb)
 {
+	struct au0828_dev *dev = vb2_get_drv_priv(vb->vb2_queue);
 	struct au0828_buffer *buf = container_of(vb, struct au0828_buffer, vb);
-	free_buffer(q, buf);
+	struct au0828_dmaqueue *vbiq = &dev->vbiq;
+	unsigned long flags = 0;
+
+	buf->mem = vb2_plane_vaddr(vb, 0);
+	buf->length = vb2_plane_size(vb, 0);
+
+	spin_lock_irqsave(&dev->slock, flags);
+	list_add_tail(&buf->list, &vbiq->active);
+	spin_unlock_irqrestore(&dev->slock, flags);
 }
 
-struct videobuf_queue_ops au0828_vbi_qops = {
-	.buf_setup    = vbi_setup,
-	.buf_prepare  = vbi_prepare,
-	.buf_queue    = vbi_queue,
-	.buf_release  = vbi_release,
+struct vb2_ops au0828_vbi_qops = {
+	.queue_setup     = vbi_queue_setup,
+	.buf_prepare     = vbi_buffer_prepare,
+	.buf_queue       = vbi_buffer_queue,
+	.start_streaming = au0828_start_analog_streaming,
+	.stop_streaming  = au0828_stop_vbi_streaming,
+	.wait_prepare    = vb2_ops_wait_prepare,
+	.wait_finish     = vb2_ops_wait_finish,
 };
diff --git a/drivers/media/usb/au0828/au0828-video.c b/drivers/media/usb/au0828/au0828-video.c
index 8a7a547..e427913 100644
--- a/drivers/media/usb/au0828/au0828-video.c
+++ b/drivers/media/usb/au0828/au0828-video.c
@@ -218,9 +218,6 @@ static int au0828_init_isoc(struct au0828_dev *dev, int max_packets,
 
 	au0828_isocdbg("au0828: called au0828_prepare_isoc\n");
 
-	/* De-allocates all pending stuff */
-	au0828_uninit_isoc(dev);
-
 	dev->isoc_ctl.isoc_copy = isoc_copy;
 	dev->isoc_ctl.num_bufs = num_bufs;
 
@@ -284,8 +281,6 @@ static int au0828_init_isoc(struct au0828_dev *dev, int max_packets,
 		}
 	}
 
-	init_waitqueue_head(&dma_q->wq);
-
 	/* submit urbs and enables IRQ */
 	for (i = 0; i < dev->isoc_ctl.num_bufs; i++) {
 		rc = usb_submit_urb(dev->isoc_ctl.urb[i], GFP_ATOMIC);
@@ -308,16 +303,12 @@ static inline void buffer_filled(struct au0828_dev *dev,
 				  struct au0828_buffer *buf)
 {
 	/* Advice that buffer was filled */
-	au0828_isocdbg("[%p/%d] wakeup\n", buf, buf->vb.i);
-
-	buf->vb.state = VIDEOBUF_DONE;
-	buf->vb.field_count++;
-	v4l2_get_timestamp(&buf->vb.ts);
+	au0828_isocdbg("[%p/%d] wakeup\n", buf, buf->top_field);
 
-	dev->isoc_ctl.buf = NULL;
-
-	list_del(&buf->vb.queue);
-	wake_up(&buf->vb.done);
+	buf->vb.v4l2_buf.sequence = dev->frame_count++;
+	buf->vb.v4l2_buf.field = V4L2_FIELD_INTERLACED;
+	v4l2_get_timestamp(&buf->vb.v4l2_buf.timestamp);
+	vb2_buffer_done(&buf->vb, VB2_BUF_STATE_DONE);
 }
 
 static inline void vbi_buffer_filled(struct au0828_dev *dev,
@@ -325,16 +316,12 @@ static inline void vbi_buffer_filled(struct au0828_dev *dev,
 				     struct au0828_buffer *buf)
 {
 	/* Advice that buffer was filled */
-	au0828_isocdbg("[%p/%d] wakeup\n", buf, buf->vb.i);
-
-	buf->vb.state = VIDEOBUF_DONE;
-	buf->vb.field_count++;
-	v4l2_get_timestamp(&buf->vb.ts);
+	au0828_isocdbg("[%p/%d] wakeup\n", buf, buf->top_field);
 
-	dev->isoc_ctl.vbi_buf = NULL;
-
-	list_del(&buf->vb.queue);
-	wake_up(&buf->vb.done);
+	buf->vb.v4l2_buf.sequence = dev->vbi_frame_count++;
+	buf->vb.v4l2_buf.field = V4L2_FIELD_INTERLACED;
+	v4l2_get_timestamp(&buf->vb.v4l2_buf.timestamp);
+	vb2_buffer_done(&buf->vb, VB2_BUF_STATE_DONE);
 }
 
 /*
@@ -353,8 +340,8 @@ static void au0828_copy_video(struct au0828_dev *dev,
 	if (len == 0)
 		return;
 
-	if (dma_q->pos + len > buf->vb.size)
-		len = buf->vb.size - dma_q->pos;
+	if (dma_q->pos + len > buf->length)
+		len = buf->length - dma_q->pos;
 
 	startread = p;
 	remain = len;
@@ -372,11 +359,11 @@ static void au0828_copy_video(struct au0828_dev *dev,
 	lencopy = bytesperline - currlinedone;
 	lencopy = lencopy > remain ? remain : lencopy;
 
-	if ((char *)startwrite + lencopy > (char *)outp + buf->vb.size) {
+	if ((char *)startwrite + lencopy > (char *)outp + buf->length) {
 		au0828_isocdbg("Overflow of %zi bytes past buffer end (1)\n",
 			       ((char *)startwrite + lencopy) -
-			       ((char *)outp + buf->vb.size));
-		remain = (char *)outp + buf->vb.size - (char *)startwrite;
+			       ((char *)outp + buf->length));
+		remain = (char *)outp + buf->length - (char *)startwrite;
 		lencopy = remain;
 	}
 	if (lencopy <= 0)
@@ -394,11 +381,11 @@ static void au0828_copy_video(struct au0828_dev *dev,
 			lencopy = bytesperline;
 
 		if ((char *)startwrite + lencopy > (char *)outp +
-		    buf->vb.size) {
+		    buf->length) {
 			au0828_isocdbg("Overflow %zi bytes past buf end (2)\n",
 				       ((char *)startwrite + lencopy) -
-				       ((char *)outp + buf->vb.size));
-			lencopy = remain = (char *)outp + buf->vb.size -
+				       ((char *)outp + buf->length));
+			lencopy = remain = (char *)outp + buf->length -
 					   (char *)startwrite;
 		}
 		if (lencopy <= 0)
@@ -434,7 +421,11 @@ static inline void get_next_buf(struct au0828_dmaqueue *dma_q,
 	}
 
 	/* Get the next buffer */
-	*buf = list_entry(dma_q->active.next, struct au0828_buffer, vb.queue);
+	*buf = list_entry(dma_q->active.next, struct au0828_buffer, list);
+	/* Cleans up buffer - Useful for testing for frame/URB loss */
+	list_del(&(*buf)->list);
+	dma_q->pos = 0;
+	(*buf)->vb_buf = (*buf)->mem;
 	dev->isoc_ctl.buf = *buf;
 
 	return;
@@ -472,8 +463,8 @@ static void au0828_copy_vbi(struct au0828_dev *dev,
 
 	bytesperline = dev->vbi_width;
 
-	if (dma_q->pos + len > buf->vb.size)
-		len = buf->vb.size - dma_q->pos;
+	if (dma_q->pos + len > buf->length)
+		len = buf->length - dma_q->pos;
 
 	startread = p;
 	startwrite = outp + (dma_q->pos / 2);
@@ -496,7 +487,6 @@ static inline void vbi_get_next_buf(struct au0828_dmaqueue *dma_q,
 				    struct au0828_buffer **buf)
 {
 	struct au0828_dev *dev = container_of(dma_q, struct au0828_dev, vbiq);
-	char *outp;
 
 	if (list_empty(&dma_q->active)) {
 		au0828_isocdbg("No active queue to serve\n");
@@ -506,13 +496,12 @@ static inline void vbi_get_next_buf(struct au0828_dmaqueue *dma_q,
 	}
 
 	/* Get the next buffer */
-	*buf = list_entry(dma_q->active.next, struct au0828_buffer, vb.queue);
+	*buf = list_entry(dma_q->active.next, struct au0828_buffer, list);
 	/* Cleans up buffer - Useful for testing for frame/URB loss */
-	outp = videobuf_to_vmalloc(&(*buf)->vb);
-	memset(outp, 0x00, (*buf)->vb.size);
-
+	list_del(&(*buf)->list);
+	dma_q->pos = 0;
+	(*buf)->vb_buf = (*buf)->mem;
 	dev->isoc_ctl.vbi_buf = *buf;
-
 	return;
 }
 
@@ -548,11 +537,11 @@ static inline int au0828_isoc_copy(struct au0828_dev *dev, struct urb *urb)
 
 	buf = dev->isoc_ctl.buf;
 	if (buf != NULL)
-		outp = videobuf_to_vmalloc(&buf->vb);
+		outp = vb2_plane_vaddr(&buf->vb, 0);
 
 	vbi_buf = dev->isoc_ctl.vbi_buf;
 	if (vbi_buf != NULL)
-		vbioutp = videobuf_to_vmalloc(&vbi_buf->vb);
+		vbioutp = vb2_plane_vaddr(&vbi_buf->vb, 0);
 
 	for (i = 0; i < urb->number_of_packets; i++) {
 		int status = urb->iso_frame_desc[i].status;
@@ -592,8 +581,8 @@ static inline int au0828_isoc_copy(struct au0828_dev *dev, struct urb *urb)
 				if (vbi_buf == NULL)
 					vbioutp = NULL;
 				else
-					vbioutp = videobuf_to_vmalloc(
-						&vbi_buf->vb);
+					vbioutp = vb2_plane_vaddr(
+						&vbi_buf->vb, 0);
 
 				/* Video */
 				if (buf != NULL)
@@ -602,7 +591,7 @@ static inline int au0828_isoc_copy(struct au0828_dev *dev, struct urb *urb)
 				if (buf == NULL)
 					outp = NULL;
 				else
-					outp = videobuf_to_vmalloc(&buf->vb);
+					outp = vb2_plane_vaddr(&buf->vb, 0);
 
 				/* As long as isoc traffic is arriving, keep
 				   resetting the timer */
@@ -656,130 +645,59 @@ static inline int au0828_isoc_copy(struct au0828_dev *dev, struct urb *urb)
 	return rc;
 }
 
-static int
-buffer_setup(struct videobuf_queue *vq, unsigned int *count,
-	     unsigned int *size)
+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 au0828_fh *fh = vq->priv_data;
-	*size = (fh->dev->width * fh->dev->height * 16 + 7) >> 3;
-
-	if (0 == *count)
-		*count = AU0828_DEF_BUF;
+	struct au0828_dev *dev = vb2_get_drv_priv(vq);
+	unsigned long img_size = dev->height * dev->bytesperline;
+	unsigned long size;
 
-	if (*count < AU0828_MIN_BUF)
-		*count = AU0828_MIN_BUF;
-	return 0;
-}
+	size = fmt ? fmt->fmt.pix.sizeimage : img_size;
+	if (size < img_size)
+		return -EINVAL;
 
-/* This is called *without* dev->slock held; please keep it that way */
-static void free_buffer(struct videobuf_queue *vq, struct au0828_buffer *buf)
-{
-	struct au0828_fh     *fh  = vq->priv_data;
-	struct au0828_dev    *dev = fh->dev;
-	unsigned long flags = 0;
-	if (in_interrupt())
-		BUG();
-
-	/* We used to wait for the buffer to finish here, but this didn't work
-	   because, as we were keeping the state as VIDEOBUF_QUEUED,
-	   videobuf_queue_cancel marked it as finished for us.
-	   (Also, it could wedge forever if the hardware was misconfigured.)
-
-	   This should be safe; by the time we get here, the buffer isn't
-	   queued anymore. If we ever start marking the buffers as
-	   VIDEOBUF_ACTIVE, it won't be, though.
-	*/
-	spin_lock_irqsave(&dev->slock, flags);
-	if (dev->isoc_ctl.buf == buf)
-		dev->isoc_ctl.buf = NULL;
-	spin_unlock_irqrestore(&dev->slock, flags);
+	*nplanes = 1;
+	sizes[0] = size;
 
-	videobuf_vmalloc_free(&buf->vb);
-	buf->vb.state = VIDEOBUF_NEEDS_INIT;
+	return 0;
 }
 
 static int
-buffer_prepare(struct videobuf_queue *vq, struct videobuf_buffer *vb,
-						enum v4l2_field field)
+buffer_prepare(struct vb2_buffer *vb)
 {
-	struct au0828_fh     *fh  = vq->priv_data;
 	struct au0828_buffer *buf = container_of(vb, struct au0828_buffer, vb);
-	struct au0828_dev    *dev = fh->dev;
-	int                  rc = 0, urb_init = 0;
+	struct au0828_dev    *dev = vb2_get_drv_priv(vb->vb2_queue);
 
-	buf->vb.size = (fh->dev->width * fh->dev->height * 16 + 7) >> 3;
+	buf->length = dev->height * dev->bytesperline;
 
-	if (0 != buf->vb.baddr  &&  buf->vb.bsize < buf->vb.size)
+	if (vb2_plane_size(vb, 0) < buf->length) {
+		pr_err("%s data will not fit into plane (%lu < %lu)\n",
+			__func__, vb2_plane_size(vb, 0), buf->length);
 		return -EINVAL;
-
-	buf->vb.width  = dev->width;
-	buf->vb.height = dev->height;
-	buf->vb.field  = field;
-
-	if (VIDEOBUF_NEEDS_INIT == buf->vb.state) {
-		rc = videobuf_iolock(vq, &buf->vb, NULL);
-		if (rc < 0) {
-			pr_info("videobuf_iolock failed\n");
-			goto fail;
-		}
 	}
-
-	if (!dev->isoc_ctl.num_bufs)
-		urb_init = 1;
-
-	if (urb_init) {
-		rc = au0828_init_isoc(dev, AU0828_ISO_PACKETS_PER_URB,
-				      AU0828_MAX_ISO_BUFS, dev->max_pkt_size,
-				      au0828_isoc_copy);
-		if (rc < 0) {
-			pr_info("au0828_init_isoc failed\n");
-			goto fail;
-		}
-	}
-
-	buf->vb.state = VIDEOBUF_PREPARED;
+	vb2_set_plane_payload(&buf->vb, 0, buf->length);
 	return 0;
-
-fail:
-	free_buffer(vq, buf);
-	return rc;
 }
 
 static void
-buffer_queue(struct videobuf_queue *vq, struct videobuf_buffer *vb)
+buffer_queue(struct vb2_buffer *vb)
 {
 	struct au0828_buffer    *buf     = container_of(vb,
 							struct au0828_buffer,
 							vb);
-	struct au0828_fh        *fh      = vq->priv_data;
-	struct au0828_dev       *dev     = fh->dev;
+	struct au0828_dev       *dev     = vb2_get_drv_priv(vb->vb2_queue);
 	struct au0828_dmaqueue  *vidq    = &dev->vidq;
+	unsigned long flags = 0;
 
-	buf->vb.state = VIDEOBUF_QUEUED;
-	list_add_tail(&buf->vb.queue, &vidq->active);
-}
-
-static void buffer_release(struct videobuf_queue *vq,
-				struct videobuf_buffer *vb)
-{
-	struct au0828_buffer   *buf  = container_of(vb,
-						    struct au0828_buffer,
-						    vb);
+	buf->mem = vb2_plane_vaddr(vb, 0);
+	buf->length = vb2_plane_size(vb, 0);
 
-	free_buffer(vq, buf);
+	spin_lock_irqsave(&dev->slock, flags);
+	list_add_tail(&buf->list, &vidq->active);
+	spin_unlock_irqrestore(&dev->slock, flags);
 }
 
-static struct videobuf_queue_ops au0828_video_qops = {
-	.buf_setup      = buffer_setup,
-	.buf_prepare    = buffer_prepare,
-	.buf_queue      = buffer_queue,
-	.buf_release    = buffer_release,
-};
-
-/* ------------------------------------------------------------------
-   V4L2 interface
-   ------------------------------------------------------------------*/
-
 static int au0828_i2s_init(struct au0828_dev *dev)
 {
 	/* Enable i2s mode */
@@ -828,7 +746,7 @@ static int au0828_analog_stream_enable(struct au0828_dev *d)
 	return 0;
 }
 
-int au0828_analog_stream_disable(struct au0828_dev *d)
+static int au0828_analog_stream_disable(struct au0828_dev *d)
 {
 	dprintk(1, "au0828_analog_stream_disable called\n");
 	au0828_writereg(d, AU0828_SENSORCTRL_100, 0x0);
@@ -861,78 +779,141 @@ static int au0828_stream_interrupt(struct au0828_dev *dev)
 	return 0;
 }
 
-/*
- * au0828_release_resources
- * unregister v4l2 devices
- */
-void au0828_analog_unregister(struct au0828_dev *dev)
+int au0828_start_analog_streaming(struct vb2_queue *vq, unsigned int count)
 {
-	dprintk(1, "au0828_release_resources called\n");
-	mutex_lock(&au0828_sysfs_lock);
+	struct au0828_dev *dev = vb2_get_drv_priv(vq);
+	int rc = 0;
 
-	if (dev->vdev)
-		video_unregister_device(dev->vdev);
-	if (dev->vbi_dev)
-		video_unregister_device(dev->vbi_dev);
+	dprintk(1, "au0828_start_analog_streaming called %d\n",
+		dev->streaming_users);
 
-	mutex_unlock(&au0828_sysfs_lock);
-}
+	if (vq->type == V4L2_BUF_TYPE_VIDEO_CAPTURE)
+		dev->frame_count = 0;
+	else
+		dev->vbi_frame_count = 0;
+
+	if (dev->streaming_users == 0) {
+		/* If we were doing ac97 instead of i2s, it would go here...*/
+		au0828_i2s_init(dev);
+		rc = au0828_init_isoc(dev, AU0828_ISO_PACKETS_PER_URB,
+				   AU0828_MAX_ISO_BUFS, dev->max_pkt_size,
+				   au0828_isoc_copy);
+		if (rc < 0) {
+			pr_info("au0828_init_isoc failed\n");
+			return rc;
+		}
 
+		if (vq->type == V4L2_BUF_TYPE_VIDEO_CAPTURE) {
+			v4l2_device_call_all(&dev->v4l2_dev, 0, video,
+						s_stream, 1);
+			dev->vid_timeout_running = 1;
+			mod_timer(&dev->vid_timeout, jiffies + (HZ / 10));
+		} else if (vq->type == V4L2_BUF_TYPE_VBI_CAPTURE) {
+			dev->vbi_timeout_running = 1;
+			mod_timer(&dev->vbi_timeout, jiffies + (HZ / 10));
+		}
+	}
+	dev->streaming_users++;
+	return rc;
+}
 
-/* Usage lock check functions */
-static int res_get(struct au0828_fh *fh, unsigned int bit)
+static void au0828_stop_streaming(struct vb2_queue *vq)
 {
-	struct au0828_dev    *dev = fh->dev;
+	struct au0828_dev *dev = vb2_get_drv_priv(vq);
+	struct au0828_dmaqueue *vidq = &dev->vidq;
+	unsigned long flags = 0;
+	int i;
 
-	if (fh->resources & bit)
-		/* have it already allocated */
-		return 1;
+	dprintk(1, "au0828_stop_streaming called %d\n", dev->streaming_users);
 
-	/* is it free? */
-	if (dev->resources & bit) {
-		/* no, someone else uses it */
-		return 0;
+	if (dev->streaming_users-- == 1)
+		au0828_uninit_isoc(dev);
+
+	spin_lock_irqsave(&dev->slock, flags);
+	if (dev->isoc_ctl.buf != NULL) {
+		vb2_buffer_done(&dev->isoc_ctl.buf->vb, VB2_BUF_STATE_ERROR);
+		dev->isoc_ctl.buf = NULL;
 	}
-	/* it's free, grab it */
-	fh->resources  |= bit;
-	dev->resources |= bit;
-	dprintk(1, "res: get %d\n", bit);
+	while (!list_empty(&vidq->active)) {
+		struct au0828_buffer *buf;
 
-	return 1;
-}
+		buf = list_entry(vidq->active.next, struct au0828_buffer, list);
+		vb2_buffer_done(&buf->vb, VB2_BUF_STATE_ERROR);
+		list_del(&buf->list);
+	}
 
-static int res_check(struct au0828_fh *fh, unsigned int bit)
-{
-	return fh->resources & bit;
-}
+	v4l2_device_call_all(&dev->v4l2_dev, 0, video, s_stream, 0);
 
-static int res_locked(struct au0828_dev *dev, unsigned int bit)
-{
-	return dev->resources & bit;
+	for (i = 0; i < AU0828_MAX_INPUT; i++) {
+		if (AUVI_INPUT(i).audio_setup == NULL)
+			continue;
+		(AUVI_INPUT(i).audio_setup)(dev, 0);
+	}
+	spin_unlock_irqrestore(&dev->slock, flags);
+
+	dev->vid_timeout_running = 0;
+	del_timer_sync(&dev->vid_timeout);
 }
 
-static void res_free(struct au0828_fh *fh, unsigned int bits)
+void au0828_stop_vbi_streaming(struct vb2_queue *vq)
 {
-	struct au0828_dev    *dev = fh->dev;
+	struct au0828_dev *dev = vb2_get_drv_priv(vq);
+	struct au0828_dmaqueue *vbiq = &dev->vbiq;
+	unsigned long flags = 0;
+
+	dprintk(1, "au0828_stop_vbi_streaming called %d\n",
+		dev->streaming_users);
+
+	if (dev->streaming_users-- == 1)
+		au0828_uninit_isoc(dev);
 
-	BUG_ON((fh->resources & bits) != bits);
+	spin_lock_irqsave(&dev->slock, flags);
+	if (dev->isoc_ctl.vbi_buf != NULL) {
+		vb2_buffer_done(&dev->isoc_ctl.vbi_buf->vb,
+				VB2_BUF_STATE_ERROR);
+		dev->isoc_ctl.vbi_buf = NULL;
+	}
+	while (!list_empty(&vbiq->active)) {
+		struct au0828_buffer *buf;
+
+		buf = list_entry(vbiq->active.next, struct au0828_buffer, list);
+		list_del(&buf->list);
+		vb2_buffer_done(&buf->vb, VB2_BUF_STATE_ERROR);
+	}
+	spin_unlock_irqrestore(&dev->slock, flags);
 
-	fh->resources  &= ~bits;
-	dev->resources &= ~bits;
-	dprintk(1, "res: put %d\n", bits);
+	dev->vbi_timeout_running = 0;
+	del_timer_sync(&dev->vbi_timeout);
 }
 
-static int get_ressource(struct au0828_fh *fh)
+static struct vb2_ops au0828_video_qops = {
+	.queue_setup     = queue_setup,
+	.buf_prepare     = buffer_prepare,
+	.buf_queue       = buffer_queue,
+	.start_streaming = au0828_start_analog_streaming,
+	.stop_streaming  = au0828_stop_streaming,
+	.wait_prepare    = vb2_ops_wait_prepare,
+	.wait_finish     = vb2_ops_wait_finish,
+};
+
+/* ------------------------------------------------------------------
+   V4L2 interface
+   ------------------------------------------------------------------*/
+/*
+ * au0828_analog_unregister
+ * unregister v4l2 devices
+ */
+void au0828_analog_unregister(struct au0828_dev *dev)
 {
-	switch (fh->type) {
-	case V4L2_BUF_TYPE_VIDEO_CAPTURE:
-		return AU0828_RESOURCE_VIDEO;
-	case V4L2_BUF_TYPE_VBI_CAPTURE:
-		return AU0828_RESOURCE_VBI;
-	default:
-		BUG();
-		return 0;
-	}
+	dprintk(1, "au0828_analog_unregister called\n");
+	mutex_lock(&au0828_sysfs_lock);
+
+	if (dev->vdev)
+		video_unregister_device(dev->vdev);
+	if (dev->vbi_dev)
+		video_unregister_device(dev->vbi_dev);
+
+	mutex_unlock(&au0828_sysfs_lock);
 }
 
 /* This function ensures that video frames continue to be delivered even if
@@ -950,8 +931,8 @@ static void au0828_vid_buffer_timeout(unsigned long data)
 
 	buf = dev->isoc_ctl.buf;
 	if (buf != NULL) {
-		vid_data = videobuf_to_vmalloc(&buf->vb);
-		memset(vid_data, 0x00, buf->vb.size); /* Blank green frame */
+		vid_data = vb2_plane_vaddr(&buf->vb, 0);
+		memset(vid_data, 0x00, buf->length); /* Blank green frame */
 		buffer_filled(dev, dma_q, buf);
 	}
 	get_next_buf(dma_q, &buf);
@@ -974,8 +955,8 @@ static void au0828_vbi_buffer_timeout(unsigned long data)
 
 	buf = dev->isoc_ctl.vbi_buf;
 	if (buf != NULL) {
-		vbi_data = videobuf_to_vmalloc(&buf->vb);
-		memset(vbi_data, 0x00, buf->vb.size);
+		vbi_data = vb2_plane_vaddr(&buf->vb, 0);
+		memset(vbi_data, 0x00, buf->length);
 		vbi_buffer_filled(dev, dma_q, buf);
 	}
 	vbi_get_next_buf(dma_q, &buf);
@@ -985,105 +966,65 @@ static void au0828_vbi_buffer_timeout(unsigned long data)
 	spin_unlock_irqrestore(&dev->slock, flags);
 }
 
-
 static int au0828_v4l2_open(struct file *filp)
 {
-	int ret = 0;
-	struct video_device *vdev = video_devdata(filp);
 	struct au0828_dev *dev = video_drvdata(filp);
-	struct au0828_fh *fh;
-	int type;
-
-	switch (vdev->vfl_type) {
-	case VFL_TYPE_GRABBER:
-		type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
-		break;
-	case VFL_TYPE_VBI:
-		type = V4L2_BUF_TYPE_VBI_CAPTURE;
-		break;
-	default:
-		return -EINVAL;
-	}
-
-	fh = kzalloc(sizeof(struct au0828_fh), GFP_KERNEL);
-	if (NULL == fh) {
-		dprintk(1, "Failed allocate au0828_fh struct!\n");
-		return -ENOMEM;
-	}
+	int ret = 0;
 
-	fh->type = type;
-	fh->dev = dev;
-	v4l2_fh_init(&fh->fh, vdev);
-	filp->private_data = fh;
+	dprintk(1,
+		"%s called std_set %d dev_state %d stream users %d users %d\n",
+		__func__, dev->std_set_in_tuner_core, dev->dev_state,
+		dev->streaming_users, dev->users);
 
-	if (mutex_lock_interruptible(&dev->lock)) {
-		kfree(fh);
+	if (mutex_lock_interruptible(&dev->lock))
 		return -ERESTARTSYS;
+
+	ret = v4l2_fh_open(filp);
+	if (ret) {
+		au0828_isocdbg("%s: v4l2_fh_open() returned error %d\n",
+				__func__, ret);
+		mutex_unlock(&dev->lock);
+		return ret;
 	}
+
 	if (dev->users == 0) {
 		au0828_analog_stream_enable(dev);
 		au0828_analog_stream_reset(dev);
-
-		/* If we were doing ac97 instead of i2s, it would go here...*/
-		au0828_i2s_init(dev);
-
 		dev->stream_state = STREAM_OFF;
 		dev->dev_state |= DEV_INITIALIZED;
 	}
-
 	dev->users++;
 	mutex_unlock(&dev->lock);
-
-	videobuf_queue_vmalloc_init(&fh->vb_vidq, &au0828_video_qops,
-				    NULL, &dev->slock,
-				    V4L2_BUF_TYPE_VIDEO_CAPTURE,
-				    V4L2_FIELD_INTERLACED,
-				    sizeof(struct au0828_buffer), fh,
-				    &dev->lock);
-
-	/* VBI Setup */
-	videobuf_queue_vmalloc_init(&fh->vb_vbiq, &au0828_vbi_qops,
-				    NULL, &dev->slock,
-				    V4L2_BUF_TYPE_VBI_CAPTURE,
-				    V4L2_FIELD_SEQ_TB,
-				    sizeof(struct au0828_buffer), fh,
-				    &dev->lock);
-	v4l2_fh_add(&fh->fh);
 	return ret;
 }
 
 static int au0828_v4l2_close(struct file *filp)
 {
 	int ret;
-	struct au0828_fh *fh = filp->private_data;
-	struct au0828_dev *dev = fh->dev;
+	struct au0828_dev *dev = video_drvdata(filp);
+	struct video_device *vdev = video_devdata(filp);
+
+	dprintk(1,
+		"%s called std_set %d dev_state %d stream users %d users %d\n",
+		__func__, dev->std_set_in_tuner_core, dev->dev_state,
+		dev->streaming_users, dev->users);
 
-	v4l2_fh_del(&fh->fh);
-	v4l2_fh_exit(&fh->fh);
 	mutex_lock(&dev->lock);
-	if (res_check(fh, AU0828_RESOURCE_VIDEO)) {
+	if (vdev->vfl_type == VFL_TYPE_GRABBER && dev->vid_timeout_running) {
 		/* Cancel timeout thread in case they didn't call streamoff */
 		dev->vid_timeout_running = 0;
 		del_timer_sync(&dev->vid_timeout);
-
-		videobuf_stop(&fh->vb_vidq);
-		res_free(fh, AU0828_RESOURCE_VIDEO);
-	}
-
-	if (res_check(fh, AU0828_RESOURCE_VBI)) {
+	} else if (vdev->vfl_type == VFL_TYPE_VBI &&
+			dev->vbi_timeout_running) {
 		/* Cancel timeout thread in case they didn't call streamoff */
 		dev->vbi_timeout_running = 0;
 		del_timer_sync(&dev->vbi_timeout);
-
-		videobuf_stop(&fh->vb_vbiq);
-		res_free(fh, AU0828_RESOURCE_VBI);
 	}
 
-	if (dev->users == 1 && video_is_registered(video_devdata(filp))) {
-		au0828_analog_stream_disable(dev);
-
-		au0828_uninit_isoc(dev);
+	if (dev->dev_state == DEV_DISCONNECTED)
+		goto end;
 
+	if (dev->users == 1) {
 		/* Save some power by putting tuner to sleep */
 		v4l2_device_call_all(&dev->v4l2_dev, 0, core, s_power, 0);
 		dev->std_set_in_tuner_core = 0;
@@ -1094,13 +1035,10 @@ static int au0828_v4l2_close(struct file *filp)
 		if (ret < 0)
 			pr_info("Au0828 can't set alternate to 0!\n");
 	}
-	mutex_unlock(&dev->lock);
-
-	videobuf_mmap_free(&fh->vb_vidq);
-	videobuf_mmap_free(&fh->vb_vbiq);
-	kfree(fh);
+end:
+	_vb2_fop_release(filp, NULL);
 	dev->users--;
-	wake_up_interruptible_nr(&dev->open, 1);
+	mutex_unlock(&dev->lock);
 	return 0;
 }
 
@@ -1112,6 +1050,9 @@ static void au0828_init_tuner(struct au0828_dev *dev)
 		.type = V4L2_TUNER_ANALOG_TV,
 	};
 
+	dprintk(1, "%s called std_set %d dev_state %d\n", __func__,
+		dev->std_set_in_tuner_core, dev->dev_state);
+
 	if (dev->std_set_in_tuner_core)
 		return;
 	dev->std_set_in_tuner_core = 1;
@@ -1124,98 +1065,6 @@ static void au0828_init_tuner(struct au0828_dev *dev)
 	i2c_gate_ctrl(dev, 0);
 }
 
-static ssize_t au0828_v4l2_read(struct file *filp, char __user *buf,
-				size_t count, loff_t *pos)
-{
-	struct au0828_fh *fh = filp->private_data;
-	struct au0828_dev *dev = fh->dev;
-	int rc;
-
-	rc = check_dev(dev);
-	if (rc < 0)
-		return rc;
-
-	if (mutex_lock_interruptible(&dev->lock))
-		return -ERESTARTSYS;
-	au0828_init_tuner(dev);
-	mutex_unlock(&dev->lock);
-
-	if (fh->type == V4L2_BUF_TYPE_VIDEO_CAPTURE) {
-		if (res_locked(dev, AU0828_RESOURCE_VIDEO))
-			return -EBUSY;
-
-		return videobuf_read_stream(&fh->vb_vidq, buf, count, pos, 0,
-					filp->f_flags & O_NONBLOCK);
-	}
-
-	if (fh->type == V4L2_BUF_TYPE_VBI_CAPTURE) {
-		if (!res_get(fh, AU0828_RESOURCE_VBI))
-			return -EBUSY;
-
-		if (dev->vbi_timeout_running == 0) {
-			/* Handle case where caller tries to read without
-			   calling streamon first */
-			dev->vbi_timeout_running = 1;
-			mod_timer(&dev->vbi_timeout, jiffies + (HZ / 10));
-		}
-
-		return videobuf_read_stream(&fh->vb_vbiq, buf, count, pos, 0,
-					    filp->f_flags & O_NONBLOCK);
-	}
-
-	return 0;
-}
-
-static unsigned int au0828_v4l2_poll(struct file *filp, poll_table *wait)
-{
-	struct au0828_fh *fh = filp->private_data;
-	struct au0828_dev *dev = fh->dev;
-	unsigned long req_events = poll_requested_events(wait);
-	unsigned int res;
-
-	if (check_dev(dev) < 0)
-		return POLLERR;
-
-	res = v4l2_ctrl_poll(filp, wait);
-	if (!(req_events & (POLLIN | POLLRDNORM)))
-		return res;
-
-	if (mutex_lock_interruptible(&dev->lock))
-		return -ERESTARTSYS;
-	au0828_init_tuner(dev);
-	mutex_unlock(&dev->lock);
-
-	if (fh->type == V4L2_BUF_TYPE_VIDEO_CAPTURE) {
-		if (!res_get(fh, AU0828_RESOURCE_VIDEO))
-			return POLLERR;
-		return res | videobuf_poll_stream(filp, &fh->vb_vidq, wait);
-	}
-	if (fh->type == V4L2_BUF_TYPE_VBI_CAPTURE) {
-		if (!res_get(fh, AU0828_RESOURCE_VBI))
-			return POLLERR;
-		return res | videobuf_poll_stream(filp, &fh->vb_vbiq, wait);
-	}
-	return POLLERR;
-}
-
-static int au0828_v4l2_mmap(struct file *filp, struct vm_area_struct *vma)
-{
-	struct au0828_fh *fh    = filp->private_data;
-	struct au0828_dev *dev   = fh->dev;
-	int		 rc;
-
-	rc = check_dev(dev);
-	if (rc < 0)
-		return rc;
-
-	if (fh->type == V4L2_BUF_TYPE_VIDEO_CAPTURE)
-		rc = videobuf_mmap_mapper(&fh->vb_vidq, vma);
-	else if (fh->type == V4L2_BUF_TYPE_VBI_CAPTURE)
-		rc = videobuf_mmap_mapper(&fh->vb_vbiq, vma);
-
-	return rc;
-}
-
 static int au0828_set_format(struct au0828_dev *dev, unsigned int cmd,
 			     struct v4l2_format *format)
 {
@@ -1267,13 +1116,14 @@ static int au0828_set_format(struct au0828_dev *dev, unsigned int cmd,
 	return 0;
 }
 
-
 static int vidioc_querycap(struct file *file, void  *priv,
 			   struct v4l2_capability *cap)
 {
 	struct video_device *vdev = video_devdata(file);
-	struct au0828_fh *fh = priv;
-	struct au0828_dev *dev = fh->dev;
+	struct au0828_dev *dev = video_drvdata(file);
+
+	dprintk(1, "%s called std_set %d dev_state %d\n", __func__,
+		dev->std_set_in_tuner_core, dev->dev_state);
 
 	strlcpy(cap->driver, "au0828", sizeof(cap->driver));
 	strlcpy(cap->card, dev->board.name, sizeof(cap->card));
@@ -1299,6 +1149,8 @@ static int vidioc_enum_fmt_vid_cap(struct file *file, void  *priv,
 	if (f->index)
 		return -EINVAL;
 
+	dprintk(1, "%s called\n", __func__);
+
 	f->type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
 	strcpy(f->description, "Packed YUV2");
 
@@ -1311,8 +1163,10 @@ static int vidioc_enum_fmt_vid_cap(struct file *file, void  *priv,
 static int vidioc_g_fmt_vid_cap(struct file *file, void *priv,
 					struct v4l2_format *f)
 {
-	struct au0828_fh *fh  = priv;
-	struct au0828_dev *dev = fh->dev;
+	struct au0828_dev *dev = video_drvdata(file);
+
+	dprintk(1, "%s called std_set %d dev_state %d\n", __func__,
+		dev->std_set_in_tuner_core, dev->dev_state);
 
 	f->fmt.pix.width = dev->width;
 	f->fmt.pix.height = dev->height;
@@ -1328,8 +1182,10 @@ static int vidioc_g_fmt_vid_cap(struct file *file, void *priv,
 static int vidioc_try_fmt_vid_cap(struct file *file, void *priv,
 				  struct v4l2_format *f)
 {
-	struct au0828_fh *fh  = priv;
-	struct au0828_dev *dev = fh->dev;
+	struct au0828_dev *dev = video_drvdata(file);
+
+	dprintk(1, "%s called std_set %d dev_state %d\n", __func__,
+		dev->std_set_in_tuner_core, dev->dev_state);
 
 	return au0828_set_format(dev, VIDIOC_TRY_FMT, f);
 }
@@ -1337,15 +1193,17 @@ static int vidioc_try_fmt_vid_cap(struct file *file, void *priv,
 static int vidioc_s_fmt_vid_cap(struct file *file, void *priv,
 				struct v4l2_format *f)
 {
-	struct au0828_fh *fh  = priv;
-	struct au0828_dev *dev = fh->dev;
+	struct au0828_dev *dev = video_drvdata(file);
 	int rc;
 
+	dprintk(1, "%s called std_set %d dev_state %d\n", __func__,
+		dev->std_set_in_tuner_core, dev->dev_state);
+
 	rc = check_dev(dev);
 	if (rc < 0)
 		return rc;
 
-	if (videobuf_queue_is_busy(&fh->vb_vidq)) {
+	if (vb2_is_busy(&dev->vb_vidq)) {
 		pr_info("%s queue busy\n", __func__);
 		rc = -EBUSY;
 		goto out;
@@ -1358,8 +1216,16 @@ out:
 
 static int vidioc_s_std(struct file *file, void *priv, v4l2_std_id norm)
 {
-	struct au0828_fh *fh = priv;
-	struct au0828_dev *dev = fh->dev;
+	struct au0828_dev *dev = video_drvdata(file);
+
+	dprintk(1, "%s called std_set %d dev_state %d\n", __func__,
+		dev->std_set_in_tuner_core, dev->dev_state);
+
+	if (norm == dev->std)
+		return 0;
+
+	if (dev->streaming_users > 0)
+		return -EBUSY;
 
 	dev->std = norm;
 
@@ -1382,8 +1248,10 @@ static int vidioc_s_std(struct file *file, void *priv, v4l2_std_id norm)
 
 static int vidioc_g_std(struct file *file, void *priv, v4l2_std_id *norm)
 {
-	struct au0828_fh *fh = priv;
-	struct au0828_dev *dev = fh->dev;
+	struct au0828_dev *dev = video_drvdata(file);
+
+	dprintk(1, "%s called std_set %d dev_state %d\n", __func__,
+		dev->std_set_in_tuner_core, dev->dev_state);
 
 	*norm = dev->std;
 	return 0;
@@ -1392,8 +1260,7 @@ static int vidioc_g_std(struct file *file, void *priv, v4l2_std_id *norm)
 static int vidioc_enum_input(struct file *file, void *priv,
 				struct v4l2_input *input)
 {
-	struct au0828_fh *fh = priv;
-	struct au0828_dev *dev = fh->dev;
+	struct au0828_dev *dev = video_drvdata(file);
 	unsigned int tmp;
 
 	static const char *inames[] = {
@@ -1406,6 +1273,9 @@ static int vidioc_enum_input(struct file *file, void *priv,
 		[AU0828_VMUX_DEBUG] = "tv debug"
 	};
 
+	dprintk(1, "%s called std_set %d dev_state %d\n", __func__,
+		dev->std_set_in_tuner_core, dev->dev_state);
+
 	tmp = input->index;
 
 	if (tmp >= AU0828_MAX_INPUT)
@@ -1431,8 +1301,11 @@ static int vidioc_enum_input(struct file *file, void *priv,
 
 static int vidioc_g_input(struct file *file, void *priv, unsigned int *i)
 {
-	struct au0828_fh *fh = priv;
-	struct au0828_dev *dev = fh->dev;
+	struct au0828_dev *dev = video_drvdata(file);
+
+	dprintk(1, "%s called std_set %d dev_state %d\n", __func__,
+		dev->std_set_in_tuner_core, dev->dev_state);
+
 	*i = dev->ctrl_input;
 	return 0;
 }
@@ -1441,6 +1314,9 @@ static void au0828_s_input(struct au0828_dev *dev, int index)
 {
 	int i;
 
+	dprintk(1, "%s called std_set %d dev_state %d\n", __func__,
+		dev->std_set_in_tuner_core, dev->dev_state);
+
 	switch (AUVI_INPUT(index).type) {
 	case AU0828_VMUX_SVIDEO:
 		dev->input_type = AU0828_VMUX_SVIDEO;
@@ -1490,8 +1366,7 @@ static void au0828_s_input(struct au0828_dev *dev, int index)
 
 static int vidioc_s_input(struct file *file, void *priv, unsigned int index)
 {
-	struct au0828_fh *fh = priv;
-	struct au0828_dev *dev = fh->dev;
+	struct au0828_dev *dev = video_drvdata(file);
 
 	dprintk(1, "VIDIOC_S_INPUT in function %s, input=%d\n", __func__,
 		index);
@@ -1509,6 +1384,8 @@ static int vidioc_enumaudio(struct file *file, void *priv, struct v4l2_audio *a)
 	if (a->index > 1)
 		return -EINVAL;
 
+	dprintk(1, "%s called\n", __func__);
+
 	if (a->index == 0)
 		strcpy(a->name, "Television");
 	else
@@ -1520,8 +1397,10 @@ static int vidioc_enumaudio(struct file *file, void *priv, struct v4l2_audio *a)
 
 static int vidioc_g_audio(struct file *file, void *priv, struct v4l2_audio *a)
 {
-	struct au0828_fh *fh = priv;
-	struct au0828_dev *dev = fh->dev;
+	struct au0828_dev *dev = video_drvdata(file);
+
+	dprintk(1, "%s called std_set %d dev_state %d\n", __func__,
+		dev->std_set_in_tuner_core, dev->dev_state);
 
 	a->index = dev->ctrl_ainput;
 	if (a->index == 0)
@@ -1535,22 +1414,26 @@ static int vidioc_g_audio(struct file *file, void *priv, struct v4l2_audio *a)
 
 static int vidioc_s_audio(struct file *file, void *priv, const struct v4l2_audio *a)
 {
-	struct au0828_fh *fh = priv;
-	struct au0828_dev *dev = fh->dev;
+	struct au0828_dev *dev = video_drvdata(file);
 
 	if (a->index != dev->ctrl_ainput)
 		return -EINVAL;
+
+	dprintk(1, "%s called std_set %d dev_state %d\n", __func__,
+		dev->std_set_in_tuner_core, dev->dev_state);
 	return 0;
 }
 
 static int vidioc_g_tuner(struct file *file, void *priv, struct v4l2_tuner *t)
 {
-	struct au0828_fh *fh = priv;
-	struct au0828_dev *dev = fh->dev;
+	struct au0828_dev *dev = video_drvdata(file);
 
 	if (t->index != 0)
 		return -EINVAL;
 
+	dprintk(1, "%s called std_set %d dev_state %d\n", __func__,
+		dev->std_set_in_tuner_core, dev->dev_state);
+
 	strcpy(t->name, "Auvitek tuner");
 
 	au0828_init_tuner(dev);
@@ -1563,12 +1446,14 @@ static int vidioc_g_tuner(struct file *file, void *priv, struct v4l2_tuner *t)
 static int vidioc_s_tuner(struct file *file, void *priv,
 				const struct v4l2_tuner *t)
 {
-	struct au0828_fh *fh = priv;
-	struct au0828_dev *dev = fh->dev;
+	struct au0828_dev *dev = video_drvdata(file);
 
 	if (t->index != 0)
 		return -EINVAL;
 
+	dprintk(1, "%s called std_set %d dev_state %d\n", __func__,
+		dev->std_set_in_tuner_core, dev->dev_state);
+
 	au0828_init_tuner(dev);
 	i2c_gate_ctrl(dev, 1);
 	v4l2_device_call_all(&dev->v4l2_dev, 0, tuner, s_tuner, t);
@@ -1584,11 +1469,12 @@ static int vidioc_s_tuner(struct file *file, void *priv,
 static int vidioc_g_frequency(struct file *file, void *priv,
 				struct v4l2_frequency *freq)
 {
-	struct au0828_fh *fh = priv;
-	struct au0828_dev *dev = fh->dev;
+	struct au0828_dev *dev = video_drvdata(file);
 
 	if (freq->tuner != 0)
 		return -EINVAL;
+	dprintk(1, "%s called std_set %d dev_state %d\n", __func__,
+		dev->std_set_in_tuner_core, dev->dev_state);
 	freq->frequency = dev->ctrl_freq;
 	return 0;
 }
@@ -1596,13 +1482,15 @@ static int vidioc_g_frequency(struct file *file, void *priv,
 static int vidioc_s_frequency(struct file *file, void *priv,
 				const struct v4l2_frequency *freq)
 {
-	struct au0828_fh *fh = priv;
-	struct au0828_dev *dev = fh->dev;
+	struct au0828_dev *dev = video_drvdata(file);
 	struct v4l2_frequency new_freq = *freq;
 
 	if (freq->tuner != 0)
 		return -EINVAL;
 
+	dprintk(1, "%s called std_set %d dev_state %d\n", __func__,
+		dev->std_set_in_tuner_core, dev->dev_state);
+
 	au0828_init_tuner(dev);
 	i2c_gate_ctrl(dev, 1);
 
@@ -1624,8 +1512,10 @@ static int vidioc_s_frequency(struct file *file, void *priv,
 static int vidioc_g_fmt_vbi_cap(struct file *file, void *priv,
 				struct v4l2_format *format)
 {
-	struct au0828_fh      *fh  = priv;
-	struct au0828_dev     *dev = fh->dev;
+	struct au0828_dev *dev = video_drvdata(file);
+
+	dprintk(1, "%s called std_set %d dev_state %d\n", __func__,
+		dev->std_set_in_tuner_core, dev->dev_state);
 
 	format->fmt.vbi.samples_per_line = dev->vbi_width;
 	format->fmt.vbi.sample_format = V4L2_PIX_FMT_GREY;
@@ -1645,12 +1535,14 @@ static int vidioc_g_fmt_vbi_cap(struct file *file, void *priv,
 static int vidioc_cropcap(struct file *file, void *priv,
 			  struct v4l2_cropcap *cc)
 {
-	struct au0828_fh *fh = priv;
-	struct au0828_dev *dev = fh->dev;
+	struct au0828_dev *dev = video_drvdata(file);
 
 	if (cc->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
 		return -EINVAL;
 
+	dprintk(1, "%s called std_set %d dev_state %d\n", __func__,
+		dev->std_set_in_tuner_core, dev->dev_state);
+
 	cc->bounds.left = 0;
 	cc->bounds.top = 0;
 	cc->bounds.width = dev->width;
@@ -1664,105 +1556,14 @@ static int vidioc_cropcap(struct file *file, void *priv,
 	return 0;
 }
 
-static int vidioc_streamon(struct file *file, void *priv,
-			   enum v4l2_buf_type type)
-{
-	struct au0828_fh      *fh  = priv;
-	struct au0828_dev     *dev = fh->dev;
-	int                   rc = -EINVAL;
-
-	rc = check_dev(dev);
-	if (rc < 0)
-		return rc;
-
-	if (unlikely(type != fh->type))
-		return -EINVAL;
-
-	dprintk(1, "vidioc_streamon fh=%p t=%d fh->res=%d dev->res=%d\n",
-		fh, type, fh->resources, dev->resources);
-
-	if (unlikely(!res_get(fh, get_ressource(fh))))
-		return -EBUSY;
-
-	au0828_init_tuner(dev);
-	if (type == V4L2_BUF_TYPE_VIDEO_CAPTURE) {
-		au0828_analog_stream_enable(dev);
-		v4l2_device_call_all(&dev->v4l2_dev, 0, video, s_stream, 1);
-	}
-
-	if (fh->type == V4L2_BUF_TYPE_VIDEO_CAPTURE) {
-		rc = videobuf_streamon(&fh->vb_vidq);
-		dev->vid_timeout_running = 1;
-		mod_timer(&dev->vid_timeout, jiffies + (HZ / 10));
-	} else if (fh->type == V4L2_BUF_TYPE_VBI_CAPTURE) {
-		rc = videobuf_streamon(&fh->vb_vbiq);
-		dev->vbi_timeout_running = 1;
-		mod_timer(&dev->vbi_timeout, jiffies + (HZ / 10));
-	}
-
-	return rc;
-}
-
-static int vidioc_streamoff(struct file *file, void *priv,
-			    enum v4l2_buf_type type)
-{
-	struct au0828_fh      *fh  = priv;
-	struct au0828_dev     *dev = fh->dev;
-	int                   rc;
-	int                   i;
-
-	rc = check_dev(dev);
-	if (rc < 0)
-		return rc;
-
-	if (fh->type != V4L2_BUF_TYPE_VIDEO_CAPTURE &&
-	    fh->type != V4L2_BUF_TYPE_VBI_CAPTURE)
-		return -EINVAL;
-	if (type != fh->type)
-		return -EINVAL;
-
-	dprintk(1, "vidioc_streamoff fh=%p t=%d fh->res=%d dev->res=%d\n",
-		fh, type, fh->resources, dev->resources);
-
-	if (fh->type == V4L2_BUF_TYPE_VIDEO_CAPTURE) {
-		dev->vid_timeout_running = 0;
-		del_timer_sync(&dev->vid_timeout);
-
-		au0828_analog_stream_disable(dev);
-		v4l2_device_call_all(&dev->v4l2_dev, 0, video, s_stream, 0);
-		rc = au0828_stream_interrupt(dev);
-		if (rc != 0)
-			return rc;
-
-		for (i = 0; i < AU0828_MAX_INPUT; i++) {
-			if (AUVI_INPUT(i).audio_setup == NULL)
-				continue;
-			(AUVI_INPUT(i).audio_setup)(dev, 0);
-		}
-
-		if (res_check(fh, AU0828_RESOURCE_VIDEO)) {
-			videobuf_streamoff(&fh->vb_vidq);
-			res_free(fh, AU0828_RESOURCE_VIDEO);
-		}
-	} else if (fh->type == V4L2_BUF_TYPE_VBI_CAPTURE) {
-		dev->vbi_timeout_running = 0;
-		del_timer_sync(&dev->vbi_timeout);
-
-		if (res_check(fh, AU0828_RESOURCE_VBI)) {
-			videobuf_streamoff(&fh->vb_vbiq);
-			res_free(fh, AU0828_RESOURCE_VBI);
-		}
-	}
-
-	return 0;
-}
-
 #ifdef CONFIG_VIDEO_ADV_DEBUG
 static int vidioc_g_register(struct file *file, void *priv,
 			     struct v4l2_dbg_register *reg)
 {
-	struct au0828_fh *fh = priv;
-	struct au0828_dev *dev = fh->dev;
+	struct au0828_dev *dev = video_drvdata(file);
+
+	dprintk(1, "%s called std_set %d dev_state %d\n", __func__,
+		dev->std_set_in_tuner_core, dev->dev_state);
 
 	reg->val = au0828_read(dev, reg->reg);
 	reg->size = 1;
@@ -1772,8 +1573,10 @@ static int vidioc_g_register(struct file *file, void *priv,
 static int vidioc_s_register(struct file *file, void *priv,
 			     const struct v4l2_dbg_register *reg)
 {
-	struct au0828_fh *fh = priv;
-	struct au0828_dev *dev = fh->dev;
+	struct au0828_dev *dev = video_drvdata(file);
+
+	dprintk(1, "%s called std_set %d dev_state %d\n", __func__,
+		dev->std_set_in_tuner_core, dev->dev_state);
 
 	return au0828_writereg(dev, reg->reg, reg->val);
 }
@@ -1783,93 +1586,13 @@ static int vidioc_log_status(struct file *file, void *fh)
 {
 	struct video_device *vdev = video_devdata(file);
 
+	dprintk(1, "%s called\n", __func__);
+
 	v4l2_ctrl_log_status(file, fh);
 	v4l2_device_call_all(vdev->v4l2_dev, 0, core, log_status);
 	return 0;
 }
 
-static int vidioc_reqbufs(struct file *file, void *priv,
-			  struct v4l2_requestbuffers *rb)
-{
-	struct au0828_fh *fh = priv;
-	struct au0828_dev *dev = fh->dev;
-	int rc;
-
-	rc = check_dev(dev);
-	if (rc < 0)
-		return rc;
-
-	if (fh->type == V4L2_BUF_TYPE_VIDEO_CAPTURE)
-		rc = videobuf_reqbufs(&fh->vb_vidq, rb);
-	else if (fh->type == V4L2_BUF_TYPE_VBI_CAPTURE)
-		rc = videobuf_reqbufs(&fh->vb_vbiq, rb);
-
-	return rc;
-}
-
-static int vidioc_querybuf(struct file *file, void *priv,
-			   struct v4l2_buffer *b)
-{
-	struct au0828_fh *fh = priv;
-	struct au0828_dev *dev = fh->dev;
-	int rc;
-
-	rc = check_dev(dev);
-	if (rc < 0)
-		return rc;
-
-	if (fh->type == V4L2_BUF_TYPE_VIDEO_CAPTURE)
-		rc = videobuf_querybuf(&fh->vb_vidq, b);
-	else if (fh->type == V4L2_BUF_TYPE_VBI_CAPTURE)
-		rc = videobuf_querybuf(&fh->vb_vbiq, b);
-
-	return rc;
-}
-
-static int vidioc_qbuf(struct file *file, void *priv, struct v4l2_buffer *b)
-{
-	struct au0828_fh *fh = priv;
-	struct au0828_dev *dev = fh->dev;
-	int rc;
-
-	rc = check_dev(dev);
-	if (rc < 0)
-		return rc;
-
-	if (fh->type == V4L2_BUF_TYPE_VIDEO_CAPTURE)
-		rc = videobuf_qbuf(&fh->vb_vidq, b);
-	else if (fh->type == V4L2_BUF_TYPE_VBI_CAPTURE)
-		rc = videobuf_qbuf(&fh->vb_vbiq, b);
-
-	return rc;
-}
-
-static int vidioc_dqbuf(struct file *file, void *priv, struct v4l2_buffer *b)
-{
-	struct au0828_fh *fh = priv;
-	struct au0828_dev *dev = fh->dev;
-	int rc;
-
-	rc = check_dev(dev);
-	if (rc < 0)
-		return rc;
-
-	/* Workaround for a bug in the au0828 hardware design that sometimes
-	   results in the colorspace being inverted */
-	if (dev->greenscreen_detected == 1) {
-		dprintk(1, "Detected green frame.  Resetting stream...\n");
-		au0828_analog_stream_reset(dev);
-		dev->greenscreen_detected = 0;
-	}
-
-	if (fh->type == V4L2_BUF_TYPE_VIDEO_CAPTURE)
-		rc = videobuf_dqbuf(&fh->vb_vidq, b, file->f_flags & O_NONBLOCK);
-	else if (fh->type == V4L2_BUF_TYPE_VBI_CAPTURE)
-		rc = videobuf_dqbuf(&fh->vb_vbiq, b, file->f_flags & O_NONBLOCK);
-
-	return rc;
-}
-
 void au0828_v4l2_suspend(struct au0828_dev *dev)
 {
 	struct urb *urb;
@@ -1937,9 +1660,9 @@ static struct v4l2_file_operations au0828_v4l_fops = {
 	.owner      = THIS_MODULE,
 	.open       = au0828_v4l2_open,
 	.release    = au0828_v4l2_close,
-	.read       = au0828_v4l2_read,
-	.poll       = au0828_v4l2_poll,
-	.mmap       = au0828_v4l2_mmap,
+	.read       = vb2_fop_read,
+	.poll       = vb2_fop_poll,
+	.mmap       = vb2_fop_mmap,
 	.unlocked_ioctl = video_ioctl2,
 };
 
@@ -1956,17 +1679,24 @@ static const struct v4l2_ioctl_ops video_ioctl_ops = {
 	.vidioc_g_audio             = vidioc_g_audio,
 	.vidioc_s_audio             = vidioc_s_audio,
 	.vidioc_cropcap             = vidioc_cropcap,
-	.vidioc_reqbufs             = vidioc_reqbufs,
-	.vidioc_querybuf            = vidioc_querybuf,
-	.vidioc_qbuf                = vidioc_qbuf,
-	.vidioc_dqbuf               = vidioc_dqbuf,
+
+	.vidioc_reqbufs             = vb2_ioctl_reqbufs,
+	.vidioc_create_bufs         = vb2_ioctl_create_bufs,
+	.vidioc_prepare_buf         = vb2_ioctl_prepare_buf,
+	.vidioc_querybuf            = vb2_ioctl_querybuf,
+	.vidioc_qbuf                = vb2_ioctl_qbuf,
+	.vidioc_dqbuf               = vb2_ioctl_dqbuf,
+	.vidioc_expbuf               = vb2_ioctl_expbuf,
+
 	.vidioc_s_std               = vidioc_s_std,
 	.vidioc_g_std               = vidioc_g_std,
 	.vidioc_enum_input          = vidioc_enum_input,
 	.vidioc_g_input             = vidioc_g_input,
 	.vidioc_s_input             = vidioc_s_input,
-	.vidioc_streamon            = vidioc_streamon,
-	.vidioc_streamoff           = vidioc_streamoff,
+
+	.vidioc_streamon            = vb2_ioctl_streamon,
+	.vidioc_streamoff           = vb2_ioctl_streamoff,
+
 	.vidioc_g_tuner             = vidioc_g_tuner,
 	.vidioc_s_tuner             = vidioc_s_tuner,
 	.vidioc_g_frequency         = vidioc_g_frequency,
@@ -1987,6 +1717,42 @@ static const struct video_device au0828_video_template = {
 	.tvnorms                    = V4L2_STD_NTSC_M | V4L2_STD_PAL_M,
 };
 
+static int au0828_vb2_setup(struct au0828_dev *dev)
+{
+	int rc;
+	struct vb2_queue *q;
+
+	/* Setup Videobuf2 for Video capture */
+	q = &dev->vb_vidq;
+	q->type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
+	q->io_modes = VB2_READ | VB2_MMAP | VB2_USERPTR | VB2_DMABUF;
+	q->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
+	q->drv_priv = dev;
+	q->buf_struct_size = sizeof(struct au0828_buffer);
+	q->ops = &au0828_video_qops;
+	q->mem_ops = &vb2_vmalloc_memops;
+
+	rc = vb2_queue_init(q);
+	if (rc < 0)
+		return rc;
+
+	/* Setup Videobuf2 for VBI capture */
+	q = &dev->vb_vbiq;
+	q->type = V4L2_BUF_TYPE_VBI_CAPTURE;
+	q->io_modes = VB2_READ | VB2_MMAP | VB2_USERPTR | VB2_DMABUF;
+	q->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
+	q->drv_priv = dev;
+	q->buf_struct_size = sizeof(struct au0828_buffer);
+	q->ops = &au0828_vbi_qops;
+	q->mem_ops = &vb2_vmalloc_memops;
+
+	rc = vb2_queue_init(q);
+	if (rc < 0)
+		return rc;
+
+	return 0;
+}
+
 /**************************************************************************/
 
 int au0828_analog_register(struct au0828_dev *dev,
@@ -2038,9 +1804,7 @@ int au0828_analog_register(struct au0828_dev *dev,
 
 	/* init video dma queues */
 	INIT_LIST_HEAD(&dev->vidq.active);
-	INIT_LIST_HEAD(&dev->vidq.queued);
 	INIT_LIST_HEAD(&dev->vbiq.active);
-	INIT_LIST_HEAD(&dev->vbiq.queued);
 
 	dev->vid_timeout.function = au0828_vid_buffer_timeout;
 	dev->vid_timeout.data = (unsigned long) dev;
@@ -2077,18 +1841,34 @@ int au0828_analog_register(struct au0828_dev *dev,
 		goto err_vdev;
 	}
 
+	mutex_init(&dev->vb_queue_lock);
+	mutex_init(&dev->vb_vbi_queue_lock);
+
 	/* Fill the video capture device struct */
 	*dev->vdev = au0828_video_template;
 	dev->vdev->v4l2_dev = &dev->v4l2_dev;
 	dev->vdev->lock = &dev->lock;
+	dev->vdev->queue = &dev->vb_vidq;
+	dev->vdev->queue->lock = &dev->vb_queue_lock;
 	strcpy(dev->vdev->name, "au0828a video");
 
 	/* Setup the VBI device */
 	*dev->vbi_dev = au0828_video_template;
 	dev->vbi_dev->v4l2_dev = &dev->v4l2_dev;
 	dev->vbi_dev->lock = &dev->lock;
+	dev->vbi_dev->queue = &dev->vb_vbiq;
+	dev->vbi_dev->queue->lock = &dev->vb_vbi_queue_lock;
 	strcpy(dev->vbi_dev->name, "au0828a vbi");
 
+	/* initialize videobuf2 stuff */
+	retval = au0828_vb2_setup(dev);
+	if (retval != 0) {
+		dprintk(1, "unable to setup videobuf2 queues (error = %d).\n",
+			retval);
+		ret = -ENODEV;
+		goto err_vbi_dev;
+	}
+
 	/* Register the v4l2 device */
 	video_set_drvdata(dev->vdev, dev);
 	retval = video_register_device(dev->vdev, VFL_TYPE_GRABBER, -1);
diff --git a/drivers/media/usb/au0828/au0828.h b/drivers/media/usb/au0828/au0828.h
index 36815a3..eb15187 100644
--- a/drivers/media/usb/au0828/au0828.h
+++ b/drivers/media/usb/au0828/au0828.h
@@ -28,7 +28,7 @@
 
 /* Analog */
 #include <linux/videodev2.h>
-#include <media/videobuf-vmalloc.h>
+#include <media/videobuf2-vmalloc.h>
 #include <media/v4l2-device.h>
 #include <media/v4l2-ctrls.h>
 #include <media/v4l2-fh.h>
@@ -126,17 +126,7 @@ enum au0828_dev_state {
 	DEV_MISCONFIGURED = 0x04
 };
 
-struct au0828_fh {
-	/* must be the first field of this struct! */
-	struct v4l2_fh fh;
-
-	struct au0828_dev *dev;
-	unsigned int  resources;
-
-	struct videobuf_queue        vb_vidq;
-	struct videobuf_queue        vb_vbiq;
-	enum v4l2_buf_type           type;
-};
+struct au0828_dev;
 
 struct au0828_usb_isoc_ctl {
 		/* max packet size of isoc transaction */
@@ -177,21 +167,20 @@ struct au0828_usb_isoc_ctl {
 /* buffer for one video frame */
 struct au0828_buffer {
 	/* common v4l buffer stuff -- must be first */
-	struct videobuf_buffer vb;
+	struct vb2_buffer vb;
+	struct list_head list;
 
-	struct list_head frame;
+	void *mem;
+	unsigned long length;
 	int top_field;
-	int receiving;
+	/* pointer to vmalloc memory address in vb */
+	char *vb_buf;
 };
 
 struct au0828_dmaqueue {
 	struct list_head       active;
-	struct list_head       queued;
-
-	wait_queue_head_t          wq;
-
 	/* Counters to control buffer fill */
-	int                        pos;
+	int                    pos;
 };
 
 struct au0828_dev {
@@ -220,14 +209,26 @@ struct au0828_dev {
 	struct au0828_rc *ir;
 #endif
 
-	int users;
-	unsigned int resources;	/* resources in use */
 	struct video_device *vdev;
 	struct video_device *vbi_dev;
+
+	/* Videobuf2 */
+	struct vb2_queue vb_vidq;
+	struct vb2_queue vb_vbiq;
+	struct mutex vb_queue_lock;
+	struct mutex vb_vbi_queue_lock;
+
+	unsigned int frame_count;
+	unsigned int vbi_frame_count;
+
 	struct timer_list vid_timeout;
 	int vid_timeout_running;
 	struct timer_list vbi_timeout;
 	int vbi_timeout_running;
+
+	int users;
+	int streaming_users;
+
 	int width;
 	int height;
 	int vbi_width;
@@ -242,7 +243,6 @@ struct au0828_dev {
 	__u8 isoc_in_endpointaddr;
 	u8 isoc_init_ok;
 	int greenscreen_detected;
-	unsigned int frame_count;
 	int ctrl_freq;
 	int input_type;
 	int std_set_in_tuner_core;
@@ -277,6 +277,7 @@ struct au0828_dev {
 	char *dig_transfer_buffer[URB_COUNT];
 };
 
+
 /* ----------------------------------------------------------- */
 #define au0828_read(dev, reg) au0828_readreg(dev, reg)
 #define au0828_write(dev, reg, value) au0828_writereg(dev, reg, value)
@@ -309,13 +310,15 @@ extern int au0828_i2c_unregister(struct au0828_dev *dev);
 
 /* ----------------------------------------------------------- */
 /* au0828-video.c */
-int au0828_analog_register(struct au0828_dev *dev,
+extern int au0828_analog_register(struct au0828_dev *dev,
 			   struct usb_interface *interface);
-int au0828_analog_stream_disable(struct au0828_dev *d);
-void au0828_analog_unregister(struct au0828_dev *dev);
+extern void au0828_analog_unregister(struct au0828_dev *dev);
+extern int au0828_start_analog_streaming(struct vb2_queue *vq,
+						unsigned int count);
+extern void au0828_stop_vbi_streaming(struct vb2_queue *vq);
 #ifdef CONFIG_VIDEO_AU0828_V4L2
-void au0828_v4l2_suspend(struct au0828_dev *dev);
-void au0828_v4l2_resume(struct au0828_dev *dev);
+extern void au0828_v4l2_suspend(struct au0828_dev *dev);
+extern void au0828_v4l2_resume(struct au0828_dev *dev);
 #else
 static inline void au0828_v4l2_suspend(struct au0828_dev *dev) { };
 static inline void au0828_v4l2_resume(struct au0828_dev *dev) { };
@@ -329,7 +332,7 @@ void au0828_dvb_suspend(struct au0828_dev *dev);
 void au0828_dvb_resume(struct au0828_dev *dev);
 
 /* au0828-vbi.c */
-extern struct videobuf_queue_ops au0828_vbi_qops;
+extern struct vb2_ops au0828_vbi_qops;
 
 #define dprintk(level, fmt, arg...)\
 	do { if (au0828_debug & level)\
-- 
2.1.0

Re: [PATCH v3 2/3] media: au0828 - convert to use videobuf2

[Lad, Prabhakar]

Hi Shuah,

Thanks for the patch.

On Tue, Jan 13, 2015 at 2:56 AM, Shuah Khan <shuahkh@osg.samsung.com> wrote:
> Convert au0828 to use videobuf2. Tested with NTSC.
> Tested video and vbi devices with xawtv, tvtime,
> and vlc. Ran v4l2-compliance to ensure there are
> no regressions. video now has no failures and vbi
> has 3 fewer failures.
>
> video before:
> test VIDIOC_G_FMT: FAIL 3 failures
> Total: 72, Succeeded: 69, Failed: 3, Warnings: 0
>
> Video after:
> Total: 72, Succeeded: 72, Failed: 0, Warnings: 18
>
> vbi before:
>     test VIDIOC_REQBUFS/CREATE_BUFS/QUERYBUF: FAIL
>     test VIDIOC_EXPBUF: FAIL
>     test USERPTR: FAIL
>     Total: 72, Succeeded: 66, Failed: 6, Warnings: 0
>
[Snip]
> -       init_waitqueue_head(&dma_q->wq);
> -
>         /* submit urbs and enables IRQ */
>         for (i = 0; i < dev->isoc_ctl.num_bufs; i++) {
>                 rc = usb_submit_urb(dev->isoc_ctl.urb[i], GFP_ATOMIC);
> @@ -308,16 +303,12 @@ static inline void buffer_filled(struct au0828_dev *dev,
>                                   struct au0828_buffer *buf)
>  {
>         /* Advice that buffer was filled */
> -       au0828_isocdbg("[%p/%d] wakeup\n", buf, buf->vb.i);
> -
> -       buf->vb.state = VIDEOBUF_DONE;
> -       buf->vb.field_count++;
> -       v4l2_get_timestamp(&buf->vb.ts);
> +       au0828_isocdbg("[%p/%d] wakeup\n", buf, buf->top_field);
>
> -       dev->isoc_ctl.buf = NULL;
> -
> -       list_del(&buf->vb.queue);
> -       wake_up(&buf->vb.done);
> +       buf->vb.v4l2_buf.sequence = dev->frame_count++;
> +       buf->vb.v4l2_buf.field = V4L2_FIELD_INTERLACED;
> +       v4l2_get_timestamp(&buf->vb.v4l2_buf.timestamp);
> +       vb2_buffer_done(&buf->vb, VB2_BUF_STATE_DONE);
>  }
>
>  static inline void vbi_buffer_filled(struct au0828_dev *dev,
> @@ -325,16 +316,12 @@ static inline void vbi_buffer_filled(struct au0828_dev *dev,
>                                      struct au0828_buffer *buf)
>  {
>         /* Advice that buffer was filled */
> -       au0828_isocdbg("[%p/%d] wakeup\n", buf, buf->vb.i);
> -
> -       buf->vb.state = VIDEOBUF_DONE;
> -       buf->vb.field_count++;
> -       v4l2_get_timestamp(&buf->vb.ts);
> +       au0828_isocdbg("[%p/%d] wakeup\n", buf, buf->top_field);
>
> -       dev->isoc_ctl.vbi_buf = NULL;
> -
> -       list_del(&buf->vb.queue);
> -       wake_up(&buf->vb.done);
> +       buf->vb.v4l2_buf.sequence = dev->vbi_frame_count++;
> +       buf->vb.v4l2_buf.field = V4L2_FIELD_INTERLACED;
> +       v4l2_get_timestamp(&buf->vb.v4l2_buf.timestamp);
> +       vb2_buffer_done(&buf->vb, VB2_BUF_STATE_DONE);
>  }
>
Why not just have one single buffer_filled function ? just check the
queue type and assign the dev->isoc_ctl.buf/ dev->isoc_ctl.vbi_buf
to NULL.

>  /*
> @@ -353,8 +340,8 @@ static void au0828_copy_video(struct au0828_dev *dev,
>         if (len == 0)
>                 return;
>
> -       if (dma_q->pos + len > buf->vb.size)
> -               len = buf->vb.size - dma_q->pos;
> +       if (dma_q->pos + len > buf->length)
> +               len = buf->length - dma_q->pos;
>
>         startread = p;
>         remain = len;
> @@ -372,11 +359,11 @@ static void au0828_copy_video(struct au0828_dev *dev,
>         lencopy = bytesperline - currlinedone;
>         lencopy = lencopy > remain ? remain : lencopy;
>
> -       if ((char *)startwrite + lencopy > (char *)outp + buf->vb.size) {
> +       if ((char *)startwrite + lencopy > (char *)outp + buf->length) {
>                 au0828_isocdbg("Overflow of %zi bytes past buffer end (1)\n",
>                                ((char *)startwrite + lencopy) -
> -                              ((char *)outp + buf->vb.size));
> -               remain = (char *)outp + buf->vb.size - (char *)startwrite;
> +                              ((char *)outp + buf->length));
> +               remain = (char *)outp + buf->length - (char *)startwrite;
>                 lencopy = remain;
>         }
>         if (lencopy <= 0)
> @@ -394,11 +381,11 @@ static void au0828_copy_video(struct au0828_dev *dev,
>                         lencopy = bytesperline;
>
>                 if ((char *)startwrite + lencopy > (char *)outp +
> -                   buf->vb.size) {
> +                   buf->length) {
>                         au0828_isocdbg("Overflow %zi bytes past buf end (2)\n",
>                                        ((char *)startwrite + lencopy) -
> -                                      ((char *)outp + buf->vb.size));
> -                       lencopy = remain = (char *)outp + buf->vb.size -
> +                                      ((char *)outp + buf->length));
> +                       lencopy = remain = (char *)outp + buf->length -
>                                            (char *)startwrite;
>                 }
>                 if (lencopy <= 0)
> @@ -434,7 +421,11 @@ static inline void get_next_buf(struct au0828_dmaqueue *dma_q,
>         }
>
>         /* Get the next buffer */
> -       *buf = list_entry(dma_q->active.next, struct au0828_buffer, vb.queue);
> +       *buf = list_entry(dma_q->active.next, struct au0828_buffer, list);
> +       /* Cleans up buffer - Useful for testing for frame/URB loss */
> +       list_del(&(*buf)->list);
> +       dma_q->pos = 0;
> +       (*buf)->vb_buf = (*buf)->mem;
>         dev->isoc_ctl.buf = *buf;
>
>         return;
> @@ -472,8 +463,8 @@ static void au0828_copy_vbi(struct au0828_dev *dev,
>
>         bytesperline = dev->vbi_width;
>
> -       if (dma_q->pos + len > buf->vb.size)
> -               len = buf->vb.size - dma_q->pos;
> +       if (dma_q->pos + len > buf->length)
> +               len = buf->length - dma_q->pos;
>
>         startread = p;
>         startwrite = outp + (dma_q->pos / 2);
> @@ -496,7 +487,6 @@ static inline void vbi_get_next_buf(struct au0828_dmaqueue *dma_q,
>                                     struct au0828_buffer **buf)
>  {
>         struct au0828_dev *dev = container_of(dma_q, struct au0828_dev, vbiq);
> -       char *outp;
>
>         if (list_empty(&dma_q->active)) {
>                 au0828_isocdbg("No active queue to serve\n");
> @@ -506,13 +496,12 @@ static inline void vbi_get_next_buf(struct au0828_dmaqueue *dma_q,
>         }
>
>         /* Get the next buffer */
> -       *buf = list_entry(dma_q->active.next, struct au0828_buffer, vb.queue);
> +       *buf = list_entry(dma_q->active.next, struct au0828_buffer, list);
>         /* Cleans up buffer - Useful for testing for frame/URB loss */
> -       outp = videobuf_to_vmalloc(&(*buf)->vb);
> -       memset(outp, 0x00, (*buf)->vb.size);
> -
> +       list_del(&(*buf)->list);
> +       dma_q->pos = 0;
> +       (*buf)->vb_buf = (*buf)->mem;
>         dev->isoc_ctl.vbi_buf = *buf;
> -
>         return;
>  }
>
> @@ -548,11 +537,11 @@ static inline int au0828_isoc_copy(struct au0828_dev *dev, struct urb *urb)
>
>         buf = dev->isoc_ctl.buf;
>         if (buf != NULL)
> -               outp = videobuf_to_vmalloc(&buf->vb);
> +               outp = vb2_plane_vaddr(&buf->vb, 0);
>
>         vbi_buf = dev->isoc_ctl.vbi_buf;
>         if (vbi_buf != NULL)
> -               vbioutp = videobuf_to_vmalloc(&vbi_buf->vb);
> +               vbioutp = vb2_plane_vaddr(&vbi_buf->vb, 0);
>
>         for (i = 0; i < urb->number_of_packets; i++) {
>                 int status = urb->iso_frame_desc[i].status;
> @@ -592,8 +581,8 @@ static inline int au0828_isoc_copy(struct au0828_dev *dev, struct urb *urb)
>                                 if (vbi_buf == NULL)
>                                         vbioutp = NULL;
>                                 else
> -                                       vbioutp = videobuf_to_vmalloc(
> -                                               &vbi_buf->vb);
> +                                       vbioutp = vb2_plane_vaddr(
> +                                               &vbi_buf->vb, 0);
>
>                                 /* Video */
>                                 if (buf != NULL)
> @@ -602,7 +591,7 @@ static inline int au0828_isoc_copy(struct au0828_dev *dev, struct urb *urb)
>                                 if (buf == NULL)
>                                         outp = NULL;
>                                 else
> -                                       outp = videobuf_to_vmalloc(&buf->vb);
> +                                       outp = vb2_plane_vaddr(&buf->vb, 0);
>
>                                 /* As long as isoc traffic is arriving, keep
>                                    resetting the timer */
> @@ -656,130 +645,59 @@ static inline int au0828_isoc_copy(struct au0828_dev *dev, struct urb *urb)
>         return rc;
>  }
>
> -static int
> -buffer_setup(struct videobuf_queue *vq, unsigned int *count,
> -            unsigned int *size)
> +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 au0828_fh *fh = vq->priv_data;
> -       *size = (fh->dev->width * fh->dev->height * 16 + 7) >> 3;
> -
> -       if (0 == *count)
> -               *count = AU0828_DEF_BUF;
> +       struct au0828_dev *dev = vb2_get_drv_priv(vq);
> +       unsigned long img_size = dev->height * dev->bytesperline;
> +       unsigned long size;
>
> -       if (*count < AU0828_MIN_BUF)
> -               *count = AU0828_MIN_BUF;
> -       return 0;
> -}
> +       size = fmt ? fmt->fmt.pix.sizeimage : img_size;
> +       if (size < img_size)
> +               return -EINVAL;
>
> -/* This is called *without* dev->slock held; please keep it that way */
> -static void free_buffer(struct videobuf_queue *vq, struct au0828_buffer *buf)
> -{
> -       struct au0828_fh     *fh  = vq->priv_data;
> -       struct au0828_dev    *dev = fh->dev;
> -       unsigned long flags = 0;
> -       if (in_interrupt())
> -               BUG();
> -
> -       /* We used to wait for the buffer to finish here, but this didn't work
> -          because, as we were keeping the state as VIDEOBUF_QUEUED,
> -          videobuf_queue_cancel marked it as finished for us.
> -          (Also, it could wedge forever if the hardware was misconfigured.)
> -
> -          This should be safe; by the time we get here, the buffer isn't
> -          queued anymore. If we ever start marking the buffers as
> -          VIDEOBUF_ACTIVE, it won't be, though.
> -       */
> -       spin_lock_irqsave(&dev->slock, flags);
> -       if (dev->isoc_ctl.buf == buf)
> -               dev->isoc_ctl.buf = NULL;
> -       spin_unlock_irqrestore(&dev->slock, flags);
> +       *nplanes = 1;
> +       sizes[0] = size;
>
> -       videobuf_vmalloc_free(&buf->vb);
> -       buf->vb.state = VIDEOBUF_NEEDS_INIT;
> +       return 0;
>  }
>
>  static int
> -buffer_prepare(struct videobuf_queue *vq, struct videobuf_buffer *vb,
> -                                               enum v4l2_field field)
> +buffer_prepare(struct vb2_buffer *vb)
>  {
> -       struct au0828_fh     *fh  = vq->priv_data;
>         struct au0828_buffer *buf = container_of(vb, struct au0828_buffer, vb);
> -       struct au0828_dev    *dev = fh->dev;
> -       int                  rc = 0, urb_init = 0;
> +       struct au0828_dev    *dev = vb2_get_drv_priv(vb->vb2_queue);
>
> -       buf->vb.size = (fh->dev->width * fh->dev->height * 16 + 7) >> 3;
> +       buf->length = dev->height * dev->bytesperline;
>
> -       if (0 != buf->vb.baddr  &&  buf->vb.bsize < buf->vb.size)
> +       if (vb2_plane_size(vb, 0) < buf->length) {
> +               pr_err("%s data will not fit into plane (%lu < %lu)\n",
> +                       __func__, vb2_plane_size(vb, 0), buf->length);
>                 return -EINVAL;
> -
> -       buf->vb.width  = dev->width;
> -       buf->vb.height = dev->height;
> -       buf->vb.field  = field;
> -
> -       if (VIDEOBUF_NEEDS_INIT == buf->vb.state) {
> -               rc = videobuf_iolock(vq, &buf->vb, NULL);
> -               if (rc < 0) {
> -                       pr_info("videobuf_iolock failed\n");
> -                       goto fail;
> -               }
>         }
> -
> -       if (!dev->isoc_ctl.num_bufs)
> -               urb_init = 1;
> -
> -       if (urb_init) {
> -               rc = au0828_init_isoc(dev, AU0828_ISO_PACKETS_PER_URB,
> -                                     AU0828_MAX_ISO_BUFS, dev->max_pkt_size,
> -                                     au0828_isoc_copy);
> -               if (rc < 0) {
> -                       pr_info("au0828_init_isoc failed\n");
> -                       goto fail;
> -               }
> -       }
> -
> -       buf->vb.state = VIDEOBUF_PREPARED;
> +       vb2_set_plane_payload(&buf->vb, 0, buf->length);
>         return 0;
> -
> -fail:
> -       free_buffer(vq, buf);
> -       return rc;
>  }
>
>  static void
> -buffer_queue(struct videobuf_queue *vq, struct videobuf_buffer *vb)
> +buffer_queue(struct vb2_buffer *vb)
>  {
>         struct au0828_buffer    *buf     = container_of(vb,
>                                                         struct au0828_buffer,
>                                                         vb);
> -       struct au0828_fh        *fh      = vq->priv_data;
> -       struct au0828_dev       *dev     = fh->dev;
> +       struct au0828_dev       *dev     = vb2_get_drv_priv(vb->vb2_queue);
>         struct au0828_dmaqueue  *vidq    = &dev->vidq;
> +       unsigned long flags = 0;
>
> -       buf->vb.state = VIDEOBUF_QUEUED;
> -       list_add_tail(&buf->vb.queue, &vidq->active);
> -}
> -
> -static void buffer_release(struct videobuf_queue *vq,
> -                               struct videobuf_buffer *vb)
> -{
> -       struct au0828_buffer   *buf  = container_of(vb,
> -                                                   struct au0828_buffer,
> -                                                   vb);
> +       buf->mem = vb2_plane_vaddr(vb, 0);
> +       buf->length = vb2_plane_size(vb, 0);
>
> -       free_buffer(vq, buf);
> +       spin_lock_irqsave(&dev->slock, flags);
> +       list_add_tail(&buf->list, &vidq->active);
> +       spin_unlock_irqrestore(&dev->slock, flags);
>  }
>
> -static struct videobuf_queue_ops au0828_video_qops = {
> -       .buf_setup      = buffer_setup,
> -       .buf_prepare    = buffer_prepare,
> -       .buf_queue      = buffer_queue,
> -       .buf_release    = buffer_release,
> -};
> -
> -/* ------------------------------------------------------------------
> -   V4L2 interface
> -   ------------------------------------------------------------------*/
> -
>  static int au0828_i2s_init(struct au0828_dev *dev)
>  {
>         /* Enable i2s mode */
> @@ -828,7 +746,7 @@ static int au0828_analog_stream_enable(struct au0828_dev *d)
>         return 0;
>  }
>
> -int au0828_analog_stream_disable(struct au0828_dev *d)
> +static int au0828_analog_stream_disable(struct au0828_dev *d)
>  {
>         dprintk(1, "au0828_analog_stream_disable called\n");
>         au0828_writereg(d, AU0828_SENSORCTRL_100, 0x0);
> @@ -861,78 +779,141 @@ static int au0828_stream_interrupt(struct au0828_dev *dev)
>         return 0;
>  }
>
> -/*
> - * au0828_release_resources
> - * unregister v4l2 devices
> - */
> -void au0828_analog_unregister(struct au0828_dev *dev)
> +int au0828_start_analog_streaming(struct vb2_queue *vq, unsigned int count)
>  {
> -       dprintk(1, "au0828_release_resources called\n");
> -       mutex_lock(&au0828_sysfs_lock);
> +       struct au0828_dev *dev = vb2_get_drv_priv(vq);
> +       int rc = 0;
>
> -       if (dev->vdev)
> -               video_unregister_device(dev->vdev);
> -       if (dev->vbi_dev)
> -               video_unregister_device(dev->vbi_dev);
> +       dprintk(1, "au0828_start_analog_streaming called %d\n",
> +               dev->streaming_users);
>
> -       mutex_unlock(&au0828_sysfs_lock);
> -}
> +       if (vq->type == V4L2_BUF_TYPE_VIDEO_CAPTURE)
> +               dev->frame_count = 0;
> +       else
> +               dev->vbi_frame_count = 0;
> +
> +       if (dev->streaming_users == 0) {
> +               /* If we were doing ac97 instead of i2s, it would go here...*/
> +               au0828_i2s_init(dev);
> +               rc = au0828_init_isoc(dev, AU0828_ISO_PACKETS_PER_URB,
> +                                  AU0828_MAX_ISO_BUFS, dev->max_pkt_size,
> +                                  au0828_isoc_copy);
> +               if (rc < 0) {
> +                       pr_info("au0828_init_isoc failed\n");
> +                       return rc;
> +               }
>
> +               if (vq->type == V4L2_BUF_TYPE_VIDEO_CAPTURE) {
> +                       v4l2_device_call_all(&dev->v4l2_dev, 0, video,
> +                                               s_stream, 1);
> +                       dev->vid_timeout_running = 1;
> +                       mod_timer(&dev->vid_timeout, jiffies + (HZ / 10));
> +               } else if (vq->type == V4L2_BUF_TYPE_VBI_CAPTURE) {
> +                       dev->vbi_timeout_running = 1;
> +                       mod_timer(&dev->vbi_timeout, jiffies + (HZ / 10));
> +               }
> +       }
> +       dev->streaming_users++;
> +       return rc;
> +}
>
> -/* Usage lock check functions */
> -static int res_get(struct au0828_fh *fh, unsigned int bit)
> +static void au0828_stop_streaming(struct vb2_queue *vq)
>  {
> -       struct au0828_dev    *dev = fh->dev;
> +       struct au0828_dev *dev = vb2_get_drv_priv(vq);
> +       struct au0828_dmaqueue *vidq = &dev->vidq;
> +       unsigned long flags = 0;
> +       int i;
>
> -       if (fh->resources & bit)
> -               /* have it already allocated */
> -               return 1;
> +       dprintk(1, "au0828_stop_streaming called %d\n", dev->streaming_users);
>
> -       /* is it free? */
> -       if (dev->resources & bit) {
> -               /* no, someone else uses it */
> -               return 0;
> +       if (dev->streaming_users-- == 1)
> +               au0828_uninit_isoc(dev);
> +
> +       spin_lock_irqsave(&dev->slock, flags);
> +       if (dev->isoc_ctl.buf != NULL) {
> +               vb2_buffer_done(&dev->isoc_ctl.buf->vb, VB2_BUF_STATE_ERROR);
> +               dev->isoc_ctl.buf = NULL;
>         }
> -       /* it's free, grab it */
> -       fh->resources  |= bit;
> -       dev->resources |= bit;
> -       dprintk(1, "res: get %d\n", bit);
> +       while (!list_empty(&vidq->active)) {
> +               struct au0828_buffer *buf;
>
> -       return 1;
> -}
> +               buf = list_entry(vidq->active.next, struct au0828_buffer, list);
> +               vb2_buffer_done(&buf->vb, VB2_BUF_STATE_ERROR);
> +               list_del(&buf->list);
> +       }
>
> -static int res_check(struct au0828_fh *fh, unsigned int bit)
> -{
> -       return fh->resources & bit;
> -}
> +       v4l2_device_call_all(&dev->v4l2_dev, 0, video, s_stream, 0);
>
> -static int res_locked(struct au0828_dev *dev, unsigned int bit)
> -{
> -       return dev->resources & bit;
> +       for (i = 0; i < AU0828_MAX_INPUT; i++) {
> +               if (AUVI_INPUT(i).audio_setup == NULL)
> +                       continue;
> +               (AUVI_INPUT(i).audio_setup)(dev, 0);
> +       }
> +       spin_unlock_irqrestore(&dev->slock, flags);
> +
> +       dev->vid_timeout_running = 0;
> +       del_timer_sync(&dev->vid_timeout);
>  }
>
> -static void res_free(struct au0828_fh *fh, unsigned int bits)
> +void au0828_stop_vbi_streaming(struct vb2_queue *vq)
>  {
> -       struct au0828_dev    *dev = fh->dev;
> +       struct au0828_dev *dev = vb2_get_drv_priv(vq);
> +       struct au0828_dmaqueue *vbiq = &dev->vbiq;
> +       unsigned long flags = 0;
> +
> +       dprintk(1, "au0828_stop_vbi_streaming called %d\n",
> +               dev->streaming_users);
> +
> +       if (dev->streaming_users-- == 1)
> +               au0828_uninit_isoc(dev);
>
> -       BUG_ON((fh->resources & bits) != bits);
> +       spin_lock_irqsave(&dev->slock, flags);
> +       if (dev->isoc_ctl.vbi_buf != NULL) {
> +               vb2_buffer_done(&dev->isoc_ctl.vbi_buf->vb,
> +                               VB2_BUF_STATE_ERROR);
> +               dev->isoc_ctl.vbi_buf = NULL;
> +       }
> +       while (!list_empty(&vbiq->active)) {
> +               struct au0828_buffer *buf;
> +
> +               buf = list_entry(vbiq->active.next, struct au0828_buffer, list);
> +               list_del(&buf->list);
> +               vb2_buffer_done(&buf->vb, VB2_BUF_STATE_ERROR);
> +       }
> +       spin_unlock_irqrestore(&dev->slock, flags);
>
> -       fh->resources  &= ~bits;
> -       dev->resources &= ~bits;
> -       dprintk(1, "res: put %d\n", bits);
> +       dev->vbi_timeout_running = 0;
> +       del_timer_sync(&dev->vbi_timeout);
>  }
>
> -static int get_ressource(struct au0828_fh *fh)
> +static struct vb2_ops au0828_video_qops = {
> +       .queue_setup     = queue_setup,
> +       .buf_prepare     = buffer_prepare,
> +       .buf_queue       = buffer_queue,
> +       .start_streaming = au0828_start_analog_streaming,
> +       .stop_streaming  = au0828_stop_streaming,
> +       .wait_prepare    = vb2_ops_wait_prepare,
> +       .wait_finish     = vb2_ops_wait_finish,
> +};
> +
> +/* ------------------------------------------------------------------
> +   V4L2 interface
> +   ------------------------------------------------------------------*/
> +/*
> + * au0828_analog_unregister
> + * unregister v4l2 devices
> + */
> +void au0828_analog_unregister(struct au0828_dev *dev)
>  {
> -       switch (fh->type) {
> -       case V4L2_BUF_TYPE_VIDEO_CAPTURE:
> -               return AU0828_RESOURCE_VIDEO;
> -       case V4L2_BUF_TYPE_VBI_CAPTURE:
> -               return AU0828_RESOURCE_VBI;
> -       default:
> -               BUG();
> -               return 0;
> -       }
> +       dprintk(1, "au0828_analog_unregister called\n");
> +       mutex_lock(&au0828_sysfs_lock);
> +
> +       if (dev->vdev)
> +               video_unregister_device(dev->vdev);
> +       if (dev->vbi_dev)
> +               video_unregister_device(dev->vbi_dev);
> +
> +       mutex_unlock(&au0828_sysfs_lock);
>  }
>
>  /* This function ensures that video frames continue to be delivered even if
> @@ -950,8 +931,8 @@ static void au0828_vid_buffer_timeout(unsigned long data)
>
>         buf = dev->isoc_ctl.buf;
>         if (buf != NULL) {
> -               vid_data = videobuf_to_vmalloc(&buf->vb);
> -               memset(vid_data, 0x00, buf->vb.size); /* Blank green frame */
> +               vid_data = vb2_plane_vaddr(&buf->vb, 0);
> +               memset(vid_data, 0x00, buf->length); /* Blank green frame */
>                 buffer_filled(dev, dma_q, buf);
>         }
>         get_next_buf(dma_q, &buf);
> @@ -974,8 +955,8 @@ static void au0828_vbi_buffer_timeout(unsigned long data)
>
>         buf = dev->isoc_ctl.vbi_buf;
>         if (buf != NULL) {
> -               vbi_data = videobuf_to_vmalloc(&buf->vb);
> -               memset(vbi_data, 0x00, buf->vb.size);
> +               vbi_data = vb2_plane_vaddr(&buf->vb, 0);
> +               memset(vbi_data, 0x00, buf->length);
>                 vbi_buffer_filled(dev, dma_q, buf);
>         }
>         vbi_get_next_buf(dma_q, &buf);
> @@ -985,105 +966,65 @@ static void au0828_vbi_buffer_timeout(unsigned long data)
>         spin_unlock_irqrestore(&dev->slock, flags);
>  }
>
> -
>  static int au0828_v4l2_open(struct file *filp)
>  {
> -       int ret = 0;
> -       struct video_device *vdev = video_devdata(filp);
>         struct au0828_dev *dev = video_drvdata(filp);
> -       struct au0828_fh *fh;
> -       int type;
> -
> -       switch (vdev->vfl_type) {
> -       case VFL_TYPE_GRABBER:
> -               type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
> -               break;
> -       case VFL_TYPE_VBI:
> -               type = V4L2_BUF_TYPE_VBI_CAPTURE;
> -               break;
> -       default:
> -               return -EINVAL;
> -       }
> -
> -       fh = kzalloc(sizeof(struct au0828_fh), GFP_KERNEL);
> -       if (NULL == fh) {
> -               dprintk(1, "Failed allocate au0828_fh struct!\n");
> -               return -ENOMEM;
> -       }
> +       int ret = 0;
>
No need to assign it to zero.

> -       fh->type = type;
> -       fh->dev = dev;
> -       v4l2_fh_init(&fh->fh, vdev);
> -       filp->private_data = fh;
> +       dprintk(1,
> +               "%s called std_set %d dev_state %d stream users %d users %d\n",
> +               __func__, dev->std_set_in_tuner_core, dev->dev_state,
> +               dev->streaming_users, dev->users);
>
> -       if (mutex_lock_interruptible(&dev->lock)) {
> -               kfree(fh);
> +       if (mutex_lock_interruptible(&dev->lock))
>                 return -ERESTARTSYS;
> +
> +       ret = v4l2_fh_open(filp);
> +       if (ret) {
> +               au0828_isocdbg("%s: v4l2_fh_open() returned error %d\n",
> +                               __func__, ret);
> +               mutex_unlock(&dev->lock);
> +               return ret;
>         }
> +
>         if (dev->users == 0) {

you can use v4l2_fh_is_singular_file() and get rid of users member ?

Thanks,
--Prabhakar Lad

>                 au0828_analog_stream_enable(dev);
>                 au0828_analog_stream_reset(dev);
> -
> -               /* If we were doing ac97 instead of i2s, it would go here...*/
> -               au0828_i2s_init(dev);
> -
>                 dev->stream_state = STREAM_OFF;
>                 dev->dev_state |= DEV_INITIALIZED;
>         }
> -
>         dev->users++;
>         mutex_unlock(&dev->lock);
> -
> -       videobuf_queue_vmalloc_init(&fh->vb_vidq, &au0828_video_qops,
> -                                   NULL, &dev->slock,
> -                                   V4L2_BUF_TYPE_VIDEO_CAPTURE,
> -                                   V4L2_FIELD_INTERLACED,
> -                                   sizeof(struct au0828_buffer), fh,
> -                                   &dev->lock);
> -
> -       /* VBI Setup */
> -       videobuf_queue_vmalloc_init(&fh->vb_vbiq, &au0828_vbi_qops,
> -                                   NULL, &dev->slock,
> -                                   V4L2_BUF_TYPE_VBI_CAPTURE,
> -                                   V4L2_FIELD_SEQ_TB,
> -                                   sizeof(struct au0828_buffer), fh,
> -                                   &dev->lock);
> -       v4l2_fh_add(&fh->fh);
>         return ret;
>  }
>
>  static int au0828_v4l2_close(struct file *filp)
>  {
>         int ret;
> -       struct au0828_fh *fh = filp->private_data;
> -       struct au0828_dev *dev = fh->dev;
> +       struct au0828_dev *dev = video_drvdata(filp);
> +       struct video_device *vdev = video_devdata(filp);
> +
> +       dprintk(1,
> +               "%s called std_set %d dev_state %d stream users %d users %d\n",
> +               __func__, dev->std_set_in_tuner_core, dev->dev_state,
> +               dev->streaming_users, dev->users);
>
> -       v4l2_fh_del(&fh->fh);
> -       v4l2_fh_exit(&fh->fh);
>         mutex_lock(&dev->lock);
> -       if (res_check(fh, AU0828_RESOURCE_VIDEO)) {
> +       if (vdev->vfl_type == VFL_TYPE_GRABBER && dev->vid_timeout_running) {
>                 /* Cancel timeout thread in case they didn't call streamoff */
>                 dev->vid_timeout_running = 0;
>                 del_timer_sync(&dev->vid_timeout);
> -
> -               videobuf_stop(&fh->vb_vidq);
> -               res_free(fh, AU0828_RESOURCE_VIDEO);
> -       }
> -
> -       if (res_check(fh, AU0828_RESOURCE_VBI)) {
> +       } else if (vdev->vfl_type == VFL_TYPE_VBI &&
> +                       dev->vbi_timeout_running) {
>                 /* Cancel timeout thread in case they didn't call streamoff */
>                 dev->vbi_timeout_running = 0;
>                 del_timer_sync(&dev->vbi_timeout);
> -
> -               videobuf_stop(&fh->vb_vbiq);
> -               res_free(fh, AU0828_RESOURCE_VBI);
>         }
>
> -       if (dev->users == 1 && video_is_registered(video_devdata(filp))) {
> -               au0828_analog_stream_disable(dev);
> -
> -               au0828_uninit_isoc(dev);
> +       if (dev->dev_state == DEV_DISCONNECTED)
> +               goto end;
>
> +       if (dev->users == 1) {
>                 /* Save some power by putting tuner to sleep */
>                 v4l2_device_call_all(&dev->v4l2_dev, 0, core, s_power, 0);
>                 dev->std_set_in_tuner_core = 0;
> @@ -1094,13 +1035,10 @@ static int au0828_v4l2_close(struct file *filp)
>                 if (ret < 0)
>                         pr_info("Au0828 can't set alternate to 0!\n");
>         }
> -       mutex_unlock(&dev->lock);
> -
> -       videobuf_mmap_free(&fh->vb_vidq);
> -       videobuf_mmap_free(&fh->vb_vbiq);
> -       kfree(fh);
> +end:
> +       _vb2_fop_release(filp, NULL);
>         dev->users--;
> -       wake_up_interruptible_nr(&dev->open, 1);
> +       mutex_unlock(&dev->lock);
>         return 0;
>  }
>
> @@ -1112,6 +1050,9 @@ static void au0828_init_tuner(struct au0828_dev *dev)
>                 .type = V4L2_TUNER_ANALOG_TV,
>         };
>
> +       dprintk(1, "%s called std_set %d dev_state %d\n", __func__,
> +               dev->std_set_in_tuner_core, dev->dev_state);
> +
>         if (dev->std_set_in_tuner_core)
>                 return;
>         dev->std_set_in_tuner_core = 1;
> @@ -1124,98 +1065,6 @@ static void au0828_init_tuner(struct au0828_dev *dev)
>         i2c_gate_ctrl(dev, 0);
>  }
>
> -static ssize_t au0828_v4l2_read(struct file *filp, char __user *buf,
> -                               size_t count, loff_t *pos)
> -{
> -       struct au0828_fh *fh = filp->private_data;
> -       struct au0828_dev *dev = fh->dev;
> -       int rc;
> -
> -       rc = check_dev(dev);
> -       if (rc < 0)
> -               return rc;
> -
> -       if (mutex_lock_interruptible(&dev->lock))
> -               return -ERESTARTSYS;
> -       au0828_init_tuner(dev);
> -       mutex_unlock(&dev->lock);
> -
> -       if (fh->type == V4L2_BUF_TYPE_VIDEO_CAPTURE) {
> -               if (res_locked(dev, AU0828_RESOURCE_VIDEO))
> -                       return -EBUSY;
> -
> -               return videobuf_read_stream(&fh->vb_vidq, buf, count, pos, 0,
> -                                       filp->f_flags & O_NONBLOCK);
> -       }
> -
> -       if (fh->type == V4L2_BUF_TYPE_VBI_CAPTURE) {
> -               if (!res_get(fh, AU0828_RESOURCE_VBI))
> -                       return -EBUSY;
> -
> -               if (dev->vbi_timeout_running == 0) {
> -                       /* Handle case where caller tries to read without
> -                          calling streamon first */
> -                       dev->vbi_timeout_running = 1;
> -                       mod_timer(&dev->vbi_timeout, jiffies + (HZ / 10));
> -               }
> -
> -               return videobuf_read_stream(&fh->vb_vbiq, buf, count, pos, 0,
> -                                           filp->f_flags & O_NONBLOCK);
> -       }
> -
> -       return 0;
> -}
> -
> -static unsigned int au0828_v4l2_poll(struct file *filp, poll_table *wait)
> -{
> -       struct au0828_fh *fh = filp->private_data;
> -       struct au0828_dev *dev = fh->dev;
> -       unsigned long req_events = poll_requested_events(wait);
> -       unsigned int res;
> -
> -       if (check_dev(dev) < 0)
> -               return POLLERR;
> -
> -       res = v4l2_ctrl_poll(filp, wait);
> -       if (!(req_events & (POLLIN | POLLRDNORM)))
> -               return res;
> -
> -       if (mutex_lock_interruptible(&dev->lock))
> -               return -ERESTARTSYS;
> -       au0828_init_tuner(dev);
> -       mutex_unlock(&dev->lock);
> -
> -       if (fh->type == V4L2_BUF_TYPE_VIDEO_CAPTURE) {
> -               if (!res_get(fh, AU0828_RESOURCE_VIDEO))
> -                       return POLLERR;
> -               return res | videobuf_poll_stream(filp, &fh->vb_vidq, wait);
> -       }
> -       if (fh->type == V4L2_BUF_TYPE_VBI_CAPTURE) {
> -               if (!res_get(fh, AU0828_RESOURCE_VBI))
> -                       return POLLERR;
> -               return res | videobuf_poll_stream(filp, &fh->vb_vbiq, wait);
> -       }
> -       return POLLERR;
> -}
> -
> -static int au0828_v4l2_mmap(struct file *filp, struct vm_area_struct *vma)
> -{
> -       struct au0828_fh *fh    = filp->private_data;
> -       struct au0828_dev *dev   = fh->dev;
> -       int              rc;
> -
> -       rc = check_dev(dev);
> -       if (rc < 0)
> -               return rc;
> -
> -       if (fh->type == V4L2_BUF_TYPE_VIDEO_CAPTURE)
> -               rc = videobuf_mmap_mapper(&fh->vb_vidq, vma);
> -       else if (fh->type == V4L2_BUF_TYPE_VBI_CAPTURE)
> -               rc = videobuf_mmap_mapper(&fh->vb_vbiq, vma);
> -
> -       return rc;
> -}
> -
>  static int au0828_set_format(struct au0828_dev *dev, unsigned int cmd,
>                              struct v4l2_format *format)
>  {
> @@ -1267,13 +1116,14 @@ static int au0828_set_format(struct au0828_dev *dev, unsigned int cmd,
>         return 0;
>  }
>
> -
>  static int vidioc_querycap(struct file *file, void  *priv,
>                            struct v4l2_capability *cap)
>  {
>         struct video_device *vdev = video_devdata(file);
> -       struct au0828_fh *fh = priv;
> -       struct au0828_dev *dev = fh->dev;
> +       struct au0828_dev *dev = video_drvdata(file);
> +
> +       dprintk(1, "%s called std_set %d dev_state %d\n", __func__,
> +               dev->std_set_in_tuner_core, dev->dev_state);
>
>         strlcpy(cap->driver, "au0828", sizeof(cap->driver));
>         strlcpy(cap->card, dev->board.name, sizeof(cap->card));
> @@ -1299,6 +1149,8 @@ static int vidioc_enum_fmt_vid_cap(struct file *file, void  *priv,
>         if (f->index)
>                 return -EINVAL;
>
> +       dprintk(1, "%s called\n", __func__);
> +
>         f->type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
>         strcpy(f->description, "Packed YUV2");
>
> @@ -1311,8 +1163,10 @@ static int vidioc_enum_fmt_vid_cap(struct file *file, void  *priv,
>  static int vidioc_g_fmt_vid_cap(struct file *file, void *priv,
>                                         struct v4l2_format *f)
>  {
> -       struct au0828_fh *fh  = priv;
> -       struct au0828_dev *dev = fh->dev;
> +       struct au0828_dev *dev = video_drvdata(file);
> +
> +       dprintk(1, "%s called std_set %d dev_state %d\n", __func__,
> +               dev->std_set_in_tuner_core, dev->dev_state);
>
>         f->fmt.pix.width = dev->width;
>         f->fmt.pix.height = dev->height;
> @@ -1328,8 +1182,10 @@ static int vidioc_g_fmt_vid_cap(struct file *file, void *priv,
>  static int vidioc_try_fmt_vid_cap(struct file *file, void *priv,
>                                   struct v4l2_format *f)
>  {
> -       struct au0828_fh *fh  = priv;
> -       struct au0828_dev *dev = fh->dev;
> +       struct au0828_dev *dev = video_drvdata(file);
> +
> +       dprintk(1, "%s called std_set %d dev_state %d\n", __func__,
> +               dev->std_set_in_tuner_core, dev->dev_state);
>
>         return au0828_set_format(dev, VIDIOC_TRY_FMT, f);
>  }
> @@ -1337,15 +1193,17 @@ static int vidioc_try_fmt_vid_cap(struct file *file, void *priv,
>  static int vidioc_s_fmt_vid_cap(struct file *file, void *priv,
>                                 struct v4l2_format *f)
>  {
> -       struct au0828_fh *fh  = priv;
> -       struct au0828_dev *dev = fh->dev;
> +       struct au0828_dev *dev = video_drvdata(file);
>         int rc;
>
> +       dprintk(1, "%s called std_set %d dev_state %d\n", __func__,
> +               dev->std_set_in_tuner_core, dev->dev_state);
> +
>         rc = check_dev(dev);
>         if (rc < 0)
>                 return rc;
>
> -       if (videobuf_queue_is_busy(&fh->vb_vidq)) {
> +       if (vb2_is_busy(&dev->vb_vidq)) {
>                 pr_info("%s queue busy\n", __func__);
>                 rc = -EBUSY;
>                 goto out;
> @@ -1358,8 +1216,16 @@ out:
>
>  static int vidioc_s_std(struct file *file, void *priv, v4l2_std_id norm)
>  {
> -       struct au0828_fh *fh = priv;
> -       struct au0828_dev *dev = fh->dev;
> +       struct au0828_dev *dev = video_drvdata(file);
> +
> +       dprintk(1, "%s called std_set %d dev_state %d\n", __func__,
> +               dev->std_set_in_tuner_core, dev->dev_state);
> +
> +       if (norm == dev->std)
> +               return 0;
> +
> +       if (dev->streaming_users > 0)
> +               return -EBUSY;
>
>         dev->std = norm;
>
> @@ -1382,8 +1248,10 @@ static int vidioc_s_std(struct file *file, void *priv, v4l2_std_id norm)
>
>  static int vidioc_g_std(struct file *file, void *priv, v4l2_std_id *norm)
>  {
> -       struct au0828_fh *fh = priv;
> -       struct au0828_dev *dev = fh->dev;
> +       struct au0828_dev *dev = video_drvdata(file);
> +
> +       dprintk(1, "%s called std_set %d dev_state %d\n", __func__,
> +               dev->std_set_in_tuner_core, dev->dev_state);
>
>         *norm = dev->std;
>         return 0;
> @@ -1392,8 +1260,7 @@ static int vidioc_g_std(struct file *file, void *priv, v4l2_std_id *norm)
>  static int vidioc_enum_input(struct file *file, void *priv,
>                                 struct v4l2_input *input)
>  {
> -       struct au0828_fh *fh = priv;
> -       struct au0828_dev *dev = fh->dev;
> +       struct au0828_dev *dev = video_drvdata(file);
>         unsigned int tmp;
>
>         static const char *inames[] = {
> @@ -1406,6 +1273,9 @@ static int vidioc_enum_input(struct file *file, void *priv,
>                 [AU0828_VMUX_DEBUG] = "tv debug"
>         };
>
> +       dprintk(1, "%s called std_set %d dev_state %d\n", __func__,
> +               dev->std_set_in_tuner_core, dev->dev_state);
> +
>         tmp = input->index;
>
>         if (tmp >= AU0828_MAX_INPUT)
> @@ -1431,8 +1301,11 @@ static int vidioc_enum_input(struct file *file, void *priv,
>
>  static int vidioc_g_input(struct file *file, void *priv, unsigned int *i)
>  {
> -       struct au0828_fh *fh = priv;
> -       struct au0828_dev *dev = fh->dev;
> +       struct au0828_dev *dev = video_drvdata(file);
> +
> +       dprintk(1, "%s called std_set %d dev_state %d\n", __func__,
> +               dev->std_set_in_tuner_core, dev->dev_state);
> +
>         *i = dev->ctrl_input;
>         return 0;
>  }
> @@ -1441,6 +1314,9 @@ static void au0828_s_input(struct au0828_dev *dev, int index)
>  {
>         int i;
>
> +       dprintk(1, "%s called std_set %d dev_state %d\n", __func__,
> +               dev->std_set_in_tuner_core, dev->dev_state);
> +
>         switch (AUVI_INPUT(index).type) {
>         case AU0828_VMUX_SVIDEO:
>                 dev->input_type = AU0828_VMUX_SVIDEO;
> @@ -1490,8 +1366,7 @@ static void au0828_s_input(struct au0828_dev *dev, int index)
>
>  static int vidioc_s_input(struct file *file, void *priv, unsigned int index)
>  {
> -       struct au0828_fh *fh = priv;
> -       struct au0828_dev *dev = fh->dev;
> +       struct au0828_dev *dev = video_drvdata(file);
>
>         dprintk(1, "VIDIOC_S_INPUT in function %s, input=%d\n", __func__,
>                 index);
> @@ -1509,6 +1384,8 @@ static int vidioc_enumaudio(struct file *file, void *priv, struct v4l2_audio *a)
>         if (a->index > 1)
>                 return -EINVAL;
>
> +       dprintk(1, "%s called\n", __func__);
> +
>         if (a->index == 0)
>                 strcpy(a->name, "Television");
>         else
> @@ -1520,8 +1397,10 @@ static int vidioc_enumaudio(struct file *file, void *priv, struct v4l2_audio *a)
>
>  static int vidioc_g_audio(struct file *file, void *priv, struct v4l2_audio *a)
>  {
> -       struct au0828_fh *fh = priv;
> -       struct au0828_dev *dev = fh->dev;
> +       struct au0828_dev *dev = video_drvdata(file);
> +
> +       dprintk(1, "%s called std_set %d dev_state %d\n", __func__,
> +               dev->std_set_in_tuner_core, dev->dev_state);
>
>         a->index = dev->ctrl_ainput;
>         if (a->index == 0)
> @@ -1535,22 +1414,26 @@ static int vidioc_g_audio(struct file *file, void *priv, struct v4l2_audio *a)
>
>  static int vidioc_s_audio(struct file *file, void *priv, const struct v4l2_audio *a)
>  {
> -       struct au0828_fh *fh = priv;
> -       struct au0828_dev *dev = fh->dev;
> +       struct au0828_dev *dev = video_drvdata(file);
>
>         if (a->index != dev->ctrl_ainput)
>                 return -EINVAL;
> +
> +       dprintk(1, "%s called std_set %d dev_state %d\n", __func__,
> +               dev->std_set_in_tuner_core, dev->dev_state);
>         return 0;
>  }
>
>  static int vidioc_g_tuner(struct file *file, void *priv, struct v4l2_tuner *t)
>  {
> -       struct au0828_fh *fh = priv;
> -       struct au0828_dev *dev = fh->dev;
> +       struct au0828_dev *dev = video_drvdata(file);
>
>         if (t->index != 0)
>                 return -EINVAL;
>
> +       dprintk(1, "%s called std_set %d dev_state %d\n", __func__,
> +               dev->std_set_in_tuner_core, dev->dev_state);
> +
>         strcpy(t->name, "Auvitek tuner");
>
>         au0828_init_tuner(dev);
> @@ -1563,12 +1446,14 @@ static int vidioc_g_tuner(struct file *file, void *priv, struct v4l2_tuner *t)
>  static int vidioc_s_tuner(struct file *file, void *priv,
>                                 const struct v4l2_tuner *t)
>  {
> -       struct au0828_fh *fh = priv;
> -       struct au0828_dev *dev = fh->dev;
> +       struct au0828_dev *dev = video_drvdata(file);
>
>         if (t->index != 0)
>                 return -EINVAL;
>
> +       dprintk(1, "%s called std_set %d dev_state %d\n", __func__,
> +               dev->std_set_in_tuner_core, dev->dev_state);
> +
>         au0828_init_tuner(dev);
>         i2c_gate_ctrl(dev, 1);
>         v4l2_device_call_all(&dev->v4l2_dev, 0, tuner, s_tuner, t);
> @@ -1584,11 +1469,12 @@ static int vidioc_s_tuner(struct file *file, void *priv,
>  static int vidioc_g_frequency(struct file *file, void *priv,
>                                 struct v4l2_frequency *freq)
>  {
> -       struct au0828_fh *fh = priv;
> -       struct au0828_dev *dev = fh->dev;
> +       struct au0828_dev *dev = video_drvdata(file);
>
>         if (freq->tuner != 0)
>                 return -EINVAL;
> +       dprintk(1, "%s called std_set %d dev_state %d\n", __func__,
> +               dev->std_set_in_tuner_core, dev->dev_state);
>         freq->frequency = dev->ctrl_freq;
>         return 0;
>  }
> @@ -1596,13 +1482,15 @@ static int vidioc_g_frequency(struct file *file, void *priv,
>  static int vidioc_s_frequency(struct file *file, void *priv,
>                                 const struct v4l2_frequency *freq)
>  {
> -       struct au0828_fh *fh = priv;
> -       struct au0828_dev *dev = fh->dev;
> +       struct au0828_dev *dev = video_drvdata(file);
>         struct v4l2_frequency new_freq = *freq;
>
>         if (freq->tuner != 0)
>                 return -EINVAL;
>
> +       dprintk(1, "%s called std_set %d dev_state %d\n", __func__,
> +               dev->std_set_in_tuner_core, dev->dev_state);
> +
>         au0828_init_tuner(dev);
>         i2c_gate_ctrl(dev, 1);
>
> @@ -1624,8 +1512,10 @@ static int vidioc_s_frequency(struct file *file, void *priv,
>  static int vidioc_g_fmt_vbi_cap(struct file *file, void *priv,
>                                 struct v4l2_format *format)
>  {
> -       struct au0828_fh      *fh  = priv;
> -       struct au0828_dev     *dev = fh->dev;
> +       struct au0828_dev *dev = video_drvdata(file);
> +
> +       dprintk(1, "%s called std_set %d dev_state %d\n", __func__,
> +               dev->std_set_in_tuner_core, dev->dev_state);
>
>         format->fmt.vbi.samples_per_line = dev->vbi_width;
>         format->fmt.vbi.sample_format = V4L2_PIX_FMT_GREY;
> @@ -1645,12 +1535,14 @@ static int vidioc_g_fmt_vbi_cap(struct file *file, void *priv,
>  static int vidioc_cropcap(struct file *file, void *priv,
>                           struct v4l2_cropcap *cc)
>  {
> -       struct au0828_fh *fh = priv;
> -       struct au0828_dev *dev = fh->dev;
> +       struct au0828_dev *dev = video_drvdata(file);
>
>         if (cc->type != V4L2_BUF_TYPE_VIDEO_CAPTURE)
>                 return -EINVAL;
>
> +       dprintk(1, "%s called std_set %d dev_state %d\n", __func__,
> +               dev->std_set_in_tuner_core, dev->dev_state);
> +
>         cc->bounds.left = 0;
>         cc->bounds.top = 0;
>         cc->bounds.width = dev->width;
> @@ -1664,105 +1556,14 @@ static int vidioc_cropcap(struct file *file, void *priv,
>         return 0;
>  }
>
> -static int vidioc_streamon(struct file *file, void *priv,
> -                          enum v4l2_buf_type type)
> -{
> -       struct au0828_fh      *fh  = priv;
> -       struct au0828_dev     *dev = fh->dev;
> -       int                   rc = -EINVAL;
> -
> -       rc = check_dev(dev);
> -       if (rc < 0)
> -               return rc;
> -
> -       if (unlikely(type != fh->type))
> -               return -EINVAL;
> -
> -       dprintk(1, "vidioc_streamon fh=%p t=%d fh->res=%d dev->res=%d\n",
> -               fh, type, fh->resources, dev->resources);
> -
> -       if (unlikely(!res_get(fh, get_ressource(fh))))
> -               return -EBUSY;
> -
> -       au0828_init_tuner(dev);
> -       if (type == V4L2_BUF_TYPE_VIDEO_CAPTURE) {
> -               au0828_analog_stream_enable(dev);
> -               v4l2_device_call_all(&dev->v4l2_dev, 0, video, s_stream, 1);
> -       }
> -
> -       if (fh->type == V4L2_BUF_TYPE_VIDEO_CAPTURE) {
> -               rc = videobuf_streamon(&fh->vb_vidq);
> -               dev->vid_timeout_running = 1;
> -               mod_timer(&dev->vid_timeout, jiffies + (HZ / 10));
> -       } else if (fh->type == V4L2_BUF_TYPE_VBI_CAPTURE) {
> -               rc = videobuf_streamon(&fh->vb_vbiq);
> -               dev->vbi_timeout_running = 1;
> -               mod_timer(&dev->vbi_timeout, jiffies + (HZ / 10));
> -       }
> -
> -       return rc;
> -}
> -
> -static int vidioc_streamoff(struct file *file, void *priv,
> -                           enum v4l2_buf_type type)
> -{
> -       struct au0828_fh      *fh  = priv;
> -       struct au0828_dev     *dev = fh->dev;
> -       int                   rc;
> -       int                   i;
> -
> -       rc = check_dev(dev);
> -       if (rc < 0)
> -               return rc;
> -
> -       if (fh->type != V4L2_BUF_TYPE_VIDEO_CAPTURE &&
> -           fh->type != V4L2_BUF_TYPE_VBI_CAPTURE)
> -               return -EINVAL;
> -       if (type != fh->type)
> -               return -EINVAL;
> -
> -       dprintk(1, "vidioc_streamoff fh=%p t=%d fh->res=%d dev->res=%d\n",
> -               fh, type, fh->resources, dev->resources);
> -
> -       if (fh->type == V4L2_BUF_TYPE_VIDEO_CAPTURE) {
> -               dev->vid_timeout_running = 0;
> -               del_timer_sync(&dev->vid_timeout);
> -
> -               au0828_analog_stream_disable(dev);
> -               v4l2_device_call_all(&dev->v4l2_dev, 0, video, s_stream, 0);
> -               rc = au0828_stream_interrupt(dev);
> -               if (rc != 0)
> -                       return rc;
> -
> -               for (i = 0; i < AU0828_MAX_INPUT; i++) {
> -                       if (AUVI_INPUT(i).audio_setup == NULL)
> -                               continue;
> -                       (AUVI_INPUT(i).audio_setup)(dev, 0);
> -               }
> -
> -               if (res_check(fh, AU0828_RESOURCE_VIDEO)) {
> -                       videobuf_streamoff(&fh->vb_vidq);
> -                       res_free(fh, AU0828_RESOURCE_VIDEO);
> -               }
> -       } else if (fh->type == V4L2_BUF_TYPE_VBI_CAPTURE) {
> -               dev->vbi_timeout_running = 0;
> -               del_timer_sync(&dev->vbi_timeout);
> -
> -               if (res_check(fh, AU0828_RESOURCE_VBI)) {
> -                       videobuf_streamoff(&fh->vb_vbiq);
> -                       res_free(fh, AU0828_RESOURCE_VBI);
> -               }
> -       }
> -
> -       return 0;
> -}
> -
>  #ifdef CONFIG_VIDEO_ADV_DEBUG
>  static int vidioc_g_register(struct file *file, void *priv,
>                              struct v4l2_dbg_register *reg)
>  {
> -       struct au0828_fh *fh = priv;
> -       struct au0828_dev *dev = fh->dev;
> +       struct au0828_dev *dev = video_drvdata(file);
> +
> +       dprintk(1, "%s called std_set %d dev_state %d\n", __func__,
> +               dev->std_set_in_tuner_core, dev->dev_state);
>
>         reg->val = au0828_read(dev, reg->reg);
>         reg->size = 1;
> @@ -1772,8 +1573,10 @@ static int vidioc_g_register(struct file *file, void *priv,
>  static int vidioc_s_register(struct file *file, void *priv,
>                              const struct v4l2_dbg_register *reg)
>  {
> -       struct au0828_fh *fh = priv;
> -       struct au0828_dev *dev = fh->dev;
> +       struct au0828_dev *dev = video_drvdata(file);
> +
> +       dprintk(1, "%s called std_set %d dev_state %d\n", __func__,
> +               dev->std_set_in_tuner_core, dev->dev_state);
>
>         return au0828_writereg(dev, reg->reg, reg->val);
>  }
> @@ -1783,93 +1586,13 @@ static int vidioc_log_status(struct file *file, void *fh)
>  {
>         struct video_device *vdev = video_devdata(file);
>
> +       dprintk(1, "%s called\n", __func__);
> +
>         v4l2_ctrl_log_status(file, fh);
>         v4l2_device_call_all(vdev->v4l2_dev, 0, core, log_status);
>         return 0;
>  }
>
> -static int vidioc_reqbufs(struct file *file, void *priv,
> -                         struct v4l2_requestbuffers *rb)
> -{
> -       struct au0828_fh *fh = priv;
> -       struct au0828_dev *dev = fh->dev;
> -       int rc;
> -
> -       rc = check_dev(dev);
> -       if (rc < 0)
> -               return rc;
> -
> -       if (fh->type == V4L2_BUF_TYPE_VIDEO_CAPTURE)
> -               rc = videobuf_reqbufs(&fh->vb_vidq, rb);
> -       else if (fh->type == V4L2_BUF_TYPE_VBI_CAPTURE)
> -               rc = videobuf_reqbufs(&fh->vb_vbiq, rb);
> -
> -       return rc;
> -}
> -
> -static int vidioc_querybuf(struct file *file, void *priv,
> -                          struct v4l2_buffer *b)
> -{
> -       struct au0828_fh *fh = priv;
> -       struct au0828_dev *dev = fh->dev;
> -       int rc;
> -
> -       rc = check_dev(dev);
> -       if (rc < 0)
> -               return rc;
> -
> -       if (fh->type == V4L2_BUF_TYPE_VIDEO_CAPTURE)
> -               rc = videobuf_querybuf(&fh->vb_vidq, b);
> -       else if (fh->type == V4L2_BUF_TYPE_VBI_CAPTURE)
> -               rc = videobuf_querybuf(&fh->vb_vbiq, b);
> -
> -       return rc;
> -}
> -
> -static int vidioc_qbuf(struct file *file, void *priv, struct v4l2_buffer *b)
> -{
> -       struct au0828_fh *fh = priv;
> -       struct au0828_dev *dev = fh->dev;
> -       int rc;
> -
> -       rc = check_dev(dev);
> -       if (rc < 0)
> -               return rc;
> -
> -       if (fh->type == V4L2_BUF_TYPE_VIDEO_CAPTURE)
> -               rc = videobuf_qbuf(&fh->vb_vidq, b);
> -       else if (fh->type == V4L2_BUF_TYPE_VBI_CAPTURE)
> -               rc = videobuf_qbuf(&fh->vb_vbiq, b);
> -
> -       return rc;
> -}
> -
> -static int vidioc_dqbuf(struct file *file, void *priv, struct v4l2_buffer *b)
> -{
> -       struct au0828_fh *fh = priv;
> -       struct au0828_dev *dev = fh->dev;
> -       int rc;
> -
> -       rc = check_dev(dev);
> -       if (rc < 0)
> -               return rc;
> -
> -       /* Workaround for a bug in the au0828 hardware design that sometimes
> -          results in the colorspace being inverted */
> -       if (dev->greenscreen_detected == 1) {
> -               dprintk(1, "Detected green frame.  Resetting stream...\n");
> -               au0828_analog_stream_reset(dev);
> -               dev->greenscreen_detected = 0;
> -       }
> -
> -       if (fh->type == V4L2_BUF_TYPE_VIDEO_CAPTURE)
> -               rc = videobuf_dqbuf(&fh->vb_vidq, b, file->f_flags & O_NONBLOCK);
> -       else if (fh->type == V4L2_BUF_TYPE_VBI_CAPTURE)
> -               rc = videobuf_dqbuf(&fh->vb_vbiq, b, file->f_flags & O_NONBLOCK);
> -
> -       return rc;
> -}
> -
>  void au0828_v4l2_suspend(struct au0828_dev *dev)
>  {
>         struct urb *urb;
> @@ -1937,9 +1660,9 @@ static struct v4l2_file_operations au0828_v4l_fops = {
>         .owner      = THIS_MODULE,
>         .open       = au0828_v4l2_open,
>         .release    = au0828_v4l2_close,
> -       .read       = au0828_v4l2_read,
> -       .poll       = au0828_v4l2_poll,
> -       .mmap       = au0828_v4l2_mmap,
> +       .read       = vb2_fop_read,
> +       .poll       = vb2_fop_poll,
> +       .mmap       = vb2_fop_mmap,
>         .unlocked_ioctl = video_ioctl2,
>  };
>
> @@ -1956,17 +1679,24 @@ static const struct v4l2_ioctl_ops video_ioctl_ops = {
>         .vidioc_g_audio             = vidioc_g_audio,
>         .vidioc_s_audio             = vidioc_s_audio,
>         .vidioc_cropcap             = vidioc_cropcap,
> -       .vidioc_reqbufs             = vidioc_reqbufs,
> -       .vidioc_querybuf            = vidioc_querybuf,
> -       .vidioc_qbuf                = vidioc_qbuf,
> -       .vidioc_dqbuf               = vidioc_dqbuf,
> +
> +       .vidioc_reqbufs             = vb2_ioctl_reqbufs,
> +       .vidioc_create_bufs         = vb2_ioctl_create_bufs,
> +       .vidioc_prepare_buf         = vb2_ioctl_prepare_buf,
> +       .vidioc_querybuf            = vb2_ioctl_querybuf,
> +       .vidioc_qbuf                = vb2_ioctl_qbuf,
> +       .vidioc_dqbuf               = vb2_ioctl_dqbuf,
> +       .vidioc_expbuf               = vb2_ioctl_expbuf,
> +
>         .vidioc_s_std               = vidioc_s_std,
>         .vidioc_g_std               = vidioc_g_std,
>         .vidioc_enum_input          = vidioc_enum_input,
>         .vidioc_g_input             = vidioc_g_input,
>         .vidioc_s_input             = vidioc_s_input,
> -       .vidioc_streamon            = vidioc_streamon,
> -       .vidioc_streamoff           = vidioc_streamoff,
> +
> +       .vidioc_streamon            = vb2_ioctl_streamon,
> +       .vidioc_streamoff           = vb2_ioctl_streamoff,
> +
>         .vidioc_g_tuner             = vidioc_g_tuner,
>         .vidioc_s_tuner             = vidioc_s_tuner,
>         .vidioc_g_frequency         = vidioc_g_frequency,
> @@ -1987,6 +1717,42 @@ static const struct video_device au0828_video_template = {
>         .tvnorms                    = V4L2_STD_NTSC_M | V4L2_STD_PAL_M,
>  };
>
> +static int au0828_vb2_setup(struct au0828_dev *dev)
> +{
> +       int rc;
> +       struct vb2_queue *q;
> +
> +       /* Setup Videobuf2 for Video capture */
> +       q = &dev->vb_vidq;
> +       q->type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
> +       q->io_modes = VB2_READ | VB2_MMAP | VB2_USERPTR | VB2_DMABUF;
> +       q->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
> +       q->drv_priv = dev;
> +       q->buf_struct_size = sizeof(struct au0828_buffer);
> +       q->ops = &au0828_video_qops;
> +       q->mem_ops = &vb2_vmalloc_memops;
> +
> +       rc = vb2_queue_init(q);
> +       if (rc < 0)
> +               return rc;
> +
> +       /* Setup Videobuf2 for VBI capture */
> +       q = &dev->vb_vbiq;
> +       q->type = V4L2_BUF_TYPE_VBI_CAPTURE;
> +       q->io_modes = VB2_READ | VB2_MMAP | VB2_USERPTR | VB2_DMABUF;
> +       q->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
> +       q->drv_priv = dev;
> +       q->buf_struct_size = sizeof(struct au0828_buffer);
> +       q->ops = &au0828_vbi_qops;
> +       q->mem_ops = &vb2_vmalloc_memops;
> +
> +       rc = vb2_queue_init(q);
> +       if (rc < 0)
> +               return rc;
> +
> +       return 0;
> +}
> +
>  /**************************************************************************/
>
>  int au0828_analog_register(struct au0828_dev *dev,
> @@ -2038,9 +1804,7 @@ int au0828_analog_register(struct au0828_dev *dev,
>
>         /* init video dma queues */
>         INIT_LIST_HEAD(&dev->vidq.active);
> -       INIT_LIST_HEAD(&dev->vidq.queued);
>         INIT_LIST_HEAD(&dev->vbiq.active);
> -       INIT_LIST_HEAD(&dev->vbiq.queued);
>
>         dev->vid_timeout.function = au0828_vid_buffer_timeout;
>         dev->vid_timeout.data = (unsigned long) dev;
> @@ -2077,18 +1841,34 @@ int au0828_analog_register(struct au0828_dev *dev,
>                 goto err_vdev;
>         }
>
> +       mutex_init(&dev->vb_queue_lock);
> +       mutex_init(&dev->vb_vbi_queue_lock);
> +
>         /* Fill the video capture device struct */
>         *dev->vdev = au0828_video_template;
>         dev->vdev->v4l2_dev = &dev->v4l2_dev;
>         dev->vdev->lock = &dev->lock;
> +       dev->vdev->queue = &dev->vb_vidq;
> +       dev->vdev->queue->lock = &dev->vb_queue_lock;
>         strcpy(dev->vdev->name, "au0828a video");
>
>         /* Setup the VBI device */
>         *dev->vbi_dev = au0828_video_template;
>         dev->vbi_dev->v4l2_dev = &dev->v4l2_dev;
>         dev->vbi_dev->lock = &dev->lock;
> +       dev->vbi_dev->queue = &dev->vb_vbiq;
> +       dev->vbi_dev->queue->lock = &dev->vb_vbi_queue_lock;
>         strcpy(dev->vbi_dev->name, "au0828a vbi");
>
> +       /* initialize videobuf2 stuff */
> +       retval = au0828_vb2_setup(dev);
> +       if (retval != 0) {
> +               dprintk(1, "unable to setup videobuf2 queues (error = %d).\n",
> +                       retval);
> +               ret = -ENODEV;
> +               goto err_vbi_dev;
> +       }
> +
>         /* Register the v4l2 device */
>         video_set_drvdata(dev->vdev, dev);
>         retval = video_register_device(dev->vdev, VFL_TYPE_GRABBER, -1);
> diff --git a/drivers/media/usb/au0828/au0828.h b/drivers/media/usb/au0828/au0828.h
> index 36815a3..eb15187 100644
> --- a/drivers/media/usb/au0828/au0828.h
> +++ b/drivers/media/usb/au0828/au0828.h
> @@ -28,7 +28,7 @@
>
>  /* Analog */
>  #include <linux/videodev2.h>
> -#include <media/videobuf-vmalloc.h>
> +#include <media/videobuf2-vmalloc.h>
>  #include <media/v4l2-device.h>
>  #include <media/v4l2-ctrls.h>
>  #include <media/v4l2-fh.h>
> @@ -126,17 +126,7 @@ enum au0828_dev_state {
>         DEV_MISCONFIGURED = 0x04
>  };
>
> -struct au0828_fh {
> -       /* must be the first field of this struct! */
> -       struct v4l2_fh fh;
> -
> -       struct au0828_dev *dev;
> -       unsigned int  resources;
> -
> -       struct videobuf_queue        vb_vidq;
> -       struct videobuf_queue        vb_vbiq;
> -       enum v4l2_buf_type           type;
> -};
> +struct au0828_dev;
>
>  struct au0828_usb_isoc_ctl {
>                 /* max packet size of isoc transaction */
> @@ -177,21 +167,20 @@ struct au0828_usb_isoc_ctl {
>  /* buffer for one video frame */
>  struct au0828_buffer {
>         /* common v4l buffer stuff -- must be first */
> -       struct videobuf_buffer vb;
> +       struct vb2_buffer vb;
> +       struct list_head list;
>
> -       struct list_head frame;
> +       void *mem;
> +       unsigned long length;
>         int top_field;
> -       int receiving;
> +       /* pointer to vmalloc memory address in vb */
> +       char *vb_buf;
>  };
>
>  struct au0828_dmaqueue {
>         struct list_head       active;
> -       struct list_head       queued;
> -
> -       wait_queue_head_t          wq;
> -
>         /* Counters to control buffer fill */
> -       int                        pos;
> +       int                    pos;
>  };
>
>  struct au0828_dev {
> @@ -220,14 +209,26 @@ struct au0828_dev {
>         struct au0828_rc *ir;
>  #endif
>
> -       int users;
> -       unsigned int resources; /* resources in use */
>         struct video_device *vdev;
>         struct video_device *vbi_dev;
> +
> +       /* Videobuf2 */
> +       struct vb2_queue vb_vidq;
> +       struct vb2_queue vb_vbiq;
> +       struct mutex vb_queue_lock;
> +       struct mutex vb_vbi_queue_lock;
> +
> +       unsigned int frame_count;
> +       unsigned int vbi_frame_count;
> +
>         struct timer_list vid_timeout;
>         int vid_timeout_running;
>         struct timer_list vbi_timeout;
>         int vbi_timeout_running;
> +
> +       int users;
> +       int streaming_users;
> +
>         int width;
>         int height;
>         int vbi_width;
> @@ -242,7 +243,6 @@ struct au0828_dev {
>         __u8 isoc_in_endpointaddr;
>         u8 isoc_init_ok;
>         int greenscreen_detected;
> -       unsigned int frame_count;
>         int ctrl_freq;
>         int input_type;
>         int std_set_in_tuner_core;
> @@ -277,6 +277,7 @@ struct au0828_dev {
>         char *dig_transfer_buffer[URB_COUNT];
>  };
>
> +
>  /* ----------------------------------------------------------- */
>  #define au0828_read(dev, reg) au0828_readreg(dev, reg)
>  #define au0828_write(dev, reg, value) au0828_writereg(dev, reg, value)
> @@ -309,13 +310,15 @@ extern int au0828_i2c_unregister(struct au0828_dev *dev);
>
>  /* ----------------------------------------------------------- */
>  /* au0828-video.c */
> -int au0828_analog_register(struct au0828_dev *dev,
> +extern int au0828_analog_register(struct au0828_dev *dev,
>                            struct usb_interface *interface);
> -int au0828_analog_stream_disable(struct au0828_dev *d);
> -void au0828_analog_unregister(struct au0828_dev *dev);
> +extern void au0828_analog_unregister(struct au0828_dev *dev);
> +extern int au0828_start_analog_streaming(struct vb2_queue *vq,
> +                                               unsigned int count);
> +extern void au0828_stop_vbi_streaming(struct vb2_queue *vq);
>  #ifdef CONFIG_VIDEO_AU0828_V4L2
> -void au0828_v4l2_suspend(struct au0828_dev *dev);
> -void au0828_v4l2_resume(struct au0828_dev *dev);
> +extern void au0828_v4l2_suspend(struct au0828_dev *dev);
> +extern void au0828_v4l2_resume(struct au0828_dev *dev);
>  #else
>  static inline void au0828_v4l2_suspend(struct au0828_dev *dev) { };
>  static inline void au0828_v4l2_resume(struct au0828_dev *dev) { };
> @@ -329,7 +332,7 @@ void au0828_dvb_suspend(struct au0828_dev *dev);
>  void au0828_dvb_resume(struct au0828_dev *dev);
>
>  /* au0828-vbi.c */
> -extern struct videobuf_queue_ops au0828_vbi_qops;
> +extern struct vb2_ops au0828_vbi_qops;
>
>  #define dprintk(level, fmt, arg...)\
>         do { if (au0828_debug & level)\
> --
> 2.1.0
>

Re: [PATCH v3 2/3] media: au0828 - convert to use videobuf2

[Devin Heitmueller]

>> -       fh->type = type;
>> -       fh->dev = dev;
>> -       v4l2_fh_init(&fh->fh, vdev);
>> -       filp->private_data = fh;
>> +       dprintk(1,
>> +               "%s called std_set %d dev_state %d stream users %d users %d\n",
>> +               __func__, dev->std_set_in_tuner_core, dev->dev_state,
>> +               dev->streaming_users, dev->users);
>>
>> -       if (mutex_lock_interruptible(&dev->lock)) {
>> -               kfree(fh);
>> +       if (mutex_lock_interruptible(&dev->lock))
>>                 return -ERESTARTSYS;
>> +
>> +       ret = v4l2_fh_open(filp);
>> +       if (ret) {
>> +               au0828_isocdbg("%s: v4l2_fh_open() returned error %d\n",
>> +                               __func__, ret);
>> +               mutex_unlock(&dev->lock);
>> +               return ret;
>>         }
>> +
>>         if (dev->users == 0) {
>
> you can use v4l2_fh_is_singular_file() and get rid of users member ?

That won't work because the underlying resources are shared between
/dev/videoX and /dev/vbiX device nodes.  Hence if you were to move to
v4l2_fh_is_singular_file(), the video device would get opened, the
stream would get reset, the VBI device would get opened, and that
would cause the analog stream to get enabled/reset *again*.

Devin

-- 
Devin J. Heitmueller - Kernel Labs
http://www.kernellabs.com

Re: [PATCH v3 2/3] media: au0828 - convert to use videobuf2

[Shuah Khan]

On 01/22/2015 08:00 AM, Devin Heitmueller wrote:
>>> -       fh->type = type;
>>> -       fh->dev = dev;
>>> -       v4l2_fh_init(&fh->fh, vdev);
>>> -       filp->private_data = fh;
>>> +       dprintk(1,
>>> +               "%s called std_set %d dev_state %d stream users %d users %d\n",
>>> +               __func__, dev->std_set_in_tuner_core, dev->dev_state,
>>> +               dev->streaming_users, dev->users);
>>>
>>> -       if (mutex_lock_interruptible(&dev->lock)) {
>>> -               kfree(fh);
>>> +       if (mutex_lock_interruptible(&dev->lock))
>>>                 return -ERESTARTSYS;
>>> +
>>> +       ret = v4l2_fh_open(filp);
>>> +       if (ret) {
>>> +               au0828_isocdbg("%s: v4l2_fh_open() returned error %d\n",
>>> +                               __func__, ret);
>>> +               mutex_unlock(&dev->lock);
>>> +               return ret;
>>>         }
>>> +
>>>         if (dev->users == 0) {
>>
>> you can use v4l2_fh_is_singular_file() and get rid of users member ?
> 
> That won't work because the underlying resources are shared between
> /dev/videoX and /dev/vbiX device nodes.  Hence if you were to move to
> v4l2_fh_is_singular_file(), the video device would get opened, the
> stream would get reset, the VBI device would get opened, and that
> would cause the analog stream to get enabled/reset *again*.
> 

Thanks Devin for a detailed explanation. I did see this behavior when I
was removed users and used v4l2_fh_is_singular_file() instead. I didn't
understand that this is due to resource sharing between /dev/videoX and
/dev/vbiX .

thanks,
-- Shuah


-- 
Shuah Khan
Sr. Linux Kernel Developer
Open Source Innovation Group
Samsung Research America (Silicon Valley)
shuahkh@osg.samsung.com | (970) 217-8978

Re: [PATCH v3 2/3] media: au0828 - convert to use videobuf2

[Shuah Khan]

Hi Prabhakar,

thanks for the review, please see in-line.

On 01/22/2015 05:41 AM, Lad, Prabhakar wrote:
> Hi Shuah,
> 
> Thanks for the patch.
> 
> On Tue, Jan 13, 2015 at 2:56 AM, Shuah Khan <shuahkh@osg.samsung.com> wrote:
>> Convert au0828 to use videobuf2. Tested with NTSC.
>> Tested video and vbi devices with xawtv, tvtime,
>> and vlc. Ran v4l2-compliance to ensure there are
>> no regressions. video now has no failures and vbi
>> has 3 fewer failures.
>>
>> video before:
>> test VIDIOC_G_FMT: FAIL 3 failures
>> Total: 72, Succeeded: 69, Failed: 3, Warnings: 0
>>
>> Video after:
>> Total: 72, Succeeded: 72, Failed: 0, Warnings: 18
>>
>> vbi before:
>>     test VIDIOC_REQBUFS/CREATE_BUFS/QUERYBUF: FAIL
>>     test VIDIOC_EXPBUF: FAIL
>>     test USERPTR: FAIL
>>     Total: 72, Succeeded: 66, Failed: 6, Warnings: 0
>>
> [Snip]
>> -       init_waitqueue_head(&dma_q->wq);
>> -
>>         /* submit urbs and enables IRQ */
>>         for (i = 0; i < dev->isoc_ctl.num_bufs; i++) {
>>                 rc = usb_submit_urb(dev->isoc_ctl.urb[i], GFP_ATOMIC);
>> @@ -308,16 +303,12 @@ static inline void buffer_filled(struct au0828_dev *dev,
>>                                   struct au0828_buffer *buf)
>>  {
>>         /* Advice that buffer was filled */
>> -       au0828_isocdbg("[%p/%d] wakeup\n", buf, buf->vb.i);
>> -
>> -       buf->vb.state = VIDEOBUF_DONE;
>> -       buf->vb.field_count++;
>> -       v4l2_get_timestamp(&buf->vb.ts);
>> +       au0828_isocdbg("[%p/%d] wakeup\n", buf, buf->top_field);
>>
>> -       dev->isoc_ctl.buf = NULL;
>> -
>> -       list_del(&buf->vb.queue);
>> -       wake_up(&buf->vb.done);
>> +       buf->vb.v4l2_buf.sequence = dev->frame_count++;
>> +       buf->vb.v4l2_buf.field = V4L2_FIELD_INTERLACED;
>> +       v4l2_get_timestamp(&buf->vb.v4l2_buf.timestamp);
>> +       vb2_buffer_done(&buf->vb, VB2_BUF_STATE_DONE);
>>  }
>>
>>  static inline void vbi_buffer_filled(struct au0828_dev *dev,
>> @@ -325,16 +316,12 @@ static inline void vbi_buffer_filled(struct au0828_dev *dev,
>>                                      struct au0828_buffer *buf)
>>  {
>>         /* Advice that buffer was filled */
>> -       au0828_isocdbg("[%p/%d] wakeup\n", buf, buf->vb.i);
>> -
>> -       buf->vb.state = VIDEOBUF_DONE;
>> -       buf->vb.field_count++;
>> -       v4l2_get_timestamp(&buf->vb.ts);
>> +       au0828_isocdbg("[%p/%d] wakeup\n", buf, buf->top_field);
>>
>> -       dev->isoc_ctl.vbi_buf = NULL;
>> -
>> -       list_del(&buf->vb.queue);
>> -       wake_up(&buf->vb.done);
>> +       buf->vb.v4l2_buf.sequence = dev->vbi_frame_count++;
>> +       buf->vb.v4l2_buf.field = V4L2_FIELD_INTERLACED;
>> +       v4l2_get_timestamp(&buf->vb.v4l2_buf.timestamp);
>> +       vb2_buffer_done(&buf->vb, VB2_BUF_STATE_DONE);
>>  }
>>
> Why not just have one single buffer_filled function ? just check the
> queue type and assign the dev->isoc_ctl.buf/ dev->isoc_ctl.vbi_buf
> to NULL.

Yes. These two routines could be collapsed into a single. Is it okay if
I made that change in a separate patch?

> 
>>  /*
>> @@ -353,8 +340,8 @@ static void au0828_copy_video(struct au0828_dev *dev,
>>         if (len == 0)
>>                 return;
>>
>> -       if (dma_q->pos + len > buf->vb.size)
>> -               len = buf->vb.size - dma_q->pos;
>> +       if (dma_q->pos + len > buf->length)
>> +               len = buf->length - dma_q->pos;
>>
>>         startread = p;
>>         remain = len;
>> @@ -372,11 +359,11 @@ static void au0828_copy_video(struct au0828_dev *dev,
>>         lencopy = bytesperline - currlinedone;
>>         lencopy = lencopy > remain ? remain : lencopy;
>>
>> -       if ((char *)startwrite + lencopy > (char *)outp + buf->vb.size) {
>> +       if ((char *)startwrite + lencopy > (char *)outp + buf->length) {
>>                 au0828_isocdbg("Overflow of %zi bytes past buffer end (1)\n",
>>                                ((char *)startwrite + lencopy) -
>> -                              ((char *)outp + buf->vb.size));
>> -               remain = (char *)outp + buf->vb.size - (char *)startwrite;
>> +                              ((char *)outp + buf->length));
>> +               remain = (char *)outp + buf->length - (char *)startwrite;
>>                 lencopy = remain;
>>         }
>>         if (lencopy <= 0)
>> @@ -394,11 +381,11 @@ static void au0828_copy_video(struct au0828_dev *dev,
>>                         lencopy = bytesperline;
>>
>>                 if ((char *)startwrite + lencopy > (char *)outp +
>> -                   buf->vb.size) {
>> +                   buf->length) {
>>                         au0828_isocdbg("Overflow %zi bytes past buf end (2)\n",
>>                                        ((char *)startwrite + lencopy) -
>> -                                      ((char *)outp + buf->vb.size));
>> -                       lencopy = remain = (char *)outp + buf->vb.size -
>> +                                      ((char *)outp + buf->length));
>> +                       lencopy = remain = (char *)outp + buf->length -
>>                                            (char *)startwrite;
>>                 }
>>                 if (lencopy <= 0)
>> @@ -434,7 +421,11 @@ static inline void get_next_buf(struct au0828_dmaqueue *dma_q,
>>         }
>>
>>         /* Get the next buffer */
>> -       *buf = list_entry(dma_q->active.next, struct au0828_buffer, vb.queue);
>> +       *buf = list_entry(dma_q->active.next, struct au0828_buffer, list);
>> +       /* Cleans up buffer - Useful for testing for frame/URB loss */
>> +       list_del(&(*buf)->list);
>> +       dma_q->pos = 0;
>> +       (*buf)->vb_buf = (*buf)->mem;
>>         dev->isoc_ctl.buf = *buf;
>>
>>         return;
>> @@ -472,8 +463,8 @@ static void au0828_copy_vbi(struct au0828_dev *dev,
>>
>>         bytesperline = dev->vbi_width;
>>
>> -       if (dma_q->pos + len > buf->vb.size)
>> -               len = buf->vb.size - dma_q->pos;
>> +       if (dma_q->pos + len > buf->length)
>> +               len = buf->length - dma_q->pos;
>>
>>         startread = p;
>>         startwrite = outp + (dma_q->pos / 2);
>> @@ -496,7 +487,6 @@ static inline void vbi_get_next_buf(struct au0828_dmaqueue *dma_q,
>>                                     struct au0828_buffer **buf)
>>  {
>>         struct au0828_dev *dev = container_of(dma_q, struct au0828_dev, vbiq);
>> -       char *outp;
>>
>>         if (list_empty(&dma_q->active)) {
>>                 au0828_isocdbg("No active queue to serve\n");
>> @@ -506,13 +496,12 @@ static inline void vbi_get_next_buf(struct au0828_dmaqueue *dma_q,
>>         }
>>
>>         /* Get the next buffer */
>> -       *buf = list_entry(dma_q->active.next, struct au0828_buffer, vb.queue);
>> +       *buf = list_entry(dma_q->active.next, struct au0828_buffer, list);
>>         /* Cleans up buffer - Useful for testing for frame/URB loss */
>> -       outp = videobuf_to_vmalloc(&(*buf)->vb);
>> -       memset(outp, 0x00, (*buf)->vb.size);
>> -
>> +       list_del(&(*buf)->list);
>> +       dma_q->pos = 0;
>> +       (*buf)->vb_buf = (*buf)->mem;
>>         dev->isoc_ctl.vbi_buf = *buf;
>> -
>>         return;
>>  }
>>
>> @@ -548,11 +537,11 @@ static inline int au0828_isoc_copy(struct au0828_dev *dev, struct urb *urb)
>>
>>         buf = dev->isoc_ctl.buf;
>>         if (buf != NULL)
>> -               outp = videobuf_to_vmalloc(&buf->vb);
>> +               outp = vb2_plane_vaddr(&buf->vb, 0);
>>
>>         vbi_buf = dev->isoc_ctl.vbi_buf;
>>         if (vbi_buf != NULL)
>> -               vbioutp = videobuf_to_vmalloc(&vbi_buf->vb);
>> +               vbioutp = vb2_plane_vaddr(&vbi_buf->vb, 0);
>>
>>         for (i = 0; i < urb->number_of_packets; i++) {
>>                 int status = urb->iso_frame_desc[i].status;
>> @@ -592,8 +581,8 @@ static inline int au0828_isoc_copy(struct au0828_dev *dev, struct urb *urb)
>>                                 if (vbi_buf == NULL)
>>                                         vbioutp = NULL;
>>                                 else
>> -                                       vbioutp = videobuf_to_vmalloc(
>> -                                               &vbi_buf->vb);
>> +                                       vbioutp = vb2_plane_vaddr(
>> +                                               &vbi_buf->vb, 0);
>>
>>                                 /* Video */
>>                                 if (buf != NULL)
>> @@ -602,7 +591,7 @@ static inline int au0828_isoc_copy(struct au0828_dev *dev, struct urb *urb)
>>                                 if (buf == NULL)
>>                                         outp = NULL;
>>                                 else
>> -                                       outp = videobuf_to_vmalloc(&buf->vb);
>> +                                       outp = vb2_plane_vaddr(&buf->vb, 0);
>>
>>                                 /* As long as isoc traffic is arriving, keep
>>                                    resetting the timer */
>> @@ -656,130 +645,59 @@ static inline int au0828_isoc_copy(struct au0828_dev *dev, struct urb *urb)
>>         return rc;
>>  }
>>
>> -static int
>> -buffer_setup(struct videobuf_queue *vq, unsigned int *count,
>> -            unsigned int *size)
>> +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 au0828_fh *fh = vq->priv_data;
>> -       *size = (fh->dev->width * fh->dev->height * 16 + 7) >> 3;
>> -
>> -       if (0 == *count)
>> -               *count = AU0828_DEF_BUF;
>> +       struct au0828_dev *dev = vb2_get_drv_priv(vq);
>> +       unsigned long img_size = dev->height * dev->bytesperline;
>> +       unsigned long size;
>>
>> -       if (*count < AU0828_MIN_BUF)
>> -               *count = AU0828_MIN_BUF;
>> -       return 0;
>> -}
>> +       size = fmt ? fmt->fmt.pix.sizeimage : img_size;
>> +       if (size < img_size)
>> +               return -EINVAL;
>>
>> -/* This is called *without* dev->slock held; please keep it that way */
>> -static void free_buffer(struct videobuf_queue *vq, struct au0828_buffer *buf)
>> -{
>> -       struct au0828_fh     *fh  = vq->priv_data;
>> -       struct au0828_dev    *dev = fh->dev;
>> -       unsigned long flags = 0;
>> -       if (in_interrupt())
>> -               BUG();
>> -
>> -       /* We used to wait for the buffer to finish here, but this didn't work
>> -          because, as we were keeping the state as VIDEOBUF_QUEUED,
>> -          videobuf_queue_cancel marked it as finished for us.
>> -          (Also, it could wedge forever if the hardware was misconfigured.)
>> -
>> -          This should be safe; by the time we get here, the buffer isn't
>> -          queued anymore. If we ever start marking the buffers as
>> -          VIDEOBUF_ACTIVE, it won't be, though.
>> -       */
>> -       spin_lock_irqsave(&dev->slock, flags);
>> -       if (dev->isoc_ctl.buf == buf)
>> -               dev->isoc_ctl.buf = NULL;
>> -       spin_unlock_irqrestore(&dev->slock, flags);
>> +       *nplanes = 1;
>> +       sizes[0] = size;
>>
>> -       videobuf_vmalloc_free(&buf->vb);
>> -       buf->vb.state = VIDEOBUF_NEEDS_INIT;
>> +       return 0;
>>  }
>>
>>  static int
>> -buffer_prepare(struct videobuf_queue *vq, struct videobuf_buffer *vb,
>> -                                               enum v4l2_field field)
>> +buffer_prepare(struct vb2_buffer *vb)
>>  {
>> -       struct au0828_fh     *fh  = vq->priv_data;
>>         struct au0828_buffer *buf = container_of(vb, struct au0828_buffer, vb);
>> -       struct au0828_dev    *dev = fh->dev;
>> -       int                  rc = 0, urb_init = 0;
>> +       struct au0828_dev    *dev = vb2_get_drv_priv(vb->vb2_queue);
>>
>> -       buf->vb.size = (fh->dev->width * fh->dev->height * 16 + 7) >> 3;
>> +       buf->length = dev->height * dev->bytesperline;
>>
>> -       if (0 != buf->vb.baddr  &&  buf->vb.bsize < buf->vb.size)
>> +       if (vb2_plane_size(vb, 0) < buf->length) {
>> +               pr_err("%s data will not fit into plane (%lu < %lu)\n",
>> +                       __func__, vb2_plane_size(vb, 0), buf->length);
>>                 return -EINVAL;
>> -
>> -       buf->vb.width  = dev->width;
>> -       buf->vb.height = dev->height;
>> -       buf->vb.field  = field;
>> -
>> -       if (VIDEOBUF_NEEDS_INIT == buf->vb.state) {
>> -               rc = videobuf_iolock(vq, &buf->vb, NULL);
>> -               if (rc < 0) {
>> -                       pr_info("videobuf_iolock failed\n");
>> -                       goto fail;
>> -               }
>>         }
>> -
>> -       if (!dev->isoc_ctl.num_bufs)
>> -               urb_init = 1;
>> -
>> -       if (urb_init) {
>> -               rc = au0828_init_isoc(dev, AU0828_ISO_PACKETS_PER_URB,
>> -                                     AU0828_MAX_ISO_BUFS, dev->max_pkt_size,
>> -                                     au0828_isoc_copy);
>> -               if (rc < 0) {
>> -                       pr_info("au0828_init_isoc failed\n");
>> -                       goto fail;
>> -               }
>> -       }
>> -
>> -       buf->vb.state = VIDEOBUF_PREPARED;
>> +       vb2_set_plane_payload(&buf->vb, 0, buf->length);
>>         return 0;
>> -
>> -fail:
>> -       free_buffer(vq, buf);
>> -       return rc;
>>  }
>>
>>  static void
>> -buffer_queue(struct videobuf_queue *vq, struct videobuf_buffer *vb)
>> +buffer_queue(struct vb2_buffer *vb)
>>  {
>>         struct au0828_buffer    *buf     = container_of(vb,
>>                                                         struct au0828_buffer,
>>                                                         vb);
>> -       struct au0828_fh        *fh      = vq->priv_data;
>> -       struct au0828_dev       *dev     = fh->dev;
>> +       struct au0828_dev       *dev     = vb2_get_drv_priv(vb->vb2_queue);
>>         struct au0828_dmaqueue  *vidq    = &dev->vidq;
>> +       unsigned long flags = 0;
>>
>> -       buf->vb.state = VIDEOBUF_QUEUED;
>> -       list_add_tail(&buf->vb.queue, &vidq->active);
>> -}
>> -
>> -static void buffer_release(struct videobuf_queue *vq,
>> -                               struct videobuf_buffer *vb)
>> -{
>> -       struct au0828_buffer   *buf  = container_of(vb,
>> -                                                   struct au0828_buffer,
>> -                                                   vb);
>> +       buf->mem = vb2_plane_vaddr(vb, 0);
>> +       buf->length = vb2_plane_size(vb, 0);
>>
>> -       free_buffer(vq, buf);
>> +       spin_lock_irqsave(&dev->slock, flags);
>> +       list_add_tail(&buf->list, &vidq->active);
>> +       spin_unlock_irqrestore(&dev->slock, flags);
>>  }
>>
>> -static struct videobuf_queue_ops au0828_video_qops = {
>> -       .buf_setup      = buffer_setup,
>> -       .buf_prepare    = buffer_prepare,
>> -       .buf_queue      = buffer_queue,
>> -       .buf_release    = buffer_release,
>> -};
>> -
>> -/* ------------------------------------------------------------------
>> -   V4L2 interface
>> -   ------------------------------------------------------------------*/
>> -
>>  static int au0828_i2s_init(struct au0828_dev *dev)
>>  {
>>         /* Enable i2s mode */
>> @@ -828,7 +746,7 @@ static int au0828_analog_stream_enable(struct au0828_dev *d)
>>         return 0;
>>  }
>>
>> -int au0828_analog_stream_disable(struct au0828_dev *d)
>> +static int au0828_analog_stream_disable(struct au0828_dev *d)
>>  {
>>         dprintk(1, "au0828_analog_stream_disable called\n");
>>         au0828_writereg(d, AU0828_SENSORCTRL_100, 0x0);
>> @@ -861,78 +779,141 @@ static int au0828_stream_interrupt(struct au0828_dev *dev)
>>         return 0;
>>  }
>>
>> -/*
>> - * au0828_release_resources
>> - * unregister v4l2 devices
>> - */
>> -void au0828_analog_unregister(struct au0828_dev *dev)
>> +int au0828_start_analog_streaming(struct vb2_queue *vq, unsigned int count)
>>  {
>> -       dprintk(1, "au0828_release_resources called\n");
>> -       mutex_lock(&au0828_sysfs_lock);
>> +       struct au0828_dev *dev = vb2_get_drv_priv(vq);
>> +       int rc = 0;
>>
>> -       if (dev->vdev)
>> -               video_unregister_device(dev->vdev);
>> -       if (dev->vbi_dev)
>> -               video_unregister_device(dev->vbi_dev);
>> +       dprintk(1, "au0828_start_analog_streaming called %d\n",
>> +               dev->streaming_users);
>>
>> -       mutex_unlock(&au0828_sysfs_lock);
>> -}
>> +       if (vq->type == V4L2_BUF_TYPE_VIDEO_CAPTURE)
>> +               dev->frame_count = 0;
>> +       else
>> +               dev->vbi_frame_count = 0;
>> +
>> +       if (dev->streaming_users == 0) {
>> +               /* If we were doing ac97 instead of i2s, it would go here...*/
>> +               au0828_i2s_init(dev);
>> +               rc = au0828_init_isoc(dev, AU0828_ISO_PACKETS_PER_URB,
>> +                                  AU0828_MAX_ISO_BUFS, dev->max_pkt_size,
>> +                                  au0828_isoc_copy);
>> +               if (rc < 0) {
>> +                       pr_info("au0828_init_isoc failed\n");
>> +                       return rc;
>> +               }
>>
>> +               if (vq->type == V4L2_BUF_TYPE_VIDEO_CAPTURE) {
>> +                       v4l2_device_call_all(&dev->v4l2_dev, 0, video,
>> +                                               s_stream, 1);
>> +                       dev->vid_timeout_running = 1;
>> +                       mod_timer(&dev->vid_timeout, jiffies + (HZ / 10));
>> +               } else if (vq->type == V4L2_BUF_TYPE_VBI_CAPTURE) {
>> +                       dev->vbi_timeout_running = 1;
>> +                       mod_timer(&dev->vbi_timeout, jiffies + (HZ / 10));
>> +               }
>> +       }
>> +       dev->streaming_users++;
>> +       return rc;
>> +}
>>
>> -/* Usage lock check functions */
>> -static int res_get(struct au0828_fh *fh, unsigned int bit)
>> +static void au0828_stop_streaming(struct vb2_queue *vq)
>>  {
>> -       struct au0828_dev    *dev = fh->dev;
>> +       struct au0828_dev *dev = vb2_get_drv_priv(vq);
>> +       struct au0828_dmaqueue *vidq = &dev->vidq;
>> +       unsigned long flags = 0;
>> +       int i;
>>
>> -       if (fh->resources & bit)
>> -               /* have it already allocated */
>> -               return 1;
>> +       dprintk(1, "au0828_stop_streaming called %d\n", dev->streaming_users);
>>
>> -       /* is it free? */
>> -       if (dev->resources & bit) {
>> -               /* no, someone else uses it */
>> -               return 0;
>> +       if (dev->streaming_users-- == 1)
>> +               au0828_uninit_isoc(dev);
>> +
>> +       spin_lock_irqsave(&dev->slock, flags);
>> +       if (dev->isoc_ctl.buf != NULL) {
>> +               vb2_buffer_done(&dev->isoc_ctl.buf->vb, VB2_BUF_STATE_ERROR);
>> +               dev->isoc_ctl.buf = NULL;
>>         }
>> -       /* it's free, grab it */
>> -       fh->resources  |= bit;
>> -       dev->resources |= bit;
>> -       dprintk(1, "res: get %d\n", bit);
>> +       while (!list_empty(&vidq->active)) {
>> +               struct au0828_buffer *buf;
>>
>> -       return 1;
>> -}
>> +               buf = list_entry(vidq->active.next, struct au0828_buffer, list);
>> +               vb2_buffer_done(&buf->vb, VB2_BUF_STATE_ERROR);
>> +               list_del(&buf->list);
>> +       }
>>
>> -static int res_check(struct au0828_fh *fh, unsigned int bit)
>> -{
>> -       return fh->resources & bit;
>> -}
>> +       v4l2_device_call_all(&dev->v4l2_dev, 0, video, s_stream, 0);
>>
>> -static int res_locked(struct au0828_dev *dev, unsigned int bit)
>> -{
>> -       return dev->resources & bit;
>> +       for (i = 0; i < AU0828_MAX_INPUT; i++) {
>> +               if (AUVI_INPUT(i).audio_setup == NULL)
>> +                       continue;
>> +               (AUVI_INPUT(i).audio_setup)(dev, 0);
>> +       }
>> +       spin_unlock_irqrestore(&dev->slock, flags);
>> +
>> +       dev->vid_timeout_running = 0;
>> +       del_timer_sync(&dev->vid_timeout);
>>  }
>>
>> -static void res_free(struct au0828_fh *fh, unsigned int bits)
>> +void au0828_stop_vbi_streaming(struct vb2_queue *vq)
>>  {
>> -       struct au0828_dev    *dev = fh->dev;
>> +       struct au0828_dev *dev = vb2_get_drv_priv(vq);
>> +       struct au0828_dmaqueue *vbiq = &dev->vbiq;
>> +       unsigned long flags = 0;
>> +
>> +       dprintk(1, "au0828_stop_vbi_streaming called %d\n",
>> +               dev->streaming_users);
>> +
>> +       if (dev->streaming_users-- == 1)
>> +               au0828_uninit_isoc(dev);
>>
>> -       BUG_ON((fh->resources & bits) != bits);
>> +       spin_lock_irqsave(&dev->slock, flags);
>> +       if (dev->isoc_ctl.vbi_buf != NULL) {
>> +               vb2_buffer_done(&dev->isoc_ctl.vbi_buf->vb,
>> +                               VB2_BUF_STATE_ERROR);
>> +               dev->isoc_ctl.vbi_buf = NULL;
>> +       }
>> +       while (!list_empty(&vbiq->active)) {
>> +               struct au0828_buffer *buf;
>> +
>> +               buf = list_entry(vbiq->active.next, struct au0828_buffer, list);
>> +               list_del(&buf->list);
>> +               vb2_buffer_done(&buf->vb, VB2_BUF_STATE_ERROR);
>> +       }
>> +       spin_unlock_irqrestore(&dev->slock, flags);
>>
>> -       fh->resources  &= ~bits;
>> -       dev->resources &= ~bits;
>> -       dprintk(1, "res: put %d\n", bits);
>> +       dev->vbi_timeout_running = 0;
>> +       del_timer_sync(&dev->vbi_timeout);
>>  }
>>
>> -static int get_ressource(struct au0828_fh *fh)
>> +static struct vb2_ops au0828_video_qops = {
>> +       .queue_setup     = queue_setup,
>> +       .buf_prepare     = buffer_prepare,
>> +       .buf_queue       = buffer_queue,
>> +       .start_streaming = au0828_start_analog_streaming,
>> +       .stop_streaming  = au0828_stop_streaming,
>> +       .wait_prepare    = vb2_ops_wait_prepare,
>> +       .wait_finish     = vb2_ops_wait_finish,
>> +};
>> +
>> +/* ------------------------------------------------------------------
>> +   V4L2 interface
>> +   ------------------------------------------------------------------*/
>> +/*
>> + * au0828_analog_unregister
>> + * unregister v4l2 devices
>> + */
>> +void au0828_analog_unregister(struct au0828_dev *dev)
>>  {
>> -       switch (fh->type) {
>> -       case V4L2_BUF_TYPE_VIDEO_CAPTURE:
>> -               return AU0828_RESOURCE_VIDEO;
>> -       case V4L2_BUF_TYPE_VBI_CAPTURE:
>> -               return AU0828_RESOURCE_VBI;
>> -       default:
>> -               BUG();
>> -               return 0;
>> -       }
>> +       dprintk(1, "au0828_analog_unregister called\n");
>> +       mutex_lock(&au0828_sysfs_lock);
>> +
>> +       if (dev->vdev)
>> +               video_unregister_device(dev->vdev);
>> +       if (dev->vbi_dev)
>> +               video_unregister_device(dev->vbi_dev);
>> +
>> +       mutex_unlock(&au0828_sysfs_lock);
>>  }
>>
>>  /* This function ensures that video frames continue to be delivered even if
>> @@ -950,8 +931,8 @@ static void au0828_vid_buffer_timeout(unsigned long data)
>>
>>         buf = dev->isoc_ctl.buf;
>>         if (buf != NULL) {
>> -               vid_data = videobuf_to_vmalloc(&buf->vb);
>> -               memset(vid_data, 0x00, buf->vb.size); /* Blank green frame */
>> +               vid_data = vb2_plane_vaddr(&buf->vb, 0);
>> +               memset(vid_data, 0x00, buf->length); /* Blank green frame */
>>                 buffer_filled(dev, dma_q, buf);
>>         }
>>         get_next_buf(dma_q, &buf);
>> @@ -974,8 +955,8 @@ static void au0828_vbi_buffer_timeout(unsigned long data)
>>
>>         buf = dev->isoc_ctl.vbi_buf;
>>         if (buf != NULL) {
>> -               vbi_data = videobuf_to_vmalloc(&buf->vb);
>> -               memset(vbi_data, 0x00, buf->vb.size);
>> +               vbi_data = vb2_plane_vaddr(&buf->vb, 0);
>> +               memset(vbi_data, 0x00, buf->length);
>>                 vbi_buffer_filled(dev, dma_q, buf);
>>         }
>>         vbi_get_next_buf(dma_q, &buf);
>> @@ -985,105 +966,65 @@ static void au0828_vbi_buffer_timeout(unsigned long data)
>>         spin_unlock_irqrestore(&dev->slock, flags);
>>  }
>>
>> -
>>  static int au0828_v4l2_open(struct file *filp)
>>  {
>> -       int ret = 0;
>> -       struct video_device *vdev = video_devdata(filp);
>>         struct au0828_dev *dev = video_drvdata(filp);
>> -       struct au0828_fh *fh;
>> -       int type;
>> -
>> -       switch (vdev->vfl_type) {
>> -       case VFL_TYPE_GRABBER:
>> -               type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
>> -               break;
>> -       case VFL_TYPE_VBI:
>> -               type = V4L2_BUF_TYPE_VBI_CAPTURE;
>> -               break;
>> -       default:
>> -               return -EINVAL;
>> -       }
>> -
>> -       fh = kzalloc(sizeof(struct au0828_fh), GFP_KERNEL);
>> -       if (NULL == fh) {
>> -               dprintk(1, "Failed allocate au0828_fh struct!\n");
>> -               return -ENOMEM;
>> -       }
>> +       int ret = 0;
>>
> No need to assign it to zero.

Yes. Looks like there are a few other routines that zero out
ret. If you want to see all of those cleaned up, I can do a
separate patch cleaning all these instances. If not I can send
patch v4? Please let me know your preference.

> 
>> -       fh->type = type;
>> -       fh->dev = dev;
>> -       v4l2_fh_init(&fh->fh, vdev);
>> -       filp->private_data = fh;
>> +       dprintk(1,
>> +               "%s called std_set %d dev_state %d stream users %d users %d\n",
>> +               __func__, dev->std_set_in_tuner_core, dev->dev_state,
>> +               dev->streaming_users, dev->users);
>>
>> -       if (mutex_lock_interruptible(&dev->lock)) {
>> -               kfree(fh);
>> +       if (mutex_lock_interruptible(&dev->lock))
>>                 return -ERESTARTSYS;
>> +
>> +       ret = v4l2_fh_open(filp);
>> +       if (ret) {
>> +               au0828_isocdbg("%s: v4l2_fh_open() returned error %d\n",
>> +                               __func__, ret);
>> +               mutex_unlock(&dev->lock);
>> +               return ret;
>>         }
>> +
>>         if (dev->users == 0) {
> 
> you can use v4l2_fh_is_singular_file() and get rid of users member ?

Please see Devin's response on this. I have been debugging the change
I made to use v4l2_fh_is_singular_file() instead of users and seeing
problem with reset stream and couldn't really figure out why until
Devin explained it.

thanks,
-- Shuah

-- 
Shuah Khan
Sr. Linux Kernel Developer
Open Source Innovation Group
Samsung Research America (Silicon Valley)
shuahkh@osg.samsung.com | (970) 217-8978

Re: [PATCH v3 2/3] media: au0828 - convert to use videobuf2

[Lad, Prabhakar]

Hi Shuah,

On Thu, Jan 22, 2015 at 4:16 PM, Shuah Khan <shuahkh@osg.samsung.com> wrote:
> Hi Prabhakar,
>
[snip]
>>> +       buf->vb.v4l2_buf.field = V4L2_FIELD_INTERLACED;
>>> +       v4l2_get_timestamp(&buf->vb.v4l2_buf.timestamp);
>>> +       vb2_buffer_done(&buf->vb, VB2_BUF_STATE_DONE);
>>>  }
>>>
>> Why not just have one single buffer_filled function ? just check the
>> queue type and assign the dev->isoc_ctl.buf/ dev->isoc_ctl.vbi_buf
>> to NULL.
>
> Yes. These two routines could be collapsed into a single. Is it okay if
> I made that change in a separate patch?
>
hmm.. this can go as a separate patch.

>>
>>>  /*
>>> @@ -353,8 +340,8 @@ static void au0828_copy_video(struct au0828_dev *dev,
>>>         if (len == 0)
>>>                 return;
>>>
>>> -       if (dma_q->pos + len > buf->vb.size)
>>> -               len = buf->vb.size - dma_q->pos;
>>> +       if (dma_q->pos + len > buf->length)
>>> +               len = buf->length - dma_q->pos;
>>>
>>>         startread = p;
>>>         remain = len;
>>> @@ -372,11 +359,11 @@ static void au0828_copy_video(struct au0828_dev *dev,
>>>         lencopy = bytesperline - currlinedone;
>>>         lencopy = lencopy > remain ? remain : lencopy;
>>>
>>> -       if ((char *)startwrite + lencopy > (char *)outp + buf->vb.size) {
>>> +       if ((char *)startwrite + lencopy > (char *)outp + buf->length) {
>>>                 au0828_isocdbg("Overflow of %zi bytes past buffer end (1)\n",
>>>                                ((char *)startwrite + lencopy) -
>>> -                              ((char *)outp + buf->vb.size));
>>> -               remain = (char *)outp + buf->vb.size - (char *)startwrite;
>>> +                              ((char *)outp + buf->length));
>>> +               remain = (char *)outp + buf->length - (char *)startwrite;
>>>                 lencopy = remain;
>>>         }
>>>         if (lencopy <= 0)
>>> @@ -394,11 +381,11 @@ static void au0828_copy_video(struct au0828_dev *dev,
>>>                         lencopy = bytesperline;
>>>
>>>                 if ((char *)startwrite + lencopy > (char *)outp +
>>> -                   buf->vb.size) {
>>> +                   buf->length) {
>>>                         au0828_isocdbg("Overflow %zi bytes past buf end (2)\n",
>>>                                        ((char *)startwrite + lencopy) -
>>> -                                      ((char *)outp + buf->vb.size));
>>> -                       lencopy = remain = (char *)outp + buf->vb.size -
>>> +                                      ((char *)outp + buf->length));
>>> +                       lencopy = remain = (char *)outp + buf->length -
>>>                                            (char *)startwrite;
>>>                 }
>>>                 if (lencopy <= 0)
>>> @@ -434,7 +421,11 @@ static inline void get_next_buf(struct au0828_dmaqueue *dma_q,
>>>         }
>>>
>>>         /* Get the next buffer */
>>> -       *buf = list_entry(dma_q->active.next, struct au0828_buffer, vb.queue);
>>> +       *buf = list_entry(dma_q->active.next, struct au0828_buffer, list);
>>> +       /* Cleans up buffer - Useful for testing for frame/URB loss */
>>> +       list_del(&(*buf)->list);
>>> +       dma_q->pos = 0;
>>> +       (*buf)->vb_buf = (*buf)->mem;
>>>         dev->isoc_ctl.buf = *buf;
>>>
>>>         return;
>>> @@ -472,8 +463,8 @@ static void au0828_copy_vbi(struct au0828_dev *dev,
>>>
>>>         bytesperline = dev->vbi_width;
>>>
>>> -       if (dma_q->pos + len > buf->vb.size)
>>> -               len = buf->vb.size - dma_q->pos;
>>> +       if (dma_q->pos + len > buf->length)
>>> +               len = buf->length - dma_q->pos;
>>>
>>>         startread = p;
>>>         startwrite = outp + (dma_q->pos / 2);
>>> @@ -496,7 +487,6 @@ static inline void vbi_get_next_buf(struct au0828_dmaqueue *dma_q,
>>>                                     struct au0828_buffer **buf)
>>>  {
>>>         struct au0828_dev *dev = container_of(dma_q, struct au0828_dev, vbiq);
>>> -       char *outp;
>>>
>>>         if (list_empty(&dma_q->active)) {
>>>                 au0828_isocdbg("No active queue to serve\n");
>>> @@ -506,13 +496,12 @@ static inline void vbi_get_next_buf(struct au0828_dmaqueue *dma_q,
>>>         }
>>>
>>>         /* Get the next buffer */
>>> -       *buf = list_entry(dma_q->active.next, struct au0828_buffer, vb.queue);
>>> +       *buf = list_entry(dma_q->active.next, struct au0828_buffer, list);
>>>         /* Cleans up buffer - Useful for testing for frame/URB loss */
>>> -       outp = videobuf_to_vmalloc(&(*buf)->vb);
>>> -       memset(outp, 0x00, (*buf)->vb.size);
>>> -
>>> +       list_del(&(*buf)->list);
>>> +       dma_q->pos = 0;
>>> +       (*buf)->vb_buf = (*buf)->mem;
>>>         dev->isoc_ctl.vbi_buf = *buf;
>>> -
>>>         return;
>>>  }
>>>
>>> @@ -548,11 +537,11 @@ static inline int au0828_isoc_copy(struct au0828_dev *dev, struct urb *urb)
>>>
>>>         buf = dev->isoc_ctl.buf;
>>>         if (buf != NULL)
>>> -               outp = videobuf_to_vmalloc(&buf->vb);
>>> +               outp = vb2_plane_vaddr(&buf->vb, 0);
>>>
>>>         vbi_buf = dev->isoc_ctl.vbi_buf;
>>>         if (vbi_buf != NULL)
>>> -               vbioutp = videobuf_to_vmalloc(&vbi_buf->vb);
>>> +               vbioutp = vb2_plane_vaddr(&vbi_buf->vb, 0);
>>>
>>>         for (i = 0; i < urb->number_of_packets; i++) {
>>>                 int status = urb->iso_frame_desc[i].status;
>>> @@ -592,8 +581,8 @@ static inline int au0828_isoc_copy(struct au0828_dev *dev, struct urb *urb)
>>>                                 if (vbi_buf == NULL)
>>>                                         vbioutp = NULL;
>>>                                 else
>>> -                                       vbioutp = videobuf_to_vmalloc(
>>> -                                               &vbi_buf->vb);
>>> +                                       vbioutp = vb2_plane_vaddr(
>>> +                                               &vbi_buf->vb, 0);
>>>
>>>                                 /* Video */
>>>                                 if (buf != NULL)
>>> @@ -602,7 +591,7 @@ static inline int au0828_isoc_copy(struct au0828_dev *dev, struct urb *urb)
>>>                                 if (buf == NULL)
>>>                                         outp = NULL;
>>>                                 else
>>> -                                       outp = videobuf_to_vmalloc(&buf->vb);
>>> +                                       outp = vb2_plane_vaddr(&buf->vb, 0);
>>>
>>>                                 /* As long as isoc traffic is arriving, keep
>>>                                    resetting the timer */
>>> @@ -656,130 +645,59 @@ static inline int au0828_isoc_copy(struct au0828_dev *dev, struct urb *urb)
>>>         return rc;
>>>  }
>>>
>>> -static int
>>> -buffer_setup(struct videobuf_queue *vq, unsigned int *count,
>>> -            unsigned int *size)
>>> +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 au0828_fh *fh = vq->priv_data;
>>> -       *size = (fh->dev->width * fh->dev->height * 16 + 7) >> 3;
>>> -
>>> -       if (0 == *count)
>>> -               *count = AU0828_DEF_BUF;
>>> +       struct au0828_dev *dev = vb2_get_drv_priv(vq);
>>> +       unsigned long img_size = dev->height * dev->bytesperline;
>>> +       unsigned long size;
>>>
>>> -       if (*count < AU0828_MIN_BUF)
>>> -               *count = AU0828_MIN_BUF;
>>> -       return 0;
>>> -}
>>> +       size = fmt ? fmt->fmt.pix.sizeimage : img_size;
>>> +       if (size < img_size)
>>> +               return -EINVAL;
>>>
>>> -/* This is called *without* dev->slock held; please keep it that way */
>>> -static void free_buffer(struct videobuf_queue *vq, struct au0828_buffer *buf)
>>> -{
>>> -       struct au0828_fh     *fh  = vq->priv_data;
>>> -       struct au0828_dev    *dev = fh->dev;
>>> -       unsigned long flags = 0;
>>> -       if (in_interrupt())
>>> -               BUG();
>>> -
>>> -       /* We used to wait for the buffer to finish here, but this didn't work
>>> -          because, as we were keeping the state as VIDEOBUF_QUEUED,
>>> -          videobuf_queue_cancel marked it as finished for us.
>>> -          (Also, it could wedge forever if the hardware was misconfigured.)
>>> -
>>> -          This should be safe; by the time we get here, the buffer isn't
>>> -          queued anymore. If we ever start marking the buffers as
>>> -          VIDEOBUF_ACTIVE, it won't be, though.
>>> -       */
>>> -       spin_lock_irqsave(&dev->slock, flags);
>>> -       if (dev->isoc_ctl.buf == buf)
>>> -               dev->isoc_ctl.buf = NULL;
>>> -       spin_unlock_irqrestore(&dev->slock, flags);
>>> +       *nplanes = 1;
>>> +       sizes[0] = size;
>>>
>>> -       videobuf_vmalloc_free(&buf->vb);
>>> -       buf->vb.state = VIDEOBUF_NEEDS_INIT;
>>> +       return 0;
>>>  }
>>>
>>>  static int
>>> -buffer_prepare(struct videobuf_queue *vq, struct videobuf_buffer *vb,
>>> -                                               enum v4l2_field field)
>>> +buffer_prepare(struct vb2_buffer *vb)
>>>  {
>>> -       struct au0828_fh     *fh  = vq->priv_data;
>>>         struct au0828_buffer *buf = container_of(vb, struct au0828_buffer, vb);
>>> -       struct au0828_dev    *dev = fh->dev;
>>> -       int                  rc = 0, urb_init = 0;
>>> +       struct au0828_dev    *dev = vb2_get_drv_priv(vb->vb2_queue);
>>>
>>> -       buf->vb.size = (fh->dev->width * fh->dev->height * 16 + 7) >> 3;
>>> +       buf->length = dev->height * dev->bytesperline;
>>>
>>> -       if (0 != buf->vb.baddr  &&  buf->vb.bsize < buf->vb.size)
>>> +       if (vb2_plane_size(vb, 0) < buf->length) {
>>> +               pr_err("%s data will not fit into plane (%lu < %lu)\n",
>>> +                       __func__, vb2_plane_size(vb, 0), buf->length);
>>>                 return -EINVAL;
>>> -
>>> -       buf->vb.width  = dev->width;
>>> -       buf->vb.height = dev->height;
>>> -       buf->vb.field  = field;
>>> -
>>> -       if (VIDEOBUF_NEEDS_INIT == buf->vb.state) {
>>> -               rc = videobuf_iolock(vq, &buf->vb, NULL);
>>> -               if (rc < 0) {
>>> -                       pr_info("videobuf_iolock failed\n");
>>> -                       goto fail;
>>> -               }
>>>         }
>>> -
>>> -       if (!dev->isoc_ctl.num_bufs)
>>> -               urb_init = 1;
>>> -
>>> -       if (urb_init) {
>>> -               rc = au0828_init_isoc(dev, AU0828_ISO_PACKETS_PER_URB,
>>> -                                     AU0828_MAX_ISO_BUFS, dev->max_pkt_size,
>>> -                                     au0828_isoc_copy);
>>> -               if (rc < 0) {
>>> -                       pr_info("au0828_init_isoc failed\n");
>>> -                       goto fail;
>>> -               }
>>> -       }
>>> -
>>> -       buf->vb.state = VIDEOBUF_PREPARED;
>>> +       vb2_set_plane_payload(&buf->vb, 0, buf->length);
>>>         return 0;
>>> -
>>> -fail:
>>> -       free_buffer(vq, buf);
>>> -       return rc;
>>>  }
>>>
>>>  static void
>>> -buffer_queue(struct videobuf_queue *vq, struct videobuf_buffer *vb)
>>> +buffer_queue(struct vb2_buffer *vb)
>>>  {
>>>         struct au0828_buffer    *buf     = container_of(vb,
>>>                                                         struct au0828_buffer,
>>>                                                         vb);
>>> -       struct au0828_fh        *fh      = vq->priv_data;
>>> -       struct au0828_dev       *dev     = fh->dev;
>>> +       struct au0828_dev       *dev     = vb2_get_drv_priv(vb->vb2_queue);
>>>         struct au0828_dmaqueue  *vidq    = &dev->vidq;
>>> +       unsigned long flags = 0;
>>>
>>> -       buf->vb.state = VIDEOBUF_QUEUED;
>>> -       list_add_tail(&buf->vb.queue, &vidq->active);
>>> -}
>>> -
>>> -static void buffer_release(struct videobuf_queue *vq,
>>> -                               struct videobuf_buffer *vb)
>>> -{
>>> -       struct au0828_buffer   *buf  = container_of(vb,
>>> -                                                   struct au0828_buffer,
>>> -                                                   vb);
>>> +       buf->mem = vb2_plane_vaddr(vb, 0);
>>> +       buf->length = vb2_plane_size(vb, 0);
>>>
>>> -       free_buffer(vq, buf);
>>> +       spin_lock_irqsave(&dev->slock, flags);
>>> +       list_add_tail(&buf->list, &vidq->active);
>>> +       spin_unlock_irqrestore(&dev->slock, flags);
>>>  }
>>>
>>> -static struct videobuf_queue_ops au0828_video_qops = {
>>> -       .buf_setup      = buffer_setup,
>>> -       .buf_prepare    = buffer_prepare,
>>> -       .buf_queue      = buffer_queue,
>>> -       .buf_release    = buffer_release,
>>> -};
>>> -
>>> -/* ------------------------------------------------------------------
>>> -   V4L2 interface
>>> -   ------------------------------------------------------------------*/
>>> -
>>>  static int au0828_i2s_init(struct au0828_dev *dev)
>>>  {
>>>         /* Enable i2s mode */
>>> @@ -828,7 +746,7 @@ static int au0828_analog_stream_enable(struct au0828_dev *d)
>>>         return 0;
>>>  }
>>>
>>> -int au0828_analog_stream_disable(struct au0828_dev *d)
>>> +static int au0828_analog_stream_disable(struct au0828_dev *d)
>>>  {
>>>         dprintk(1, "au0828_analog_stream_disable called\n");
>>>         au0828_writereg(d, AU0828_SENSORCTRL_100, 0x0);
>>> @@ -861,78 +779,141 @@ static int au0828_stream_interrupt(struct au0828_dev *dev)
>>>         return 0;
>>>  }
>>>
>>> -/*
>>> - * au0828_release_resources
>>> - * unregister v4l2 devices
>>> - */
>>> -void au0828_analog_unregister(struct au0828_dev *dev)
>>> +int au0828_start_analog_streaming(struct vb2_queue *vq, unsigned int count)
>>>  {
>>> -       dprintk(1, "au0828_release_resources called\n");
>>> -       mutex_lock(&au0828_sysfs_lock);
>>> +       struct au0828_dev *dev = vb2_get_drv_priv(vq);
>>> +       int rc = 0;
>>>
>>> -       if (dev->vdev)
>>> -               video_unregister_device(dev->vdev);
>>> -       if (dev->vbi_dev)
>>> -               video_unregister_device(dev->vbi_dev);
>>> +       dprintk(1, "au0828_start_analog_streaming called %d\n",
>>> +               dev->streaming_users);
>>>
>>> -       mutex_unlock(&au0828_sysfs_lock);
>>> -}
>>> +       if (vq->type == V4L2_BUF_TYPE_VIDEO_CAPTURE)
>>> +               dev->frame_count = 0;
>>> +       else
>>> +               dev->vbi_frame_count = 0;
>>> +
>>> +       if (dev->streaming_users == 0) {
>>> +               /* If we were doing ac97 instead of i2s, it would go here...*/
>>> +               au0828_i2s_init(dev);
>>> +               rc = au0828_init_isoc(dev, AU0828_ISO_PACKETS_PER_URB,
>>> +                                  AU0828_MAX_ISO_BUFS, dev->max_pkt_size,
>>> +                                  au0828_isoc_copy);
>>> +               if (rc < 0) {
>>> +                       pr_info("au0828_init_isoc failed\n");
>>> +                       return rc;
>>> +               }
>>>
>>> +               if (vq->type == V4L2_BUF_TYPE_VIDEO_CAPTURE) {
>>> +                       v4l2_device_call_all(&dev->v4l2_dev, 0, video,
>>> +                                               s_stream, 1);
>>> +                       dev->vid_timeout_running = 1;
>>> +                       mod_timer(&dev->vid_timeout, jiffies + (HZ / 10));
>>> +               } else if (vq->type == V4L2_BUF_TYPE_VBI_CAPTURE) {
>>> +                       dev->vbi_timeout_running = 1;
>>> +                       mod_timer(&dev->vbi_timeout, jiffies + (HZ / 10));
>>> +               }
>>> +       }
>>> +       dev->streaming_users++;
>>> +       return rc;
>>> +}
>>>
>>> -/* Usage lock check functions */
>>> -static int res_get(struct au0828_fh *fh, unsigned int bit)
>>> +static void au0828_stop_streaming(struct vb2_queue *vq)
>>>  {
>>> -       struct au0828_dev    *dev = fh->dev;
>>> +       struct au0828_dev *dev = vb2_get_drv_priv(vq);
>>> +       struct au0828_dmaqueue *vidq = &dev->vidq;
>>> +       unsigned long flags = 0;
>>> +       int i;
>>>
>>> -       if (fh->resources & bit)
>>> -               /* have it already allocated */
>>> -               return 1;
>>> +       dprintk(1, "au0828_stop_streaming called %d\n", dev->streaming_users);
>>>
>>> -       /* is it free? */
>>> -       if (dev->resources & bit) {
>>> -               /* no, someone else uses it */
>>> -               return 0;
>>> +       if (dev->streaming_users-- == 1)
>>> +               au0828_uninit_isoc(dev);
>>> +
>>> +       spin_lock_irqsave(&dev->slock, flags);
>>> +       if (dev->isoc_ctl.buf != NULL) {
>>> +               vb2_buffer_done(&dev->isoc_ctl.buf->vb, VB2_BUF_STATE_ERROR);
>>> +               dev->isoc_ctl.buf = NULL;
>>>         }
>>> -       /* it's free, grab it */
>>> -       fh->resources  |= bit;
>>> -       dev->resources |= bit;
>>> -       dprintk(1, "res: get %d\n", bit);
>>> +       while (!list_empty(&vidq->active)) {
>>> +               struct au0828_buffer *buf;
>>>
>>> -       return 1;
>>> -}
>>> +               buf = list_entry(vidq->active.next, struct au0828_buffer, list);
>>> +               vb2_buffer_done(&buf->vb, VB2_BUF_STATE_ERROR);
>>> +               list_del(&buf->list);
>>> +       }
>>>
>>> -static int res_check(struct au0828_fh *fh, unsigned int bit)
>>> -{
>>> -       return fh->resources & bit;
>>> -}
>>> +       v4l2_device_call_all(&dev->v4l2_dev, 0, video, s_stream, 0);
>>>
>>> -static int res_locked(struct au0828_dev *dev, unsigned int bit)
>>> -{
>>> -       return dev->resources & bit;
>>> +       for (i = 0; i < AU0828_MAX_INPUT; i++) {
>>> +               if (AUVI_INPUT(i).audio_setup == NULL)
>>> +                       continue;
>>> +               (AUVI_INPUT(i).audio_setup)(dev, 0);
>>> +       }
>>> +       spin_unlock_irqrestore(&dev->slock, flags);
>>> +
>>> +       dev->vid_timeout_running = 0;
>>> +       del_timer_sync(&dev->vid_timeout);
>>>  }
>>>
>>> -static void res_free(struct au0828_fh *fh, unsigned int bits)
>>> +void au0828_stop_vbi_streaming(struct vb2_queue *vq)
>>>  {
>>> -       struct au0828_dev    *dev = fh->dev;
>>> +       struct au0828_dev *dev = vb2_get_drv_priv(vq);
>>> +       struct au0828_dmaqueue *vbiq = &dev->vbiq;
>>> +       unsigned long flags = 0;
>>> +
>>> +       dprintk(1, "au0828_stop_vbi_streaming called %d\n",
>>> +               dev->streaming_users);
>>> +
>>> +       if (dev->streaming_users-- == 1)
>>> +               au0828_uninit_isoc(dev);
>>>
>>> -       BUG_ON((fh->resources & bits) != bits);
>>> +       spin_lock_irqsave(&dev->slock, flags);
>>> +       if (dev->isoc_ctl.vbi_buf != NULL) {
>>> +               vb2_buffer_done(&dev->isoc_ctl.vbi_buf->vb,
>>> +                               VB2_BUF_STATE_ERROR);
>>> +               dev->isoc_ctl.vbi_buf = NULL;
>>> +       }
>>> +       while (!list_empty(&vbiq->active)) {
>>> +               struct au0828_buffer *buf;
>>> +
>>> +               buf = list_entry(vbiq->active.next, struct au0828_buffer, list);
>>> +               list_del(&buf->list);
>>> +               vb2_buffer_done(&buf->vb, VB2_BUF_STATE_ERROR);
>>> +       }
>>> +       spin_unlock_irqrestore(&dev->slock, flags);
>>>
>>> -       fh->resources  &= ~bits;
>>> -       dev->resources &= ~bits;
>>> -       dprintk(1, "res: put %d\n", bits);
>>> +       dev->vbi_timeout_running = 0;
>>> +       del_timer_sync(&dev->vbi_timeout);
>>>  }
>>>
>>> -static int get_ressource(struct au0828_fh *fh)
>>> +static struct vb2_ops au0828_video_qops = {
>>> +       .queue_setup     = queue_setup,
>>> +       .buf_prepare     = buffer_prepare,
>>> +       .buf_queue       = buffer_queue,
>>> +       .start_streaming = au0828_start_analog_streaming,
>>> +       .stop_streaming  = au0828_stop_streaming,
>>> +       .wait_prepare    = vb2_ops_wait_prepare,
>>> +       .wait_finish     = vb2_ops_wait_finish,
>>> +};
>>> +
>>> +/* ------------------------------------------------------------------
>>> +   V4L2 interface
>>> +   ------------------------------------------------------------------*/
>>> +/*
>>> + * au0828_analog_unregister
>>> + * unregister v4l2 devices
>>> + */
>>> +void au0828_analog_unregister(struct au0828_dev *dev)
>>>  {
>>> -       switch (fh->type) {
>>> -       case V4L2_BUF_TYPE_VIDEO_CAPTURE:
>>> -               return AU0828_RESOURCE_VIDEO;
>>> -       case V4L2_BUF_TYPE_VBI_CAPTURE:
>>> -               return AU0828_RESOURCE_VBI;
>>> -       default:
>>> -               BUG();
>>> -               return 0;
>>> -       }
>>> +       dprintk(1, "au0828_analog_unregister called\n");
>>> +       mutex_lock(&au0828_sysfs_lock);
>>> +
>>> +       if (dev->vdev)
>>> +               video_unregister_device(dev->vdev);
>>> +       if (dev->vbi_dev)
>>> +               video_unregister_device(dev->vbi_dev);
>>> +
>>> +       mutex_unlock(&au0828_sysfs_lock);
>>>  }
>>>
>>>  /* This function ensures that video frames continue to be delivered even if
>>> @@ -950,8 +931,8 @@ static void au0828_vid_buffer_timeout(unsigned long data)
>>>
>>>         buf = dev->isoc_ctl.buf;
>>>         if (buf != NULL) {
>>> -               vid_data = videobuf_to_vmalloc(&buf->vb);
>>> -               memset(vid_data, 0x00, buf->vb.size); /* Blank green frame */
>>> +               vid_data = vb2_plane_vaddr(&buf->vb, 0);
>>> +               memset(vid_data, 0x00, buf->length); /* Blank green frame */
>>>                 buffer_filled(dev, dma_q, buf);
>>>         }
>>>         get_next_buf(dma_q, &buf);
>>> @@ -974,8 +955,8 @@ static void au0828_vbi_buffer_timeout(unsigned long data)
>>>
>>>         buf = dev->isoc_ctl.vbi_buf;
>>>         if (buf != NULL) {
>>> -               vbi_data = videobuf_to_vmalloc(&buf->vb);
>>> -               memset(vbi_data, 0x00, buf->vb.size);
>>> +               vbi_data = vb2_plane_vaddr(&buf->vb, 0);
>>> +               memset(vbi_data, 0x00, buf->length);
>>>                 vbi_buffer_filled(dev, dma_q, buf);
>>>         }
>>>         vbi_get_next_buf(dma_q, &buf);
>>> @@ -985,105 +966,65 @@ static void au0828_vbi_buffer_timeout(unsigned long data)
>>>         spin_unlock_irqrestore(&dev->slock, flags);
>>>  }
>>>
>>> -
>>>  static int au0828_v4l2_open(struct file *filp)
>>>  {
>>> -       int ret = 0;
>>> -       struct video_device *vdev = video_devdata(filp);
>>>         struct au0828_dev *dev = video_drvdata(filp);
>>> -       struct au0828_fh *fh;
>>> -       int type;
>>> -
>>> -       switch (vdev->vfl_type) {
>>> -       case VFL_TYPE_GRABBER:
>>> -               type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
>>> -               break;
>>> -       case VFL_TYPE_VBI:
>>> -               type = V4L2_BUF_TYPE_VBI_CAPTURE;
>>> -               break;
>>> -       default:
>>> -               return -EINVAL;
>>> -       }
>>> -
>>> -       fh = kzalloc(sizeof(struct au0828_fh), GFP_KERNEL);
>>> -       if (NULL == fh) {
>>> -               dprintk(1, "Failed allocate au0828_fh struct!\n");
>>> -               return -ENOMEM;
>>> -       }
>>> +       int ret = 0;
>>>
>> No need to assign it to zero.
>
> Yes. Looks like there are a few other routines that zero out
> ret. If you want to see all of those cleaned up, I can do a
> separate patch cleaning all these instances. If not I can send
> patch v4? Please let me know your preference.
>
No it needs to go in this patch itself for here.

>>
>>> -       fh->type = type;
>>> -       fh->dev = dev;
>>> -       v4l2_fh_init(&fh->fh, vdev);
>>> -       filp->private_data = fh;
>>> +       dprintk(1,
>>> +               "%s called std_set %d dev_state %d stream users %d users %d\n",
>>> +               __func__, dev->std_set_in_tuner_core, dev->dev_state,
>>> +               dev->streaming_users, dev->users);
>>>
>>> -       if (mutex_lock_interruptible(&dev->lock)) {
>>> -               kfree(fh);
>>> +       if (mutex_lock_interruptible(&dev->lock))
>>>                 return -ERESTARTSYS;
>>> +
>>> +       ret = v4l2_fh_open(filp);
>>> +       if (ret) {
>>> +               au0828_isocdbg("%s: v4l2_fh_open() returned error %d\n",
>>> +                               __func__, ret);
>>> +               mutex_unlock(&dev->lock);
>>> +               return ret;
>>>         }
>>> +
>>>         if (dev->users == 0) {
>>
>> you can use v4l2_fh_is_singular_file() and get rid of users member ?
>
> Please see Devin's response on this. I have been debugging the change
> I made to use v4l2_fh_is_singular_file() instead of users and seeing
> problem with reset stream and couldn't really figure out why until
> Devin explained it.
>
Agreed.

Thanks,
--Prabhakar Lad

Re: [PATCH v3 2/3] media: au0828 - convert to use videobuf2

[Shuah Khan]

On 01/22/2015 01:47 PM, Lad, Prabhakar wrote:
> Hi Shuah,
> 
> On Thu, Jan 22, 2015 at 4:16 PM, Shuah Khan <shuahkh@osg.samsung.com> wrote:
>> Hi Prabhakar,
>>
> [snip]
>>>> +       buf->vb.v4l2_buf.field = V4L2_FIELD_INTERLACED;
>>>> +       v4l2_get_timestamp(&buf->vb.v4l2_buf.timestamp);
>>>> +       vb2_buffer_done(&buf->vb, VB2_BUF_STATE_DONE);
>>>>  }
>>>>
>>> Why not just have one single buffer_filled function ? just check the
>>> queue type and assign the dev->isoc_ctl.buf/ dev->isoc_ctl.vbi_buf
>>> to NULL.
>>
>> Yes. These two routines could be collapsed into a single. Is it okay if
>> I made that change in a separate patch?
>>
> hmm.. this can go as a separate patch.

Thanks. Looked into this a bit more. It is definitely better done as a
separate patch.

> 
>>>
>>>>  /*
>>>> @@ -353,8 +340,8 @@ static void au0828_copy_video(struct au0828_dev *dev,
>>>>         if (len == 0)
>>>>                 return;
>>>>
>>>> -       if (dma_q->pos + len > buf->vb.size)
>>>> -               len = buf->vb.size - dma_q->pos;
>>>> +       if (dma_q->pos + len > buf->length)
>>>> +               len = buf->length - dma_q->pos;
>>>>
>>>>         startread = p;
>>>>         remain = len;
>>>> @@ -372,11 +359,11 @@ static void au0828_copy_video(struct au0828_dev *dev,
>>>>         lencopy = bytesperline - currlinedone;
>>>>         lencopy = lencopy > remain ? remain : lencopy;
>>>>
>>>> -       if ((char *)startwrite + lencopy > (char *)outp + buf->vb.size) {
>>>> +       if ((char *)startwrite + lencopy > (char *)outp + buf->length) {
>>>>                 au0828_isocdbg("Overflow of %zi bytes past buffer end (1)\n",
>>>>                                ((char *)startwrite + lencopy) -
>>>> -                              ((char *)outp + buf->vb.size));
>>>> -               remain = (char *)outp + buf->vb.size - (char *)startwrite;
>>>> +                              ((char *)outp + buf->length));
>>>> +               remain = (char *)outp + buf->length - (char *)startwrite;
>>>>                 lencopy = remain;
>>>>         }
>>>>         if (lencopy <= 0)
>>>> @@ -394,11 +381,11 @@ static void au0828_copy_video(struct au0828_dev *dev,
>>>>                         lencopy = bytesperline;
>>>>
>>>>                 if ((char *)startwrite + lencopy > (char *)outp +
>>>> -                   buf->vb.size) {
>>>> +                   buf->length) {
>>>>                         au0828_isocdbg("Overflow %zi bytes past buf end (2)\n",
>>>>                                        ((char *)startwrite + lencopy) -
>>>> -                                      ((char *)outp + buf->vb.size));
>>>> -                       lencopy = remain = (char *)outp + buf->vb.size -
>>>> +                                      ((char *)outp + buf->length));
>>>> +                       lencopy = remain = (char *)outp + buf->length -
>>>>                                            (char *)startwrite;
>>>>                 }
>>>>                 if (lencopy <= 0)
>>>> @@ -434,7 +421,11 @@ static inline void get_next_buf(struct au0828_dmaqueue *dma_q,
>>>>         }
>>>>
>>>>         /* Get the next buffer */
>>>> -       *buf = list_entry(dma_q->active.next, struct au0828_buffer, vb.queue);
>>>> +       *buf = list_entry(dma_q->active.next, struct au0828_buffer, list);
>>>> +       /* Cleans up buffer - Useful for testing for frame/URB loss */
>>>> +       list_del(&(*buf)->list);
>>>> +       dma_q->pos = 0;
>>>> +       (*buf)->vb_buf = (*buf)->mem;
>>>>         dev->isoc_ctl.buf = *buf;
>>>>
>>>>         return;
>>>> @@ -472,8 +463,8 @@ static void au0828_copy_vbi(struct au0828_dev *dev,
>>>>
>>>>         bytesperline = dev->vbi_width;
>>>>
>>>> -       if (dma_q->pos + len > buf->vb.size)
>>>> -               len = buf->vb.size - dma_q->pos;
>>>> +       if (dma_q->pos + len > buf->length)
>>>> +               len = buf->length - dma_q->pos;
>>>>
>>>>         startread = p;
>>>>         startwrite = outp + (dma_q->pos / 2);
>>>> @@ -496,7 +487,6 @@ static inline void vbi_get_next_buf(struct au0828_dmaqueue *dma_q,
>>>>                                     struct au0828_buffer **buf)
>>>>  {
>>>>         struct au0828_dev *dev = container_of(dma_q, struct au0828_dev, vbiq);
>>>> -       char *outp;
>>>>
>>>>         if (list_empty(&dma_q->active)) {
>>>>                 au0828_isocdbg("No active queue to serve\n");
>>>> @@ -506,13 +496,12 @@ static inline void vbi_get_next_buf(struct au0828_dmaqueue *dma_q,
>>>>         }
>>>>
>>>>         /* Get the next buffer */
>>>> -       *buf = list_entry(dma_q->active.next, struct au0828_buffer, vb.queue);
>>>> +       *buf = list_entry(dma_q->active.next, struct au0828_buffer, list);
>>>>         /* Cleans up buffer - Useful for testing for frame/URB loss */
>>>> -       outp = videobuf_to_vmalloc(&(*buf)->vb);
>>>> -       memset(outp, 0x00, (*buf)->vb.size);
>>>> -
>>>> +       list_del(&(*buf)->list);
>>>> +       dma_q->pos = 0;
>>>> +       (*buf)->vb_buf = (*buf)->mem;
>>>>         dev->isoc_ctl.vbi_buf = *buf;
>>>> -
>>>>         return;
>>>>  }
>>>>
>>>> @@ -548,11 +537,11 @@ static inline int au0828_isoc_copy(struct au0828_dev *dev, struct urb *urb)
>>>>
>>>>         buf = dev->isoc_ctl.buf;
>>>>         if (buf != NULL)
>>>> -               outp = videobuf_to_vmalloc(&buf->vb);
>>>> +               outp = vb2_plane_vaddr(&buf->vb, 0);
>>>>
>>>>         vbi_buf = dev->isoc_ctl.vbi_buf;
>>>>         if (vbi_buf != NULL)
>>>> -               vbioutp = videobuf_to_vmalloc(&vbi_buf->vb);
>>>> +               vbioutp = vb2_plane_vaddr(&vbi_buf->vb, 0);
>>>>
>>>>         for (i = 0; i < urb->number_of_packets; i++) {
>>>>                 int status = urb->iso_frame_desc[i].status;
>>>> @@ -592,8 +581,8 @@ static inline int au0828_isoc_copy(struct au0828_dev *dev, struct urb *urb)
>>>>                                 if (vbi_buf == NULL)
>>>>                                         vbioutp = NULL;
>>>>                                 else
>>>> -                                       vbioutp = videobuf_to_vmalloc(
>>>> -                                               &vbi_buf->vb);
>>>> +                                       vbioutp = vb2_plane_vaddr(
>>>> +                                               &vbi_buf->vb, 0);
>>>>
>>>>                                 /* Video */
>>>>                                 if (buf != NULL)
>>>> @@ -602,7 +591,7 @@ static inline int au0828_isoc_copy(struct au0828_dev *dev, struct urb *urb)
>>>>                                 if (buf == NULL)
>>>>                                         outp = NULL;
>>>>                                 else
>>>> -                                       outp = videobuf_to_vmalloc(&buf->vb);
>>>> +                                       outp = vb2_plane_vaddr(&buf->vb, 0);
>>>>
>>>>                                 /* As long as isoc traffic is arriving, keep
>>>>                                    resetting the timer */
>>>> @@ -656,130 +645,59 @@ static inline int au0828_isoc_copy(struct au0828_dev *dev, struct urb *urb)
>>>>         return rc;
>>>>  }
>>>>
>>>> -static int
>>>> -buffer_setup(struct videobuf_queue *vq, unsigned int *count,
>>>> -            unsigned int *size)
>>>> +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 au0828_fh *fh = vq->priv_data;
>>>> -       *size = (fh->dev->width * fh->dev->height * 16 + 7) >> 3;
>>>> -
>>>> -       if (0 == *count)
>>>> -               *count = AU0828_DEF_BUF;
>>>> +       struct au0828_dev *dev = vb2_get_drv_priv(vq);
>>>> +       unsigned long img_size = dev->height * dev->bytesperline;
>>>> +       unsigned long size;
>>>>
>>>> -       if (*count < AU0828_MIN_BUF)
>>>> -               *count = AU0828_MIN_BUF;
>>>> -       return 0;
>>>> -}
>>>> +       size = fmt ? fmt->fmt.pix.sizeimage : img_size;
>>>> +       if (size < img_size)
>>>> +               return -EINVAL;
>>>>
>>>> -/* This is called *without* dev->slock held; please keep it that way */
>>>> -static void free_buffer(struct videobuf_queue *vq, struct au0828_buffer *buf)
>>>> -{
>>>> -       struct au0828_fh     *fh  = vq->priv_data;
>>>> -       struct au0828_dev    *dev = fh->dev;
>>>> -       unsigned long flags = 0;
>>>> -       if (in_interrupt())
>>>> -               BUG();
>>>> -
>>>> -       /* We used to wait for the buffer to finish here, but this didn't work
>>>> -          because, as we were keeping the state as VIDEOBUF_QUEUED,
>>>> -          videobuf_queue_cancel marked it as finished for us.
>>>> -          (Also, it could wedge forever if the hardware was misconfigured.)
>>>> -
>>>> -          This should be safe; by the time we get here, the buffer isn't
>>>> -          queued anymore. If we ever start marking the buffers as
>>>> -          VIDEOBUF_ACTIVE, it won't be, though.
>>>> -       */
>>>> -       spin_lock_irqsave(&dev->slock, flags);
>>>> -       if (dev->isoc_ctl.buf == buf)
>>>> -               dev->isoc_ctl.buf = NULL;
>>>> -       spin_unlock_irqrestore(&dev->slock, flags);
>>>> +       *nplanes = 1;
>>>> +       sizes[0] = size;
>>>>
>>>> -       videobuf_vmalloc_free(&buf->vb);
>>>> -       buf->vb.state = VIDEOBUF_NEEDS_INIT;
>>>> +       return 0;
>>>>  }
>>>>
>>>>  static int
>>>> -buffer_prepare(struct videobuf_queue *vq, struct videobuf_buffer *vb,
>>>> -                                               enum v4l2_field field)
>>>> +buffer_prepare(struct vb2_buffer *vb)
>>>>  {
>>>> -       struct au0828_fh     *fh  = vq->priv_data;
>>>>         struct au0828_buffer *buf = container_of(vb, struct au0828_buffer, vb);
>>>> -       struct au0828_dev    *dev = fh->dev;
>>>> -       int                  rc = 0, urb_init = 0;
>>>> +       struct au0828_dev    *dev = vb2_get_drv_priv(vb->vb2_queue);
>>>>
>>>> -       buf->vb.size = (fh->dev->width * fh->dev->height * 16 + 7) >> 3;
>>>> +       buf->length = dev->height * dev->bytesperline;
>>>>
>>>> -       if (0 != buf->vb.baddr  &&  buf->vb.bsize < buf->vb.size)
>>>> +       if (vb2_plane_size(vb, 0) < buf->length) {
>>>> +               pr_err("%s data will not fit into plane (%lu < %lu)\n",
>>>> +                       __func__, vb2_plane_size(vb, 0), buf->length);
>>>>                 return -EINVAL;
>>>> -
>>>> -       buf->vb.width  = dev->width;
>>>> -       buf->vb.height = dev->height;
>>>> -       buf->vb.field  = field;
>>>> -
>>>> -       if (VIDEOBUF_NEEDS_INIT == buf->vb.state) {
>>>> -               rc = videobuf_iolock(vq, &buf->vb, NULL);
>>>> -               if (rc < 0) {
>>>> -                       pr_info("videobuf_iolock failed\n");
>>>> -                       goto fail;
>>>> -               }
>>>>         }
>>>> -
>>>> -       if (!dev->isoc_ctl.num_bufs)
>>>> -               urb_init = 1;
>>>> -
>>>> -       if (urb_init) {
>>>> -               rc = au0828_init_isoc(dev, AU0828_ISO_PACKETS_PER_URB,
>>>> -                                     AU0828_MAX_ISO_BUFS, dev->max_pkt_size,
>>>> -                                     au0828_isoc_copy);
>>>> -               if (rc < 0) {
>>>> -                       pr_info("au0828_init_isoc failed\n");
>>>> -                       goto fail;
>>>> -               }
>>>> -       }
>>>> -
>>>> -       buf->vb.state = VIDEOBUF_PREPARED;
>>>> +       vb2_set_plane_payload(&buf->vb, 0, buf->length);
>>>>         return 0;
>>>> -
>>>> -fail:
>>>> -       free_buffer(vq, buf);
>>>> -       return rc;
>>>>  }
>>>>
>>>>  static void
>>>> -buffer_queue(struct videobuf_queue *vq, struct videobuf_buffer *vb)
>>>> +buffer_queue(struct vb2_buffer *vb)
>>>>  {
>>>>         struct au0828_buffer    *buf     = container_of(vb,
>>>>                                                         struct au0828_buffer,
>>>>                                                         vb);
>>>> -       struct au0828_fh        *fh      = vq->priv_data;
>>>> -       struct au0828_dev       *dev     = fh->dev;
>>>> +       struct au0828_dev       *dev     = vb2_get_drv_priv(vb->vb2_queue);
>>>>         struct au0828_dmaqueue  *vidq    = &dev->vidq;
>>>> +       unsigned long flags = 0;
>>>>
>>>> -       buf->vb.state = VIDEOBUF_QUEUED;
>>>> -       list_add_tail(&buf->vb.queue, &vidq->active);
>>>> -}
>>>> -
>>>> -static void buffer_release(struct videobuf_queue *vq,
>>>> -                               struct videobuf_buffer *vb)
>>>> -{
>>>> -       struct au0828_buffer   *buf  = container_of(vb,
>>>> -                                                   struct au0828_buffer,
>>>> -                                                   vb);
>>>> +       buf->mem = vb2_plane_vaddr(vb, 0);
>>>> +       buf->length = vb2_plane_size(vb, 0);
>>>>
>>>> -       free_buffer(vq, buf);
>>>> +       spin_lock_irqsave(&dev->slock, flags);
>>>> +       list_add_tail(&buf->list, &vidq->active);
>>>> +       spin_unlock_irqrestore(&dev->slock, flags);
>>>>  }
>>>>
>>>> -static struct videobuf_queue_ops au0828_video_qops = {
>>>> -       .buf_setup      = buffer_setup,
>>>> -       .buf_prepare    = buffer_prepare,
>>>> -       .buf_queue      = buffer_queue,
>>>> -       .buf_release    = buffer_release,
>>>> -};
>>>> -
>>>> -/* ------------------------------------------------------------------
>>>> -   V4L2 interface
>>>> -   ------------------------------------------------------------------*/
>>>> -
>>>>  static int au0828_i2s_init(struct au0828_dev *dev)
>>>>  {
>>>>         /* Enable i2s mode */
>>>> @@ -828,7 +746,7 @@ static int au0828_analog_stream_enable(struct au0828_dev *d)
>>>>         return 0;
>>>>  }
>>>>
>>>> -int au0828_analog_stream_disable(struct au0828_dev *d)
>>>> +static int au0828_analog_stream_disable(struct au0828_dev *d)
>>>>  {
>>>>         dprintk(1, "au0828_analog_stream_disable called\n");
>>>>         au0828_writereg(d, AU0828_SENSORCTRL_100, 0x0);
>>>> @@ -861,78 +779,141 @@ static int au0828_stream_interrupt(struct au0828_dev *dev)
>>>>         return 0;
>>>>  }
>>>>
>>>> -/*
>>>> - * au0828_release_resources
>>>> - * unregister v4l2 devices
>>>> - */
>>>> -void au0828_analog_unregister(struct au0828_dev *dev)
>>>> +int au0828_start_analog_streaming(struct vb2_queue *vq, unsigned int count)
>>>>  {
>>>> -       dprintk(1, "au0828_release_resources called\n");
>>>> -       mutex_lock(&au0828_sysfs_lock);
>>>> +       struct au0828_dev *dev = vb2_get_drv_priv(vq);
>>>> +       int rc = 0;
>>>>
>>>> -       if (dev->vdev)
>>>> -               video_unregister_device(dev->vdev);
>>>> -       if (dev->vbi_dev)
>>>> -               video_unregister_device(dev->vbi_dev);
>>>> +       dprintk(1, "au0828_start_analog_streaming called %d\n",
>>>> +               dev->streaming_users);
>>>>
>>>> -       mutex_unlock(&au0828_sysfs_lock);
>>>> -}
>>>> +       if (vq->type == V4L2_BUF_TYPE_VIDEO_CAPTURE)
>>>> +               dev->frame_count = 0;
>>>> +       else
>>>> +               dev->vbi_frame_count = 0;
>>>> +
>>>> +       if (dev->streaming_users == 0) {
>>>> +               /* If we were doing ac97 instead of i2s, it would go here...*/
>>>> +               au0828_i2s_init(dev);
>>>> +               rc = au0828_init_isoc(dev, AU0828_ISO_PACKETS_PER_URB,
>>>> +                                  AU0828_MAX_ISO_BUFS, dev->max_pkt_size,
>>>> +                                  au0828_isoc_copy);
>>>> +               if (rc < 0) {
>>>> +                       pr_info("au0828_init_isoc failed\n");
>>>> +                       return rc;
>>>> +               }
>>>>
>>>> +               if (vq->type == V4L2_BUF_TYPE_VIDEO_CAPTURE) {
>>>> +                       v4l2_device_call_all(&dev->v4l2_dev, 0, video,
>>>> +                                               s_stream, 1);
>>>> +                       dev->vid_timeout_running = 1;
>>>> +                       mod_timer(&dev->vid_timeout, jiffies + (HZ / 10));
>>>> +               } else if (vq->type == V4L2_BUF_TYPE_VBI_CAPTURE) {
>>>> +                       dev->vbi_timeout_running = 1;
>>>> +                       mod_timer(&dev->vbi_timeout, jiffies + (HZ / 10));
>>>> +               }
>>>> +       }
>>>> +       dev->streaming_users++;
>>>> +       return rc;
>>>> +}
>>>>
>>>> -/* Usage lock check functions */
>>>> -static int res_get(struct au0828_fh *fh, unsigned int bit)
>>>> +static void au0828_stop_streaming(struct vb2_queue *vq)
>>>>  {
>>>> -       struct au0828_dev    *dev = fh->dev;
>>>> +       struct au0828_dev *dev = vb2_get_drv_priv(vq);
>>>> +       struct au0828_dmaqueue *vidq = &dev->vidq;
>>>> +       unsigned long flags = 0;
>>>> +       int i;
>>>>
>>>> -       if (fh->resources & bit)
>>>> -               /* have it already allocated */
>>>> -               return 1;
>>>> +       dprintk(1, "au0828_stop_streaming called %d\n", dev->streaming_users);
>>>>
>>>> -       /* is it free? */
>>>> -       if (dev->resources & bit) {
>>>> -               /* no, someone else uses it */
>>>> -               return 0;
>>>> +       if (dev->streaming_users-- == 1)
>>>> +               au0828_uninit_isoc(dev);
>>>> +
>>>> +       spin_lock_irqsave(&dev->slock, flags);
>>>> +       if (dev->isoc_ctl.buf != NULL) {
>>>> +               vb2_buffer_done(&dev->isoc_ctl.buf->vb, VB2_BUF_STATE_ERROR);
>>>> +               dev->isoc_ctl.buf = NULL;
>>>>         }
>>>> -       /* it's free, grab it */
>>>> -       fh->resources  |= bit;
>>>> -       dev->resources |= bit;
>>>> -       dprintk(1, "res: get %d\n", bit);
>>>> +       while (!list_empty(&vidq->active)) {
>>>> +               struct au0828_buffer *buf;
>>>>
>>>> -       return 1;
>>>> -}
>>>> +               buf = list_entry(vidq->active.next, struct au0828_buffer, list);
>>>> +               vb2_buffer_done(&buf->vb, VB2_BUF_STATE_ERROR);
>>>> +               list_del(&buf->list);
>>>> +       }
>>>>
>>>> -static int res_check(struct au0828_fh *fh, unsigned int bit)
>>>> -{
>>>> -       return fh->resources & bit;
>>>> -}
>>>> +       v4l2_device_call_all(&dev->v4l2_dev, 0, video, s_stream, 0);
>>>>
>>>> -static int res_locked(struct au0828_dev *dev, unsigned int bit)
>>>> -{
>>>> -       return dev->resources & bit;
>>>> +       for (i = 0; i < AU0828_MAX_INPUT; i++) {
>>>> +               if (AUVI_INPUT(i).audio_setup == NULL)
>>>> +                       continue;
>>>> +               (AUVI_INPUT(i).audio_setup)(dev, 0);
>>>> +       }
>>>> +       spin_unlock_irqrestore(&dev->slock, flags);
>>>> +
>>>> +       dev->vid_timeout_running = 0;
>>>> +       del_timer_sync(&dev->vid_timeout);
>>>>  }
>>>>
>>>> -static void res_free(struct au0828_fh *fh, unsigned int bits)
>>>> +void au0828_stop_vbi_streaming(struct vb2_queue *vq)
>>>>  {
>>>> -       struct au0828_dev    *dev = fh->dev;
>>>> +       struct au0828_dev *dev = vb2_get_drv_priv(vq);
>>>> +       struct au0828_dmaqueue *vbiq = &dev->vbiq;
>>>> +       unsigned long flags = 0;
>>>> +
>>>> +       dprintk(1, "au0828_stop_vbi_streaming called %d\n",
>>>> +               dev->streaming_users);
>>>> +
>>>> +       if (dev->streaming_users-- == 1)
>>>> +               au0828_uninit_isoc(dev);
>>>>
>>>> -       BUG_ON((fh->resources & bits) != bits);
>>>> +       spin_lock_irqsave(&dev->slock, flags);
>>>> +       if (dev->isoc_ctl.vbi_buf != NULL) {
>>>> +               vb2_buffer_done(&dev->isoc_ctl.vbi_buf->vb,
>>>> +                               VB2_BUF_STATE_ERROR);
>>>> +               dev->isoc_ctl.vbi_buf = NULL;
>>>> +       }
>>>> +       while (!list_empty(&vbiq->active)) {
>>>> +               struct au0828_buffer *buf;
>>>> +
>>>> +               buf = list_entry(vbiq->active.next, struct au0828_buffer, list);
>>>> +               list_del(&buf->list);
>>>> +               vb2_buffer_done(&buf->vb, VB2_BUF_STATE_ERROR);
>>>> +       }
>>>> +       spin_unlock_irqrestore(&dev->slock, flags);
>>>>
>>>> -       fh->resources  &= ~bits;
>>>> -       dev->resources &= ~bits;
>>>> -       dprintk(1, "res: put %d\n", bits);
>>>> +       dev->vbi_timeout_running = 0;
>>>> +       del_timer_sync(&dev->vbi_timeout);
>>>>  }
>>>>
>>>> -static int get_ressource(struct au0828_fh *fh)
>>>> +static struct vb2_ops au0828_video_qops = {
>>>> +       .queue_setup     = queue_setup,
>>>> +       .buf_prepare     = buffer_prepare,
>>>> +       .buf_queue       = buffer_queue,
>>>> +       .start_streaming = au0828_start_analog_streaming,
>>>> +       .stop_streaming  = au0828_stop_streaming,
>>>> +       .wait_prepare    = vb2_ops_wait_prepare,
>>>> +       .wait_finish     = vb2_ops_wait_finish,
>>>> +};
>>>> +
>>>> +/* ------------------------------------------------------------------
>>>> +   V4L2 interface
>>>> +   ------------------------------------------------------------------*/
>>>> +/*
>>>> + * au0828_analog_unregister
>>>> + * unregister v4l2 devices
>>>> + */
>>>> +void au0828_analog_unregister(struct au0828_dev *dev)
>>>>  {
>>>> -       switch (fh->type) {
>>>> -       case V4L2_BUF_TYPE_VIDEO_CAPTURE:
>>>> -               return AU0828_RESOURCE_VIDEO;
>>>> -       case V4L2_BUF_TYPE_VBI_CAPTURE:
>>>> -               return AU0828_RESOURCE_VBI;
>>>> -       default:
>>>> -               BUG();
>>>> -               return 0;
>>>> -       }
>>>> +       dprintk(1, "au0828_analog_unregister called\n");
>>>> +       mutex_lock(&au0828_sysfs_lock);
>>>> +
>>>> +       if (dev->vdev)
>>>> +               video_unregister_device(dev->vdev);
>>>> +       if (dev->vbi_dev)
>>>> +               video_unregister_device(dev->vbi_dev);
>>>> +
>>>> +       mutex_unlock(&au0828_sysfs_lock);
>>>>  }
>>>>
>>>>  /* This function ensures that video frames continue to be delivered even if
>>>> @@ -950,8 +931,8 @@ static void au0828_vid_buffer_timeout(unsigned long data)
>>>>
>>>>         buf = dev->isoc_ctl.buf;
>>>>         if (buf != NULL) {
>>>> -               vid_data = videobuf_to_vmalloc(&buf->vb);
>>>> -               memset(vid_data, 0x00, buf->vb.size); /* Blank green frame */
>>>> +               vid_data = vb2_plane_vaddr(&buf->vb, 0);
>>>> +               memset(vid_data, 0x00, buf->length); /* Blank green frame */
>>>>                 buffer_filled(dev, dma_q, buf);
>>>>         }
>>>>         get_next_buf(dma_q, &buf);
>>>> @@ -974,8 +955,8 @@ static void au0828_vbi_buffer_timeout(unsigned long data)
>>>>
>>>>         buf = dev->isoc_ctl.vbi_buf;
>>>>         if (buf != NULL) {
>>>> -               vbi_data = videobuf_to_vmalloc(&buf->vb);
>>>> -               memset(vbi_data, 0x00, buf->vb.size);
>>>> +               vbi_data = vb2_plane_vaddr(&buf->vb, 0);
>>>> +               memset(vbi_data, 0x00, buf->length);
>>>>                 vbi_buffer_filled(dev, dma_q, buf);
>>>>         }
>>>>         vbi_get_next_buf(dma_q, &buf);
>>>> @@ -985,105 +966,65 @@ static void au0828_vbi_buffer_timeout(unsigned long data)
>>>>         spin_unlock_irqrestore(&dev->slock, flags);
>>>>  }
>>>>
>>>> -
>>>>  static int au0828_v4l2_open(struct file *filp)
>>>>  {
>>>> -       int ret = 0;
>>>> -       struct video_device *vdev = video_devdata(filp);
>>>>         struct au0828_dev *dev = video_drvdata(filp);
>>>> -       struct au0828_fh *fh;
>>>> -       int type;
>>>> -
>>>> -       switch (vdev->vfl_type) {
>>>> -       case VFL_TYPE_GRABBER:
>>>> -               type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
>>>> -               break;
>>>> -       case VFL_TYPE_VBI:
>>>> -               type = V4L2_BUF_TYPE_VBI_CAPTURE;
>>>> -               break;
>>>> -       default:
>>>> -               return -EINVAL;
>>>> -       }
>>>> -
>>>> -       fh = kzalloc(sizeof(struct au0828_fh), GFP_KERNEL);
>>>> -       if (NULL == fh) {
>>>> -               dprintk(1, "Failed allocate au0828_fh struct!\n");
>>>> -               return -ENOMEM;
>>>> -       }
>>>> +       int ret = 0;
>>>>
>>> No need to assign it to zero.
>>
>> Yes. Looks like there are a few other routines that zero out
>> ret. If you want to see all of those cleaned up, I can do a
>> separate patch cleaning all these instances. If not I can send
>> patch v4? Please let me know your preference.
>>
> No it needs to go in this patch itself for here.

No problem. I will send patch v4 for this patch with this change.

> 
>>>
>>>> -       fh->type = type;
>>>> -       fh->dev = dev;
>>>> -       v4l2_fh_init(&fh->fh, vdev);
>>>> -       filp->private_data = fh;
>>>> +       dprintk(1,
>>>> +               "%s called std_set %d dev_state %d stream users %d users %d\n",
>>>> +               __func__, dev->std_set_in_tuner_core, dev->dev_state,
>>>> +               dev->streaming_users, dev->users);
>>>>
>>>> -       if (mutex_lock_interruptible(&dev->lock)) {
>>>> -               kfree(fh);
>>>> +       if (mutex_lock_interruptible(&dev->lock))
>>>>                 return -ERESTARTSYS;
>>>> +
>>>> +       ret = v4l2_fh_open(filp);
>>>> +       if (ret) {
>>>> +               au0828_isocdbg("%s: v4l2_fh_open() returned error %d\n",
>>>> +                               __func__, ret);
>>>> +               mutex_unlock(&dev->lock);
>>>> +               return ret;
>>>>         }
>>>> +
>>>>         if (dev->users == 0) {
>>>
>>> you can use v4l2_fh_is_singular_file() and get rid of users member ?
>>
>> Please see Devin's response on this. I have been debugging the change
>> I made to use v4l2_fh_is_singular_file() instead of users and seeing
>> problem with reset stream and couldn't really figure out why until
>> Devin explained it.
>>
> Agreed.

Thanks,
-- Shuah


-- 
Shuah Khan
Sr. Linux Kernel Developer
Open Source Innovation Group
Samsung Research America (Silicon Valley)
shuahkh@osg.samsung.com | (970) 217-8978

[PATCH v3 3/3] media: au0828 remove video and vbi buffer timeout work-around

[Shuah Khan]

au0828 does video and vbi buffer timeout handling to prevent
applications such as tvtime from hanging by ensuring that the
video frames continue to be delivered even when the ITU-656
input isn't receiving any data. This work-around is complex
as it introduces set and clear timer code paths in start/stop
streaming, and close interfaces. However, tvtime will hang
without the following tvtime change:

'tvtime: don't block indefinitely waiting for frames' with

this change to remove timeout, if there is no valid video data.

Signed-off-by: Shuah Khan <shuahkh@osg.samsung.com>
---
 drivers/media/usb/au0828/au0828-video.c | 103 +-------------------------------
 drivers/media/usb/au0828/au0828.h       |   5 --
 2 files changed, 1 insertion(+), 107 deletions(-)

diff --git a/drivers/media/usb/au0828/au0828-video.c b/drivers/media/usb/au0828/au0828-video.c
index e427913..08b1e96 100644
--- a/drivers/media/usb/au0828/au0828-video.c
+++ b/drivers/media/usb/au0828/au0828-video.c
@@ -592,15 +592,6 @@ static inline int au0828_isoc_copy(struct au0828_dev *dev, struct urb *urb)
 					outp = NULL;
 				else
 					outp = vb2_plane_vaddr(&buf->vb, 0);
-
-				/* As long as isoc traffic is arriving, keep
-				   resetting the timer */
-				if (dev->vid_timeout_running)
-					mod_timer(&dev->vid_timeout,
-						  jiffies + (HZ / 10));
-				if (dev->vbi_timeout_running)
-					mod_timer(&dev->vbi_timeout,
-						  jiffies + (HZ / 10));
 			}
 
 			if (buf != NULL) {
@@ -803,15 +794,9 @@ int au0828_start_analog_streaming(struct vb2_queue *vq, unsigned int count)
 			return rc;
 		}
 
-		if (vq->type == V4L2_BUF_TYPE_VIDEO_CAPTURE) {
+		if (vq->type == V4L2_BUF_TYPE_VIDEO_CAPTURE)
 			v4l2_device_call_all(&dev->v4l2_dev, 0, video,
 						s_stream, 1);
-			dev->vid_timeout_running = 1;
-			mod_timer(&dev->vid_timeout, jiffies + (HZ / 10));
-		} else if (vq->type == V4L2_BUF_TYPE_VBI_CAPTURE) {
-			dev->vbi_timeout_running = 1;
-			mod_timer(&dev->vbi_timeout, jiffies + (HZ / 10));
-		}
 	}
 	dev->streaming_users++;
 	return rc;
@@ -850,9 +835,6 @@ static void au0828_stop_streaming(struct vb2_queue *vq)
 		(AUVI_INPUT(i).audio_setup)(dev, 0);
 	}
 	spin_unlock_irqrestore(&dev->slock, flags);
-
-	dev->vid_timeout_running = 0;
-	del_timer_sync(&dev->vid_timeout);
 }
 
 void au0828_stop_vbi_streaming(struct vb2_queue *vq)
@@ -881,9 +863,6 @@ void au0828_stop_vbi_streaming(struct vb2_queue *vq)
 		vb2_buffer_done(&buf->vb, VB2_BUF_STATE_ERROR);
 	}
 	spin_unlock_irqrestore(&dev->slock, flags);
-
-	dev->vbi_timeout_running = 0;
-	del_timer_sync(&dev->vbi_timeout);
 }
 
 static struct vb2_ops au0828_video_qops = {
@@ -916,56 +895,6 @@ void au0828_analog_unregister(struct au0828_dev *dev)
 	mutex_unlock(&au0828_sysfs_lock);
 }
 
-/* This function ensures that video frames continue to be delivered even if
-   the ITU-656 input isn't receiving any data (thereby preventing applications
-   such as tvtime from hanging) */
-static void au0828_vid_buffer_timeout(unsigned long data)
-{
-	struct au0828_dev *dev = (struct au0828_dev *) data;
-	struct au0828_dmaqueue *dma_q = &dev->vidq;
-	struct au0828_buffer *buf;
-	unsigned char *vid_data;
-	unsigned long flags = 0;
-
-	spin_lock_irqsave(&dev->slock, flags);
-
-	buf = dev->isoc_ctl.buf;
-	if (buf != NULL) {
-		vid_data = vb2_plane_vaddr(&buf->vb, 0);
-		memset(vid_data, 0x00, buf->length); /* Blank green frame */
-		buffer_filled(dev, dma_q, buf);
-	}
-	get_next_buf(dma_q, &buf);
-
-	if (dev->vid_timeout_running == 1)
-		mod_timer(&dev->vid_timeout, jiffies + (HZ / 10));
-
-	spin_unlock_irqrestore(&dev->slock, flags);
-}
-
-static void au0828_vbi_buffer_timeout(unsigned long data)
-{
-	struct au0828_dev *dev = (struct au0828_dev *) data;
-	struct au0828_dmaqueue *dma_q = &dev->vbiq;
-	struct au0828_buffer *buf;
-	unsigned char *vbi_data;
-	unsigned long flags = 0;
-
-	spin_lock_irqsave(&dev->slock, flags);
-
-	buf = dev->isoc_ctl.vbi_buf;
-	if (buf != NULL) {
-		vbi_data = vb2_plane_vaddr(&buf->vb, 0);
-		memset(vbi_data, 0x00, buf->length);
-		vbi_buffer_filled(dev, dma_q, buf);
-	}
-	vbi_get_next_buf(dma_q, &buf);
-
-	if (dev->vbi_timeout_running == 1)
-		mod_timer(&dev->vbi_timeout, jiffies + (HZ / 10));
-	spin_unlock_irqrestore(&dev->slock, flags);
-}
-
 static int au0828_v4l2_open(struct file *filp)
 {
 	struct au0828_dev *dev = video_drvdata(filp);
@@ -1002,7 +931,6 @@ static int au0828_v4l2_close(struct file *filp)
 {
 	int ret;
 	struct au0828_dev *dev = video_drvdata(filp);
-	struct video_device *vdev = video_devdata(filp);
 
 	dprintk(1,
 		"%s called std_set %d dev_state %d stream users %d users %d\n",
@@ -1010,17 +938,6 @@ static int au0828_v4l2_close(struct file *filp)
 		dev->streaming_users, dev->users);
 
 	mutex_lock(&dev->lock);
-	if (vdev->vfl_type == VFL_TYPE_GRABBER && dev->vid_timeout_running) {
-		/* Cancel timeout thread in case they didn't call streamoff */
-		dev->vid_timeout_running = 0;
-		del_timer_sync(&dev->vid_timeout);
-	} else if (vdev->vfl_type == VFL_TYPE_VBI &&
-			dev->vbi_timeout_running) {
-		/* Cancel timeout thread in case they didn't call streamoff */
-		dev->vbi_timeout_running = 0;
-		del_timer_sync(&dev->vbi_timeout);
-	}
-
 	if (dev->dev_state == DEV_DISCONNECTED)
 		goto end;
 
@@ -1614,11 +1531,6 @@ void au0828_v4l2_suspend(struct au0828_dev *dev)
 			}
 		}
 	}
-
-	if (dev->vid_timeout_running)
-		del_timer_sync(&dev->vid_timeout);
-	if (dev->vbi_timeout_running)
-		del_timer_sync(&dev->vbi_timeout);
 }
 
 void au0828_v4l2_resume(struct au0828_dev *dev)
@@ -1632,11 +1544,6 @@ void au0828_v4l2_resume(struct au0828_dev *dev)
 		au0828_init_tuner(dev);
 	}
 
-	if (dev->vid_timeout_running)
-		mod_timer(&dev->vid_timeout, jiffies + (HZ / 10));
-	if (dev->vbi_timeout_running)
-		mod_timer(&dev->vbi_timeout, jiffies + (HZ / 10));
-
 	/* If we were doing ac97 instead of i2s, it would go here...*/
 	au0828_i2s_init(dev);
 
@@ -1806,14 +1713,6 @@ int au0828_analog_register(struct au0828_dev *dev,
 	INIT_LIST_HEAD(&dev->vidq.active);
 	INIT_LIST_HEAD(&dev->vbiq.active);
 
-	dev->vid_timeout.function = au0828_vid_buffer_timeout;
-	dev->vid_timeout.data = (unsigned long) dev;
-	init_timer(&dev->vid_timeout);
-
-	dev->vbi_timeout.function = au0828_vbi_buffer_timeout;
-	dev->vbi_timeout.data = (unsigned long) dev;
-	init_timer(&dev->vbi_timeout);
-
 	dev->width = NTSC_STD_W;
 	dev->height = NTSC_STD_H;
 	dev->field_size = dev->width * dev->height;
diff --git a/drivers/media/usb/au0828/au0828.h b/drivers/media/usb/au0828/au0828.h
index eb15187..9dac92e 100644
--- a/drivers/media/usb/au0828/au0828.h
+++ b/drivers/media/usb/au0828/au0828.h
@@ -221,11 +221,6 @@ struct au0828_dev {
 	unsigned int frame_count;
 	unsigned int vbi_frame_count;
 
-	struct timer_list vid_timeout;
-	int vid_timeout_running;
-	struct timer_list vbi_timeout;
-	int vbi_timeout_running;
-
 	int users;
 	int streaming_users;
 
-- 
2.1.0

Re: [PATCH v3 3/3] media: au0828 remove video and vbi buffer timeout work-around

[Devin Heitmueller]

Hi Shuah,

On Mon, Jan 12, 2015 at 9:56 PM, Shuah Khan <shuahkh@osg.samsung.com> wrote:
> au0828 does video and vbi buffer timeout handling to prevent
> applications such as tvtime from hanging by ensuring that the
> video frames continue to be delivered even when the ITU-656
> input isn't receiving any data. This work-around is complex
> as it introduces set and clear timer code paths in start/stop
> streaming, and close interfaces. However, tvtime will hang
> without the following tvtime change:

I'm confused.  When we last debated whether this patch would be
accepted, the last message from Mauro said the following:

> That means that we'll need to keep holding such timeout code for
> years, until all distros update to a new tvtime, of course assuming
> that this is the only one application with such issue.

In other words, the timeout code has to stay in there since otherwise
it will cause ABI breakage.  It's great that Hans has submitted a
patch to improve tvtime, and over the next couple of years that patch
might actually start to appear in distributions.  That unfortunately
doesn't change the fact that everybody who updates their kernel (or
has it updated for them as part of their distribution) will go from
"works fine" to "completely broken".

The driver was working before the VB2 conversion, so if there is now
instability then it's likely that some bug was introduced during the
conversion to VB2.  Simply ripping out the timeout code seems like the
wrong approach to addressing a regression likely caused by your own
VB2 conversion patch.

Devin

-- 
Devin J. Heitmueller - Kernel Labs
http://www.kernellabs.com

Re: [PATCH v3 3/3] media: au0828 remove video and vbi buffer timeout work-around

[Shuah Khan]

On 01/12/2015 09:44 PM, Devin Heitmueller wrote:
> Hi Shuah,
> 
> On Mon, Jan 12, 2015 at 9:56 PM, Shuah Khan <shuahkh@osg.samsung.com> wrote:
>> au0828 does video and vbi buffer timeout handling to prevent
>> applications such as tvtime from hanging by ensuring that the
>> video frames continue to be delivered even when the ITU-656
>> input isn't receiving any data. This work-around is complex
>> as it introduces set and clear timer code paths in start/stop
>> streaming, and close interfaces. However, tvtime will hang
>> without the following tvtime change:
> 
> I'm confused.  When we last debated whether this patch would be
> accepted, the last message from Mauro said the following:
> 
>> That means that we'll need to keep holding such timeout code for
>> years, until all distros update to a new tvtime, of course assuming
>> that this is the only one application with such issue.
> 
> In other words, the timeout code has to stay in there since otherwise
> it will cause ABI breakage.  It's great that Hans has submitted a
> patch to improve tvtime, and over the next couple of years that patch
> might actually start to appear in distributions.  That unfortunately
> doesn't change the fact that everybody who updates their kernel (or
> has it updated for them as part of their distribution) will go from
> "works fine" to "completely broken".
> 
> The driver was working before the VB2 conversion, so if there is now
> instability then it's likely that some bug was introduced during the
> conversion to VB2.  Simply ripping out the timeout code seems like the
> wrong approach to addressing a regression likely caused by your own
> VB2 conversion patch.
> 

I couldn't reproduce what I was seeing when I did patch v2 series
work. What I noticed was that I was seeing a few too many green screens
and I had to re-tune xawtv when the timeout code is in place. My
thinking was that this timeout handling could be introducing blank
green frames when there is no need. However, I can't reproduce the
problem on 3.19-rc4 base which is what I am using to test the changes
to the patch series. Hence, I am not positive if the timeout code
indeed was doing anything bad.

I am seeing tvtime hangs without the timeout. I am fine with this
patch not going. It does make the code cleaner and also reduces
buffer handling during streaming. However, there is a clear regression
to tvtime.

thanks,
-- Shuah


-- 
Shuah Khan
Sr. Linux Kernel Developer
Open Source Innovation Group
Samsung Research America (Silicon Valley)
shuahkh@osg.samsung.com | (970) 217-8978

Re: [PATCH v3 3/3] media: au0828 remove video and vbi buffer timeout work-around

[Devin Heitmueller]

> I couldn't reproduce what I was seeing when I did patch v2 series
> work. What I noticed was that I was seeing a few too many green screens
> and I had to re-tune xawtv when the timeout code is in place. My
> thinking was that this timeout handling could be introducing blank
> green frames when there is no need. However, I can't reproduce the
> problem on 3.19-rc4 base which is what I am using to test the changes
> to the patch series. Hence, I am not positive if the timeout code
> indeed was doing anything bad.

IIRC, the timer was set for 40ms, so if a complete video frame doesn't
arrive within that interval we generate a green frame.  It was never
really intended to have perfect clocking (i.e. 29.97 FPS), but is
really just there to prevent the tvtime user interface from blocking
indefinitely.

If you weren't seeing it in the V2 series, then I guess you fixed
whatever bug was present in V1.

> I am seeing tvtime hangs without the timeout. I am fine with this
> patch not going. It does make the code cleaner and also reduces
> buffer handling during streaming. However, there is a clear regression
> to tvtime.

Correct.  I think everybody agrees that the timer code is ugly and it
would be cleaner if it wasn't needed - except it clearly is needed to
prevent regressions in tvtime.

All that said, I'm quite excited to see the driver converted to VB2.  Nice job!

Devin

-- 
Devin J. Heitmueller - Kernel Labs
http://www.kernellabs.com