[media] pwc: Replace private buffer management code with videobuf2

Looking at the pwc buffer management code has made it clear to me it needed
some serious fixing. Not only was there a ton of code duplication even
internally to pwc (read and mmap wait for frame code was duplicated), the
code also was outright buggy. With the worst offender being dqbuf, which
just round robin returned all the mmap buffers, without paying any attention
to them being queued by the app with qbuf or not. And qbuf itself was a noop.

So I set out to fix this and already had some cleanups in place when
I read Jonathan Corbet's lwn article on videobuf2, this inspired me to just
rip out the buffer management code and replace it with videobuf2, greatly
reducing the amount of code, and fixing all bugs in one go:

Many thanks to Jonathan for the timely article on this !

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Signed-off-by: Mauro Carvalho Chehab <mchehab@redhat.com>

1 files changed, 233 insertions, 614 deletions

diff --git a/drivers/media/video/pwc/pwc-if.c b/drivers/media/video/pwc/pwc-if.c
index 3d3ff6030987..907db23fdb5d 100644
--- a/drivers/media/video/pwc/pwc-if.c
+++ b/drivers/media/video/pwc/pwc-if.c
@@ -116,6 +116,7 @@ MODULE_DEVICE_TABLE(usb, pwc_device_table);
 
 static int usb_pwc_probe(struct usb_interface *intf, const struct usb_device_id *id);
 static void usb_pwc_disconnect(struct usb_interface *intf);
+static void pwc_isoc_cleanup(struct pwc_device *pdev);
 
 static struct usb_driver pwc_driver = {
 	.name =			"Philips webcam",	/* name */
@@ -129,8 +130,6 @@ static struct usb_driver pwc_driver = {
 
 static int default_size = PSZ_QCIF;
 static int default_fps = 10;
-static int default_fbufs = 3;   /* Default number of frame buffers */
-	int pwc_mbufs = 2;	/* Default number of mmap() buffers */
 #ifdef CONFIG_USB_PWC_DEBUG
 	int pwc_trace = PWC_DEBUG_LEVEL;
 #endif
@@ -173,52 +172,6 @@ static struct video_device pwc_template = {
 /***************************************************************************/
 /* Private functions */
 
-/* Here we want the physical address of the memory.
- * This is used when initializing the contents of the area.
- */
-
-
-
-static void *pwc_rvmalloc(unsigned long size)
-{
-	void * mem;
-	unsigned long adr;
-
-	mem=vmalloc_32(size);
-	if (!mem)
-		return NULL;
-
-	memset(mem, 0, size); /* Clear the ram out, no junk to the user */
-	adr=(unsigned long) mem;
-	while (size > 0)
-	 {
-	   SetPageReserved(vmalloc_to_page((void *)adr));
-	   adr  += PAGE_SIZE;
-	   size -= PAGE_SIZE;
-	 }
-	return mem;
-}
-
-static void pwc_rvfree(void * mem, unsigned long size)
-{
-	unsigned long adr;
-
-	if (!mem)
-		return;
-
-	adr=(unsigned long) mem;
-	while ((long) size > 0)
-	 {
-	   ClearPageReserved(vmalloc_to_page((void *)adr));
-	   adr  += PAGE_SIZE;
-	   size -= PAGE_SIZE;
-	 }
-	vfree(mem);
-}
-
-
-
-
 static int pwc_allocate_buffers(struct pwc_device *pdev)
 {
 	int i, err;
@@ -239,30 +192,6 @@ static int pwc_allocate_buffers(struct pwc_device *pdev)
 		}
 	}
 
-	/* Allocate frame buffer structure */
-	if (pdev->fbuf == NULL) {
-		kbuf = kzalloc(default_fbufs * sizeof(struct pwc_frame_buf), GFP_KERNEL);
-		if (kbuf == NULL) {
-			PWC_ERROR("Failed to allocate frame buffer structure.\n");
-			return -ENOMEM;
-		}
-		PWC_DEBUG_MEMORY("Allocated frame buffer structure at %p.\n", kbuf);
-		pdev->fbuf = kbuf;
-	}
-
-	/* create frame buffers, and make circular ring */
-	for (i = 0; i < default_fbufs; i++) {
-		if (pdev->fbuf[i].data == NULL) {
-			kbuf = vzalloc(PWC_FRAME_SIZE); /* need vmalloc since frame buffer > 128K */
-			if (kbuf == NULL) {
-				PWC_ERROR("Failed to allocate frame buffer %d.\n", i);
-				return -ENOMEM;
-			}
-			PWC_DEBUG_MEMORY("Allocated frame buffer %d at %p.\n", i, kbuf);
-			pdev->fbuf[i].data = kbuf;
-		}
-	}
-
 	/* Allocate decompressor table space */
 	if (DEVICE_USE_CODEC1(pdev->type))
 		err = pwc_dec1_alloc(pdev);
@@ -274,25 +203,6 @@ static int pwc_allocate_buffers(struct pwc_device *pdev)
 		return err;
 	}
 
-	/* Allocate image buffer; double buffer for mmap() */
-	kbuf = pwc_rvmalloc(pwc_mbufs * pdev->len_per_image);
-	if (kbuf == NULL) {
-		PWC_ERROR("Failed to allocate image buffer(s). needed (%d)\n",
-				pwc_mbufs * pdev->len_per_image);
-		return -ENOMEM;
-	}
-	PWC_DEBUG_MEMORY("Allocated image buffer at %p.\n", kbuf);
-	pdev->image_data = kbuf;
-	for (i = 0; i < pwc_mbufs; i++) {
-		pdev->images[i].offset = i * pdev->len_per_image;
-		pdev->images[i].vma_use_count = 0;
-	}
-	for (; i < MAX_IMAGES; i++) {
-		pdev->images[i].offset = 0;
-	}
-
-	kbuf = NULL;
-
 	PWC_DEBUG_MEMORY("<< pwc_allocate_buffers()\n");
 	return 0;
 }
