Hi Kieran,
On Wednesday, 7 November 2018 01:31:29 EET Laurent Pinchart wrote:
> On Tuesday, 6 November 2018 23:27:11 EET Kieran Bingham wrote:
> > From: Kieran Bingham <kieran.bingham@xxxxxxxxxxxxxxxx>
> >
> > The Linux UVC driver has long provided adequate performance capabilities
> > for web-cams and low data rate video devices in Linux while resolutions
> > were low.
> >
> > Modern USB cameras are now capable of high data rates thanks to USB3 with
> > 1080p, and even 4k capture resolutions supported.
> >
> > Cameras such as the Stereolabs ZED (bulk transfers) or the Logitech BRIO
> > (isochronous transfers) can generate more data than an embedded ARM core
> > is able to process on a single core, resulting in frame loss.
> >
> > A large part of this performance impact is from the requirement to
> > ‘memcpy’ frames out from URB packets to destination frames. This
> > unfortunate requirement is due to the UVC protocol allowing a variable
> > length header, and thus it is not possible to provide the target frame
> > buffers directly.
> >
> > Extra throughput is possible by moving the actual memcpy actions to a work
> > queue, and moving the memcpy out of interrupt context thus allowing work
> > tasks to be scheduled across multiple cores.
> >
> > This series has been tested on both the ZED and BRIO cameras on arm64
> > platforms, and with thanks to Randy Dunlap, a Dynex 1.3MP Webcam, a Sonix
> > USB2 Camera, and a built in Toshiba Laptop camera, and with thanks to
> > Philipp Zabel for testing on a Lite-On internal Laptop Webcam, Logitech
> > C910 (USB2 isoc), Oculus Sensor (USB3 isoc), and Microsoft HoloLens
> > Sensors (USB3 bulk).
> >
> > As far as I am aware iSight devices, and devices which use UVC to encode
> > data (output device) have not yet been tested - but should find no ill
> > effect (at least not until they are tested of course :D )
>
> :-D
>
> I'm not sure whether anyone is still using those devices with Linux. I
> wouldn't be surprised if we realized down the road that they already don't
> work.
>
> > Tested-by: Randy Dunlap <rdunlap@xxxxxxxxxxxxx>
> > Tested-by: Philipp Zabel <philipp.zabel@xxxxxxxxx>
> >
> > v2:
> > - Fix race reported by Guennadi
> >
> > v3:
> > - Fix similar race reported by Laurent
> > - Only queue work if required (encode/isight do not queue work)
> > - Refactor/Rename variables for clarity
> >
> > v4:
> > - (Yet another) Rework of the uninitialise path.
> >
> > This time to hopefully clean up the shutdown races for good.
> > use usb_poison_urb() to halt all URBs, then flush the work queue
> > before freeing.
> >
> > - Rebase to latest linux-media/master
> >
> > v5:
> > - Provide lockdep validation
> > - rename uvc_queue_requeue -> uvc_queue_buffer_requeue()
> > - Fix comments and periods throughout
> > - Rebase to media/v4.20-2
> > - Use GFP_KERNEL allocation in uvc_video_copy_data_work()
> > - Fix function documentation for uvc_video_copy_data_work()
> > - Add periods to the end of sentences
> > - Rename 'decode' variable to 'op' in uvc_video_decode_data()
> > - Move uvc_urb->async_operations initialisation to before use
> > - Move async workqueue to match uvc_streaming lifetime instead of
> >
> > streamon/streamoff
> >
> > - bracket the for_each_uvc_urb() macro
> >
> > - New patches added to series:
> > media: uvcvideo: Split uvc_video_enable into two
> > media: uvcvideo: Rename uvc_{un,}init_video()
> > media: uvcvideo: Utilise for_each_uvc_urb iterator
> >
> > Kieran Bingham (9):
> > media: uvcvideo: Refactor URB descriptors
> > media: uvcvideo: Convert decode functions to use new context structure
> > media: uvcvideo: Protect queue internals with helper
> > media: uvcvideo: queue: Simplify spin-lock usage
> > media: uvcvideo: queue: Support asynchronous buffer handling
> > media: uvcvideo: Move decode processing to process context
> > media: uvcvideo: Split uvc_video_enable into two
>
> I've taken the above patches in my tree.
I'm afraid I'll have to drop them :-( If I disconnect the camera while in use,
I get the following oops:
[ 237.514625] usb 2-1.4: USB disconnect, device number 5
[ 237.516123] uvcvideo: Failed to resubmit video URB (-19).
[ 237.549470] BUG: unable to handle kernel paging request at 000000004bec0091
[ 237.549476] PGD 0 P4D 0
[ 237.549481] Oops: 0000 [#1] PREEMPT SMP PTI
[ 237.549485] CPU: 3 PID: 5332 Comm: luvcview Tainted: G W O 4.18.16-gentoo #1
[ 237.549487] Hardware name: Dell Inc. Latitude E6420/0K0DNP, BIOS A08 10/18/2011
[ 237.549493] RIP: 0010:flush_workqueue_prep_pwqs+0x49/0x120
[ 237.549494] Code: 47 48 85 c0 0f 85 e1 00 00 00 c7 45 48 01 00 00 00 48 8b 45 00 4c 8d 78 90 48 39 c5 0f 84 d0 00 00 00 c6 44 24 07 00 4d 63 f4 <4d> 8b 2f 4c 89 ef e8 5c b3 89 00 45 85 e4 78 1d 41 83 7f 14 ff 75
[ 237.549525] RSP: 0018:ffffc9000154fc50 EFLAGS: 00010296
[ 237.549527] RAX: 000000004bec0101 RBX: 0000000000000002 RCX: 0000000000000002
[ 237.549529] RDX: 0000000000000002 RSI: 0000000000000001 RDI: ffff88030241c600
[ 237.549530] RBP: ffff88030241c600 R08: 0000000000000000 R09: 0000000000000000
[ 237.549532] R10: ffffc9000154fd30 R11: ffff8802faa09398 R12: 0000000000000001
[ 237.549534] R13: ffff88030241c668 R14: 0000000000000001 R15: 000000004bec0091
[ 237.549536] FS: 0000000000000000(0000) GS:ffff88032dd80000(0000) knlGS:0000000000000000
[ 237.549537] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 237.549539] CR2: 000000004bec0091 CR3: 000000000220a001 CR4: 00000000000606e0
[ 237.549540] Call Trace:
[ 237.549545] flush_workqueue+0x161/0x390
[ 237.549549] ? preempt_count_add+0x8d/0x90
[ 237.549557] ? uvc_uninit_video+0x51/0xa0 [uvcvideo]
[ 237.549560] uvc_uninit_video+0x51/0xa0 [uvcvideo]
[ 237.549565] uvc_video_stop_streaming+0xe/0x80 [uvcvideo]
[ 237.549568] uvc_stop_streaming+0x17/0x40 [uvcvideo]
[ 237.549573] __vb2_queue_cancel+0x25/0x1c0 [videobuf2_common]
[ 237.549576] vb2_core_queue_release+0x19/0x40 [videobuf2_common]
[ 237.549580] uvc_queue_release+0x1c/0x30 [uvcvideo]
[ 237.549584] uvc_v4l2_release+0x9d/0xd0 [uvcvideo]
[ 237.549594] v4l2_release+0x2d/0x80 [videodev]
[ 237.549598] __fput+0x95/0x1d0
[ 237.549602] task_work_run+0x84/0xa0
[ 237.549605] do_exit+0x2a3/0xad0
[ 237.549608] do_group_exit+0x2e/0xa0
[ 237.549610] __x64_sys_exit_group+0xf/0x10
[ 237.549613] do_syscall_64+0x3d/0xf0
[ 237.549617] entry_SYSCALL_64_after_hwframe+0x44/0xa9
[ 237.549619] RIP: 0033:0x7f2b0b6097a6
[ 237.549620] Code: Bad RIP value.
[ 237.549625] RSP: 002b:00007ffd2ddf5088 EFLAGS: 00000246 ORIG_RAX: 00000000000000e7
[ 237.549627] RAX: ffffffffffffffda RBX: 00007f2b0b8f8740 RCX: 00007f2b0b6097a6
[ 237.549629] RDX: 0000000000000000 RSI: 000000000000003c RDI: 0000000000000000
[ 237.549630] RBP: 0000000000000000 R08: 00000000000000e7 R09: ffffffffffffff80
[ 237.549632] R10: 00007f2b05e37168 R11: 0000000000000246 R12: 00007f2b0b8f8740
[ 237.549634] R13: 0000000000000001 R14: 00007f2b0b9016e8 R15: 0000000000000000
[ 237.549636] Modules linked in: uvcvideo(O) videobuf2_vmalloc(O) videobuf2_memops(O) videobuf2_v4l2(O) videobuf2_common(O) v4l2_common(O) videodev(O) media(O) xt_conntrack ip6table_filter binfmt_misc dell_laptop snd_hda_codec_hdmi dell_smbios dcdbas coretemp hwmon kvm_intel kvm irqbypass snd_hda_codec_idt snd_hda_codec_generic iwldvm snd_hda_intel snd_hda_codec snd_hwdep snd_hda_core snd_pcm snd_timer iwlwifi snd [last unloaded: media]
[ 237.549659] CR2: 000000004bec0091
[ 237.549662] ---[ end trace 574358b4f247dc6f ]---
[ 237.549664] RIP: 0010:flush_workqueue_prep_pwqs+0x49/0x120
[ 237.549665] Code: 47 48 85 c0 0f 85 e1 00 00 00 c7 45 48 01 00 00 00 48 8b 45 00 4c 8d 78 90 48 39 c5 0f 84 d0 00 00 00 c6 44 24 07 00 4d 63 f4 <4d> 8b 2f 4c 89 ef e8 5c b3 89 00 45 85 e4 78 1d 41 83 7f 14 ff 75
[ 237.549696] RSP: 0018:ffffc9000154fc50 EFLAGS: 00010296
[ 237.549699] RAX: 000000004bec0101 RBX: 0000000000000002 RCX: 0000000000000002
[ 237.549700] RDX: 0000000000000002 RSI: 0000000000000001 RDI: ffff88030241c600
[ 237.549702] RBP: ffff88030241c600 R08: 0000000000000000 R09: 0000000000000000
[ 237.549704] R10: ffffc9000154fd30 R11: ffff8802faa09398 R12: 0000000000000001
[ 237.549706] R13: ffff88030241c668 R14: 0000000000000001 R15: 000000004bec0091
[ 237.549708] FS: 0000000000000000(0000) GS:ffff88032dd80000(0000) knlGS:0000000000000000
[ 237.549710] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 237.549712] CR2: 00007f2b0b60977c CR3: 000000000220a001 CR4: 00000000000606e0
[ 237.549714] Fixing recursive fault but reboot is needed!
> > media: uvcvideo: Rename uvc_{un,}init_video()
> > media: uvcvideo: Utilise for_each_uvc_urb iterator
>
> And I've sent review comments for these two.
>
> > drivers/media/usb/uvc/uvc_driver.c | 2 +-
> > drivers/media/usb/uvc/uvc_isight.c | 6 +-
> > drivers/media/usb/uvc/uvc_queue.c | 110 +++++++++---
> > drivers/media/usb/uvc/uvc_video.c | 282 +++++++++++++++++++-----------
> > drivers/media/usb/uvc/uvcvideo.h | 65 ++++++-
> > 5 files changed, 331 insertions(+), 134 deletions(-)
> >
> > base-commit: dafb7f9aef2fd44991ff1691721ff765a23be27b
--
Regards,
Laurent Pinchart
