Video/UVC: Port mainlined uvc video driver to NOMMU
PATCH 1/1 Video/UVC: Port mainlined uvc video driver to NOMMU
From: Michael Hennerich <[email protected]>
Add NOMMU mmap support.
--- drivers/media/video/uvc/uvc_v4l2.c | 14 ++++++++++++++ 1 files changed, 14 insertions(+), 0 deletions(-)
drivers/media/video/uvc/uvc_v4l2.c +14 -0
@@ -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);
Signature · 1 line
1.5.6.3
Hi Bryan, Michael,
Quoted 23 lines · Bryan Wu
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(-)(quoted diff) +0
-0
b/drivers/media/video/uvc/uvc_v4l2.cindex 758dfef..2237f5e 100644drivers/media/video/uvc/uvc_v4l2.c +1
-0@@ -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;
drivers/media/video/uvc/uvc_v4l2.c static int uvc_v4l2_mmap(struct file *file, struct · line 1077
Quoted 6 lines
drivers/media/video/uvc/uvc_v4l2.c +3
-0@@ -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 ?
drivers/media/video/uvc/uvc_v4l2.c static int uvc_v4l2_mmap(struct file *file, struct vm_area_struct *vma) · line 1082
Quoted 5 lines
drivers/media/video/uvc/uvc_v4l2.c +5
-0+ 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 ?
drivers/media/video/uvc/uvc_v4l2.c static int uvc_v4l2_mmap(struct file *file, struct vm_area_struct *vma) · line 1091
Quoted 9 lines
drivers/media/video/uvc/uvc_v4l2.c +5
-0+ + 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
--
video4linux-list mailing list
Unsubscribe mailto:[email protected]?subject=unsubscribe
https://www.redhat.com/mailman/listinfo/video4linux-list
Quoted 7 lines
-----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-
Quoted 2 lines
[email protected]; Michael Hennerich
Subject: Re: [PATCH] Video/UVC: Port mainlined uvc video driver to
NOMMU
Quoted 14 lines
Hi Bryan, Michael,
Quoted 10 lines · Bryan Wu
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(-)
Quoted 5 lines
drivers/media/video/uvc/uvc_v4l2.c static int uvc_v4l2_mmap(struct file *file,
Quoted 5 lines
(quoted diff) +0
-0
b/drivers/media/video/uvc/uvc_v4l2.cindex 758dfef..2237f5e 100644drivers/media/video/uvc/uvc_v4l2.c +0
-0@@ -1050,6 +1050,7 @@ static int uvc_v4l2_mmap(struct file *file,
struct
Quoted 8 lines
drivers/media/video/uvc/uvc_v4l2.c static int uvc_v4l2_mmap(struct file *file,
Quoted 8 lines
drivers/media/video/uvc/uvc_v4l2.c +1
-0vm_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
Quoted 9 lines
drivers/media/video/uvc/uvc_v4l2.c static int uvc_v4l2_mmap(struct file *file, struct vm_area_struct *vma) · line 1077
Quoted 6 lines
drivers/media/video/uvc/uvc_v4l2.c +3
-0vm_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_ALIGNsize 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().
Quoted 6 lines
drivers/media/video/uvc/uvc_v4l2.c static int uvc_v4l2_mmap(struct file *file, struct vm_area_struct *vma)
Quoted 6 lines
drivers/media/video/uvc/uvc_v4l2.c +5
-0+ ret = -EINVAL ; + goto done; + } + + vma->vm_flags |=VM_IO |VM_MAYSHARE ;/*
documentation/nommu-mmap.txt
Quoted 3 lines
*/
VM_MAYSHAREis not documented anywhere inDocumentation/in Linux
2.6.28-
Quoted 2 lines
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
Quoted 14 lines
drivers/media/video/uvc/uvc_v4l2.c static int uvc_v4l2_mmap(struct file *file, struct vm_area_struct *vma) · line 1091
Quoted 10 lines
drivers/media/video/uvc/uvc_v4l2.c +5
-0+ + 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,
Quoted 12 lines · Michael
Quoted 11 lines · Bryan Wu
drivers/media/video/uvc/uvc_v4l2.c static int uvc_v4l2_mmap(struct file *file, · line 1077
Quoted 7 lines
drivers/media/video/uvc/uvc_v4l2.c +3
-0@@ -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_ALIGNsize for non-MMU platforms ?
Quoted 3 lines
Size and
video->queue.buf_sizeis not the 100% same size (off by a few bytes < pagesize), I think it's because on NOMMU the kernel callskmalloc()to allocate the buffer, notget_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.
Quoted 9 lines
Quoted 8 lines
drivers/media/video/uvc/uvc_v4l2.c static int uvc_v4l2_mmap(struct file *file, struct vm_area_struct *vma)
Quoted 5 lines
drivers/media/video/uvc/uvc_v4l2.c +5
-0+ ret = -EINVAL ; + goto done; + } + + vma->vm_flags |=VM_IO |VM_MAYSHARE ;/* documentation/nommu-mmap.txt
VM_MAYSHAREis not documented anywhere inDocumentation/in Linux 2.6.28-rc3. Why is it needed for non-MMU architectures only ?
Quoted 5 lines
mmap on NOMMU is a bit tricky and very restricted.
In case user does aMAP_SHAREDwith some combination of the PROT_### Flags the mmap fails. What's allowed and what's not is documented indocumentation/nommu-mmap.txt
SettingVM_MAYSHAREallows userMAP_PRIVATEmappings.
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
Quoted 9 lines
-----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
Quoted 6 lines
Hi Michael,
Quoted 2 lines · Michael
Quoted 2 lines · Bryan Wu
Review static int uvc_v4l2_mmap(struct file
Quoted 1 line
(quoted diff) +0
-0@@ -1071,7 +1072,20 @@ static int uvc_v4l2_mmap(struct file
*file,
Quoted 8 lines
Quoted 8 lines
Quoted 8 lines
drivers/media/video/uvc/uvc_v4l2.c static int uvc_v4l2_mmap(struct file *file, struct vm_area_struct *vma) · line 1077
Quoted 6 lines
drivers/media/video/uvc/uvc_v4l2.c +3
-0struct 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_ALIGNsize for
non-MMU
Quoted 3 lines
Quoted 3 lines
Quoted 1 line
platforms ?
Size and
video->queue.buf_sizeis not the 100% same size (off by a
few
Quoted 6 lines
Quoted 2 lines
bytes < pagesize), I think it's because on NOMMU the kernel calls
kmalloc()to allocate the buffer, notget_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 willread()device data
into
Quoted 9 lines
the buffer when mapping the device (at least on NOMMU platforms). Only shared
mappings make sense in this case.Quoted 5 lines
Quoted 5 lines
drivers/media/video/uvc/uvc_v4l2.c static int uvc_v4l2_mmap(struct file *file, struct vm_area_struct *vma)
Quoted 5 lines
drivers/media/video/uvc/uvc_v4l2.c +5
-0+ ret = -EINVAL ; + goto done; + } + + vma->vm_flags |=VM_IO |VM_MAYSHARE ;/*
documentation/nommu-
Quoted 5 lines
mmap.txt
Quoted 4 lines
Quoted 3 lines
VM_MAYSHAREis not documented anywhere inDocumentation/in Linux 2.6.28-rc3. Why is it needed for non-MMU architectures only ?
Quoted 7 lines
Quoted 5 lines
mmap on NOMMU is a bit tricky and very restricted.
In case user does aMAP_SHAREDwith some combination of the PROT_### Flags the mmap fails. What's allowed and what's not is documented indocumentation/nommu-mmap.txt
SettingVM_MAYSHAREallows userMAP_PRIVATEmappings.There's something I don't understand. I've had a quick look at NOMMU
mmap
Quoted 1 line
(
mm/nommu.c) and it seems neitherMAP_SHAREDnorMAP_PRIVATEcan
succeed
Quoted 4 lines
with
UVC devices.The uvcvideo driver doesn't implements the read and
get_unmapped_area
file
Quoted 1 line
operations.
validate_mmap_requestwill thus clear the
BDI_CAP_MAP_DIRECT
Quoted 2 lines
and
BDI_CAP_MAP_COPYfrom the device mapping capabilities. As shared
mappings
Quoted 1 line
require
BDI_CAP_MAP_DIRECTand private mappings require
BDI_CAP_MAP_COPY
Quoted 3 lines
validate_mmap_requestwill return an error and mmap will fail.I might be wrong in my analysis, but if mapping a UVC device is
impossible
Quoted 2 lines
on
a NOMMU platform the patch doesn't make much sense. Feel free to prove
me
Quoted 1 line
wrong and send me back to
mm/if you've been able to map a UVC device
on a
Quoted 1 line
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
Quoted 2 lines
Laurent Pinchart
Quoted 13 lines · Laurent Pinchart
Quoted 12 lines · Michael
Quoted 11 lines · Bryan Wu
drivers/media/video/uvc/uvc_v4l2.c static int uvc_v4l2_mmap(struct file *file, · line 1077
Quoted 7 lines
drivers/media/video/uvc/uvc_v4l2.c +3
-0@@ -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_ALIGNsize for non-MMU platforms ?
Quoted 9 lines
Quoted 3 lines
Size and
video->queue.buf_sizeis not the 100% same size (off by a few bytes < pagesize), I think it's because on NOMMU the kernel callskmalloc()to allocate the buffer, notget_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.
Quoted 39 lines · Paul Mundt
Quoted 13 lines · Laurent Pinchart
Quoted 12 lines · Michael
Quoted 11 lines · Bryan Wu
drivers/media/video/uvc/uvc_v4l2.c static int uvc_v4l2_mmap(struct file *file, · line 1077
Quoted 7 lines
drivers/media/video/uvc/uvc_v4l2.c +3
-0@@ -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_ALIGNsize for non-MMU platforms ?Quoted 9 lines
Quoted 3 lines
Size and
video->queue.buf_sizeis not the 100% same size (off by a few bytes < pagesize), I think it's because on NOMMU the kernel callskmalloc()to allocate the buffer, notget_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 invm_insert_page()would likely give the desired result, although we ought to fix upvm_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 timevma_insert_page()is called.
Quoted 7 lines
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'smmap()routine. This sort of thing has absolutely no place in the driver, though.
Hi Laurent,
Does this patch make sense? I hope you understand the tricky things in nommu mapping world.
Thanks
-Bryan
Hi Bryan,
Quoted 47 lines · Bryan Wu
Quoted 39 lines · Paul Mundt
Quoted 13 lines · Laurent Pinchart
Quoted 12 lines · Michael
Quoted 11 lines · Bryan Wu
drivers/media/video/uvc/uvc_v4l2.c static int uvc_v4l2_mmap(struct file *file, · line 1077
Quoted 7 lines
drivers/media/video/uvc/uvc_v4l2.c +3
-0@@ -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_ALIGNsize for non-MMU platforms ?Quoted 8 lines
Quoted 3 lines
Size and
video->queue.buf_sizeis not the 100% same size (off by a few bytes < pagesize), I think it's because on NOMMU the kernel callskmalloc()to allocate the buffer, notget_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 invm_insert_page()would likely give the desired result, although we ought to fix upvm_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 timevma_insert_page()is called.Quoted 6 lines
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'smmap()routine. This sort of thing has absolutely no place in the driver, though.
Quoted 4 lines
Hi Laurent,
Does this patch make sense? I hope you understand the tricky things in nommu mapping world.
I agree with Paul, the problem lies in vm_insert_page() and a fix is needed there.
Best regards,
Laurent Pinchart