@@ -311,19 +221,6 @@ static void pwc_free_buffers(struct pwc_device *pdev)
 			pdev->sbuf[i].data = NULL;
 		}
 
-	/* The same for frame buffers */
-	if (pdev->fbuf != NULL) {
-		for (i = 0; i < default_fbufs; i++) {
-			if (pdev->fbuf[i].data != NULL) {
-				PWC_DEBUG_MEMORY("Freeing frame buffer %d at %p.\n", i, pdev->fbuf[i].data);
-				vfree(pdev->fbuf[i].data);
-				pdev->fbuf[i].data = NULL;
-			}
-		}
-		kfree(pdev->fbuf);
-		pdev->fbuf = NULL;
-	}
-
 	/* Intermediate decompression buffer & tables */
 	if (pdev->decompress_data != NULL) {
 		PWC_DEBUG_MEMORY("Freeing decompression buffer at %p.\n", pdev->decompress_data);
@@ -331,226 +228,23 @@ static void pwc_free_buffers(struct pwc_device *pdev)
 		pdev->decompress_data = NULL;
 	}
 
-	/* Release image buffers */
-	if (pdev->image_data != NULL) {
-		PWC_DEBUG_MEMORY("Freeing image buffer at %p.\n", pdev->image_data);
-		pwc_rvfree(pdev->image_data, pwc_mbufs * pdev->len_per_image);
-	}
-	pdev->image_data = NULL;
-
 	PWC_DEBUG_MEMORY("Leaving free_buffers().\n");
 }
 
