Hi, On Saturday 31 July 2010 01:49:09 P F wrote: > Hi again Laurent, > > --- On Fri, 7/23/10, P F <public_fil...@yahoo.com> wrote: > > Adding: > > vma->vm_page_prot = > > pgprot_noncached(vma->vm_page_prot); > > to uvc_v4l2_mmap. This resolved the corruption. > > gspca_zc3xx has the same > > issue, and this patch worked there as well. > > I am happy to report that all instances of UVC JPEG corruption on my > platform have been resolved. There were actually two sources of > corruption, and I was encountering both of them. This may explain why > debugging was so bewildering.
Congratulations :-) > One form of corruption was due to CPU starvation and not properly handling > lost isochronous packets. Your patch here: > http://lists.berlios.de/pipermail/linux-uvc-devel/2010-July/005824.html > definitely resolved this for me. > > The other form of corruption is less well understood, but it was resolved > by the change I described above. > > I have two pathological test cases which triggered these bugs. > Unfortunately, the causes are totally independent, and I was seeing the > "caching bug" when trying your isochronous patch, and thus saw no > improvement. However, after resolving the "caching bug," I moved on to a > different test case which revealed corruption again, and here your patch > was the solution. > > I would like to ask you to push your patch upstream, Done. The patch has been queued for 2.6.36. > and to investigate the efficacy of mine. I don't know anything about VMAs, > but I know this works for me. I can't accept that patch as-is, for several reasons. First of all, the patch maps V4L2 buffers to userspace in non-cacheable mode. This will potentially degrade performances. Platforms that have a PIPT or non- aliasing VIPT cache (x86 comes to mind, as well as several ARM processors) don't require this patch, so there will be a performance impact for the vast majority of uvcvideo users with no added benefit (I'm thinking about the x86 desktop). Then, even on ARM platforms with a VIVT or aliasing VIPT cache, the patch is not correct. The V4L2 buffers are mapped once in kernel space using a normal mapping. Your patch makes the second (userspace) mapping uncacheable, and the ARM specification clearly forbids different mappings with different attributes for the same memory region. The only correct fix I can think of at the moment is to munmap() buffers before queuing them, and mmap()ing them after dequeuing them. There's obviously a performance hit, but that's the price to pay for correctness. Other solutions might be possible, but they won't be straightforward and will require changes to the ARM memory management core. Maybe this should be explored on the linux-arm-kernel mailing list. -- Regards, Laurent Pinchart _______________________________________________ Linux-uvc-devel mailing list Linux-uvc-devel@lists.berlios.de https://lists.berlios.de/mailman/listinfo/linux-uvc-devel
