Re: New USB core API to change interval and max packet size
On Wed, Oct 02, 2013 at 12:16:04AM +0300, Xenia Ragiadakou wrote: On 10/01/2013 11:45 PM, Sarah Sharp wrote: Sure. I would actually suggest you first finish up the patch to issue a configure endpoint if userspace wants to clear a halt, but the endpoint isn't actually halted. Did your most current patch work? I can't remember what the status was. Sarah Sharp Thanks for the clarification, I understand better now. As far as concerns the reset endpoint fix, I am not sure if it works I have to send an RFC so that it can be further tested but I have a lot of pending RFCs for xhci on the mailing list so i was waiting for those to be reviewed before sending new ones. Or it is not necessary to wait and just send the RFC based on current usb-next tree? Go ahead and send that one as well. Sarah Sharp -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: New USB core API to change interval and max packet size
On Wed, Oct 02, 2013 at 10:22:52AM -0400, Alan Stern wrote: On Tue, 1 Oct 2013, Sarah Sharp wrote: When you say a new API, what do you mean? New functions in usbcore to be used by usb device drivers? Yes. You would export the function in the USB core, and put a prototype in a USB include file (probably in include/linux/usb.h). Let's say that function is called usb_change_ep_bandwidth. Drivers could call into that function when they needed to change either the bInterval or wMaxPacketSize of a particular endpoint. This could be during the driver's probe function, or before switching alternate interface settings, or even after the alt setting is in place, but userspace dictates the driver use a different bandwidth. Drivers should pass usb_change_ep_bandwidth a pointer to the endpoint they need to change, along with the bInterval and wMaxPacketSize values they would like the endpoint to have. Those values could be stored as new values in struct usb_host_endpoint. usb_change_ep_bandwidth would then call into the xHCI driver to drop the endpoint, re-add it, and then issue a bandwidth change. The xHCI driver would have to be changed to look at the new fields in usb_host_endpoint, and set up the endpoint contexts with the interval and packet size from those fields, instead of the endpoint descriptor. We should probably set the new values in usb_host_endpoint to zero after the driver unbinds from the device. Not sure if they should be reset after the driver switches interfaces. I would have to see the use cases in the driver. We should consider this before rushing into a new API. Yes, I agree. :) That's why I'd like to see some cases in the media drivers code where it could benefit from changing the interval or maxpacket size, so that we can see what use cases we have. Mauro, can you point is to places in drivers that would need this? In particular, do we want to go around changing single endpoints, one at a time? Or do we want to change all the endpoints in an interface at once? Given that a change to one endpoint may require the entire schedule to be recomputed, it seems to make more sense to do all of them at once. For example, drivers could call a routine to set the desired endpoint parameters, and then the new parameters could take effect when the driver calls usb_set_interface(). I think it makes sense to change several endpoints, and then ask the host to recompute the schedule. Perhaps the driver could issue several calls to usb_change_ep_bandwidth and then ask the USB core to update the host's schedule? I'm not sure that usb_set_interface() is the right function for the new parameters to take effect. What if the driver is trying to modify the current alt setting? Would they need to call usb_set_interface() again? In any case, the question about what to do when the interface setting gets switched never really arises. Each usb_host_endpoint structure is referenced from only one altsetting. If the driver wants the new parameters applied to an endpoint in several altsettings, it will have to change each altsetting separately. Ok, so it sounds like we want to keep the changes the endpoints across alt setting changes. But we still want to reset the values after the driver unbinds, correct? Sarah Sharp -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
New USB core API to change interval and max packet size
On Tue, Oct 01, 2013 at 10:01:08PM +0300, Xenia Ragiadakou wrote: Hi Sarah, I read the mail on 'possible conflict between xhci_hcd and a patched usbhid'. For reference to others: http://marc.info/?l=linux-usbm=138064948726038w=2 http://marc.info/?l=linux-usbm=138065201426880w=2 I looked in xhci and the problem arises in xhci_queue_intr_tx() when if (xhci_interval != ep_interval) { ... urb-interval = xhci_interval; } right? Yes. The underlying problem is that the xHCI host sets up the endpoint contexts during the Configure Endpoint command, using the interval from the device's endpoint descriptors. It also uses the endpoint descriptor wMaxPacketSize, which can be wrong as well. If the device driver wants to use a different urb-interval than is in the endpoint descriptor, the xHCI driver will simply ignore it. (I'm Ccing the linux-media list, as I've discussed some of these devices with broken descriptors before.) When you say a new API, what do you mean? New functions in usbcore to be used by usb device drivers? Yes. You would export the function in the USB core, and put a prototype in a USB include file (probably in include/linux/usb.h). Let's say that function is called usb_change_ep_bandwidth. Drivers could call into that function when they needed to change either the bInterval or wMaxPacketSize of a particular endpoint. This could be during the driver's probe function, or before switching alternate interface settings, or even after the alt setting is in place, but userspace dictates the driver use a different bandwidth. Drivers should pass usb_change_ep_bandwidth a pointer to the endpoint they need to change, along with the bInterval and wMaxPacketSize values they would like the endpoint to have. Those values could be stored as new values in struct usb_host_endpoint. usb_change_ep_bandwidth would then call into the xHCI driver to drop the endpoint, re-add it, and then issue a bandwidth change. The xHCI driver would have to be changed to look at the new fields in usb_host_endpoint, and set up the endpoint contexts with the interval and packet size from those fields, instead of the endpoint descriptor. We should probably set the new values in usb_host_endpoint to zero after the driver unbinds from the device. Not sure if they should be reset after the driver switches interfaces. I would have to see the use cases in the driver. Here, it is needed to change the endpoint descriptors with the new value in urb so that xhci takes the correct value? I mean the fix should be made in usbcore and xhci shall remain intact? No, we need to fix both the xHCI driver and the USB core. I have time to work on that but i 'm not sure that i understood. Sure. I would actually suggest you first finish up the patch to issue a configure endpoint if userspace wants to clear a halt, but the endpoint isn't actually halted. Did your most current patch work? I can't remember what the status was. Sarah Sharp -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: issue with uvcvideo and xhci
On Wed, Dec 12, 2012 at 12:47:24PM +0100, Javier Martinez Canillas wrote: Hello, We have an issue when trying to use USB cameras on a particular machine using the latest mainline Linux 3.7 kernel. This is not a regression since the same issue is present with older kernels (i.e: 3.5). The cameras work fine when plugged to an USB2.0 port (using the EHCI HCD host controller driver) but they don't when using the USB3.0 port (using the xHCI HCD host controller driver). The machine's USB3.0 host controller is a NEC Corporation uPD720200 USB 3.0 Host Controller (rev 04). When enabling trace on the uvcvideo driver I see that most frames are lost: Dec 12 11:07:58 thinclient kernel: [ 4965.597637] uvcvideo: USB isochronous frame lost (-18). Dec 12 11:07:58 thinclient kernel: [ 4965.597642] uvcvideo: USB isochronous frame lost (-18). Dec 12 11:07:58 thinclient kernel: [ 4965.597647] uvcvideo: Marking buffer as bad (error bit set). Dec 12 11:07:58 thinclient kernel: [ 4965.597651] uvcvideo: Frame complete (EOF found). Dec 12 11:07:58 thinclient kernel: [ 4965.597655] uvcvideo: EOF in empty payload. Dec 12 11:07:58 thinclient kernel: [ 4965.597661] uvcvideo: Dropping payload (out of sync). Dec 12 11:07:58 thinclient kernel: [ 4965.813294] uvcvideo: frame 486 stats: 0/2/8 packets, 0/0/8 pts The uvcvideo checks if urb-iso_frame_desc[i].status 0 on the uvc_video_decode_isoc() function (drivers/media/usb/uvc/uvc_video.c). I checked on the xhci driver and the only place where this error code (-EXDEV) is assigned to frame-status is inside the skip_isoc_td() function (drivers/usb/host/xhci-ring.c). At this point I'm not sure if this is a bug on the xhci driver, another quirk needed by the XHCI_NEC_HOST, a camera misconfiguration on the USB Video Class driver or a firmware/hardware bug. It's a known performance issue, although it's not clear whether it's on the xHCI driver side or the host controller side. When an interface setting is enabled where the isochronous endpoint requires two additional transfers per service interval, the NEC host controller starts reporting many missed service intervals. The xHCI driver then finds all the frame buffers that were skipped and marks them with the -EXDEV status. An error status of Missed Service Interval means the host controller could not access the transfer memory fast enough through the PCI bus to service the endpoint in time. It could be a host hardware issue, or it could be software slowing down the system to a crawl. I lean towards a software issue since, as you said, the Windows driver works fine. (Although who knows what NEC quirks the Windows driver is working around...) The NEC xHCI host controller is a 0.96 revision, which doesn't support the Block Event Interrupt (BEI) flag which cuts down on the number of interrupts per URB submitted. So the xHCI driver's interrupt routine gets called on every single service interval, rather than being called once per URB. Since the Linux xHCI driver isn't really optimized for performance yet, the interrupt handler is probably pretty slow and could cause delays in submitting future URBs. The high amount of interrupts is probably causing other systems to be starved, possibly leading to the xHCI host controller not being able to access memory fast enough to service the endpoint. The cameras are reported to work on the same machine but using another operating system (Windows). Windows probably uses Event Data TRBs to cut the interrupts down to one per URB. It would take a major effort to make the xHCI driver use Event Data TRBs. I was wondering if you can give me some pointers on how to be sure what's the issue or if this rings any bells to you. I don't have time to work on performance issues right now, as I have several other critical bugs (mostly around failed S3/S4). However, if you want to try to fix this issue yourself, I suggest you run perf and see where the bottle necks in the xHCI interrupt handler are. I suspect that part of it is that the interrupt handler reads the xHCI status register. That PCI register read is pretty costly, and it's not necessary since 99% of the time the host controller is going to report an OK status. And there's no guarantee that when the host does have an error that it will set a bad status. But without an analysis by perf, we won't really know where the bottlenecks are. Sarah Sharp -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: USB mini-summit at LinuxCon Vancouver
On Tue, Aug 09, 2011 at 09:52:44AM +0200, Hans de Goede wrote: Hi, On 08/08/2011 07:58 PM, Sarah Sharp wrote: On Fri, Aug 05, 2011 at 09:45:56AM +0200, Hans de Goede wrote: Hi, On 08/05/2011 12:56 AM, Greg KH wrote: On Thu, Aug 04, 2011 at 07:21:47PM -0300, Mauro Carvalho Chehab wrote: I think it is important to separate oranges from apples here, there are at least 3 different problem classes which all seem to have gotten thrown onto a pile here: 1) The reason Mauro suggested having some discussion on this at the USB summit is because of a discussion about dual mode cameras on the linux media list. ... 3) Re-direction of usb devices to virtual machines. This works by using the userspace usbfs interface from qemu / vmware / virtualbox / whatever. The basics of this work fine, but it lacks proper locking / safeguards for when a vm takes over a usb device from the in kernel driver. Hi Hans and Mauro, We have do room in the schedule for the USB mini-summit for this discussion, since the schedule is still pretty flexible. The preliminary schedule is up here: http://userweb.kernel.org/~sarah/linuxcon-usb-minisummit.html I think it's best to discuss the VM redirection in the afternoon when some of the KVM folks join us after Hans' talk on USB redirection over the network. That seems best to me too. I'm available at both proposed time slots, and I would like to join both discussions. It sounds like we need a separate topic for the dual mode cameras and TV tuners. Mauro, do you want to lead that discussion in the early morning (in a 9:30 to 10:30 slot) or in the late afternoon (in a 15:30 to 16:30 slot)? I want to be sure we have all the video/media developers who are interested in this topic present, and I don't know if they will be going to the KVM forum. I would really like to see the dual mode camera and TV tuner discussion separated. They are 2 different issues AFAIK. Ok, great, I've put both topics in the morning, in separate slots. Sarah Sharp -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: USB mini-summit at LinuxCon Vancouver
On Fri, Aug 05, 2011 at 09:45:56AM +0200, Hans de Goede wrote: Hi, On 08/05/2011 12:56 AM, Greg KH wrote: On Thu, Aug 04, 2011 at 07:21:47PM -0300, Mauro Carvalho Chehab wrote: I think it is important to separate oranges from apples here, there are at least 3 different problem classes which all seem to have gotten thrown onto a pile here: 1) The reason Mauro suggested having some discussion on this at the USB summit is because of a discussion about dual mode cameras on the linux media list. ... 3) Re-direction of usb devices to virtual machines. This works by using the userspace usbfs interface from qemu / vmware / virtualbox / whatever. The basics of this work fine, but it lacks proper locking / safeguards for when a vm takes over a usb device from the in kernel driver. Hi Hans and Mauro, We have do room in the schedule for the USB mini-summit for this discussion, since the schedule is still pretty flexible. The preliminary schedule is up here: http://userweb.kernel.org/~sarah/linuxcon-usb-minisummit.html I think it's best to discuss the VM redirection in the afternoon when some of the KVM folks join us after Hans' talk on USB redirection over the network. It sounds like we need a separate topic for the dual mode cameras and TV tuners. Mauro, do you want to lead that discussion in the early morning (in a 9:30 to 10:30 slot) or in the late afternoon (in a 15:30 to 16:30 slot)? I want to be sure we have all the video/media developers who are interested in this topic present, and I don't know if they will be going to the KVM forum. Link PM was originally slated for the 10am slot, but since we're missing several people for that discussion (Alan Stern, Andiry Xu, and any of the Intel folks who did the Moorestown USB 2.0 LPM), I think it will be more useful to have that discussion on-list or at another conference. Sarah Sharp -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: USB mini-summit at LinuxCon Vancouver
On Mon, Aug 08, 2011 at 01:23:56PM -0500, Theodore Kilgore wrote: On Mon, 8 Aug 2011, Sarah Sharp wrote: On Fri, Aug 05, 2011 at 09:45:56AM +0200, Hans de Goede wrote: Hi, On 08/05/2011 12:56 AM, Greg KH wrote: On Thu, Aug 04, 2011 at 07:21:47PM -0300, Mauro Carvalho Chehab wrote: I think it is important to separate oranges from apples here, there are at least 3 different problem classes which all seem to have gotten thrown onto a pile here: 1) The reason Mauro suggested having some discussion on this at the USB summit is because of a discussion about dual mode cameras on the linux media list. ... 3) Re-direction of usb devices to virtual machines. This works by using the userspace usbfs interface from qemu / vmware / virtualbox / whatever. The basics of this work fine, but it lacks proper locking / safeguards for when a vm takes over a usb device from the in kernel driver. Hi Hans and Mauro, We have do room in the schedule for the USB mini-summit for this discussion, since the schedule is still pretty flexible. The preliminary schedule is up here: http://userweb.kernel.org/~sarah/linuxcon-usb-minisummit.html I think it's best to discuss the VM redirection in the afternoon when some of the KVM folks join us after Hans' talk on USB redirection over the network. It sounds like we need a separate topic for the dual mode cameras and TV tuners. Mauro, do you want to lead that discussion in the early morning (in a 9:30 to 10:30 slot) or in the late afternoon (in a 15:30 to 16:30 slot)? I want to be sure we have all the video/media developers who are interested in this topic present, and I don't know if they will be going to the KVM forum. Sarah, Alas. I would suspect that I am one of the people most interested in the topic of dual-mode cameras, since I have worked on supporting them both in libgphoto2 and in the kernel. But I teach in a university for a living, and the first classes of Fall Semester 2011 start on August 17 in Auburn, Alabama. Knowing this, I decided, months ago, that I simply could not attend a conference which starts on August 16 in Vancouver. So, after starting all of the current mailing-list discussion on the topic I will not be at the conference. I can only hope that those who do attend will keep me current about what gets discussed. No worries, I'll be taking notes and post them to the Linux USB and Linux media lists. Sarah Sharp -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 0/2] USB: EHCI: Allow users to override 80% max periodic bandwidth
On Fri, Jun 24, 2011 at 08:48:06PM +0400, Kirill Smelkov wrote: Changes since v1: - dropped RFC status as this seems like the sort of feature somebody might reasonably want to use -- if they know exactly what they're doing; - new preparatory patch (1/2) which moves already-in-there sysfs code into ehci-sysfs.c; - moved uframe_periodic_max parameter from module option to sysfs attribute, so that it can be set per controller and at runtime, added validity checks; - clarified a bit bandwith analysis for 96% max periodic setup as noticed by Alan Stern; - clarified patch description saying that set in stone 80% max periodic is specified by USB 2.0; Have you tested this patch by maxing out this bandwidth on various types of host controllers? It's entirely possible that you'll run into vendor-specific bugs if you try to pack the schedule with isochronous transfers. I don't think any hardware designer would seriously test or validate their hardware with a schedule that is basically a violation of the USB bus spec (more than 80% for periodic transfers). But if Alan is fine with giving users a way to shoot themselves in the foot, and it's disabled by default, then I don't particularly mind this patch. Sarah Sharp -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: uvcvideo failure under xHCI
On Fri, Jun 17, 2011 at 10:18:39AM +0200, Laurent Pinchart wrote: On Thursday 16 June 2011 22:20:22 Alan Stern wrote: On Thu, 16 Jun 2011, Sarah Sharp wrote: On Thu, Jun 16, 2011 at 03:39:11PM -0400, Alan Stern wrote: That's appropriate. But nobody should ever set an isochronous URB's status field to -EPROTO, no matter whether the device is connected or not and no matter whether the host controller is alive or not. But the individual frame status be set to -EPROTO, correct? That's what Alex was told to do when an isochronous TD had a completion code of Incompatible Device Error. Right. -EPROTO is a perfectly reasonable code for a frame's status. But not for an isochronous URB's status. There's no reason for uvcvideo to test for it. The uvcvideo driver tests for -EPROTO for interrupt URBs only. For isochronous URBs it tests for -ENOENT, -ECONNRESET and -ESHUTDOWN. So is uvc_status_complete() shared between interrupt and isochronous URBs then? Sarah Sharp -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: uvcvideo failure under xHCI
On Fri, Jun 17, 2011 at 07:01:10PM +0200, Laurent Pinchart wrote: Hi Sarah, On Friday 17 June 2011 18:46:20 Sarah Sharp wrote: On Fri, Jun 17, 2011 at 10:18:39AM +0200, Laurent Pinchart wrote: On Thursday 16 June 2011 22:20:22 Alan Stern wrote: On Thu, 16 Jun 2011, Sarah Sharp wrote: On Thu, Jun 16, 2011 at 03:39:11PM -0400, Alan Stern wrote: That's appropriate. But nobody should ever set an isochronous URB's status field to -EPROTO, no matter whether the device is connected or not and no matter whether the host controller is alive or not. But the individual frame status be set to -EPROTO, correct? That's what Alex was told to do when an isochronous TD had a completion code of Incompatible Device Error. Right. -EPROTO is a perfectly reasonable code for a frame's status. But not for an isochronous URB's status. There's no reason for uvcvideo to test for it. The uvcvideo driver tests for -EPROTO for interrupt URBs only. For isochronous URBs it tests for -ENOENT, -ECONNRESET and -ESHUTDOWN. So is uvc_status_complete() shared between interrupt and isochronous URBs then? No, uvc_status_complete() handles status URBs (interrupt only), and uvc_video_complete() handles video URBs (isochronous or bulk, depending on the device). Huh, that's very odd then. I could have sworn I was getting missed service interval events (which are only for isochronous transfers) and then seeing the Non-zero message. And the userspace video definitely froze before my patch and did not freeze after the patch was applied. I'll have to look more closely at the logs. Sarah Sharp -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: uvcvideo failure under xHCI
On Thu, Jun 16, 2011 at 10:35:49AM -0400, Alan Stern wrote: On Thu, 16 Jun 2011, Laurent Pinchart wrote: On Thursday 16 June 2011 04:59:57 Sarah Sharp wrote: On Wed, Jun 15, 2011 at 06:39:57PM -0700, Sarah Sharp wrote: I've grepped through drivers/media/video, and it seems like none of the drivers handle the -EXDEV status. What should the xHCI driver be setting the URB's status and frame status to when the xHCI host controller skips over transfers? -EREMOTEIO? Or does it need to set the URB's status to zero, but only set the individual frame status to -EXDEV? Ok, looking at both EHCI and UHCI, they seem to set the urb-status to zero, regardless of what they set the frame descriptor field to. Alan, does that seem correct? The description of the behavior of ehci-hcd and uhci-hcd is correct. ohci-hcd behaves the same way too. And they all agree with the behavior described in the kerneldoc for struct urb in include/linux/usb.h. Ah, you mean this bit? * @status: This is read in non-iso completion functions to get the * status of the particular request. ISO requests only use it * to tell whether the URB was unlinked; detailed status for * each frame is in the fields of the iso_frame-desc. According to Documentation/usb/error-codes.txt, host controller drivers should set the status to -EXDEV. However, no device drivers seem to handle that, probably because the EHCI/UHCI drivers don't use that error code. Drivers are clearly out of sync with the documentation, so we should fix one of them. Under the circumstances, the documentation file should be changed. Sarah, can you do that along with the change to xhci-hcd? Sure. It feels like there should be a note about which values isochronous URBs might have in the urb-status field. The USB core is the only one that would be setting those, so which values would it set? uvcvideo tests for these error codes: case -ENOENT: /* usb_kill_urb() called. */ case -ECONNRESET: /* usb_unlink_urb() called. */ case -ESHUTDOWN:/* The endpoint is being disabled. */ case -EPROTO: /* Device is disconnected (reported by some * host controller). */ Are there any others. Sarah Sharp -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: uvcvideo failure under xHCI
On Thu, Jun 16, 2011 at 01:39:43PM -0400, Alan Stern wrote: On Thu, 16 Jun 2011, Sarah Sharp wrote: Alan, does that seem correct? The description of the behavior of ehci-hcd and uhci-hcd is correct. ohci-hcd behaves the same way too. And they all agree with the behavior described in the kerneldoc for struct urb in include/linux/usb.h. Ah, you mean this bit? * @status: This is read in non-iso completion functions to get the * status of the particular request. ISO requests only use it * to tell whether the URB was unlinked; detailed status for * each frame is in the fields of the iso_frame-desc. Right. There's also some more near the end: * Completion Callbacks: * * The completion callback is made in_interrupt(), and one of the first * things that a completion handler should do is check the status field. * The status field is provided for all URBs. It is used to report * unlinked URBs, and status for all non-ISO transfers. It should not * be examined before the URB is returned to the completion handler. Under the circumstances, the documentation file should be changed. Sarah, can you do that along with the change to xhci-hcd? Sure. It feels like there should be a note about which values isochronous URBs might have in the urb-status field. The USB core is the only one that would be setting those, so which values would it set? uvcvideo tests for these error codes: case -ENOENT: /* usb_kill_urb() called. */ case -ECONNRESET: /* usb_unlink_urb() called. */ case -ESHUTDOWN:/* The endpoint is being disabled. */ case -EPROTO: /* Device is disconnected (reported by some * host controller). */ Are there any others. -EREMOTEIO, in the unlikely event that URB_SHORT_NOT_OK is set, but no others. Are you saying that the USB core will only set -EREMOTEIO for isochronous URBs? Or do you mean that in addition to the status values that uvcvideo checks, the USB core can also set -EREMOTEIO? And I wasn't aware of that last one... Host controller drivers should report -ESHUTDOWN to mean the device has been disconnected, not -EPROTO. But usually HCD don't take these events into account when determining URB status codes. The xHCI driver will return -ESHUTDOWN as a status for URBs when the host controller is dying. Sarah Sharp -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: uvcvideo failure under xHCI
On Thu, Jun 16, 2011 at 03:39:11PM -0400, Alan Stern wrote: That's appropriate. But nobody should ever set an isochronous URB's status field to -EPROTO, no matter whether the device is connected or not and no matter whether the host controller is alive or not. But the individual frame status be set to -EPROTO, correct? That's what Alex was told to do when an isochronous TD had a completion code of Incompatible Device Error. Sarah Sharp -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
uvcvideo failure under xHCI
Hi Laurent, I think this issue has been happening for a while now, but my recent patches to remove most of the xHCI debugging have finally allowed me to use a webcam under xHCI with debugging on. Unfortunately, it doesn't work very well. When I plug in a webcam under an xHCI host controller in 3.0-rc3+ (basically top of Greg's usb-linus branch) with xHCI debugging turned on, the host controller occasionally cannot keep up with the isochronous transfers, and it tells the xHCI driver that it had to skip several microframes of transfers. These Missed Service Intervals aren't supposed to be fatal errors, just an indication that something was hogging the PCI memory bandwidth. The xHCI driver then sets the URB's status to -EXDEV, to indicate that some of the iso_frame_desc transferred, and sets at least one frame's status to -EXDEV: static int skip_isoc_td(struct xhci_hcd *xhci, struct xhci_td *td, struct xhci_transfer_event *event, struct xhci_virt_ep *ep, int *status) { struct xhci_ring *ep_ring; struct urb_priv *urb_priv; struct usb_iso_packet_descriptor *frame; int idx; ep_ring = xhci_dma_to_transfer_ring(ep, le64_to_cpu(event-buffer)); urb_priv = td-urb-hcpriv; idx = urb_priv-td_cnt; frame = td-urb-iso_frame_desc[idx]; /* The transfer is partly done */ *status = -EXDEV; frame-status = -EXDEV; /* calc actual length */ frame-actual_length = 0; /* Update ring dequeue pointer */ while (ep_ring-dequeue != td-last_trb) inc_deq(xhci, ep_ring, false); inc_deq(xhci, ep_ring, false); return finish_td(xhci, td, NULL, event, ep, status, true); } The urb-status causes uvcvideo code in uvc_status.c:uvc_status_complete() to fail with the message: Jun 15 17:37:11 talon kernel: [ 117.987769] uvcvideo: Non-zero status (-18) in video completion handler. It doesn't resubmit the isochronous URB in that case, and the userspace video freezes. If I restart the application, the video comes back until the next Missed Service Interval event from the xHCI driver. Ideally, the video driver would just resubmit the URB, and the xHCI host controller would complete transfers as best it can. I think the frames with -EXDEV status should be treated like short transfers. I've grepped through drivers/media/video, and it seems like none of the drivers handle the -EXDEV status. What should the xHCI driver be setting the URB's status and frame status to when the xHCI host controller skips over transfers? -EREMOTEIO? Or does it need to set the URB's status to zero, but only set the individual frame status to -EXDEV? Sarah Sharp -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: uvcvideo failure under xHCI
On Wed, Jun 15, 2011 at 06:39:57PM -0700, Sarah Sharp wrote: When I plug in a webcam under an xHCI host controller in 3.0-rc3+ (basically top of Greg's usb-linus branch) with xHCI debugging turned on, the host controller occasionally cannot keep up with the isochronous transfers, and it tells the xHCI driver that it had to skip several microframes of transfers. These Missed Service Intervals aren't supposed to be fatal errors, just an indication that something was hogging the PCI memory bandwidth. The xHCI driver then sets the URB's status to -EXDEV, to indicate that some of the iso_frame_desc transferred, and sets at least one frame's status to -EXDEV: ... The urb-status causes uvcvideo code in uvc_status.c:uvc_status_complete() to fail with the message: Jun 15 17:37:11 talon kernel: [ 117.987769] uvcvideo: Non-zero status (-18) in video completion handler. ... I've grepped through drivers/media/video, and it seems like none of the drivers handle the -EXDEV status. What should the xHCI driver be setting the URB's status and frame status to when the xHCI host controller skips over transfers? -EREMOTEIO? Or does it need to set the URB's status to zero, but only set the individual frame status to -EXDEV? Ok, looking at both EHCI and UHCI, they seem to set the urb-status to zero, regardless of what they set the frame descriptor field to. Alan, does that seem correct? I've created a patch to do the same thing in the xHCI driver, and I seem to be getting consistent video with xHCI debugging turned on, despite lots of Missed Service Interval events. Sarah Sharp -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: USB mini-summit at LinuxCon Vancouver
On Thu, Jun 09, 2011 at 08:18:05PM -0700, Greg KH wrote: On Thu, Jun 09, 2011 at 05:21:03PM -0700, Sarah Sharp wrote: Topic 1 --- The KVM folks suggested that it would be good to get USB and virtualization developers together to talk about how to virtualize the xHCI host controller. The xHCI spec architect worked closely with VMWare to get some extra goodies in the spec to help virtualization, and I'd like to see the other virtualization developers take advantage of that. I'd also like us to hash out any issues they have been finding in the USB core or xHCI driver during the virtualization effort. Do people really want to virtualize the whole xHCI controller, or just specific ports or devices to the guest operating system? A host OS could chose to virtualize the whole xHCI controller if it wanted to. That's part of the reason why xHCI does all the bandwidth checking in hardware, not in software. If just specific ports, would something like usbip be better for virtual machines, with the USB traffic going over the network connection between the guest/host? It could be done that way too. But that doesn't help if you're trying to run Windows under Linux, right? Only if all the guest OSes use the same USB IP protocol then it would work. Hope to see you there! Thanks for putting this together. Do we need to sign up somewhere to give the organizers a sense of how many people will be attending? I private ack by email would be great. Or you can ack by facebook: https://www.facebook.com/event.php?eid=229532200405657 I could add the event in upcoming.yahoo.com if anyone uses that. Sarah Sharp -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
USB mini-summit at LinuxCon Vancouver
I'm pleased to announce a USB mini-summit at LinuxCon Vancouver. What: USB mini-summit When: Tuesday, August 16th, all day Where: At the conference venue, room TBD pending confirmation from Angela Brown. Proposed topics include USB virtualization, and improved bandwidth APIs between the USB core and drivers (especially webcam drivers). See the detailed topic list below. Anyone is also welcome to propose or show up with a USB related topic. MUSB? USB 3.0 gadget drivers? USB-IP? The USB mini-summit does overlap with the virtualization mini-summit by a day, but I'm hoping we can schedule talks so some of the virtualization folks can make it to the USB mini-summit. The other option was on Friday during the conference which was not ideal. Proposed topics: Topic 1 --- The KVM folks suggested that it would be good to get USB and virtualization developers together to talk about how to virtualize the xHCI host controller. The xHCI spec architect worked closely with VMWare to get some extra goodies in the spec to help virtualization, and I'd like to see the other virtualization developers take advantage of that. I'd also like us to hash out any issues they have been finding in the USB core or xHCI driver during the virtualization effort. Topic 2 --- I'd also like to get the V4L and audio developers who work with USB devices together with the core USB folks to talk about bandwidth management under xHCI. One of the issues is that since the xHCI hardware does bandwidth management, not the xHCI driver, a schedule that will take too much bandwidth will get rejected much sooner than any USB driver currently expects (during a call to usb_set_interface). This poses issues, since most USB video drivers negotiate the video size and frame rate after they call usb_set_interface, so they don't know whether they can fall back to a less bandwidth-intensive setting. Currently, they just submit URBs with less and less bandwidth until one interval setting gets accepted that won't work under xHCI. A second issue is that that some drivers need less bandwidth than the device advertises, and the xHCI driver currently uses whatever periodic interval the device advertises in its descriptors. This is not what the video/audio driver wants, especially in the case of buggy high speed devices that advertise the interval in frames, not microframes. There needs to be some way for the drivers to communicate their bandwidth needs to the USB core. We've known about this issue for a while, and I think it's time to get everyone in the same room and hash out an API. (I will send out an API proposal later this month.) Hope to see you there! Sarah Sharp -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: Status of the patches under review (85 patches) and some misc notes about the devel procedures
On Sat, May 08, 2010 at 03:45:41PM -0700, Mauro Carvalho Chehab wrote: Jean-Francois Moine wrote: On Fri, 7 May 2010 09:39:16 -0300 Mauro Carvalho Chehab mche...@redhat.com wrote: == Gspca patches - Waiting Jean-Francois Moine moin...@free.fr submission/review == Feb,24 2010: gspca pac7302: add USB PID range based on heuristics http://patchwork.kernel.org/patch/81612 May, 3 2010: gspca: Try a less bandwidth-intensive alt setting. http://patchwork.kernel.org/patch/96514 Hello Mauro, I don't think the patch about pac7302 should be applied. The patch about the gspca main is in my last git pull request. (c/c Sarah) I also didn't like this patch strategy. It seems a sort of workaround for xHCI, instead of a proper fix. I'll mark this patch as rejected, and wait for a proper fix. This isn't a work around for a bug in the xHCI host. The bandwidth checking is a feature. It allows the host to reject a new interface if other devices are already taking up too much of the bus bandwidth. I expect that all drivers that use interrupt or isochronous will have to fall back to a different alternate interface setting if they can. Now, Alan Stern and I have been talking about making a different API for drivers to request a specific polling rate and frame list length for an endpoint. However, I expect that the call would have to be either before or part of the call to usb_set_interface(), because of how the xHCI hardware tracks endpoints. So even if we had the ideal interface, the drivers would still need code like this to fall back to less-bandwidth intensive alternate settings. Is there a different way you think we should handle running out of bandwidth? Sarah Sharp -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] gspca: Try a less bandwidth-intensive alt setting.
Under OHCI, UHCI, and EHCI, if an alternate interface setting took too much of the bus bandwidth, then submit_urb() would fail. The xHCI host controller does bandwidth checking when the alternate interface setting is installed, so usb_set_interface() can fail. If it does, try the next alternate interface setting. Signed-off-by: Sarah Sharp sarah.a.sh...@linux.intel.com Tested-by: Andiry Xu andiry...@amd.com --- drivers/media/video/gspca/gspca.c | 10 ++ 1 files changed, 6 insertions(+), 4 deletions(-) diff --git a/drivers/media/video/gspca/gspca.c b/drivers/media/video/gspca/gspca.c index 222af47..6de3117 100644 --- a/drivers/media/video/gspca/gspca.c +++ b/drivers/media/video/gspca/gspca.c @@ -643,6 +643,7 @@ static struct usb_host_endpoint *get_ep(struct gspca_dev *gspca_dev) xfer = gspca_dev-cam.bulk ? USB_ENDPOINT_XFER_BULK : USB_ENDPOINT_XFER_ISOC; i = gspca_dev-alt; /* previous alt setting */ +find_alt: if (gspca_dev-cam.reverse_alts) { while (++i gspca_dev-nbalt) { ep = alt_xfer(intf-altsetting[i], xfer); @@ -666,10 +667,11 @@ static struct usb_host_endpoint *get_ep(struct gspca_dev *gspca_dev) if (gspca_dev-nbalt 1) { gspca_input_destroy_urb(gspca_dev); ret = usb_set_interface(gspca_dev-dev, gspca_dev-iface, i); - if (ret 0) { - err(set alt %d err %d, i, ret); - ep = NULL; - } + /* xHCI hosts will reject set interface requests +* if they take too much bandwidth, so try again. +*/ + if (ret 0) + goto find_alt; gspca_input_create_urb(gspca_dev); } return ep; -- 1.6.3.3 -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
xHCI bandwidth error with USB webcam
I've been trying out the patches to enable isochronous transfers under xHCI, and they work fine on my USB speaker. However, I've been having trouble getting my high speed USB webcam to work. The NEC Express Card I have rejects the first alternate setting that the uvcvideo driver tries to install (altsetting 11), saying that it takes too much bandwidth. This happens even when I plug the device directly into the roothub with no other devices plugged in. I would like to know if this is correct behavior for the host, as I can't believe a device would advertise an alternate setting that took up too much bandwidth by itself. Device descriptors and a log snippet are attached. The other problem is that uvcvideo then gives up on the device when installing the alt setting fails, rather than trying the next less resource-intensive alternate setting. The past, submit_urb() might fail if there wasn't enough bandwidth for the isochronous transfers, but under an xHCI host controller, it will fail sooner, when usb_set_interface() is called. That needs to be fixed in all the USB video drivers. I figured out how to patch the gspca driver, but not uvcvideo. The patch looks a bit hackish; can with experience with that driver look it over? Can anyone tell me where to look for the usb_set_interface() in uvcvideo? Sarah Sharp 8-- From 0e6bc81b178364ee9771c64a06ab006588c73ae6 Mon Sep 17 00:00:00 2001 From: Sarah Sharp sarah.a.sh...@linux.intel.com Date: Mon, 12 Apr 2010 11:23:46 -0700 Subject: [PATCH] gspca: Try a less bandwidth-intensive alt setting. Under OHCI, UHCI, and EHCI, if an alternate interface setting took too much of the bus bandwidth, then submit_urb() would fail. The xHCI host controller does bandwidth checking when the alternate interface setting is installed, so usb_set_interface() can fail. If it does, try the next alternate interface setting. Signed-off-by: Sarah Sharp sarah.a.sh...@linux.intel.com --- drivers/media/video/gspca/gspca.c | 10 ++ 1 files changed, 6 insertions(+), 4 deletions(-) diff --git a/drivers/media/video/gspca/gspca.c b/drivers/media/video/gspca/gspca.c index 222af47..6de3117 100644 --- a/drivers/media/video/gspca/gspca.c +++ b/drivers/media/video/gspca/gspca.c @@ -643,6 +643,7 @@ static struct usb_host_endpoint *get_ep(struct gspca_dev *gspca_dev) xfer = gspca_dev-cam.bulk ? USB_ENDPOINT_XFER_BULK : USB_ENDPOINT_XFER_ISOC; i = gspca_dev-alt; /* previous alt setting */ +find_alt: if (gspca_dev-cam.reverse_alts) { while (++i gspca_dev-nbalt) { ep = alt_xfer(intf-altsetting[i], xfer); @@ -666,10 +667,11 @@ static struct usb_host_endpoint *get_ep(struct gspca_dev *gspca_dev) if (gspca_dev-nbalt 1) { gspca_input_destroy_urb(gspca_dev); ret = usb_set_interface(gspca_dev-dev, gspca_dev-iface, i); - if (ret 0) { - err(set alt %d err %d, i, ret); - ep = NULL; - } + /* xHCI hosts will reject set interface requests +* if they take too much bandwidth, so try again. +*/ + if (ret 0) + goto find_alt; gspca_input_create_urb(gspca_dev); } return ep; -- 1.6.3.3 Bus 009 Device 002: ID 046d:09a5 Logitech, Inc. Device Descriptor: bLength18 bDescriptorType 1 bcdUSB 2.00 bDeviceClass 239 Miscellaneous Device bDeviceSubClass 2 ? bDeviceProtocol 1 Interface Association bMaxPacketSize064 idVendor 0x046d Logitech, Inc. idProduct 0x09a5 bcdDevice0.06 iManufacturer 0 iProduct0 iSerial 2 0A539160 bNumConfigurations 1 Configuration Descriptor: bLength 9 bDescriptorType 2 wTotalLength 1183 bNumInterfaces 4 bConfigurationValue 1 iConfiguration 0 bmAttributes 0x80 (Bus Powered) MaxPower 500mA Interface Association: bLength 8 bDescriptorType11 bFirstInterface 0 bInterfaceCount 2 bFunctionClass 14 Video bFunctionSubClass 3 Video Interface Collection bFunctionProtocol 0 iFunction 0 Interface Descriptor: bLength 9 bDescriptorType 4 bInterfaceNumber0 bAlternateSetting 0 bNumEndpoints 1 bInterfaceClass14 Video bInterfaceSubClass 1 Video Control bInterfaceProtocol 0 iInterface 0 VideoControl Interface Descriptor: bLength13 bDescriptorType