-/* The frame & image buffer mess.
-
-   Yes, this is a mess. Well, it used to be simple, but alas...  In this
-   module, 3 buffers schemes are used to get the data from the USB bus to
-   the user program. The first scheme involves the ISO buffers (called thus
-   since they transport ISO data from the USB controller), and not really
-   interesting. Suffices to say the data from this buffer is quickly
-   gathered in an interrupt handler (pwc_isoc_handler) and placed into the
-   frame buffer.
-
-   The frame buffer is the second scheme, and is the central element here.
-   It collects the data from a single frame from the camera (hence, the
-   name). Frames are delimited by the USB camera with a short USB packet,
-   so that's easy to detect. The frame buffers form a list that is filled
-   by the camera+USB controller and drained by the user process through
-   either read() or mmap().
-
-   The image buffer is the third scheme, in which frames are decompressed
-   and converted into planar format. For mmap() there is more than
-   one image buffer available.
-
-   The frame buffers provide the image buffering. In case the user process
-   is a bit slow, this introduces lag and some undesired side-effects.
-   The problem arises when the frame buffer is full. I used to drop the last
-   frame, which makes the data in the queue stale very quickly. But dropping
-   the frame at the head of the queue proved to be a litte bit more difficult.
-   I tried a circular linked scheme, but this introduced more problems than
-   it solved.
-
-   Because filling and draining are completely asynchronous processes, this
-   requires some fiddling with pointers and mutexes.
-
-   Eventually, I came up with a system with 2 lists: an 'empty' frame list
-   and a 'full' frame list:
-     * Initially, all frame buffers but one are on the 'empty' list; the one
-       remaining buffer is our initial fill frame.
-     * If a frame is needed for filling, we try to take it from the 'empty'
-       list, unless that list is empty, in which case we take the buffer at
-       the head of the 'full' list.
-     * When our fill buffer has been filled, it is appended to the 'full'
-       list.
-     * If a frame is needed by read() or mmap(), it is taken from the head of
-       the 'full' list, handled, and then appended to the 'empty' list. If no
-       buffer is present on the 'full' list, we wait.
-   The advantage is that the buffer that is currently being decompressed/
-   converted, is on neither list, and thus not in our way (any other scheme
-   I tried had the problem of old data lingering in the queue).
-
-   Whatever strategy you choose, it always remains a tradeoff: with more
-   frame buffers the chances of a missed frame are reduced. On the other
-   hand, on slower machines it introduces lag because the queue will
-   always be full.
- */
-
-/**
-  \brief Find next frame buffer to fill. Take from empty or full list, whichever comes first.
- */
-static int pwc_next_fill_frame(struct pwc_device *pdev)
+struct pwc_frame_buf *pwc_get_next_fill_buf(struct pwc_device *pdev)
 {
-	int ret;
-	unsigned long flags;
-
-	ret = 0;
-	spin_lock_irqsave(&pdev->ptrlock, flags);
-	if (pdev->fill_frame != NULL) {
-		/* append to 'full' list */
-		if (pdev->full_frames == NULL) {
-			pdev->full_frames = pdev->fill_frame;
-			pdev->full_frames_tail = pdev->full_frames;
-		}
-		else {
-			pdev->full_frames_tail->next = pdev->fill_frame;
-			pdev->full_frames_tail = pdev->fill_frame;
-		}
-	}
-	if (pdev->empty_frames != NULL) {
-		/* We have empty frames available. That's easy */
-		pdev->fill_frame = pdev->empty_frames;
-		pdev->empty_frames = pdev->empty_frames->next;
-	}
-	else {
-		/* Hmm. Take it from the full list */
-		/* sanity check */
-		if (pdev->full_frames == NULL) {
-			PWC_ERROR("Neither empty or full frames available!\n");
-			spin_unlock_irqrestore(&pdev->ptrlock, flags);
-			return -EINVAL;
-		}
-		pdev->fill_frame = pdev->full_frames;
-		pdev->full_frames = pdev->full_frames->next;
-		ret = 1;
-	}
-	pdev->fill_frame->next = NULL;
-	spin_unlock_irqrestore(&pdev->ptrlock, flags);
-	return ret;
-}
-
-
-/**
-  \brief Reset all buffers, pointers and lists, except for the image_used[] buffer.
-
-  If the image_used[] buffer is cleared too, mmap()/VIDIOCSYNC will run into trouble.
- */
-static void pwc_reset_buffers(struct pwc_device *pdev)
-{
-	int i;
-	unsigned long flags;
-
-	PWC_DEBUG_MEMORY(">> %s __enter__\n", __func__);
-
-	spin_lock_irqsave(&pdev->ptrlock, flags);
-	pdev->full_frames = NULL;
-	pdev->full_frames_tail = NULL;
-	for (i = 0; i < default_fbufs; i++) {
-		pdev->fbuf[i].filled = 0;
-		if (i > 0)
-			pdev->fbuf[i].next = &pdev->fbuf[i - 1];
-		else
-			pdev->fbuf->next = NULL;
-	}
-	pdev->empty_frames = &pdev->fbuf[default_fbufs - 1];
-	pdev->empty_frames_tail = pdev->fbuf;
-	pdev->read_frame = NULL;
-	pdev->fill_frame = pdev->empty_frames;
-	pdev->empty_frames = pdev->empty_frames->next;
-
-	pdev->image_read_pos = 0;
-	pdev->fill_image = 0;
-	spin_unlock_irqrestore(&pdev->ptrlock, flags);
-
-	PWC_DEBUG_MEMORY("<< %s __leaving__\n", __func__);
-}
-
-
-/**
-  \brief Do all the handling for getting one frame: get pointer, decompress, advance pointers.
- */
-int pwc_handle_frame(struct pwc_device *pdev)
-{
-	int ret = 0;
-	unsigned long flags;
-
-	spin_lock_irqsave(&pdev->ptrlock, flags);
-	/* First grab our read_frame; this is removed from all lists, so
-	   we can release the lock after this without problems */
-	if (pdev->read_frame != NULL) {
-		/* This can't theoretically happen */
-		PWC_ERROR("Huh? Read frame still in use?\n");
-		spin_unlock_irqrestore(&pdev->ptrlock, flags);
-		return ret;
-	}
-
-
-	if (pdev->full_frames == NULL) {
-		PWC_ERROR("Woops. No frames ready.\n");
-	}
-	else {
-		pdev->read_frame = pdev->full_frames;
-		pdev->full_frames = pdev->full_frames->next;
-		pdev->read_frame->next = NULL;
-	}
-
-	if (pdev->read_frame != NULL) {
-		/* Decompression is a lengthy process, so it's outside of the lock.
-		   This gives the isoc_handler the opportunity to fill more frames
-		   in the mean time.
-		*/
-		spin_unlock_irqrestore(&pdev->ptrlock, flags);
-		ret = pwc_decompress(pdev);
-		spin_lock_irqsave(&pdev->ptrlock, flags);
-
-		/* We're done with read_buffer, tack it to the end of the empty buffer list */
-		if (pdev->empty_frames == NULL) {
-			pdev->empty_frames = pdev->read_frame;
-			pdev->empty_frames_tail = pdev->empty_frames;
-		}
-		else {
-			pdev->empty_frames_tail->next = pdev->read_frame;
-			pdev->empty_frames_tail = pdev->read_frame;
-		}
-		pdev->read_frame = NULL;
-	}
-	spin_unlock_irqrestore(&pdev->ptrlock, flags);
-	return ret;
-}
-
-/**
-  \brief Advance pointers of image buffer (after each user request)
-*/
-void pwc_next_image(struct pwc_device *pdev)
-{
-	pdev->image_used[pdev->fill_image] = 0;
-	pdev->fill_image = (pdev->fill_image + 1) % pwc_mbufs;
-}
-
-/**
- * Print debug information when a frame is discarded because all of our buffer
- * is full
- */
-static void pwc_frame_dumped(struct pwc_device *pdev)
-{
-	pdev->vframes_dumped++;
-	if (pdev->vframe_count < FRAME_LOWMARK)
-		return;
-
-	if (pdev->vframes_dumped < 20)
-		PWC_DEBUG_FLOW("Dumping frame %d\n", pdev->vframe_count);
-	else if (pdev->vframes_dumped == 20)
-		PWC_DEBUG_FLOW("Dumping frame %d (last message)\n",
-				pdev->vframe_count);
+	unsigned long flags = 0;
+	struct pwc_frame_buf *buf = NULL;
+
+	spin_lock_irqsave(&pdev->queued_bufs_lock, flags);
+	if (list_empty(&pdev->queued_bufs))
+		goto leave;
+
+	buf = list_entry(pdev->queued_bufs.next, struct pwc_frame_buf, list);
+	list_del(&buf->list);
+leave:
+	spin_unlock_irqrestore(&pdev->queued_bufs_lock, flags);
+	return buf;
 }
 
 static void pwc_snapshot_button(struct pwc_device *pdev, int down)
@@ -570,9 +264,9 @@ static void pwc_snapshot_button(struct pwc_device *pdev, int down)
 #endif
 }
 
