From: Michael Hennerich <[email protected]>
Add NOMMU mmap support.
Signed-off-by: Michael Hennerich <[email protected]>
Signed-off-by: Bryan Wu <[email protected]>
---
drivers/media/video/uvc/uvc_v4l2.c | 14 ++++++++++++++
1 files changed, 14 insertions(+), 0 deletions(-)
diff --git a/drivers/media/video/uvc/uvc_v4l2.c b/drivers/media/video/uvc/uvc_v4l2.c
index 758dfef..2237f5e 100644
--- a/drivers/media/video/uvc/uvc_v4l2.c
+++ b/drivers/media/video/uvc/uvc_v4l2.c
@@ -1050,6 +1050,7 @@ static int uvc_v4l2_mmap(struct file *file, struct vm_area_struct *vma)
break;
}
+#ifdef CONFIG_MMU
if (i == video->queue.count || size != video->queue.buf_size) {
ret = -EINVAL;
goto done;
@@ -1071,7 +1072,20 @@ static int uvc_v4l2_mmap(struct file *file, struct vm_area_struct *vma)
addr += PAGE_SIZE;
size -= PAGE_SIZE;
}
+#else
+ if (i == video->queue.count ||
+ PAGE_ALIGN(size) != video->queue.buf_size) {
+ ret = -EINVAL;
+ goto done;
+ }
+
+ vma->vm_flags |= VM_IO | VM_MAYSHARE; /* documentation/nommu-mmap.txt */
+
+ addr = (unsigned long)video->queue.mem + buffer->buf.m.offset;
+ vma->vm_start = addr;
+ vma->vm_end = addr + video->queue.buf_size;
+#endif
vma->vm_ops = &uvc_vm_ops;
vma->vm_private_data = buffer;
uvc_vm_open(vma);
--
1.5.6.3
Hi Bryan, Michael,
On Thursday 06 November 2008, Bryan Wu wrote:
> From: Michael Hennerich <[email protected]>
>
> Add NOMMU mmap support.
>
> Signed-off-by: Michael Hennerich <[email protected]>
> Signed-off-by: Bryan Wu <[email protected]>
> ---
> drivers/media/video/uvc/uvc_v4l2.c | 14 ++++++++++++++
> 1 files changed, 14 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/media/video/uvc/uvc_v4l2.c
> b/drivers/media/video/uvc/uvc_v4l2.c index 758dfef..2237f5e 100644
> --- a/drivers/media/video/uvc/uvc_v4l2.c
> +++ b/drivers/media/video/uvc/uvc_v4l2.c
> @@ -1050,6 +1050,7 @@ static int uvc_v4l2_mmap(struct file *file, struct
> vm_area_struct *vma) break;
> }
>
> +#ifdef CONFIG_MMU
> if (i == video->queue.count || size != video->queue.buf_size) {
> ret = -EINVAL;
> goto done;
> @@ -1071,7 +1072,20 @@ static int uvc_v4l2_mmap(struct file *file, struct
> vm_area_struct *vma) addr += PAGE_SIZE;
> size -= PAGE_SIZE;
> }
> +#else
> + if (i == video->queue.count ||
> + PAGE_ALIGN(size) != video->queue.buf_size) {
Just out of curiosity, why do you need to PAGE_ALIGN size for non-MMU
platforms ?
> + ret = -EINVAL;
> + goto done;
> + }
> +
> + vma->vm_flags |= VM_IO | VM_MAYSHARE; /* documentation/nommu-mmap.txt */
VM_MAYSHARE is not documented anywhere in Documentation/ in Linux 2.6.28-rc3.
Why is it needed for non-MMU architectures only ?
> +
> + addr = (unsigned long)video->queue.mem + buffer->buf.m.offset;
>
> + vma->vm_start = addr;
> + vma->vm_end = addr + video->queue.buf_size;
> +#endif
> vma->vm_ops = &uvc_vm_ops;
> vma->vm_private_data = buffer;
> uvc_vm_open(vma);
Best regards,
Laurent Pinchart
>-----Original Message-----
>From: Laurent Pinchart [mailto:[email protected]]
>Sent: Sunday, November 09, 2008 10:22 PM
>To: Bryan Wu
>Cc: [email protected]; [email protected];
linux-
>[email protected]; Michael Hennerich
>Subject: Re: [PATCH] Video/UVC: Port mainlined uvc video driver to
NOMMU
>
>Hi Bryan, Michael,
>
>On Thursday 06 November 2008, Bryan Wu wrote:
>> From: Michael Hennerich <[email protected]>
>>
>> Add NOMMU mmap support.
>>
>> Signed-off-by: Michael Hennerich <[email protected]>
>> Signed-off-by: Bryan Wu <[email protected]>
>> ---
>> drivers/media/video/uvc/uvc_v4l2.c | 14 ++++++++++++++
>> 1 files changed, 14 insertions(+), 0 deletions(-)
>>
>> diff --git a/drivers/media/video/uvc/uvc_v4l2.c
>> b/drivers/media/video/uvc/uvc_v4l2.c index 758dfef..2237f5e 100644
>> --- a/drivers/media/video/uvc/uvc_v4l2.c
>> +++ b/drivers/media/video/uvc/uvc_v4l2.c
>> @@ -1050,6 +1050,7 @@ static int uvc_v4l2_mmap(struct file *file,
struct
>> vm_area_struct *vma) break;
>> }
>>
>> +#ifdef CONFIG_MMU
>> if (i == video->queue.count || size != video->queue.buf_size) {
>> ret = -EINVAL;
>> goto done;
>> @@ -1071,7 +1072,20 @@ static int uvc_v4l2_mmap(struct file *file,
struct
>> vm_area_struct *vma) addr += PAGE_SIZE;
>> size -= PAGE_SIZE;
>> }
>> +#else
>> + if (i == video->queue.count ||
>> + PAGE_ALIGN(size) != video->queue.buf_size) {
>
>Just out of curiosity, why do you need to PAGE_ALIGN size for non-MMU
>platforms ?
Size and video->queue.buf_size is not the 100% same size (off by a few
bytes < pagesize), I think it's because on NOMMU the kernel calls
kmalloc() to allocate the buffer, not get_free_page().
>
>> + ret = -EINVAL;
>> + goto done;
>> + }
>> +
>> + vma->vm_flags |= VM_IO | VM_MAYSHARE; /*
documentation/nommu-mmap.txt
>*/
>
>VM_MAYSHARE is not documented anywhere in Documentation/ in Linux
2.6.28-
>rc3.
>Why is it needed for non-MMU architectures only ?
mmap on NOMMU is a bit tricky and very restricted.
In case user does a MAP_SHARED with some combination of the PROT_###
Flags the mmap fails. What's allowed and what's not is documented in
documentation/nommu-mmap.txt
Setting VM_MAYSHARE allows user MAP_PRIVATE mappings.
-Michael
>
>> +
>> + addr = (unsigned long)video->queue.mem + buffer->buf.m.offset;
>>
>> + vma->vm_start = addr;
>> + vma->vm_end = addr + video->queue.buf_size;
>> +#endif
>> vma->vm_ops = &uvc_vm_ops;
>> vma->vm_private_data = buffer;
>> uvc_vm_open(vma);
>
>Best regards,
>
>Laurent Pinchart
Hi Michael,
On Monday 10 November 2008, Hennerich, Michael wrote:
> >On Thursday 06 November 2008, Bryan Wu wrote:
> > > @@ -1071,7 +1072,20 @@ static int uvc_v4l2_mmap(struct file *file,
> > > struct vm_area_struct *vma) addr += PAGE_SIZE;
> > > size -= PAGE_SIZE;
> > > }
> > > +#else
> > > + if (i == video->queue.count ||
> > > + PAGE_ALIGN(size) != video->queue.buf_size) {
> >
> > Just out of curiosity, why do you need to PAGE_ALIGN size for non-MMU
> > platforms ?
>
> Size and video->queue.buf_size is not the 100% same size (off by a few
> bytes < pagesize), I think it's because on NOMMU the kernel calls
> kmalloc() to allocate the buffer, not get_free_page().
That's right, but only for private mappings. It makes little sense to create a
private mapping on a V4L2 device as the kernel will read() device data into
the buffer when mapping the device (at least on NOMMU platforms). Only shared
mappings make sense in this case.
> > > + ret = -EINVAL;
> > > + goto done;
> > > + }
> > > +
> > > + vma->vm_flags |= VM_IO | VM_MAYSHARE; /* documentation/nommu-mmap.txt
> >
> > VM_MAYSHARE is not documented anywhere in Documentation/ in Linux
> > 2.6.28-rc3. Why is it needed for non-MMU architectures only ?
>
> mmap on NOMMU is a bit tricky and very restricted.
> In case user does a MAP_SHARED with some combination of the PROT_###
> Flags the mmap fails. What's allowed and what's not is documented in
> documentation/nommu-mmap.txt
> Setting VM_MAYSHARE allows user MAP_PRIVATE mappings.
There's something I don't understand. I've had a quick look at NOMMU mmap
(mm/nommu.c) and it seems neither MAP_SHARED nor MAP_PRIVATE can succeed with
UVC devices.
The uvcvideo driver doesn't implements the read and get_unmapped_area file
operations. validate_mmap_request will thus clear the BDI_CAP_MAP_DIRECT and
BDI_CAP_MAP_COPY from the device mapping capabilities. As shared mappings
require BDI_CAP_MAP_DIRECT and private mappings require BDI_CAP_MAP_COPY
validate_mmap_request will return an error and mmap will fail.
I might be wrong in my analysis, but if mapping a UVC device is impossible on
a NOMMU platform the patch doesn't make much sense. Feel free to prove me
wrong and send me back to mm/ if you've been able to map a UVC device on a
NOMMU platform :-)
Laurent Pinchart
>-----Original Message-----
>From: Laurent Pinchart [mailto:[email protected]]
>Sent: Friday, November 14, 2008 12:52 AM
>To: Hennerich, Michael
>Cc: Bryan Wu; [email protected]; video4linux-
>[email protected]; [email protected]
>Subject: Re: [PATCH] Video/UVC: Port mainlined uvc video driver to
NOMMU
>
>Hi Michael,
>
>On Monday 10 November 2008, Hennerich, Michael wrote:
>> >On Thursday 06 November 2008, Bryan Wu wrote:
>> > > @@ -1071,7 +1072,20 @@ static int uvc_v4l2_mmap(struct file
*file,
>> > > struct vm_area_struct *vma) addr += PAGE_SIZE;
>> > > size -= PAGE_SIZE;
>> > > }
>> > > +#else
>> > > + if (i == video->queue.count ||
>> > > + PAGE_ALIGN(size) != video->queue.buf_size) {
>> >
>> > Just out of curiosity, why do you need to PAGE_ALIGN size for
non-MMU
>> > platforms ?
>>
>> Size and video->queue.buf_size is not the 100% same size (off by a
few
>> bytes < pagesize), I think it's because on NOMMU the kernel calls
>> kmalloc() to allocate the buffer, not get_free_page().
>
>That's right, but only for private mappings. It makes little sense to
>create a
>private mapping on a V4L2 device as the kernel will read() device data
into
>the buffer when mapping the device (at least on NOMMU platforms). Only
>shared
>mappings make sense in this case.
>
>> > > + ret = -EINVAL;
>> > > + goto done;
>> > > + }
>> > > +
>> > > + vma->vm_flags |= VM_IO | VM_MAYSHARE; /*
documentation/nommu-
>mmap.txt
>> >
>> > VM_MAYSHARE is not documented anywhere in Documentation/ in Linux
>> > 2.6.28-rc3. Why is it needed for non-MMU architectures only ?
>>
>> mmap on NOMMU is a bit tricky and very restricted.
>> In case user does a MAP_SHARED with some combination of the PROT_###
>> Flags the mmap fails. What's allowed and what's not is documented in
>> documentation/nommu-mmap.txt
>> Setting VM_MAYSHARE allows user MAP_PRIVATE mappings.
>
>There's something I don't understand. I've had a quick look at NOMMU
mmap
>(mm/nommu.c) and it seems neither MAP_SHARED nor MAP_PRIVATE can
succeed
>with
>UVC devices.
>
>The uvcvideo driver doesn't implements the read and get_unmapped_area
file
>operations. validate_mmap_request will thus clear the
BDI_CAP_MAP_DIRECT
>and
>BDI_CAP_MAP_COPY from the device mapping capabilities. As shared
mappings
>require BDI_CAP_MAP_DIRECT and private mappings require
BDI_CAP_MAP_COPY
>validate_mmap_request will return an error and mmap will fail.
>
>I might be wrong in my analysis, but if mapping a UVC device is
impossible
>on
>a NOMMU platform the patch doesn't make much sense. Feel free to prove
me
>wrong and send me back to mm/ if you've been able to map a UVC device
on a
>NOMMU platform :-)
Hi Laurent,
I can only say when we map the buffers in user space (ffmpeg, luvcview,
etc.) as MAP_PRIVTE with PROT_READ | PROT_WRITE set, this patch works on
NOMMU. It passes validate_mmap_request(),determine_vm_flags() doesn't
set VM_MAYSHARE, so we finally call do_mmap_private() which in turn
calls the uvc v4l2 private mmap function. There is no copy etc.
involved. User and kernel share the same physical contiguous addresses.
This is even more efficient on NOMMU.
-Michael
>
>Laurent Pinchart
On Fri, Nov 14, 2008 at 12:52:10AM +0100, Laurent Pinchart wrote:
> On Monday 10 November 2008, Hennerich, Michael wrote:
> > >On Thursday 06 November 2008, Bryan Wu wrote:
> > > > @@ -1071,7 +1072,20 @@ static int uvc_v4l2_mmap(struct file *file,
> > > > struct vm_area_struct *vma) addr += PAGE_SIZE;
> > > > size -= PAGE_SIZE;
> > > > }
> > > > +#else
> > > > + if (i == video->queue.count ||
> > > > + PAGE_ALIGN(size) != video->queue.buf_size) {
> > >
> > > Just out of curiosity, why do you need to PAGE_ALIGN size for non-MMU
> > > platforms ?
> >
> > Size and video->queue.buf_size is not the 100% same size (off by a few
> > bytes < pagesize), I think it's because on NOMMU the kernel calls
> > kmalloc() to allocate the buffer, not get_free_page().
>
> That's right, but only for private mappings. It makes little sense to create a
> private mapping on a V4L2 device as the kernel will read() device data into
> the buffer when mapping the device (at least on NOMMU platforms). Only shared
> mappings make sense in this case.
>
The change itself is crap anyways, since it is just working around the
fact that vm_insert_page() presently -EINVAL's out on nommu. The
vmalloc_to_page()/vm_insert_page() pair is used extensively across the
v4l drivers, and it's unrealistic to expect to patch every call site with
this sort of a workaround.
While we can't support vm_insert_page() in most cases, these sorts of
use cases where it is just iterating over a contiguous block of pages on
a VMA that has already been established is something that can at least be
handled fairly easily (in this case, even just doing nothing in
vm_insert_page() would likely give the desired result, although we ought
to fix up vm_insert_page() to do something more useful instead). Working
out the bounds of the VMA isn't exactly difficult either, since we have
all of that already filled in the VMA by the time vma_insert_page() is
called.
Regarding the VMA flag, this can be rectified by providing a dummy
get_unmapped_area() for the v4l devices which will set some default
capabilities. After that, determine_vm_flags() should take care of
setting up the proper VMA flags without hacking every driver's mmap()
routine. This sort of thing has absolutely no place in the driver,
though.