Re: [REVIEW PATCH 3/3] saa7134: convert to vb2

[Devin Heitmueller <dheitmueller@kernellabs.com>]

> This was a bit confusing following the previous paragraph. I meant to say that the
> *saa7134* userptr implementation is not USERPTR at all but PAGE_ALIGNED_USERPTR.
>
> A proper USERPTR implementation (like in bttv) can use any malloc()ed pointer as
> it should, but saa7134 can't as it requires the application to align the pointer
> to a page boundary, which is non-standard.

It's probably worth mentioning at this point that there's a bug in
videobuf2_vmalloc() where it forces the buffer provided by userptr
mode to be page aligned.  This causes issues if you hand it a buffer
that isn't actually page aligned, as it writes to an arbitrary offset
into the buffer instead of the start of the buffer you provided.

I've had the following patch in my private tree for months, but had
been hesitant to submit it since I didn't know if it would effect
other implementations.  I wasn't sure if USERPTR mode required the
buffers to be page aligned and I was violating the spec.

Devin

From: Devin Heitmueller <dheitmueller@kernellabs.com>
Date: Fri, 17 May 2013 19:53:02 +0000 (-0400)
Subject: Fix alignment bug when using VB2 with userptr mode

Fix alignment bug when using VB2 with userptr mode

If we pass in a USERPTR buffer that isn't page aligned, the driver
will align it (and not tell userland).  The result is that the driver
thinks the video starts in one place while userland thinks it starts
in another.  This manifests itself as the video being horizontally
shifted and there being garbage from the start of the field up to
that point.

This problem was seen with both the em28xx and uvc drivers when
testing on the Davinci platform (where the buffers allocated are
not necessarily page aligned).
---

diff --git a/linux/drivers/media/v4l2-core/videobuf2-vmalloc.c
b/linux/drivers/media/v4l2-core/videobuf2-vmalloc.c
index 94efa04..ad7ce7c 100644
--- a/linux/drivers/media/v4l2-core/videobuf2-vmalloc.c
+++ b/linux/drivers/media/v4l2-core/videobuf2-vmalloc.c
@@ -82,7 +82,7 @@ static void *vb2_vmalloc_get_userptr(void
*alloc_ctx, unsigned long vaddr,
  return NULL;

  buf->write = write;
- offset = vaddr & ~PAGE_MASK;
+ offset = 0;
  buf->size = size;

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