-static int pwc_rcv_short_packet(struct pwc_device *pdev, const struct pwc_frame_buf *fbuf)
+static void pwc_frame_complete(struct pwc_device *pdev)
 {
-	int awake = 0;
+	struct pwc_frame_buf *fbuf = pdev->fill_buf;
 
 	/* The ToUCam Fun CMOS sensor causes the firmware to send 2 or 3 bogus
 	   frames on the USB wire after an exposure change. This conditition is
@@ -584,7 +278,6 @@ static int pwc_rcv_short_packet(struct pwc_device *pdev, const struct pwc_frame_
 		if (ptr[1] == 1 && ptr[0] & 0x10) {
 			PWC_TRACE("Hyundai CMOS sensor bug. Dropping frame.\n");
 			pdev->drop_frames += 2;
-			pdev->vframes_error++;
 		}
 		if ((ptr[0] ^ pdev->vmirror) & 0x01) {
 			pwc_snapshot_button(pdev, ptr[0] & 0x01);
@@ -607,8 +300,7 @@ static int pwc_rcv_short_packet(struct pwc_device *pdev, const struct pwc_frame_
 		   */
 		if (fbuf->filled == 4)
 			pdev->drop_frames++;
-	}
-	else if (pdev->type == 740 || pdev->type == 720) {
+	} else if (pdev->type == 740 || pdev->type == 720) {
 		unsigned char *ptr = (unsigned char *)fbuf->data;
 		if ((ptr[0] ^ pdev->vmirror) & 0x01) {
 			pwc_snapshot_button(pdev, ptr[0] & 0x01);
@@ -616,33 +308,23 @@ static int pwc_rcv_short_packet(struct pwc_device *pdev, const struct pwc_frame_
 		pdev->vmirror = ptr[0] & 0x03;
 	}
 
-	/* In case we were instructed to drop the frame, do so silently.
-	   The buffer pointers are not updated either (but the counters are reset below).
-	   */
-	if (pdev->drop_frames > 0)
+	/* In case we were instructed to drop the frame, do so silently. */
+	if (pdev->drop_frames > 0) {
 		pdev->drop_frames--;
-	else {
+	} else {
 		/* Check for underflow first */
 		if (fbuf->filled < pdev->frame_total_size) {
 			PWC_DEBUG_FLOW("Frame buffer underflow (%d bytes);"
 				       " discarded.\n", fbuf->filled);
-			pdev->vframes_error++;
-		}
-		else {
-			/* Send only once per EOF */
-			awake = 1; /* delay wake_ups */
-
-			/* Find our next frame to fill. This will always succeed, since we
-			 * nick a frame from either empty or full list, but if we had to
-			 * take it from the full list, it means a frame got dropped.
-			 */
-			if (pwc_next_fill_frame(pdev))
-				pwc_frame_dumped(pdev);
-
+		} else {
+			fbuf->vb.v4l2_buf.field = V4L2_FIELD_NONE;
+			fbuf->vb.v4l2_buf.sequence = pdev->vframe_count;
+			vb2_buffer_done(&fbuf->vb, VB2_BUF_STATE_DONE);
+			pdev->fill_buf = NULL;
+			pdev->vsync = 0;
 		}
 	} /* !drop_frames */
 	pdev->vframe_count++;
-	return awake;
 }
 
 /* This gets called for the Isochronous pipe (video). This is done in
@@ -650,24 +332,20 @@ static int pwc_rcv_short_packet(struct pwc_device *pdev, const struct pwc_frame_
  */
 static void pwc_isoc_handler(struct urb *urb)
 {
-	struct pwc_device *pdev;
+	struct pwc_device *pdev = (struct pwc_device *)urb->context;
 	int i, fst, flen;
-	int awake;
-	struct pwc_frame_buf *fbuf;
-	unsigned char *fillptr = NULL, *iso_buf = NULL;
-
-	awake = 0;
-	pdev = (struct pwc_device *)urb->context;
-	if (pdev == NULL) {
-		PWC_ERROR("isoc_handler() called with NULL device?!\n");
-		return;
-	}
+	unsigned char *iso_buf = NULL;
 
-	if (urb->status == -ENOENT || urb->status == -ECONNRESET) {
+	if (urb->status == -ENOENT || urb->status == -ECONNRESET ||
+	    urb->status == -ESHUTDOWN) {
 		PWC_DEBUG_OPEN("URB (%p) unlinked %ssynchronuously.\n", urb, urb->status == -ENOENT ? "" : "a");
 		return;
 	}
-	if (urb->status != -EINPROGRESS && urb->status != 0) {
+
+	if (pdev->fill_buf == NULL)
+		pdev->fill_buf = pwc_get_next_fill_buf(pdev);
+
+	if (urb->status != 0) {
 		const char *errmsg;
 
 		errmsg = "Unknown";
@@ -679,29 +357,21 @@ static void pwc_isoc_handler(struct urb *urb)
 			case -EILSEQ:		errmsg = "CRC/Timeout (could be anything)"; break;
 			case -ETIME:		errmsg = "Device does not respond"; break;
 		}
-		PWC_DEBUG_FLOW("pwc_isoc_handler() called with status %d [%s].\n", urb->status, errmsg);
-		/* Give up after a number of contiguous errors on the USB bus.
-		   Appearantly something is wrong so we simulate an unplug event.
-		 */
+		PWC_ERROR("pwc_isoc_handler() called with status %d [%s].\n",
+			  urb->status, errmsg);
+		/* Give up after a number of contiguous errors */
 		if (++pdev->visoc_errors > MAX_ISOC_ERRORS)
 		{
-			PWC_INFO("Too many ISOC errors, bailing out.\n");
-			pdev->error_status = EIO;
-			awake = 1;
-			wake_up_interruptible(&pdev->frameq);
+			PWC_ERROR("Too many ISOC errors, bailing out.\n");
+			if (pdev->fill_buf) {
+				vb2_buffer_done(&pdev->fill_buf->vb,
+						VB2_BUF_STATE_ERROR);
+				pdev->fill_buf = NULL;
+			}
 		}
-		goto handler_end; // ugly, but practical
-	}
-
-	fbuf = pdev->fill_frame;
-	if (fbuf == NULL) {
-		PWC_ERROR("pwc_isoc_handler without valid fill frame.\n");
-		awake = 1;
+		pdev->vsync = 0; /* Drop the current frame */
 		goto handler_end;
 	}
-	else {
-		fillptr = fbuf->data + fbuf->filled;
-	}
 
 	/* Reset ISOC error counter. We did get here, after all. */
 	pdev->visoc_errors = 0;
@@ -715,65 +385,50 @@ static void pwc_isoc_handler(struct urb *urb)
 		fst  = urb->iso_frame_desc[i].status;
 		flen = urb->iso_frame_desc[i].actual_length;
 		iso_buf = urb->transfer_buffer + urb->iso_frame_desc[i].offset;
-		if (fst == 0) {
-			if (flen > 0) { /* if valid data... */
-				if (pdev->vsync > 0) { /* ...and we are not sync-hunting... */
-					pdev->vsync = 2;
-
-					/* ...copy data to frame buffer, if possible */
-					if (flen + fbuf->filled > pdev->frame_total_size) {
-						PWC_DEBUG_FLOW("Frame buffer overflow (flen = %d, frame_total_size = %d).\n", flen, pdev->frame_total_size);
-						pdev->vsync = 0; /* Hmm, let's wait for an EOF (end-of-frame) */
-						pdev->vframes_error++;
-					}
-					else {
-						memmove(fillptr, iso_buf, flen);
-						fillptr += flen;
-					}
-				}
+		if (fst != 0) {
+			PWC_ERROR("Iso frame %d has error %d\n", i, fst);
+			continue;
+		}
+		if (flen > 0 && pdev->vsync) {
+			struct pwc_frame_buf *fbuf = pdev->fill_buf;
+
+			if (pdev->vsync == 1) {
+				do_gettimeofday(&fbuf->vb.v4l2_buf.timestamp);
+				pdev->vsync = 2;
+			}
+
+			if (flen + fbuf->filled > pdev->frame_total_size) {
+				PWC_ERROR("Frame overflow (%d > %d)\n",
+					  flen + fbuf->filled,
+					  pdev->frame_total_size);
+				pdev->vsync = 0; /* Let's wait for an EOF */
+			} else {
+				memcpy(fbuf->data + fbuf->filled, iso_buf,
+				       flen);
 				fbuf->filled += flen;
-			} /* ..flen > 0 */
-
-			if (flen < pdev->vlast_packet_size) {
-				/* Shorter packet... We probably have the end of an image-frame;
-				   wake up read() process and let select()/poll() do something.
-				   Decompression is done in user time over there.
-				   */
-				if (pdev->vsync == 2) {
-					if (pwc_rcv_short_packet(pdev, fbuf)) {
-						awake = 1;
-						fbuf = pdev->fill_frame;
-					}
-				}
-				fbuf->filled = 0;
-				fillptr = fbuf->data;
+			}
+		}
+		if (flen < pdev->vlast_packet_size) {
+			/* Shorter packet... end of frame */
+			if (pdev->vsync == 2)
+				pwc_frame_complete(pdev);
+			if (pdev->fill_buf == NULL)
+				pdev->fill_buf = pwc_get_next_fill_buf(pdev);
+			if (pdev->fill_buf) {
+				pdev->fill_buf->filled = 0;
 				pdev->vsync = 1;
 			}
-
-			pdev->vlast_packet_size = flen;
-		} /* ..status == 0 */
-		else {
-			/* This is normally not interesting to the user, unless
-			 * you are really debugging something, default = 0 */
-			static int iso_error;
-			iso_error++;
-			if (iso_error < 20)
-				PWC_DEBUG_FLOW("Iso frame %d of USB has error %d\n", i, fst);
 		}
+		pdev->vlast_packet_size = flen;
 	}
 
 handler_end:
-	if (awake)
-		wake_up_interruptible(&pdev->frameq);
-
-	urb->dev = pdev->udev;
 	i = usb_submit_urb(urb, GFP_ATOMIC);
 	if (i != 0)
 		PWC_ERROR("Error (%d) re-submitting urb in pwc_isoc_handler.\n", i);
 }
 
-
-int pwc_isoc_init(struct pwc_device *pdev)
+static int pwc_isoc_init(struct pwc_device *pdev)
 {
 	struct usb_device *udev;
 	struct urb *urb;
@@ -784,6 +439,8 @@ int pwc_isoc_init(struct pwc_device *pdev)
 	if (pdev->iso_init)
 		return 0;
 	pdev->vsync = 0;
+	pdev->fill_buf = NULL;
+	pdev->vframe_count = 0;
 	udev = pdev->udev;
 
 	/* Get the current alternate interface, adjust packet size */
@@ -904,7 +561,7 @@ static void pwc_iso_free(struct pwc_device *pdev)
 	}
 }
 
-void pwc_isoc_cleanup(struct pwc_device *pdev)
+static void pwc_isoc_cleanup(struct pwc_device *pdev)
 {
 	PWC_DEBUG_OPEN(">> pwc_isoc_cleanup()\n");
 
@@ -926,6 +583,22 @@ void pwc_isoc_cleanup(struct pwc_device *pdev)
 	PWC_DEBUG_OPEN("<< pwc_isoc_cleanup()\n");
 }
 
+/*
+ * Release all queued buffers, no need to take queued_bufs_lock, since all
+ * iso urbs have been killed when we're called so pwc_isoc_handler won't run.
+ */
+static void pwc_cleanup_queued_bufs(struct pwc_device *pdev)
+{
+	while (!list_empty(&pdev->queued_bufs)) {
+		struct pwc_frame_buf *buf;
+
+		buf = list_entry(pdev->queued_bufs.next, struct pwc_frame_buf,
+				 list);
+		list_del(&buf->list);
+		vb2_buffer_done(&buf->vb, VB2_BUF_STATE_ERROR);
+	}
+}
+
 /*********
  * sysfs
  *********/
@@ -1086,12 +759,6 @@ static int pwc_video_open(struct file *file)
 	}
 
 	/* Reset buffers & parameters */
-	pwc_reset_buffers(pdev);
-	for (i = 0; i < pwc_mbufs; i++)
-		pdev->image_used[i] = 0;
-	pdev->vframe_count = 0;
-	pdev->vframes_dumped = 0;
-	pdev->vframes_error = 0;
 	pdev->visoc_errors = 0;
 	pdev->error_status = 0;
 	pwc_construct(pdev); /* set min/max sizes correct */
@@ -1161,19 +828,12 @@ static int pwc_video_close(struct file *file)
 	if (pdev->vopen == 0)
 		PWC_DEBUG_MODULE("video_close() called on closed device?\n");
 
-	/* Dump statistics, but only if a reasonable amount of frames were
-	   processed (to prevent endless log-entries in case of snap-shot
-	   programs)
-	 */
-	if (pdev->vframe_count > 20)
-		PWC_DEBUG_MODULE("Closing video device: %d frames received, dumped %d frames, %d frames with errors.\n", pdev->vframe_count, pdev->vframes_dumped, pdev->vframes_error);
-
 	if (DEVICE_USE_CODEC1(pdev->type))
 	    pwc_dec1_exit();
 	else
 	    pwc_dec23_exit();
 
-	pwc_isoc_cleanup(pdev);
+	vb2_queue_release(&pdev->vb_queue);
 	pwc_free_buffers(pdev);
 
 	/* Turn off LEDS and power down camera, but only when not unplugged */
@@ -1193,182 +853,155 @@ static int pwc_video_close(struct file *file)
 	return 0;
 }
 
-/*
- *	FIXME: what about two parallel reads ????
- *      ANSWER: Not supported. You can't open the device more than once,
-		despite what the V4L1 interface says. First, I don't see
-		the need, second there's no mechanism of alerting the
-		2nd/3rd/... process of events like changing image size.
-		And I don't see the point of blocking that for the
-		2nd/3rd/... process.
-		In multi-threaded environments reading parallel from any
-		device is tricky anyhow.
- */
-
 static ssize_t pwc_video_read(struct file *file, char __user *buf,
-			  size_t count, loff_t *ppos)
+			      size_t count, loff_t *ppos)
 {
 	struct video_device *vdev = file->private_data;
-	struct pwc_device *pdev;
-	int noblock = file->f_flags & O_NONBLOCK;
-	DECLARE_WAITQUEUE(wait, current);
-	int bytes_to_read, rv = 0;
-	void *image_buffer_addr;
+	struct pwc_device *pdev = video_get_drvdata(vdev);
 
-	PWC_DEBUG_READ("pwc_video_read(vdev=0x%p, buf=%p, count=%zd) called.\n",
-			vdev, buf, count);
-	pdev = video_get_drvdata(vdev);
+	if (pdev->error_status)
+		return -pdev->error_status;
 
-	if (pdev->error_status) {
-		rv = -pdev->error_status; /* Something happened, report what. */
-		goto err_out;
-	}
+	return vb2_read(&pdev->vb_queue, buf, count, ppos,
+			file->f_flags & O_NONBLOCK);
+}
 
