At Thu, 17 Feb 2011 12:23:43 -0800, Sarah Sharp wrote: > > On Thu, Feb 17, 2011 at 04:53:41PM +0100, Takashi Iwai wrote: > > At Thu, 17 Feb 2011 16:43:28 +0100, > > Takashi Iwai wrote: > > > > > > [OK, moved now to linux-usb ML...] > > > > > > At Fri, 11 Feb 2011 10:44:57 -0800, > > > Sarah Sharp wrote: > > > > > > > > On Fri, Feb 11, 2011 at 11:11:29AM +0100, Takashi Iwai wrote: > > > > > Actually the audio on xhci seems broken, so far. > > > > > Recently I tested usb audio devices with xhci on new HP laptops, and > > > > > got several different errors per kernel versions. > > > > > > > > I would be interested if you could send those errors to the linux-usb > > > > mailing list and Cc me. > > > > > > > > > I had no time to dig into the issue yet, but if it's related with > > > > > bandwidth setup, we'd need definitely to work on it together. > > > > > > > > The HP laptops have an NEC host controller in them, and I have heard > > > > some complaints about their isoc support. They do have a firmware update > > > > that improves that support, so you may want to google around for that. > > > > > > > > The other issue is the xHCI driver design. Currently, with 0.96 xHCI > > > > hardware, we get multiple interrupts per isoc URB (as many interrupts as > > > > there are isoc buffers). I have a work around in mind, but it involves > > > > updating the xHCI ring code, which is always somewhat hairy and may take > > > > a month or so. However, for xHCI 1.0 host controllers, the spec > > > > architects have made some improvements that will easily allow us to get > > > > less interrupts (once per URB, or even less if the driver submits > > > > several URBs and marks some of them with the URB_NO_INTERRUPT flag). > > > > That only involves setting one bit, so that would be an easy one day > > > > fix. > > > > > > > > Then there's the fact that the xHCI driver's ring code is recursive. :( > > > > I wrote it at 3 in the morning a couple days before a demo, and I really > > > > need to fix that ASAP. There's also still performance issues, like > > > > register reads in the interrupt path that really should be removed. > > > > > > > > But we should really pull in the linux-usb mailing list into any further > > > > discussion of this. :) > > > > > > Looking down more at the errors, it seems that the controller stalls > > > after some point. First, "short transfer on control ep" comes up once > > > or twice: > > > > > > [ 1391.490134] xhci_hcd 0000:27:00.0: WARN: short transfer on control ep > > > [ 1391.479142] xhci_hcd 0000:27:00.0: WARN: short transfer on control ep > > > > > > Then it starts "Stalled endpoint": > > > > > > [ 1391.498122] xhci_hcd 0000:27:00.0: WARN: Stalled endpoint > > > [ 1391.505114] xhci_hcd 0000:27:00.0: WARN: Stalled endpoint > > > [ 1391.512114] xhci_hcd 0000:27:00.0: WARN: Stalled endpoint > > > ... > > > > > > At each time a stall happens, 3 trbs are queued up and pending. So, > > > after repeating this cycle, it finally reaches to "no room": > > > > > > [ 1401.120101] xhci_hcd 0000:27:00.0: ERROR no room on ep ring > > > > > > The behavior looks consistent no matter which kernel version is used > > > between 2.6.32 and 2.6.38-rc5. So, the problem is before isoc things, > > > but in other points. > > > > > > FWIW, the ring causing this error is no ctrl but slot 1 ep 0. > > > > > > I tested with two USB audio 1.x devices, and both show the similar > > > behavior. > > > > > > Can this be a better hint for further looking? > > Maybe. I've got a couple patches in my queue that I need to send to > Greg that involve fixes in the ring TRB counting calculation. Can you > reset against my for-usb-linus-pending branch in the xhci.git tree on > kernel.org and see if those patches fix your issue? I'm off today, so will check it on the next Monday (the machine is in office). Or, Oliver can take a look at it quickly? > > BTW, during checking this problem, I made up a patch like below > > (for understanding the pending trbs). > > Yes, I've been meaning to rip out that code and replace it with > something simpler like your patch. > > > Doesn't the handling of stalled trbs change the number of queued > > items, or the recalculation of num_enqueued field below is needed? > > The problem with stalls is that the xHCI driver issues a command to move > the dequeue pointer past the stalled transfer, which only completes > later. At the point the command completes, we really have no idea how > far the dequeue pointer moved, so the dequeue pointer doesn't get > updated. I suppose we could look at the command on the command ring and > see where we moved the dequeue pointer. Thanks, that was a vague part in the code when I read it. > I think the dequeue pointer will get moved on the next completed > transfer. But if there's several stalled transfers, and no transfer > gets through, then we can't enqueue any more transfers, and the ring is > basically useless. I'll look and see if I can come up with a fix. > > But why is the driver crashing if we tell it we can't enqueue any more > transfers? It should be able to deal with that case. It doesn't crash. But it just spews errors and doesn't perform correctly. Sometimes the sound driver registration failed, sometimes gives errors during operations. thanks, Takashi -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