-	/* Start the stream (if not already started) */
-	rv = pwc_isoc_init(pdev);
-	if (rv)
-		goto err_out;
-
-	/* In case we're doing partial reads, we don't have to wait for a frame */
-	if (pdev->image_read_pos == 0) {
-		/* Do wait queueing according to the (doc)book */
-		add_wait_queue(&pdev->frameq, &wait);
-		while (pdev->full_frames == NULL) {
-			/* Check for unplugged/etc. here */
-			if (pdev->error_status) {
-				remove_wait_queue(&pdev->frameq, &wait);
-				set_current_state(TASK_RUNNING);
-				rv = -pdev->error_status ;
-				goto err_out;
-			}
-			if (noblock) {
-				remove_wait_queue(&pdev->frameq, &wait);
-				set_current_state(TASK_RUNNING);
-				rv = -EWOULDBLOCK;
-				goto err_out;
-			}
-			if (signal_pending(current)) {
-				remove_wait_queue(&pdev->frameq, &wait);
-				set_current_state(TASK_RUNNING);
-				rv = -ERESTARTSYS;
-				goto err_out;
-			}
-			mutex_unlock(&pdev->modlock);
-			schedule();
-			set_current_state(TASK_INTERRUPTIBLE);
-			mutex_lock(&pdev->modlock);
-		}
-		remove_wait_queue(&pdev->frameq, &wait);
-		set_current_state(TASK_RUNNING);
+static unsigned int pwc_video_poll(struct file *file, poll_table *wait)
+{
+	struct video_device *vdev = file->private_data;
+	struct pwc_device *pdev = video_get_drvdata(vdev);
 
-		/* Decompress and release frame */
-		rv = pwc_handle_frame(pdev);
-		if (rv)
-			goto err_out;
-	}
+	if (pdev->error_status)
+		return POLL_ERR;
 
-	PWC_DEBUG_READ("Copying data to user space.\n");
-	if (pdev->pixfmt != V4L2_PIX_FMT_YUV420)
-		bytes_to_read = pdev->frame_size + sizeof(struct pwc_raw_frame);
-	else
-		bytes_to_read = pdev->view.size;
-
-	/* copy bytes to user space; we allow for partial reads */
-	if (count + pdev->image_read_pos > bytes_to_read)
-		count = bytes_to_read - pdev->image_read_pos;
-	image_buffer_addr = pdev->image_data;
-	image_buffer_addr += pdev->images[pdev->fill_image].offset;
-	image_buffer_addr += pdev->image_read_pos;
-	if (copy_to_user(buf, image_buffer_addr, count)) {
-		rv = -EFAULT;
-		goto err_out;
-	}
-	pdev->image_read_pos += count;
-	if (pdev->image_read_pos >= bytes_to_read) { /* All data has been read */
-		pdev->image_read_pos = 0;
-		pwc_next_image(pdev);
-	}
-	return count;
-err_out:
-	return rv;
+	return vb2_poll(&pdev->vb_queue, file, wait);
 }
 
-static unsigned int pwc_video_poll(struct file *file, poll_table *wait)
+static int pwc_video_mmap(struct file *file, struct vm_area_struct *vma)
 {
 	struct video_device *vdev = file->private_data;
-	struct pwc_device *pdev;
-	int ret;
+	struct pwc_device *pdev = video_get_drvdata(vdev);
 
-	pdev = video_get_drvdata(vdev);
+	return vb2_mmap(&pdev->vb_queue, vma);
+}
 
-	/* Start the stream (if not already started) */
-	ret = pwc_isoc_init(pdev);
-	if (ret)
-		return ret;
+/***************************************************************************/
+/* Videobuf2 operations */
+
+static int queue_setup(struct vb2_queue *vq, unsigned int *nbuffers,
+				unsigned int *nplanes, unsigned long sizes[],
+				void *alloc_ctxs[])
+{
+	struct pwc_device *pdev = vb2_get_drv_priv(vq);
 
-	poll_wait(file, &pdev->frameq, wait);
+	if (*nbuffers < MIN_FRAMES)
+		*nbuffers = MIN_FRAMES;
+	else if (*nbuffers > MAX_FRAMES)
+		*nbuffers = MAX_FRAMES;
+
+	*nplanes = 1;
+
+	sizes[0] = PAGE_ALIGN((pdev->abs_max.x * pdev->abs_max.y * 3) / 2);
+
+	return 0;
+}
+
+static int buffer_init(struct vb2_buffer *vb)
+{
+	struct pwc_frame_buf *buf = container_of(vb, struct pwc_frame_buf, vb);
+
+	/* need vmalloc since frame buffer > 128K */
+	buf->data = vzalloc(PWC_FRAME_SIZE);
+	if (buf->data == NULL)
+		return -ENOMEM;
+
+	return 0;
+}
+
+static int buffer_prepare(struct vb2_buffer *vb)
+{
+	struct pwc_device *pdev = vb2_get_drv_priv(vb->vb2_queue);
+
+	/* Don't allow queing new buffers after device disconnection */
 	if (pdev->error_status)
-		return POLLERR;
-	if (pdev->full_frames != NULL) /* we have frames waiting */
-		return (POLLIN | POLLRDNORM);
+		return -pdev->error_status;
 
 	return 0;
 }
 
-static int pwc_video_mmap(struct file *file, struct vm_area_struct *vma)
+static int buffer_finish(struct vb2_buffer *vb)
 {
-	struct video_device *vdev = file->private_data;
-	struct pwc_device *pdev;
-	unsigned long start;
-	unsigned long size;
-	unsigned long page, pos = 0;
-	int index;
+	struct pwc_device *pdev = vb2_get_drv_priv(vb->vb2_queue);
+	struct pwc_frame_buf *buf = container_of(vb, struct pwc_frame_buf, vb);
 
-	PWC_DEBUG_MEMORY(">> %s\n", __func__);
-	pdev = video_get_drvdata(vdev);
-	size = vma->vm_end - vma->vm_start;
-	start = vma->vm_start;
+	/*
+	 * Application has called dqbuf and is getting back a buffer we've
+	 * filled, take the pwc data we've stored in buf->data and decompress
+	 * it into a usable format, storing the result in the vb2_buffer
+	 */
+	return pwc_decompress(pdev, buf);
+}
+
+static void buffer_cleanup(struct vb2_buffer *vb)
+{
+	struct pwc_frame_buf *buf = container_of(vb, struct pwc_frame_buf, vb);
+
+	vfree(buf->data);
+}
+
+static void buffer_queue(struct vb2_buffer *vb)
+{
+	struct pwc_device *pdev = vb2_get_drv_priv(vb->vb2_queue);
+	struct pwc_frame_buf *buf = container_of(vb, struct pwc_frame_buf, vb);
+	unsigned long flags = 0;
+
+	spin_lock_irqsave(&pdev->queued_bufs_lock, flags);
+	list_add_tail(&buf->list, &pdev->queued_bufs);
+	spin_unlock_irqrestore(&pdev->queued_bufs_lock, flags);
+}
+
+static int start_streaming(struct vb2_queue *vq)
+{
+	struct pwc_device *pdev = vb2_get_drv_priv(vq);
+
+	return pwc_isoc_init(pdev);
+}
+
+static int stop_streaming(struct vb2_queue *vq)
+{
+	struct pwc_device *pdev = vb2_get_drv_priv(vq);
+
+	pwc_isoc_cleanup(pdev);
+	pwc_cleanup_queued_bufs(pdev);
 
-	/* Find the idx buffer for this mapping */
-	for (index = 0; index < pwc_mbufs; index++) {
-		pos = pdev->images[index].offset;
-		if ((pos>>PAGE_SHIFT) == vma->vm_pgoff)
-			break;
-	}
-	if (index == MAX_IMAGES)
-		return -EINVAL;
-	if (index == 0) {
-		/*
-		 * Special case for v4l1. In v4l1, we map only one big buffer,
-		 * but in v4l2 each buffer is mapped
-		 */
-		unsigned long total_size;
-		total_size = pwc_mbufs * pdev->len_per_image;
-		if (size != pdev->len_per_image && size != total_size) {
-			PWC_ERROR("Wrong size (%lu) needed to be len_per_image=%d or total_size=%lu\n",
-				   size, pdev->len_per_image, total_size);
-			return -EINVAL;
-		}
-	} else if (size > pdev->len_per_image)
-		return -EINVAL;
-
-	vma->vm_flags |= VM_IO;	/* from 2.6.9-acX */
-
-	pos += (unsigned long)pdev->image_data;
-	while (size > 0) {
-		page = vmalloc_to_pfn((void *)pos);
-		if (remap_pfn_range(vma, start, page, PAGE_SIZE, PAGE_SHARED))
-			return -EAGAIN;
-		start += PAGE_SIZE;
-		pos += PAGE_SIZE;
-		if (size > PAGE_SIZE)
-			size -= PAGE_SIZE;
-		else
-			size = 0;
-	}
 	return 0;
 }
 
+static void pwc_lock(struct vb2_queue *vq)
+{
+	struct pwc_device *pdev = vb2_get_drv_priv(vq);
+	mutex_lock(&pdev->modlock);
+}
+
+static void pwc_unlock(struct vb2_queue *vq)
+{
+	struct pwc_device *pdev = vb2_get_drv_priv(vq);
+	mutex_unlock(&pdev->modlock);
+}
+
+static struct vb2_ops pwc_vb_queue_ops = {
+	.queue_setup		= queue_setup,
+	.buf_init		= buffer_init,
+	.buf_prepare		= buffer_prepare,
+	.buf_finish		= buffer_finish,
+	.buf_cleanup		= buffer_cleanup,
+	.buf_queue		= buffer_queue,
+	.start_streaming	= start_streaming,
+	.stop_streaming		= stop_streaming,
+	.wait_prepare		= pwc_unlock,
+	.wait_finish		= pwc_lock,
+};
+
 /***************************************************************************/
 /* USB functions */
 
@@ -1648,12 +1281,22 @@ static int usb_pwc_probe(struct usb_interface *intf, const struct usb_device_id
 	}
 
 	mutex_init(&pdev->modlock);
-	spin_lock_init(&pdev->ptrlock);
+	spin_lock_init(&pdev->queued_bufs_lock);
+	INIT_LIST_HEAD(&pdev->queued_bufs);
 
 	pdev->udev = udev;
-	init_waitqueue_head(&pdev->frameq);
 	pdev->vcompression = pwc_preferred_compression;
 
+	/* Init videobuf2 queue structure */
+	memset(&pdev->vb_queue, 0, sizeof(pdev->vb_queue));
+	pdev->vb_queue.type = V4L2_BUF_TYPE_VIDEO_CAPTURE;
+	pdev->vb_queue.io_modes = VB2_MMAP | VB2_USERPTR | VB2_READ;
+	pdev->vb_queue.drv_priv = pdev;
+	pdev->vb_queue.buf_struct_size = sizeof(struct pwc_frame_buf);
+	pdev->vb_queue.ops = &pwc_vb_queue_ops;
+	pdev->vb_queue.mem_ops = &vb2_vmalloc_memops;
+	vb2_queue_init(&pdev->vb_queue);
+
 	/* Init video_device structure */
 	memcpy(&pdev->vdev, &pwc_template, sizeof(pwc_template));
 	pdev->vdev.parent = &intf->dev;
@@ -1752,11 +1395,9 @@ static void usb_pwc_disconnect(struct usb_interface *intf)
 	pdev->error_status = EPIPE;
 	pdev->unplugged = 1;
 
-	/* Alert waiting processes */
-	wake_up_interruptible(&pdev->frameq);
-
 	/* No need to keep the urbs around after disconnection */
 	pwc_isoc_cleanup(pdev);
+	pwc_cleanup_queued_bufs(pdev);
 
 	mutex_unlock(&pdev->modlock);
 
@@ -1776,8 +1417,6 @@ static void usb_pwc_disconnect(struct usb_interface *intf)
 
 static char *size;
 static int fps;
-static int fbufs;
-static int mbufs;
 static int compression = -1;
 static int leds[2] = { -1, -1 };
 static unsigned int leds_nargs;
@@ -1786,8 +1425,6 @@ static unsigned int dev_hint_nargs;
 
 module_param(size, charp, 0444);
 module_param(fps, int, 0444);
-module_param(fbufs, int, 0444);
-module_param(mbufs, int, 0444);
 #ifdef CONFIG_USB_PWC_DEBUG
 module_param_named(trace, pwc_trace, int, 0644);
 #endif
@@ -1798,8 +1435,6 @@ module_param_array(dev_hint, charp, &dev_hint_nargs, 0444);
 
 MODULE_PARM_DESC(size, "Initial image size. One of sqcif, qsif, qcif, sif, cif, vga");
 MODULE_PARM_DESC(fps, "Initial frames per second. Varies with model, useful range 5-30");
-MODULE_PARM_DESC(fbufs, "Number of internal frame buffers to reserve");
-MODULE_PARM_DESC(mbufs, "Number of external (mmap()ed) image buffers");
 #ifdef CONFIG_USB_PWC_DEBUG
 MODULE_PARM_DESC(trace, "For debugging purposes");
 #endif
@@ -1847,22 +1482,6 @@ static int __init usb_pwc_init(void)
 		}
 		PWC_DEBUG_MODULE("Default image size set to %s [%dx%d].\n", sizenames[default_size], pwc_image_sizes[default_size].x, pwc_image_sizes[default_size].y);
 	}
-	if (mbufs) {
-		if (mbufs < 1 || mbufs > MAX_IMAGES) {
-			PWC_ERROR("Illegal number of mmap() buffers; use a number between 1 and %d.\n", MAX_IMAGES);
-			return -EINVAL;
-		}
-		pwc_mbufs = mbufs;
-		PWC_DEBUG_MODULE("Number of image buffers set to %d.\n", pwc_mbufs);
-	}
-	if (fbufs) {
-		if (fbufs < 2 || fbufs > MAX_FRAMES) {
-			PWC_ERROR("Illegal number of frame buffers; use a number between 2 and %d.\n", MAX_FRAMES);
-			return -EINVAL;
-		}
-		default_fbufs = fbufs;
-		PWC_DEBUG_MODULE("Number of frame buffers set to %d.\n", default_fbufs);
-	}
 #ifdef CONFIG_USB_PWC_DEBUG
 	if (pwc_trace >= 0) {
 		PWC_DEBUG_MODULE("Trace options: 0x%04x\n", pwc_trace);