Re: How should we handle isochronous underruns?
On Sun, Jul 21, 2013 at 11:28 PM, Alan Stern st...@rowland.harvard.edu wrote: On Sun, 21 Jul 2013, Ming Lei wrote: On Sun, Jul 21, 2013 at 2:00 AM, Alan Stern st...@rowland.harvard.edu wrote: On Sat, 20 Jul 2013, Ming Lei wrote: No, we don't have to change every driver using isochronous URBs. Many of them set URB_ISO_ASAP on every URB -- in fact, AFAIK snd-usb-audio is the only one that has been fixed not to do this. OK, if you are sure just snd-usb-audio and very less drivers need the change, using the API only is correct way. I haven't checked in detail, but this seems very likely. For ehci example, URB_ISO_ASAP is not checked when isoc queue is empty, so will cause a new schedule when underrun happens. ehci-hcd is indeed one of the drivers that will need to be updated. So will ohci-hcd and uhci-hcd, when they start using tasklets. The new call doesn't turn streaming on or off. The driver does that by submitting URBs or not submitting them. The new call merely separates an old stream from a new one. It tells the HCD that the old stream (if any) is finished, so that any URBs submitted in the future will belong to a new stream. In particular, it tells the HCD that the next URB for this endpoint should be assigned to a time slot that will be visible to the hardware, instead of being assigned to the next time slot after the last packet of the previous URB (which has probably expired already). OK, I got it, looks usb_new_iso_stream(udev, ep) need to be called before starting a new stream. Or after ending an old stream. Either way would work. From view of HCD, the hcd only knows a stream has been started, and doesn't know if it has been stopped. And the most possible usage for hcd is that: if (test_and_clear_bit(ISOC_NEW_STREAM, ep-stream_flag) ; /* new stream */ So looks hcd has to write the flag. Not exactly. The HCD has to keep track of the next available time slot for the endpoint. Some special value, like -1, will indicate that no stream is running yet. So the new function will merely have to set the slot value to -1. I am wondering if -1 is special enough, since in theory -1 might be a valid time slot when HCD uses time slot ANDed with (mod - 1). If we define the API as usb_iso_stream(udev, ep, start), hcd only need to read the flag, so it should be simper to use and implement. It wouldn't be simpler for drivers. And what happens if the HCD gets an URB submitted for an endpoint where the stream_flag is off? If the API is defined as usb_iso_stream(udev, ep, start), drivers are required to call usb_iso_stream(udev, ep, 1) before starting stream, and call usb_iso_stream(udev, ep, 0) after stopping stream. Also with both streaming on and off information, the HCD might improve bandwidth management in the future. I doubt it very much. The USB spec states that bandwidth for periodic endpoints gets allocated whenever a new altsetting is installed, and that is the approach the HCDs will try to take. Looks USB spec doesn't mention part of the bandwidth allocated by set_interface can't be freed. Also Clemens mentioned, there might be devices which exposes non-zero payload sizes in the default interface setting. In a normal system, the URB count won't be very much( 1000), and if you mean the overhead is from memory, I guess it won't cause real memory increase per URB for slab's sake. If you mean performance loss with set/get the single lockless/common variable, maybe we should focus on the big HCD lock, which might be the biggest bottle of USB protocol, :-) If you're talking about the hcd_urb_list_lock, note that it could easily be made hcd-specific. I haven't done that because the regions it protects are all very short, so there probably isn't very much contention. I mean the HCD private lock, and there are heavy contention on the lock. I don't have any ideas how to reduce that contention. Do you? For ehci example, one draft idea I thought about is to split the ehci-lock into three locks: ehci-periodic_lock which covers intr isoc transfer, and ehci-async_lock which covers async schedule, and ehci-lock which manages some global thing but is held with short time. But the idea is far more mature, and might be wrong, and only for discussion because I haven't considered much details already. Also I thought of one improvement on the idea of 'urb-completing': - define one percpu 'struct urb *' variable inside 'struct hcd' to record the completing urb, so HCD can see easily if the urb is being resubmitted inside its completion handler. Even on a UP system, what happens if multiple URBs get completed before the tasklet runs and one of them is resubmitted? On each CPU, for one HCD, one urb-complete() won't be preempted by any other urb-complete() either running in tasklet or hardirq handler, so we can record the completing urb in one HCD percpu variable before
Re: How should we handle isochronous underruns?
On Mon, 22 Jul 2013, Ming Lei wrote: Not exactly. The HCD has to keep track of the next available time slot for the endpoint. Some special value, like -1, will indicate that no stream is running yet. So the new function will merely have to set the slot value to -1. I am wondering if -1 is special enough, since in theory -1 might be a valid time slot when HCD uses time slot ANDed with (mod - 1). On EHCI, valid time slots run from 0 to 8191 (or less). On OHCI they run from 0 to 65535. On UHCI they run from 0 to 1023. If we define the API as usb_iso_stream(udev, ep, start), hcd only need to read the flag, so it should be simper to use and implement. It wouldn't be simpler for drivers. And what happens if the HCD gets an URB submitted for an endpoint where the stream_flag is off? If the API is defined as usb_iso_stream(udev, ep, start), drivers are required to call usb_iso_stream(udev, ep, 1) before starting stream, and call usb_iso_stream(udev, ep, 0) after stopping stream. This just adds complexity with no benefit. Also with both streaming on and off information, the HCD might improve bandwidth management in the future. I doubt it very much. The USB spec states that bandwidth for periodic endpoints gets allocated whenever a new altsetting is installed, and that is the approach the HCDs will try to take. Looks USB spec doesn't mention part of the bandwidth allocated by set_interface can't be freed. It is implicit. If bandwidth gets allocated then drivers can assume it is available. If it were then freed by the HCD, it might not be available when a driver wanted to use it. Also Clemens mentioned, there might be devices which exposes non-zero payload sizes in the default interface setting. No problem. The bandwidth will be allocated when the default interface setting is installed. I mean the HCD private lock, and there are heavy contention on the lock. I don't have any ideas how to reduce that contention. Do you? For ehci example, one draft idea I thought about is to split the ehci-lock into three locks: ehci-periodic_lock which covers intr isoc transfer, and ehci-async_lock which covers async schedule, and ehci-lock which manages some global thing but is held with short time. But the idea is far more mature, and might be wrong, and only for discussion because I haven't considered much details already. Okay. Also I thought of one improvement on the idea of 'urb-completing': - define one percpu 'struct urb *' variable inside 'struct hcd' to record the completing urb, so HCD can see easily if the urb is being resubmitted inside its completion handler. Even on a UP system, what happens if multiple URBs get completed before the tasklet runs and one of them is resubmitted? On each CPU, for one HCD, one urb-complete() won't be preempted by any other urb-complete() either running in tasklet or hardirq handler, so we can record the completing urb in one HCD percpu variable before calling urb-complete() and clear the variable after returning form urb-complete(). Then in submit path, we can find if the URB is scheduled in its own completion handler. That might work. (What if the thread is preempted while the completion handler is running and then continues on a different CPU?) But I think it would still have higher overhead than calling usb_new_iso_stream once, whenever a new stream is started. Alan Stern -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: How should we handle isochronous underruns?
On Tue, 23 Jul 2013, Ming Lei wrote: On Mon, Jul 22, 2013 at 11:09 PM, Alan Stern st...@rowland.harvard.edu wrote: On Mon, 22 Jul 2013, Ming Lei wrote: Not exactly. The HCD has to keep track of the next available time slot for the endpoint. Some special value, like -1, will indicate that no stream is running yet. So the new function will merely have to set the slot value to -1. I am wondering if -1 is special enough, since in theory -1 might be a valid time slot when HCD uses time slot ANDed with (mod - 1). On EHCI, valid time slots run from 0 to 8191 (or less). On OHCI they run from 0 to 65535. On UHCI they run from 0 to 1023. There are also lots of non-standard HCs, and going to be many in future. When one new API is introduced, maybe we can't ignore these HCs, also need consider coming HCs. The authors of those HCDs can implement the new API however they want. If the API is defined as usb_iso_stream(udev, ep, start), drivers are required to call usb_iso_stream(udev, ep, 1) before starting stream, and call usb_iso_stream(udev, ep, 0) after stopping stream. This just adds complexity with no benefit. The symmetrical(start, stop) APIs may be easy to understand and use, and looks no extra complexity introduced. I disagree. (Start) is just as easy to understand as (start,stop). Adding stop calls that do nothing is indeed extra complexity with no benefit. Think about it: What is an HCD going to do when it gets a stop call? One benefit is that we may remove the assumption of -1 being invalid frame/uframe on all HCs, another one is that HCD can know clearly if stream is on or off currently, which might be helpful in the future. This is not an assumption for all HCDs; it is just the way that _my_ HCDs will work. Other HCDs can implement the new API in a different way if the author likes. The approach is only helpful if most of isoc drivers need to use the new API. Considered most of drivers resubmit isoc URBs inside completion handler, we can use this automatic way to decide new stream for these drivers and avoid lots of changes. Have you tried to find the places where an isochronous URB is submitted without URB_ISO_ASAP? Those are the only places that will need to be changed. Alan Stern -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: How should we handle isochronous underruns?
On Mon, Jul 22, 2013 at 11:09 PM, Alan Stern st...@rowland.harvard.edu wrote: On Mon, 22 Jul 2013, Ming Lei wrote: Not exactly. The HCD has to keep track of the next available time slot for the endpoint. Some special value, like -1, will indicate that no stream is running yet. So the new function will merely have to set the slot value to -1. I am wondering if -1 is special enough, since in theory -1 might be a valid time slot when HCD uses time slot ANDed with (mod - 1). On EHCI, valid time slots run from 0 to 8191 (or less). On OHCI they run from 0 to 65535. On UHCI they run from 0 to 1023. There are also lots of non-standard HCs, and going to be many in future. When one new API is introduced, maybe we can't ignore these HCs, also need consider coming HCs. If we define the API as usb_iso_stream(udev, ep, start), hcd only need to read the flag, so it should be simper to use and implement. It wouldn't be simpler for drivers. And what happens if the HCD gets an URB submitted for an endpoint where the stream_flag is off? If the API is defined as usb_iso_stream(udev, ep, start), drivers are required to call usb_iso_stream(udev, ep, 1) before starting stream, and call usb_iso_stream(udev, ep, 0) after stopping stream. This just adds complexity with no benefit. The symmetrical(start, stop) APIs may be easy to understand and use, and looks no extra complexity introduced. One benefit is that we may remove the assumption of -1 being invalid frame/uframe on all HCs, another one is that HCD can know clearly if stream is on or off currently, which might be helpful in the future. Also with both streaming on and off information, the HCD might improve bandwidth management in the future. I doubt it very much. The USB spec states that bandwidth for periodic endpoints gets allocated whenever a new altsetting is installed, and that is the approach the HCDs will try to take. Looks USB spec doesn't mention part of the bandwidth allocated by set_interface can't be freed. It is implicit. If bandwidth gets allocated then drivers can assume it is available. If it were then freed by the HCD, it might not be available when a driver wanted to use it. Also Clemens mentioned, there might be devices which exposes non-zero payload sizes in the default interface setting. No problem. The bandwidth will be allocated when the default interface setting is installed. I mean the HCD private lock, and there are heavy contention on the lock. I don't have any ideas how to reduce that contention. Do you? For ehci example, one draft idea I thought about is to split the ehci-lock into three locks: ehci-periodic_lock which covers intr isoc transfer, and ehci-async_lock which covers async schedule, and ehci-lock which manages some global thing but is held with short time. But the idea is far more mature, and might be wrong, and only for discussion because I haven't considered much details already. Okay. Also I thought of one improvement on the idea of 'urb-completing': - define one percpu 'struct urb *' variable inside 'struct hcd' to record the completing urb, so HCD can see easily if the urb is being resubmitted inside its completion handler. Even on a UP system, what happens if multiple URBs get completed before the tasklet runs and one of them is resubmitted? On each CPU, for one HCD, one urb-complete() won't be preempted by any other urb-complete() either running in tasklet or hardirq handler, so we can record the completing urb in one HCD percpu variable before calling urb-complete() and clear the variable after returning form urb-complete(). Then in submit path, we can find if the URB is scheduled in its own completion handler. That might work. (What if the thread is preempted while the completion handler is running and then continues on a different CPU?) But I think It works under the situation because both interrupt handler and tasklet can't be preempted. If the thread of urb submission is preempted, that means it isn't run in completion handler, so no matter if it is migrated into other CPUs, the approach can work well. it would still have higher overhead than calling usb_new_iso_stream once, whenever a new stream is started. Yes, the approach introduces overhead of two stores to percpu variable in complete() path. But it can't replace the API because it can't work out in case that urb is resubmitted in tasklet or workqueue whch is scheduled in completion handler. The approach is only helpful if most of isoc drivers need to use the new API. Considered most of drivers resubmit isoc URBs inside completion handler, we can use this automatic way to decide new stream for these drivers and avoid lots of changes. Thanks, -- Ming Lei -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at
Re: How should we handle isochronous underruns?
On Sun, Jul 21, 2013 at 2:00 AM, Alan Stern st...@rowland.harvard.edu wrote: On Sat, 20 Jul 2013, Ming Lei wrote: No, we don't have to change every driver using isochronous URBs. Many of them set URB_ISO_ASAP on every URB -- in fact, AFAIK snd-usb-audio is the only one that has been fixed not to do this. OK, if you are sure just snd-usb-audio and very less drivers need the change, using the API only is correct way. I haven't checked in detail, but this seems very likely. For ehci example, URB_ISO_ASAP is not checked when isoc queue is empty, so will cause a new schedule when underrun happens. But never mind. For a new API, how about void usb_new_iso_stream(struct usb_device *, struct usb_host_endpoint *); Does that look okay? Looks you missed one parameter of 'int on'. I don't understand. Why is another parameter needed? So do you only want to set streaming on and not streaming off? If so, that looks a bit weird, could you describe its usage? The new call doesn't turn streaming on or off. The driver does that by submitting URBs or not submitting them. The new call merely separates an old stream from a new one. It tells the HCD that the old stream (if any) is finished, so that any URBs submitted in the future will belong to a new stream. In particular, it tells the HCD that the next URB for this endpoint should be assigned to a time slot that will be visible to the hardware, instead of being assigned to the next time slot after the last packet of the previous URB (which has probably expired already). OK, I got it, looks usb_new_iso_stream(udev, ep) need to be called before starting a new stream. From view of HCD, the hcd only knows a stream has been started, and doesn't know if it has been stopped. And the most possible usage for hcd is that: if (test_and_clear_bit(ISOC_NEW_STREAM, ep-stream_flag) ; /* new stream */ So looks hcd has to write the flag. If we define the API as usb_iso_stream(udev, ep, start), hcd only need to read the flag, so it should be simper to use and implement. Also with both streaming on and off information, the HCD might improve bandwidth management in the future. In short, it would simply tell the HCD to act as though the next URB has URB_ISO_ASAP set. For HCDs that act as though URB_ISO_ASAP was always set, the new call wouldn't do anything -- they would not need to implement it. (with extra overhead for every URB!) to fix a problem that ought to be In a normal system, the URB count won't be very much( 1000), and if you mean the overhead is from memory, I guess it won't cause real memory increase per URB for slab's sake. If you mean performance loss with set/get the single lockless/common variable, maybe we should focus on the big HCD lock, which might be the biggest bottle of USB protocol, :-) If you're talking about the hcd_urb_list_lock, note that it could easily be made hcd-specific. I haven't done that because the regions it protects are all very short, so there probably isn't very much contention. I mean the HCD private lock, and there are heavy contention on the lock. Also I thought of one improvement on the idea of 'urb-completing': - define one percpu 'struct urb *' variable inside 'struct hcd' to record the completing urb, so HCD can see easily if the urb is being resubmitted inside its completion handler. Anyway this approach is only helpful if much lots of drivers need changes for underrun case. Thanks, -- Ming Lei -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: How should we handle isochronous underruns?
On Sun, 21 Jul 2013, Ming Lei wrote: On Sun, Jul 21, 2013 at 2:00 AM, Alan Stern st...@rowland.harvard.edu wrote: On Sat, 20 Jul 2013, Ming Lei wrote: No, we don't have to change every driver using isochronous URBs. Many of them set URB_ISO_ASAP on every URB -- in fact, AFAIK snd-usb-audio is the only one that has been fixed not to do this. OK, if you are sure just snd-usb-audio and very less drivers need the change, using the API only is correct way. I haven't checked in detail, but this seems very likely. For ehci example, URB_ISO_ASAP is not checked when isoc queue is empty, so will cause a new schedule when underrun happens. ehci-hcd is indeed one of the drivers that will need to be updated. So will ohci-hcd and uhci-hcd, when they start using tasklets. The new call doesn't turn streaming on or off. The driver does that by submitting URBs or not submitting them. The new call merely separates an old stream from a new one. It tells the HCD that the old stream (if any) is finished, so that any URBs submitted in the future will belong to a new stream. In particular, it tells the HCD that the next URB for this endpoint should be assigned to a time slot that will be visible to the hardware, instead of being assigned to the next time slot after the last packet of the previous URB (which has probably expired already). OK, I got it, looks usb_new_iso_stream(udev, ep) need to be called before starting a new stream. Or after ending an old stream. Either way would work. From view of HCD, the hcd only knows a stream has been started, and doesn't know if it has been stopped. And the most possible usage for hcd is that: if (test_and_clear_bit(ISOC_NEW_STREAM, ep-stream_flag) ; /* new stream */ So looks hcd has to write the flag. Not exactly. The HCD has to keep track of the next available time slot for the endpoint. Some special value, like -1, will indicate that no stream is running yet. So the new function will merely have to set the slot value to -1. If we define the API as usb_iso_stream(udev, ep, start), hcd only need to read the flag, so it should be simper to use and implement. It wouldn't be simpler for drivers. And what happens if the HCD gets an URB submitted for an endpoint where the stream_flag is off? Also with both streaming on and off information, the HCD might improve bandwidth management in the future. I doubt it very much. The USB spec states that bandwidth for periodic endpoints gets allocated whenever a new altsetting is installed, and that is the approach the HCDs will try to take. In a normal system, the URB count won't be very much( 1000), and if you mean the overhead is from memory, I guess it won't cause real memory increase per URB for slab's sake. If you mean performance loss with set/get the single lockless/common variable, maybe we should focus on the big HCD lock, which might be the biggest bottle of USB protocol, :-) If you're talking about the hcd_urb_list_lock, note that it could easily be made hcd-specific. I haven't done that because the regions it protects are all very short, so there probably isn't very much contention. I mean the HCD private lock, and there are heavy contention on the lock. I don't have any ideas how to reduce that contention. Do you? Also I thought of one improvement on the idea of 'urb-completing': - define one percpu 'struct urb *' variable inside 'struct hcd' to record the completing urb, so HCD can see easily if the urb is being resubmitted inside its completion handler. Even on a UP system, what happens if multiple URBs get completed before the tasklet runs and one of them is resubmitted? Anyway this approach is only helpful if much lots of drivers need changes for underrun case. Alan Stern -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: How should we handle isochronous underruns?
On Sat, 20 Jul 2013, Ming Lei wrote: No, we don't have to change every driver using isochronous URBs. Many of them set URB_ISO_ASAP on every URB -- in fact, AFAIK snd-usb-audio is the only one that has been fixed not to do this. OK, if you are sure just snd-usb-audio and very less drivers need the change, using the API only is correct way. I haven't checked in detail, but this seems very likely. But never mind. For a new API, how about void usb_new_iso_stream(struct usb_device *, struct usb_host_endpoint *); Does that look okay? Looks you missed one parameter of 'int on'. I don't understand. Why is another parameter needed? So do you only want to set streaming on and not streaming off? If so, that looks a bit weird, could you describe its usage? The new call doesn't turn streaming on or off. The driver does that by submitting URBs or not submitting them. The new call merely separates an old stream from a new one. It tells the HCD that the old stream (if any) is finished, so that any URBs submitted in the future will belong to a new stream. In particular, it tells the HCD that the next URB for this endpoint should be assigned to a time slot that will be visible to the hardware, instead of being assigned to the next time slot after the last packet of the previous URB (which has probably expired already). In short, it would simply tell the HCD to act as though the next URB has URB_ISO_ASAP set. For HCDs that act as though URB_ISO_ASAP was always set, the new call wouldn't do anything -- they would not need to implement it. (with extra overhead for every URB!) to fix a problem that ought to be In a normal system, the URB count won't be very much( 1000), and if you mean the overhead is from memory, I guess it won't cause real memory increase per URB for slab's sake. If you mean performance loss with set/get the single lockless/common variable, maybe we should focus on the big HCD lock, which might be the biggest bottle of USB protocol, :-) If you're talking about the hcd_urb_list_lock, note that it could easily be made hcd-specific. I haven't done that because the regions it protects are all very short, so there probably isn't very much contention. fixed by changing drivers. If only very less drivers need the change, I agree with you. Okay, good. Alan Stern -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: How should we handle isochronous underruns?
Ming Lei wrote: On Fri, Jul 19, 2013 at 5:20 AM, Alan Stern st...@rowland.harvard.edu wrote: On Thu, 18 Jul 2013, Clemens Ladisch wrote: In any case, there must be _some_ mechanism to explicitly restart a stream. I do not really care if this is some URB flag or some function call. I prefer a function call over the flag. The function call can easily be issued just once, but the completion routine would have to clear the flag every time the URB gets used. IMO, one good candidate is to do it in usb_set_interface() This will result in a control request to be sent to the device, which might confuse its firmware. also it is reasonable, see blow: From 5.6.3 Isochronous Transfer Packet Size Constraints of USB spec 2.0: All device default interface settings must not include any isochronous endpoints with non-zero data payload sizes. That's what the spec says. In reality, there are devices that have non- zero data payload sizes in the default interface setting, and have no other alternate setting. that said all drivers which are using isoc endpoints have to call usb_set_interface(altsetting) explicitly before starting isoc schedule. But a set_interface request does not necessarily affect a single stream; there are devices that have multiple isochronous and bulk endpoints in a single interface, and where restarting one stream must not affect the others. Maybe we can use usb_reset_endpoint() for this purpose after all. It is a perfect fit, because we want to tell the HCD to reset the isochronous endpoint back to the start of stream state. I suggest not to introduce extra starting stream function to usb_reset_endpoint(), and if we have to do this, I suggest to add a new API for the purpose cleanly. I agree. Regards, Clemens -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: How should we handle isochronous underruns?
On Fri, Jul 19, 2013 at 1:44 AM, Clemens Ladisch clem...@ladisch.de wrote:
Alan Stern wrote:
On Thu, 18 Jul 2013, Ming Lei wrote:
On Thu, Jul 18, 2013 at 3:24 AM, Alan Stern st...@rowland.harvard.edu
wrote:
On Thu, 4 Jul 2013, Alan Stern wrote:
On Thu, 4 Jul 2013, Ming Lei wrote:
I had some more ideas about this. Instead of requiring drivers to set
URB_ISO_ASAP on just the first URB of an isochronous stream, we could
ask drivers to call usb_reset_endpoint() between streams. This would
tell the HCD that the next URB marks the start of a new stream, with no
need for a special flag.
This idea still requires changes from old drivers.
Also it might be not appropriate to call usb_reset_endpoint() in this case
because other host controller's implementation may do other unnecessary
thing for this situation.
Perhaps. I doubt that HCDs need to do anything when they reset an
isochronous endpoint, but you're right. It's safer to avoid the issue.
Another possibility, which would be even simpler, is for HCDs to assume
that if the endpoint's queue has been empty for more than 100 ms then a
new stream is starting. Then drivers wouldn't have to do anything
special at all. (Unless they are stopping and restarting streams very
rapidly,
... which happens when a stream is restarted after an error, or when two
sound files are played back-to-back. The audio driver will always
explicitly restart the stream (because checking whether the timeout has
elapsed would be too much of a bother), so the timeout is not useful
in practice.
In this case, we may use changing altsetting to decide start of streaming.
Yes indeed. But that could still require changes to old drivers.
To be even more safe, unlinking an URB should mark the end of a stream.
That's what snd-usb-audio does when it closes a stream; it kills all
the outstanding URBs.
But it might be possible that the queue is empty at that point.
In any case, there must be _some_ mechanism to explicitly restart
a stream. I do not really care if this is some URB flag or some
function call.
Thought about the problem further, looks there is one simple
approach for detecting underrun in case of empty queue:
if (list_empty (stream-td_list)) {
if (running from hcd interrupt or tasklet context)
underrun
else
new stream
}
Any comments on this way?
Thanks,
--
Ming Lei
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: How should we handle isochronous underruns?
Ming Lei wrote:
On Fri, Jul 19, 2013 at 1:44 AM, Clemens Ladisch clem...@ladisch.de wrote:
In any case, there must be _some_ mechanism to explicitly restart
a stream. I do not really care if this is some URB flag or some
function call.
Thought about the problem further, looks there is one simple
approach for detecting underrun in case of empty queue:
if (list_empty (stream-td_list)) {
if (running from hcd interrupt or tasklet context)
underrun
else
new stream
}
Why shouldn't a driver start a stream in an interrupt/tasklet,
or delay URB resubmission to a workqueue?
Regards,
Clemens
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: How should we handle isochronous underruns?
On Fri, Jul 19, 2013 at 5:06 PM, Clemens Ladisch clem...@ladisch.de wrote:
Ming Lei wrote:
On Fri, Jul 19, 2013 at 1:44 AM, Clemens Ladisch clem...@ladisch.de wrote:
In any case, there must be _some_ mechanism to explicitly restart
a stream. I do not really care if this is some URB flag or some
function call.
Thought about the problem further, looks there is one simple
approach for detecting underrun in case of empty queue:
if (list_empty (stream-td_list)) {
if (running from hcd interrupt or tasklet context)
underrun
else
new stream
}
Why shouldn't a driver start a stream in an interrupt/tasklet,
It should, but actually one driver won't do this because generally
usb_set_interface() is required before starting stream.
If stream is started in another non-isoc URB complete(), this approach
can cover this situation easily.
But do you have examples in which one isoc stream is started in another
isoc URB's complete()? Anyway we need to consider current drivers'
implementation.
or delay URB resubmission to a workqueue?
Yes, it is possible, and the isoc URBs can be resubmitted in tasklet too,
but it isn't a big deal:
- they have worked well for long time in the underrun situation before
switching to tasklet, so we needn't worry about the change introduced
by switching URB-complete() to tasklet.
or
- change the resubmission into complete() since there are very few
such usage(I only found two drivers which resubmit isoc URBs in tasklet
when I was doing urb complete() cleanup work)
Thanks,
--
Ming Lei
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: How should we handle isochronous underruns?
Ming Lei wrote:
On Fri, Jul 19, 2013 at 5:06 PM, Clemens Ladisch clem...@ladisch.de wrote:
Ming Lei wrote:
if (list_empty (stream-td_list)) {
if (running from hcd interrupt or tasklet context)
underrun
else
new stream
}
Why shouldn't a driver start a stream in an interrupt/tasklet,
It should, but actually one driver won't do this because generally
usb_set_interface() is required before starting stream.
Not always, and the usb_set_interface() call can be done in a different
context.
If stream is started in another non-isoc URB complete(), this approach
can cover this situation easily.
But do you have examples in which one isoc stream is started in another
isoc URB's complete()?
There are quite a few audio devices using implicit rate feedback, where
playback URBs are submitted from the capture completions handler.
or delay URB resubmission to a workqueue?
Yes, it is possible, and the isoc URBs can be resubmitted in tasklet too,
but it isn't a big deal:
But your proposed code would not be able to differentiate a workqueue
resubmission from a stream start.
Regards,
Clemens
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: How should we handle isochronous underruns?
On Fri, Jul 19, 2013 at 6:30 PM, Clemens Ladisch clem...@ladisch.de wrote:
Ming Lei wrote:
On Fri, Jul 19, 2013 at 5:06 PM, Clemens Ladisch clem...@ladisch.de wrote:
Ming Lei wrote:
if (list_empty (stream-td_list)) {
if (running from hcd interrupt or tasklet context)
underrun
else
new stream
}
Why shouldn't a driver start a stream in an interrupt/tasklet,
It should, but actually one driver won't do this because generally
usb_set_interface() is required before starting stream.
Not always, and the usb_set_interface() call can be done in a different
context.
If stream is started in another non-isoc URB complete(), this approach
can cover this situation easily.
But do you have examples in which one isoc stream is started in another
isoc URB's complete()?
There are quite a few audio devices using implicit rate feedback, where
playback URBs are submitted from the capture completions handler.
That still can be covered easily by the approach since isoc direction is
different, :-)
or delay URB resubmission to a workqueue?
Yes, it is possible, and the isoc URBs can be resubmitted in tasklet too,
but it isn't a big deal:
But your proposed code would not be able to differentiate a workqueue
resubmission from a stream start.
As I explained, it isn't a big deal. Either it is working well for long time
in case of underrun with empty queue, or we can change the resubmission
to complete(), right?
Thanks,
--
Ming Lei
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: How should we handle isochronous underruns?
On Fri, Jul 19, 2013 at 6:36 PM, Ming Lei tom.leim...@gmail.com wrote:
On Fri, Jul 19, 2013 at 6:30 PM, Clemens Ladisch clem...@ladisch.de wrote:
Ming Lei wrote:
On Fri, Jul 19, 2013 at 5:06 PM, Clemens Ladisch clem...@ladisch.de wrote:
Ming Lei wrote:
if (list_empty (stream-td_list)) {
if (running from hcd interrupt or tasklet context)
underrun
else
new stream
}
Why shouldn't a driver start a stream in an interrupt/tasklet,
It should, but actually one driver won't do this because generally
usb_set_interface() is required before starting stream.
Not always, and the usb_set_interface() call can be done in a different
context.
If stream is started in another non-isoc URB complete(), this approach
can cover this situation easily.
But do you have examples in which one isoc stream is started in another
isoc URB's complete()?
There are quite a few audio devices using implicit rate feedback, where
playback URBs are submitted from the capture completions handler.
That still can be covered easily by the approach since isoc direction is
different, :-)
Even we can introduce one flag of 'completing' in 'struct urb' to check if
the USB's submit is called inside its own complete(), by which we can
check if resubmission is in underrun easily.
Consider that isoc URB's resubmission in completion handler should
cover 99% cases, I think this approach is doable.
For other resubmission from tasklet or workque cases, either let them alone
or move the resubmission inside completion() handler, or introduce simple
helper to mark start of frame only for them. Anyway, there are very few isoc
resubmissions from non-completion handler (only two drivers found in my
urb complet() cleanup work), so it shouldn't be a big deal.
What do you think about this approach?
Thanks,
--
Ming Lei
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: How should we handle isochronous underruns?
On Fri, 19 Jul 2013, Ming Lei wrote: Even we can introduce one flag of 'completing' in 'struct urb' to check if the USB's submit is called inside its own complete(), by which we can check if resubmission is in underrun easily. Consider that isoc URB's resubmission in completion handler should cover 99% cases, I think this approach is doable. For other resubmission from tasklet or workque cases, either let them alone or move the resubmission inside completion() handler, or introduce simple helper to mark start of frame only for them. Anyway, there are very few isoc resubmissions from non-completion handler (only two drivers found in my urb complet() cleanup work), so it shouldn't be a big deal. What do you think about this approach? Why go to all this trouble? We already have the URB_ISO_ASAP flag. Trying to check the context of an URB submission strikes me as not robust at all. Who knows what tricks drivers will get up to in the future? Adding a new API seems like the cleanest solution. (Although I still think that usb_reset_endpoint() would be appropriate.) Actually 'start of stream' should be done inside usbcore because it is HCD-independent function, so it is better to provide the information from usbcore and let HCD use it if HCD needs the flag. I'm not quite sure what you mean. Note that usb_reset_endpoint() _is_ a function in usbcore. But never mind. For a new API, how about void usb_new_iso_stream(struct usb_device *, struct usb_host_endpoint *); Does that look okay? Alan Stern -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: How should we handle isochronous underruns?
On Fri, Jul 19, 2013 at 10:53 PM, Alan Stern st...@rowland.harvard.edu wrote:
On Fri, 19 Jul 2013, Ming Lei wrote:
Even we can introduce one flag of 'completing' in 'struct urb' to check if
the USB's submit is called inside its own complete(), by which we can
check if resubmission is in underrun easily.
Consider that isoc URB's resubmission in completion handler should
cover 99% cases, I think this approach is doable.
For other resubmission from tasklet or workque cases, either let them alone
or move the resubmission inside completion() handler, or introduce simple
helper to mark start of frame only for them. Anyway, there are very few
isoc
resubmissions from non-completion handler (only two drivers found in my
urb complet() cleanup work), so it shouldn't be a big deal.
What do you think about this approach?
Why go to all this trouble? We already have the URB_ISO_ASAP flag.
Trying to check the context of an URB submission strikes me as not
robust at all. Who knows what tricks drivers will get up to in the
future?
Firstly, it shouldn't be fragile as you image, with the below simple script, we
can find almost all isoc completion handler(276, but most of them are
false positive).
#/bin/sh
USB=drivers/usb
dirs=`ls -d drivers/* | grep -v $USB`
dirs=$dirs $USB/storage $USB/serial $USB/image $USB/class $USB/atm
$USB/misc sound/usb
export dir
for dir in ${dirs}; do
ss=$(git grep -l -w -E iso_frame_desc $dir)
if [ -n $ss ]; then
git grep -n struct\surb $dir | grep -E \(|\) | grep -v -E
;|,
fi
done
Secondly, from the above script, only very few drivers resubmits isoc URB
in tasklet or other context, and most of them do it in complete()
handler directly.
Finally, suppose drivers do some trick to resubmit URBs in tasklet, workqueue
or other context, we even can ignore them since they worked well for long time
in case of underrun with empty queue, right?
Adding a new API seems like the cleanest solution. (Although I still
think that usb_reset_endpoint() would be appropriate.)
After adding a new API, we still need to change every ISOC driver, and it
won't be simpler than only checking the completion handler, will it?
Actually 'start of stream' should be done inside usbcore because it
is HCD-independent function, so it is better to provide the information
from usbcore and let HCD use it if HCD needs the flag.
I'm not quite sure what you mean. Note that usb_reset_endpoint() _is_
a function in usbcore.
But never mind. For a new API, how about
void usb_new_iso_stream(struct usb_device *, struct usb_host_endpoint *);
Does that look okay?
Looks you missed one parameter of 'int on'.
But we need to change every ISOC driver to use the API, that is why I
suggest to add urb-completing to minimize changes on drivers.
So how about only using the API on drivers which don't resubmit isoc URBs
in its completion handler()? And I think we can document this usage.
Thanks,
--
Ming Lei
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: How should we handle isochronous underruns?
On Sat, 20 Jul 2013, Ming Lei wrote:
Trying to check the context of an URB submission strikes me as not
robust at all. Who knows what tricks drivers will get up to in the
future?
Firstly, it shouldn't be fragile as you image, with the below simple script,
we
can find almost all isoc completion handler(276, but most of them are
false positive).
#/bin/sh
USB=drivers/usb
dirs=`ls -d drivers/* | grep -v $USB`
dirs=$dirs $USB/storage $USB/serial $USB/image $USB/class $USB/atm
$USB/misc sound/usb
export dir
for dir in ${dirs}; do
ss=$(git grep -l -w -E iso_frame_desc $dir)
if [ -n $ss ]; then
git grep -n struct\surb $dir | grep -E \(|\) | grep -v -E
;|,
fi
done
Secondly, from the above script, only very few drivers resubmits isoc URB
in tasklet or other context, and most of them do it in complete()
handler directly.
This is irrelevant. We have to handle drivers that haven't been
written yet, as well as drivers that exist now.
Finally, suppose drivers do some trick to resubmit URBs in tasklet, workqueue
or other context, we even can ignore them since they worked well for long time
in case of underrun with empty queue, right?
No. This is an example of trying to be too clever. Keep it simple.
Adding a new API seems like the cleanest solution. (Although I still
think that usb_reset_endpoint() would be appropriate.)
After adding a new API, we still need to change every ISOC driver, and it
won't be simpler than only checking the completion handler, will it?
No, we don't have to change every driver using isochronous URBs. Many
of them set URB_ISO_ASAP on every URB -- in fact, AFAIK snd-usb-audio
is the only one that has been fixed not to do this.
We also don't have to change the isochronous code in every HCD. Many
of them treat every URB as though URB_ISO_ASAP was set. Only a few
make an attempt to implement proper handling of isochronous underruns
-- and those have to be changed anyway.
But never mind. For a new API, how about
void usb_new_iso_stream(struct usb_device *, struct usb_host_endpoint *);
Does that look okay?
Looks you missed one parameter of 'int on'.
I don't understand. Why is another parameter needed?
But we need to change every ISOC driver to use the API, that is why I
suggest to add urb-completing to minimize changes on drivers.
So how about only using the API on drivers which don't resubmit isoc URBs
in its completion handler()? And I think we can document this usage.
I do not like the idea of adding a complex and non-robust mechanism
(with extra overhead for every URB!) to fix a problem that ought to be
fixed by changing drivers.
Alan Stern
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: How should we handle isochronous underruns?
On Sat, Jul 20, 2013 at 11:57 AM, Alan Stern st...@rowland.harvard.edu wrote:
On Sat, 20 Jul 2013, Ming Lei wrote:
Trying to check the context of an URB submission strikes me as not
robust at all. Who knows what tricks drivers will get up to in the
future?
Firstly, it shouldn't be fragile as you image, with the below simple script,
we
can find almost all isoc completion handler(276, but most of them are
false positive).
#/bin/sh
USB=drivers/usb
dirs=`ls -d drivers/* | grep -v $USB`
dirs=$dirs $USB/storage $USB/serial $USB/image $USB/class $USB/atm
$USB/misc sound/usb
export dir
for dir in ${dirs}; do
ss=$(git grep -l -w -E iso_frame_desc $dir)
if [ -n $ss ]; then
git grep -n struct\surb $dir | grep -E \(|\) | grep -v -E
;|,
fi
done
Secondly, from the above script, only very few drivers resubmits isoc URB
in tasklet or other context, and most of them do it in complete()
handler directly.
This is irrelevant. We have to handle drivers that haven't been
written yet, as well as drivers that exist now.
Finally, suppose drivers do some trick to resubmit URBs in tasklet, workqueue
or other context, we even can ignore them since they worked well for long
time
in case of underrun with empty queue, right?
No. This is an example of trying to be too clever. Keep it simple.
Adding a new API seems like the cleanest solution. (Although I still
think that usb_reset_endpoint() would be appropriate.)
After adding a new API, we still need to change every ISOC driver, and it
won't be simpler than only checking the completion handler, will it?
No, we don't have to change every driver using isochronous URBs. Many
of them set URB_ISO_ASAP on every URB -- in fact, AFAIK snd-usb-audio
is the only one that has been fixed not to do this.
OK, if you are sure just snd-usb-audio and very less drivers need the change,
using the API only is correct way.
We also don't have to change the isochronous code in every HCD. Many
of them treat every URB as though URB_ISO_ASAP was set. Only a few
make an attempt to implement proper handling of isochronous underruns
-- and those have to be changed anyway.
Yes.
But never mind. For a new API, how about
void usb_new_iso_stream(struct usb_device *, struct usb_host_endpoint *);
Does that look okay?
Looks you missed one parameter of 'int on'.
I don't understand. Why is another parameter needed?
So do you only want to set streaming on and not streaming off?
If so, that looks a bit weird, could you describe its usage?
But we need to change every ISOC driver to use the API, that is why I
suggest to add urb-completing to minimize changes on drivers.
So how about only using the API on drivers which don't resubmit isoc URBs
in its completion handler()? And I think we can document this usage.
I do not like the idea of adding a complex and non-robust mechanism
IMO, it is very simple, and only one flag inside urb is required for meeting
the demand.
For the non-robust part(new drivers don't resubmit URBs in complete()), we
can let the new introduced API cover it.
Looks it should cover all old drivers automatically.
(with extra overhead for every URB!) to fix a problem that ought to be
In a normal system, the URB count won't be very much( 1000), and if
you mean the overhead is from memory, I guess it won't cause real memory
increase per URB for slab's sake. If you mean performance loss with
set/get the single lockless/common variable, maybe we should focus on
the big HCD lock, which might be the biggest bottle of USB protocol, :-)
fixed by changing drivers.
If only very less drivers need the change, I agree with you.
Thanks,
--
Ming Lei
--
To unsubscribe from this list: send the line unsubscribe linux-usb in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: How should we handle isochronous underruns?
On Thu, 18 Jul 2013, Ming Lei wrote: On Thu, Jul 18, 2013 at 3:24 AM, Alan Stern st...@rowland.harvard.edu wrote: On Thu, 4 Jul 2013, Alan Stern wrote: On Thu, 4 Jul 2013, Ming Lei wrote: If so, your coming change may break ABI because as you described that the flag should be set in the first URB of a new stream, but some user-space drivers may not set it before. Even USB audio driver doesn't set it in current -next tree. I had some more ideas about this. Instead of requiring drivers to set URB_ISO_ASAP on just the first URB of an isochronous stream, we could ask drivers to call usb_reset_endpoint() between streams. This would tell the HCD that the next URB marks the start of a new stream, with no need for a special flag. This idea still requires changes from old drivers. Also it might be not appropriate to call usb_reset_endpoint() in this case because other host controller's implementation may do other unnecessary thing for this situation. Perhaps. I doubt that HCDs need to do anything when they reset an isochronous endpoint, but you're right. It's safer to avoid the issue. Another possibility, which would be even simpler, is for HCDs to assume that if the endpoint's queue has been empty for more than 100 ms then a new stream is starting. Then drivers wouldn't have to do anything special at all. (Unless they are stopping and restarting streams very rapidly, in which case something like usb_reset_endpoint() would be In this case, we may use changing altsetting to decide start of streaming. Yes indeed. But that could still require changes to old drivers. To be even more safe, unlinking an URB should mark the end of a stream. That's what snd-usb-audio does when it closes a stream; it kills all the outstanding URBs. necessary.) IMO, this one should be OK, and looks it is a bit similar with unlinking empty interrupt qh. In fact, it's necessary: ehci-hcd doesn't keep track of the state of a stream beyond 512 ms or so. 100 ms might even be overkill. No stream should ever have a gap that large unless something has gone drastically wrong, in which case it doesn't much matter what we do. (Except perhaps for isochronous endpoints with a period of 128 ms or more. I have never heard of such things; have you?) Alan Stern -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: How should we handle isochronous underruns?
Alan Stern wrote: On Thu, 18 Jul 2013, Ming Lei wrote: On Thu, Jul 18, 2013 at 3:24 AM, Alan Stern st...@rowland.harvard.edu wrote: On Thu, 4 Jul 2013, Alan Stern wrote: On Thu, 4 Jul 2013, Ming Lei wrote: I had some more ideas about this. Instead of requiring drivers to set URB_ISO_ASAP on just the first URB of an isochronous stream, we could ask drivers to call usb_reset_endpoint() between streams. This would tell the HCD that the next URB marks the start of a new stream, with no need for a special flag. This idea still requires changes from old drivers. Also it might be not appropriate to call usb_reset_endpoint() in this case because other host controller's implementation may do other unnecessary thing for this situation. Perhaps. I doubt that HCDs need to do anything when they reset an isochronous endpoint, but you're right. It's safer to avoid the issue. Another possibility, which would be even simpler, is for HCDs to assume that if the endpoint's queue has been empty for more than 100 ms then a new stream is starting. Then drivers wouldn't have to do anything special at all. (Unless they are stopping and restarting streams very rapidly, ... which happens when a stream is restarted after an error, or when two sound files are played back-to-back. The audio driver will always explicitly restart the stream (because checking whether the timeout has elapsed would be too much of a bother), so the timeout is not useful in practice. In this case, we may use changing altsetting to decide start of streaming. Yes indeed. But that could still require changes to old drivers. To be even more safe, unlinking an URB should mark the end of a stream. That's what snd-usb-audio does when it closes a stream; it kills all the outstanding URBs. But it might be possible that the queue is empty at that point. In any case, there must be _some_ mechanism to explicitly restart a stream. I do not really care if this is some URB flag or some function call. Regards, Clemens -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: How should we handle isochronous underruns?
On Thu, 18 Jul 2013, Clemens Ladisch wrote: In any case, there must be _some_ mechanism to explicitly restart a stream. I do not really care if this is some URB flag or some function call. I prefer a function call over the flag. The function call can easily be issued just once, but the completion routine would have to clear the flag every time the URB gets used. Maybe we can use usb_reset_endpoint() for this purpose after all. It is a perfect fit, because we want to tell the HCD to reset the isochronous endpoint back to the start of stream state. A search under drivers/ shows that only a few HCDs other than ehci currently implement the endpoint_reset method: xhci, whci, dwc2, and ozwpan. It would not be hard to fix them up to ignore calls for isochronous endpoints. Any objections to this approach? Alan Stern -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: How should we handle isochronous underruns?
On Fri, Jul 19, 2013 at 5:20 AM, Alan Stern st...@rowland.harvard.edu wrote: On Thu, 18 Jul 2013, Clemens Ladisch wrote: In any case, there must be _some_ mechanism to explicitly restart a stream. I do not really care if this is some URB flag or some function call. I prefer a function call over the flag. The function call can easily be issued just once, but the completion routine would have to clear the flag every time the URB gets used. IMO, one good candidate is to do it in usb_set_interface(), and we may avoid changes on most of drivers which are using isoc endpoints, also it is reasonable, see blow: From 5.6.3 Isochronous Transfer Packet Size Constraints of USB spec 2.0: All device default interface settings must not include any isochronous endpoints with non-zero data payload sizes (specified via wMaxPacketSize in the endpoint descriptor). Alternate interface settings may specify non-zero data payload sizes for isochronous endpoints. that said all drivers which are using isoc endpoints have to call usb_set_interface(altsetting) explicitly before starting isoc schedule. So for isoc drivers, it is very reasonable to call usb_set_interface(altsetting) before starting streaming and call usb_set_interface(0) after stopping streaming. I think we may document this usage, then use this info as starting/stopping stream flag. Looks both uvc and uac drivers support this way. If one driver doesn't call usb_set_interface(0) after streaming off, we can think it as buggy since the device still consumes bus bandwidth unnecessary and may cause other device's bandwidth requirement not satisfied on same bus. Maybe we can use usb_reset_endpoint() for this purpose after all. It is a perfect fit, because we want to tell the HCD to reset the isochronous endpoint back to the start of stream state. I suggest not to introduce extra starting stream function to usb_reset_endpoint(), and if we have to do this, I suggest to add a new API for the purpose cleanly. A search under drivers/ shows that only a few HCDs other than ehci currently implement the endpoint_reset method: xhci, whci, dwc2, and ozwpan. It would not be hard to fix them up to ignore calls for isochronous endpoints. From xhci_endpoint_reset(), looks the hcd doesn't rule out isoc endpoint. Actually 'start of stream' should be done inside usbcore because it is HCD-independent function, so it is better to provide the information from usbcore and let HCD use it if HCD needs the flag. Thanks, -- Ming Lei -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: How should we handle isochronous underruns?
On Thu, 4 Jul 2013, Alan Stern wrote: On Thu, 4 Jul 2013, Ming Lei wrote: If so, your coming change may break ABI because as you described that the flag should be set in the first URB of a new stream, but some user-space drivers may not set it before. Even USB audio driver doesn't set it in current -next tree. I had some more ideas about this. Instead of requiring drivers to set URB_ISO_ASAP on just the first URB of an isochronous stream, we could ask drivers to call usb_reset_endpoint() between streams. This would tell the HCD that the next URB marks the start of a new stream, with no need for a special flag. Another possibility, which would be even simpler, is for HCDs to assume that if the endpoint's queue has been empty for more than 100 ms then a new stream is starting. Then drivers wouldn't have to do anything special at all. (Unless they are stopping and restarting streams very rapidly, in which case something like usb_reset_endpoint() would be necessary.) What do you think? Alan Stern -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: How should we handle isochronous underruns?
On Thu, Jul 18, 2013 at 3:24 AM, Alan Stern st...@rowland.harvard.edu wrote: On Thu, 4 Jul 2013, Alan Stern wrote: On Thu, 4 Jul 2013, Ming Lei wrote: If so, your coming change may break ABI because as you described that the flag should be set in the first URB of a new stream, but some user-space drivers may not set it before. Even USB audio driver doesn't set it in current -next tree. I had some more ideas about this. Instead of requiring drivers to set URB_ISO_ASAP on just the first URB of an isochronous stream, we could ask drivers to call usb_reset_endpoint() between streams. This would tell the HCD that the next URB marks the start of a new stream, with no need for a special flag. This idea still requires changes from old drivers. Also it might be not appropriate to call usb_reset_endpoint() in this case because other host controller's implementation may do other unnecessary thing for this situation. Another possibility, which would be even simpler, is for HCDs to assume that if the endpoint's queue has been empty for more than 100 ms then a new stream is starting. Then drivers wouldn't have to do anything special at all. (Unless they are stopping and restarting streams very rapidly, in which case something like usb_reset_endpoint() would be In this case, we may use changing altsetting to decide start of streaming. necessary.) IMO, this one should be OK, and looks it is a bit similar with unlinking empty interrupt qh. Thanks, -- Ming Lei -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: How should we handle isochronous underruns?
On Thu, 4 Jul 2013, Ming Lei wrote: If so, your coming change may break ABI because as you described that the flag should be set in the first URB of a new stream, but some user-space drivers may not set it before. Even USB audio driver doesn't set it in current -next tree. Really? I thought Clemens's changes were already merged. In any case, Looks the change isn't found in linux-next-20130701. You are right. I was thinking of commit c75c5ab575af (ALSA: USB: adjust for changed 3.8 USB API). It turns off URB_ISO_ASAP completely, but now we will need it to be turned on for the first URB in a stream. User programs probably don't matter much. In general, the practice is to leave the interface on altsetting 0 when nothing is running and then change to a different altsetting when starting an isochronous stream. Changing the altsetting is another way to tell the HCD that a stream is starting fresh. Yes, it might work, but many details need to be payed attention to, such as, changing altsetting may not be done during suspend/resume. Hmmm. I don't know how snd-usb-audio handles a suspend/resume in the middle of playback or recording. HCDs may need to be smart enough to realize that all isochronous streams must be restarted following a root-hub suspend. According to the discussion, URB_ISO_ASAP should be set on the first URB of one audio stream, and keep unset on all other URBs, which looks a new usage, IMO. It's the same as the current usage. I didn't realize that the audio driver wasn't setting URB_ISO_ASAP on the first URB of a stream -- it's not necessary with the current API. But it will be needed with the new API; that will have to be fixed. Here's a short explanation, in case the reasoning isn't clear to everyone. Suppose an isochronous stream is started, then stopped, and then restarted. If URB_ISO_ASAP isn't set in the URB that restarts the stream, then the HCD will think the stream was running all along -- the gap will look like a _very_ large underrun. Consequently, the restart will get messed up. With the old code, the HCD could tell the difference between an underrun and a genuine gap. With an underrun, the pipeline would never empty out, because each URB would be resubmitted _before_ its completion was finished. With tasklets that is no longer true; a sufficiently bad underrun could cause the pipeline to become completely empty. Alan Stern -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: How should we handle isochronous underruns?
Alan Stern wrote: On Thu, 4 Jul 2013, Ming Lei wrote: Changing the altsetting is another way to tell the HCD that a stream is starting fresh. Yes, it might work, but many details need to be payed attention to, such as, changing altsetting may not be done during suspend/resume. Hmmm. I don't know how snd-usb-audio handles a suspend/resume in the middle of playback or recording. In this case, the ALSA framework will stop the PCM stream, and on resume, report this like an underrun; the application will then reinitialize the stream. (Handling suspend/resume transparently would require that the driver can restart playing from an arbitrary position in the ring buffer; snd-usb-audio does not bother to implement this.) Regards, Clemens -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: How should we handle isochronous underruns?
On Thu, 4 Jul 2013, Clemens Ladisch wrote: Alan Stern wrote: On Thu, 4 Jul 2013, Ming Lei wrote: Changing the altsetting is another way to tell the HCD that a stream is starting fresh. Yes, it might work, but many details need to be payed attention to, such as, changing altsetting may not be done during suspend/resume. Hmmm. I don't know how snd-usb-audio handles a suspend/resume in the middle of playback or recording. In this case, the ALSA framework will stop the PCM stream, and on resume, report this like an underrun; the application will then reinitialize the stream. (Handling suspend/resume transparently would require that the driver can restart playing from an arbitrary position in the ring buffer; snd-usb-audio does not bother to implement this.) Good; that means we don't have to worry about it. :-) Alan Stern -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: How should we handle isochronous underruns?
On Wed, 3 Jul 2013, Ming Lei wrote: Yes, that is the change tasklet is bringing, so looks we need to find a way to distinguish streaming-on from underrun when stream-td_list becomes empty in iso_stream_schedule(). The difference will be the URB_ISO_ASAP flag. The flag should be set in the first URB of a new stream. It should not be set in any other URBs, unless the client driver doesn't care about losing synchronization when an underrun occurs. So sounds a better name for the flag should be URB_ISO_STREAM_ON, :-) I agree, but the existing name is already exposed to userspace in include/uapi/linux/usbdevice_fs.h. We could change the internal name while leaving the external name the same, but that would be confusing. (URB_ISO_ASAP used to mean that the HCD was responsible for scheduling the URB, and if the flag wasn't set then the value of urb-start_frame would be used directly. This doesn't make sense, because drivers should not be in charge of URB scheduling -- HCDs should take care of it. So I have felt free to alter the flag's meaning.) Also looks drivers need fix for the API change. I don't know if any drivers will need to be fixed. I suspect many of them simply set URB_ISO_ASAP on all their URBs. Clemens has already added a fix for the snd-usb-audio driver. Alan Stern -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: How should we handle isochronous underruns?
On Wed, 3 Jul 2013, Laurent Pinchart wrote: Of course, it has been true all along that the status fields in the URB's individual usb_iso_packet_descriptor structures indicate any errors. But HCDs tend to set urb-status to 0 always, for isochronous URBs. Is there any advantage to setting urb-status to -EXDEV, given that we already set urb-iso_frame_desc[i].status to -EXDEV for each i? Not that I see, for the uvcvideo driver at least. And probably not for anybody else either. With (2) or (3), the driver could also recover right away by setting the URB_ISO_ASAP flag on its next URB, but then synchronization would be lost. You wouldn't want to do this if synchronization matters. But if it doesn't, the driver could simply set URB_ISO_ASAP on every URB to avoid worrying about the problem -- the flag would have no effect in the absence of underruns. That's what the uvcvideo driver does. This could be changed, but I don't have time to investigate at the moment. (Tested) patches are of course welcome :-) Based on your earlier comments, it sounds like always using URB_ISO_ASAP will do just what uvcvideo wants. Thanks for your feedback, Alan Stern -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: How should we handle isochronous underruns?
On Thu, 4 Jul 2013, Ming Lei wrote: On Wed, Jul 3, 2013 at 10:15 PM, Alan Stern st...@rowland.harvard.edu wrote: On Wed, 3 Jul 2013, Ming Lei wrote: Yes, that is the change tasklet is bringing, so looks we need to find a way to distinguish streaming-on from underrun when stream-td_list becomes empty in iso_stream_schedule(). The difference will be the URB_ISO_ASAP flag. The flag should be set in the first URB of a new stream. It should not be set in any other URBs, unless the client driver doesn't care about losing synchronization when an underrun occurs. So sounds a better name for the flag should be URB_ISO_STREAM_ON, :-) I agree, but the existing name is already exposed to userspace in include/uapi/linux/usbdevice_fs.h. We could change the internal name If so, your coming change may break ABI because as you described that the flag should be set in the first URB of a new stream, but some user-space drivers may not set it before. Even USB audio driver doesn't set it in current -next tree. Really? I thought Clemens's changes were already merged. In any case, the audio driver should be easy enough to change. Setting the flag on the first URB isn't a big deal. User programs probably don't matter much. In general, the practice is to leave the interface on altsetting 0 when nothing is running and then change to a different altsetting when starting an isochronous stream. Changing the altsetting is another way to tell the HCD that a stream is starting fresh. while leaving the external name the same, but that would be confusing. (URB_ISO_ASAP used to mean that the HCD was responsible for scheduling the URB, and if the flag wasn't set then the value of urb-start_frame would be used directly. This doesn't make sense, because drivers should not be in charge of URB scheduling -- HCDs should take care of it. So I have felt free to alter the flag's meaning.) Anyway the name will become a bit confusing, so if we don't plan to change it, we should add detailed comment about its usage. Yes. There already are detailed comments; they will need to be updated. But I never did add anything to Documentation/usb/URB.txt; obviously that file needs attention. them simply set URB_ISO_ASAP on all their URBs. Clemens has already added a fix for the snd-usb-audio driver. I am wondering the fix has been added because it may depend on the coming change of URB_ISO_ASAP, :-) The new meaning of URB_ISO_ASAP is very close to the current meaning. Whatever changes Clemens made ought to work for both the current code and the upcoming code. Alan Stern -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: How should we handle isochronous underruns?
On Mon, Jul 1, 2013 at 10:48 PM, Alan Stern st...@rowland.harvard.edu wrote: On Mon, 1 Jul 2013, Ming Lei wrote: The fact that the delay is small doesn't matter -- what's important is that it will be 0 whereas now there is no delay between completion and resubmission (in fact, the resubmission occurs before the completion ends). Yes, that is the change tasklet is bringing, so looks we need to find a way to distinguish streaming-on from underrun when stream-td_list becomes empty in iso_stream_schedule(). I don't think the introduced tasklet delay is a problem for EHCI since per EHCI spec the maximum rate at which the host controller will issue interrupts is one microframe(125us), which means isochronous transfer completion can be reported to CPU with about 125us delay in hardware level. Irrelevant. Besides, the delay can be longer than 125 us if another interrupt is being handled. Thus, for example, even if the pipeline contains only a single URB, with the current code it will not become empty. But with the new code it will. If the load on the system is too high, the pipeline could empty out even if it normally contains two or more URBs. Single URB may not work well too when running complete() in hard irq context. It could work very well indeed, if the interval between URBs was larger than 1 ms. It is too fragile so that single isoc URB is seldom used, isn't it? Thanks, -- Ming Lei -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: How should we handle isochronous underruns?
On Tue, 2 Jul 2013, Ming Lei wrote: On Mon, Jul 1, 2013 at 10:48 PM, Alan Stern st...@rowland.harvard.edu wrote: On Mon, 1 Jul 2013, Ming Lei wrote: The fact that the delay is small doesn't matter -- what's important is that it will be 0 whereas now there is no delay between completion and resubmission (in fact, the resubmission occurs before the completion ends). Yes, that is the change tasklet is bringing, so looks we need to find a way to distinguish streaming-on from underrun when stream-td_list becomes empty in iso_stream_schedule(). The difference will be the URB_ISO_ASAP flag. The flag should be set in the first URB of a new stream. It should not be set in any other URBs, unless the client driver doesn't care about losing synchronization when an underrun occurs. Thus, for example, even if the pipeline contains only a single URB, with the current code it will not become empty. But with the new code it will. If the load on the system is too high, the pipeline could empty out even if it normally contains two or more URBs. Single URB may not work well too when running complete() in hard irq context. It could work very well indeed, if the interval between URBs was larger than 1 ms. It is too fragile so that single isoc URB is seldom used, isn't it? Maybe not for isochronous. Interrupt transfers almost always use a single URB, though, and they don't have any trouble. Alan Stern -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: How should we handle isochronous underruns?
Hi Alan, On Monday 01 July 2013 15:39:46 Alan Stern wrote: On Mon, 1 Jul 2013, Laurent Pinchart wrote: What about error recovery for insane systems? If we do get a 50-ms gap in the data stream, what's the best way for the UVC driver to learn this has happens and attempt to carry on? When isochronous packets are lost video frames get corrupted. UVC doesn't provide sequence numbers for packets, so there's no way to know exactly how many packets have been lost. However, several synchronization methods are available for the driver to detect frame boundaries (namely an End-Of-Frame bit and a Frame ID bit that toggles for every frame in isochronous packet headers). The driver already handles those bits and marks video frames as complete when the EOF bit or an FID transition is detected. For uncompressed formats the driver then checks whether the received data size matches the frame size, and marks the frame as bad if it doesn't. That check can't be performed for compressed formats as the frame size is then variable. It would thus be helpful to receive a notification when a gap in the data stream is detected, through the URB status field for instance. The driver could then mark the frame being received as faulty and reset its state machine to resynchronize to the next frame boundary. One possibility is to set urb-status to -EXDEV if an underrun has caused the entire URB to be too late. Currently, under those conditions the HCD rejects the URB submission with a -EXDEV error code, which doesn't seem to be the right approach. Drivers don't expect isochronous _submissions_ to fail, although they are prepared to see failure statuses for isochronous _completions_. Of course, it has been true all along that the status fields in the URB's individual usb_iso_packet_descriptor structures indicate any errors. But HCDs tend to set urb-status to 0 always, for isochronous URBs. Is there any advantage to setting urb-status to -EXDEV, given that we already set urb-iso_frame_desc[i].status to -EXDEV for each i? Not that I see, for the uvcvideo driver at least. (There's also an urb-error_count field, which reports how many of the isochronous packets got an error. It is hardly used; I think we could remove it.) This boils down to three possibilities for how to handle URBs that were submitted too late -- that is, so late that all the time slots for all the URB's packets are known to have expired already: (1) Reject the submission with -EXDEV -- this is what we do now. (2) Accept the submission, but have the URB complete immediately with urb-status and all the packet statuses set to -EXDEV. (3) Accept the submission, but have the URB complete immediately with urb-status set to 0 and all the packet statuses set to -EXDEV. For (1), the only way to recover is to submit an URB with URB_ISO_ASAP set. This is essentially the same as shutting down the stream and restarting it. For (2) and (3), the stream's next time slot value would be updated in the usual way. For example, if 10 slots had expired and the driver was submitting URBs containing 4 packets each, then the first and second URBs would complete right away with errors, and the first two packets of the third URB would get errors, but the last two packets of the third URB would be assigned to the two upcoming time slots and would be treated normally. Thus recovery would be automatic, at the cost of wasting two URBs. Since we expect underruns to be rare, maybe this is acceptable. With (2) or (3), the driver could also recover right away by setting the URB_ISO_ASAP flag on its next URB, but then synchronization would be lost. You wouldn't want to do this if synchronization matters. But if it doesn't, the driver could simply set URB_ISO_ASAP on every URB to avoid worrying about the problem -- the flag would have no effect in the absence of underruns. That's what the uvcvideo driver does. This could be changed, but I don't have time to investigate at the moment. (Tested) patches are of course welcome :-) (The main reason the current code uses (1) is because (2) and (3) require completion to occur _during_ submission. Resubmissions would thus be nested, consuming excessive stack space, and there would be a deadlock if the completion handler tried to acquire a lock that was held during submission. When a tasklet is used for completions, none of these problems arise.) Any preference? Clemens, what do you think? -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: How should we handle isochronous underruns?
On 6/30/2013 8:32 PM, Alan Stern wrote: There are several possible things the HCD could do when an underrun occurs: I do not have much experience in working with ISOC host.But by the experience of device side I can say that, There could be problem irrespective of tasklet or irq context, only that tasklet would be more prone to it. Should not we have same approach in handling isoc packet in either of the cases? It could schedule the URB for the first time slot known to be available, even if that means skipping some time slots which the hardware might (or might not) be able to use. IMO, this approach is better. But, what should we call as first time slot known to be available. Current code calculates it as now (current time slot) + delta microframe, and delta is kept as fixed. However, different system works with different values of delta. IMO, This delta should be dynamic and software should update it on the basis of initial value and feedback (past failure/missed isoc experience). It could try to schedule the URB for the next time slot after the last one used by the preceding URB, even if that time slot has already expired. There is no meaning of submitting an URB for expired time slot. In anyway, it is not going to be transferred and will further result in under-run/missed packet. Something in between... Making this offset dynamic will cause software to be further complex, but that would be the best way to handle such situation. Regards Pratyush What would be best for your purposes? Or do you have any different suggestions? -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: How should we handle isochronous underruns?
Pratyush Anand wrote: On 6/30/2013 8:32 PM, Alan Stern wrote: It could schedule the URB for the first time slot known to be available, even if that means skipping some time slots which the hardware might (or might not) be able to use. IMO, this approach is better. To quote the USB spec: | Isochronous Data | A stream of data whose timing is implied by its delivery rate. Isochronous transfers are usually used for real-time applications where it is better to drop a single packet than to delay *all* following packets. It could try to schedule the URB for the next time slot after the last one used by the preceding URB, even if that time slot has already expired. There is no meaning of submitting an URB for expired time slot. Of course there is. The HCD cannot exactly know whether the current slot will expire before the URB is enqueued, so it is not always possible to label the slot as expired or not during submission. The meaning of an isochronous URB submission is try to transmit this data in that frame; whether the URB actually was transferred will be reported to the completion callback. Regards, Clemens -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: How should we handle isochronous underruns?
Ming Lei wrote: On Sun, Jun 30, 2013 at 11:02 PM, Alan Stern st...@rowland.harvard.edu wrote: Thus, for example, even if the pipeline contains only a single URB, with the current code it will not become empty. But with the new code it will. If the load on the system is too high, the pipeline could empty out even if it normally contains two or more URBs. [...] But I don't know how USB audio driver uses URBs, and could it submit only one isoc URB to playback/record audio data? The audio drivers uses at least two URBs. (The actual number and length of URBs depends on how the application configures its buffer.) Regards, Clemens -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: How should we handle isochronous underruns?
On Sun, Jun 30, 2013 at 11:02 PM, Alan Stern st...@rowland.harvard.edu wrote: Naturally, under normal circumstances this won't matter, because underruns shouldn't occur. But I know from experience that people try to push the latency as far down as they can, which increases the likelihood of underruns. I understand the latency is effected by packet count in one URB, and it shouldn't be related with URB count, so looks we still can ease the problem by submitting more URBs concurrently, and at the same time make less packets per URB if guys care latency. Correct me if it is wrong... On Mon, Jul 1, 2013 at 7:36 PM, Clemens Ladisch clem...@ladisch.de wrote: The audio drivers uses at least two URBs. (The actual number and length of URBs depends on how the application configures its buffer.) Clemens, thanks for your input. Also, another important data about the problem is that how much time one isoc URB may span, which depends on the endpoint interval and packet number of URB. Thanks, -- Ming Lei -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: How should we handle isochronous underruns?
On 7/1/2013 4:48 PM, Clemens Ladisch wrote: It could try to schedule the URB for the next time slot after the last one used by the preceding URB, even if that time slot has already expired. There is no meaning of submitting an URB for expired time slot. Of course there is. The HCD cannot exactly know whether the current slot will expire before the URB is enqueued, so it is not always possible to label the slot as expired or not during submission. The meaning of an isochronous URB submission is try to transmit this data in that frame; whether the URB actually was transferred will be reported to the completion callback. Correct, I understand it. Let me explain with an example what I wanted to convey: Lets assume that URB1-8 has been submitted to HCD driver from upper layer. HCD driver has also further submitted them to the controller hardware for execution in n to (n+7)th uf respectively. Now URB3 was not able to be transmitted in time and resulted in missed isoc. This failure will cause mainly because of two reasons: 1. SW was slow enough in submission of URB to HW. It actually submitted when timing was already expired. In this case, it is most likely that following URBs (4-8) will also result in missed isoc. So, if we add further URB9 for transmission in (n + 8) uf, most likely it may also result in missed isoc. 2. SW submitted well in time, but HW was slow enough and could not fetch data in time. (In case HS it may not occur, but in case of SS it may occur sometime, so this case may not be valid for ehci-hcd). In such situation, HW will flush current descriptor and most likely will be able to transmit next URB correctly. So, What I was proposing, to go for dynamic, something like this... When an HCD gets an URB request for any isoc pipe, and if there is any pending missed isoc flag then wait.. do not submit it to controller. May be just keep them in received list(submitted_list). If during any of the pending completion callback, SW observes success then submit all from submitted_list to be scheduled in very next uf, and clear pending missed isoc flag. If none of the submitted URB results in success, then do not submit them in very next frame, rather submit for the first time slot known to be available. Regards Pratyush -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: How should we handle isochronous underruns?
Ming Lei wrote: On Sun, Jun 30, 2013 at 11:02 PM, Alan Stern st...@rowland.harvard.edu wrote: Naturally, under normal circumstances this won't matter, because underruns shouldn't occur. But I know from experience that people try to push the latency as far down as they can, which increases the likelihood of underruns. I understand the latency is effected by packet count in one URB, and it shouldn't be related with URB count, This is true only when capturing. For playback, the latency is the length of the entire pipeline. Regards, Clemens -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: How should we handle isochronous underruns?
Hello, On Monday 01 July 2013 13:19:05 Ming Lei wrote: On Sun, Jun 30, 2013 at 11:02 PM, Alan Stern wrote: Clement and Laurent: The two of you seem to be the people who make the most use of isochronous USB transfers. Since the ehci-hcd driver is being changed to use a tasklet for URB completion callbacks, it looks like I will need to reconsider how isochronous underruns get handled. Without using tasklet, the hard interrupt handler still can be delayed for some time, and switching to tasklet doesn't change the fact of the probable delay of URB handling. The basic prolem is simple enough: What should the HCD do when an URB is submitted after an isochronous pipeline has emptied out? I think some time-related data should be helpful for the discussion, for example, how long the isochronous pipeline may become empty in current audio/video driver(application) without any URB resubmit during the period? The problem will be more acute than in the past, because URB resubmissions will no longer be synchronous with URB completions, thanks to the tasklet. That is, in the current code, if the completion handler resubmits the URB, the resubmission occurs before the HCD finishes the completion callback. But in the new code, the HCD will simply hand the URB over to the tasklet, and the resubmission won't occur until some time later (when the tasklet wakes up and invokes the completion handler). As far as the HCD is concerned, the completion The delay should be very small(generally several microseconds) since isochronous URBs are completed in high priority tasklet. I don't think the introduced tasklet delay is a problem for EHCI since per EHCI spec the maximum rate at which the host controller will issue interrupts is one microframe(125us), which means isochronous transfer completion can be reported to CPU with about 125us delay in hardware level. will already be finished. Thus, for example, even if the pipeline contains only a single URB, with the current code it will not become empty. But with the new code it will. If the load on the system is too high, the pipeline could empty out even if it normally contains two or more URBs. Single URB may not work well too when running complete() in hard irq context. In UVC driver, looks it may submit at most 5 URBs which can include max 32 packets, so it will take about 5*32*125us(20ms) to make isoc pipeline empty suppose the interval is 1uF. That's correct. Unlike the UAC driver, the UVC driver doesn't care too much about exact timings. Avoiding packet loss is my main concern. As video frames are not delivered to userspace before they are fully transmitted, and given the large number of URBs required to transmit a video frame, I can submit more URBs (without reasonable limits) if that can help avoiding underruns. But I don't know how USB audio driver uses URBs, and could it submit only one isoc URB to playback/record audio data? This means that the HCD will have trouble telling the difference between an underrun and a normal restart of a stopped I/O stream. In both cases it will see an URB being submitted to an empty queue. IMO, we should try to avoid the situation, and in UVC cases looks it is impossible to see an URB being submitted to an empty queue except for at the moment of starting streaming or interrupt disabled for extremely long. A couple of users have experienced underruns in the past due to a rogue driver disabling interrupts for tens of milliseconds. I don't think switching to tasklets will have a strong adverse effect, the 20ms buffer time should be large enough to avoid underruns in sane systems. Here's where the URB_ISO_ASAP flag will be useful; if that flag is set then the URB is restarting a stopped stream, but if it isn't set then the pipeline experienced an underrun. Naturally, under normal circumstances this won't matter, because underruns shouldn't occur. But I know from experience that people try to push the latency as far down as they can, which increases the likelihood of underruns. There are several possible things the HCD could do when an underrun occurs: It could schedule the URB for the first time slot known to be available, even if that means skipping some time slots which the hardware might (or might not) be able to use. It could try to schedule the URB for the next time slot after the last one used by the preceding URB, even if that time slot has already expired. Something in between... What would be best for your purposes? Or do you have any different suggestions? -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: How should we handle isochronous underruns?
On Mon, Jul 1, 2013 at 9:06 PM, Clemens Ladisch clem...@ladisch.de wrote: Ming Lei wrote: On Sun, Jun 30, 2013 at 11:02 PM, Alan Stern st...@rowland.harvard.edu wrote: Naturally, under normal circumstances this won't matter, because underruns shouldn't occur. But I know from experience that people try to push the latency as far down as they can, which increases the likelihood of underruns. I understand the latency is effected by packet count in one URB, and it shouldn't be related with URB count, This is true only when capturing. For playback, the latency is the length of the entire pipeline. For playback, every URB submitted is added into hw table immediately, then the data will be played to speaker. I don't understand why the latency is the entire pipeline. If you submitted a bit more URBs, underruns shouldn't happen at all. Could you explain it in a bit detail? Thanks, -- Ming Lei -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: How should we handle isochronous underruns?
Ming Lei wrote: On Mon, Jul 1, 2013 at 9:06 PM, Clemens Ladisch clem...@ladisch.de wrote: Ming Lei wrote: I understand the latency is effected by packet count in one URB, and it shouldn't be related with URB count, This is true only when capturing. For playback, the latency is the length of the entire pipeline. For playback, every URB submitted is added into hw table immediately, then the data will be played to speaker. I don't understand why the latency is the entire pipeline. A submitted packet will be transmitted only after all the other packets in the pipeline have been transmitted. Regards, Clemens -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: How should we handle isochronous underruns?
On Mon, Jul 1, 2013 at 9:27 PM, Clemens Ladisch clem...@ladisch.de wrote: Ming Lei wrote: On Mon, Jul 1, 2013 at 9:06 PM, Clemens Ladisch clem...@ladisch.de wrote: Ming Lei wrote: I understand the latency is effected by packet count in one URB, and it shouldn't be related with URB count, This is true only when capturing. For playback, the latency is the length of the entire pipeline. For playback, every URB submitted is added into hw table immediately, then the data will be played to speaker. I don't understand why the latency is the entire pipeline. A submitted packet will be transmitted only after all the other packets in the pipeline have been transmitted. Yes, that is always true since EHCI HW will send out data(packet) to device one by one at the scheduled frame/uframe according to the order of URB submitting , so the USB audio driver can submit URBs in advance, can't it? Also could you provide the typical time one URB in audio driver may span? Thanks, -- Ming Lei -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: How should we handle isochronous underruns?
Pratyush Anand wrote: On 7/1/2013 4:48 PM, Clemens Ladisch wrote: There is no meaning of submitting an URB for expired time slot. Of course there is. The HCD cannot exactly know whether the current slot will expire before the URB is enqueued, so it is not always possible to label the slot as expired or not during submission. The meaning of an isochronous URB submission is try to transmit this data in that frame; whether the URB actually was transferred will be reported to the completion callback. Correct, I understand it. Let me explain with an example what I wanted to convey: Lets assume that URB1-8 has been submitted to HCD driver from upper layer. HCD driver has also further submitted them to the controller hardware for execution in n to (n+7)th uf respectively. Now URB3 was not able to be transmitted in time and resulted in missed isoc. This failure will cause mainly because of two reasons: 1. SW was slow enough in submission of URB to HW. It actually submitted when timing was already expired. In this case, it is most likely that following URBs (4-8) will also result in missed isoc. So, if we add further URB9 for transmission in (n + 8) uf, most likely it may also result in missed isoc. But there is _some_ uf that has not yet expired. If the HCD delays enqueuing URBs, it just increases the risk that that uf will expire too. 2. SW submitted well in time, but HW was slow enough and could not fetch data in time. In such situation, HW will flush current descriptor and most likely will be able to transmit next URB correctly. So, What I was proposing, to go for dynamic, something like this... When an HCD gets an URB request for any isoc pipe, and if there is any pending missed isoc flag then wait.. do not submit it to controller. May be just keep them in received list(submitted_list). If during any of the pending completion callback, SW observes success then submit all from submitted_list to be scheduled in very next uf This is not possible with a very short pipeline where there is no other packet that could complete. And even if it is possible in some cases: why should submitted packets be delayed at all? The UAC driver wants all packets at their correct position in time (or else to be dropped), and the UVC driver does not want delays which could make it lose some data sent by the device. Regards, Clemens -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: How should we handle isochronous underruns?
Ming Lei wrote: On Mon, Jul 1, 2013 at 9:27 PM, Clemens Ladisch clem...@ladisch.de wrote: Ming Lei wrote: On Mon, Jul 1, 2013 at 9:06 PM, Clemens Ladisch clem...@ladisch.de wrote: Ming Lei wrote: I understand the latency is effected by packet count in one URB, and it shouldn't be related with URB count, This is true only when capturing. For playback, the latency is the length of the entire pipeline. For playback, every URB submitted is added into hw table immediately, then the data will be played to speaker. I don't understand why the latency is the entire pipeline. A submitted packet will be transmitted only after all the other packets in the pipeline have been transmitted. Yes, that is always true since EHCI HW will send out data(packet) to device one by one at the scheduled frame/uframe according to the order of URB submitting , so the USB audio driver can submit URBs in advance, can't it? Latency is defined as the time interval between the software generating the sample data, and the data actually being played. When the driver submits URBs in advance, latency increases accordingly. (The Linux USB API does not allow changing a URB's buffer after it has been submitted.) Also could you provide the typical time one URB in audio driver may span? Anything between one microframe and twenty milliseconds. Most software ends up using eight milliseconds. Regards, Clemens -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: How should we handle isochronous underruns?
On Sun, 30 Jun 2013, Clemens Ladisch wrote: Alan Stern wrote: [...] This means that the HCD will have trouble telling the difference between an underrun and a normal restart of a stopped I/O stream. In both cases it will see an URB being submitted to an empty queue. Here's where the URB_ISO_ASAP flag will be useful; if that flag is set then the URB is restarting a stopped stream, but if it isn't set then the pipeline experienced an underrun. Naturally, under normal circumstances this won't matter, because underruns shouldn't occur. But I know from experience that people try to push the latency as far down as they can, which increases the likelihood of underruns. There are several possible things the HCD could do when an underrun occurs: It could schedule the URB for the first time slot known to be available, even if that means skipping some time slots which the hardware might (or might not) be able to use. The word might shows an important point: due to software and hardware racing against each other, it is _not possible_ to know whether some URB to be submitted will actually still be on time or not. That's a very important point. On the other hand, sometimes it _is_ possible to know that a particular slot has already expired (for example, if a register read indicates the hardware has already reached frame N+1 then we know that frame N is over). It could try to schedule the URB for the next time slot after the last one used by the preceding URB, even if that time slot has already expired. For audio, it is important to _try_ to transmit the packet at the _correct_ time; a guarantee that the packet will be transmitted at _some_ time would be worthless. (This is what isochronous is all about.) That the error reporting is delayed from submission to the completion callback is no problem. Should the behavior change if the submission is _extremely_ late? The rogue driver leaving interrupts disabled for over 50 ms is a good example (and one that I actually encountered!). Even when that happens, would you prefer to require the audio driver to submit 50-ms worth of URBs before doing any real I/O? Alan Stern -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: How should we handle isochronous underruns?
On Mon, 1 Jul 2013, Ming Lei wrote: On Sun, Jun 30, 2013 at 11:02 PM, Alan Stern st...@rowland.harvard.edu wrote: Clement and Laurent: The two of you seem to be the people who make the most use of isochronous USB transfers. Since the ehci-hcd driver is being changed to use a tasklet for URB completion callbacks, it looks like I will need to reconsider how isochronous underruns get handled. Without using tasklet, the hard interrupt handler still can be delayed for some time, and switching to tasklet doesn't change the fact of the probable delay of URB handling. Yes. That wasn't the point. The problem will be more acute than in the past, because URB resubmissions will no longer be synchronous with URB completions, thanks to the tasklet. That is, in the current code, if the completion handler resubmits the URB, the resubmission occurs before the HCD finishes the completion callback. But in the new code, the HCD will simply hand the URB over to the tasklet, and the resubmission won't occur until some time later (when the tasklet wakes up and invokes the completion handler). As far as the HCD is concerned, the completion The delay should be very small(generally several microseconds) since isochronous URBs are completed in high priority tasklet. The fact that the delay is small doesn't matter -- what's important is that it will be 0 whereas now there is no delay between completion and resubmission (in fact, the resubmission occurs before the completion ends). I don't think the introduced tasklet delay is a problem for EHCI since per EHCI spec the maximum rate at which the host controller will issue interrupts is one microframe(125us), which means isochronous transfer completion can be reported to CPU with about 125us delay in hardware level. Irrelevant. Besides, the delay can be longer than 125 us if another interrupt is being handled. Thus, for example, even if the pipeline contains only a single URB, with the current code it will not become empty. But with the new code it will. If the load on the system is too high, the pipeline could empty out even if it normally contains two or more URBs. Single URB may not work well too when running complete() in hard irq context. It could work very well indeed, if the interval between URBs was larger than 1 ms. Alan Stern -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: How should we handle isochronous underruns?
On Mon, 1 Jul 2013, Pratyush Anand wrote: On 6/30/2013 8:32 PM, Alan Stern wrote: There are several possible things the HCD could do when an underrun occurs: I do not have much experience in working with ISOC host.But by the experience of device side I can say that, There could be problem irrespective of tasklet or irq context, only that tasklet would be more prone to it. Should not we have same approach in handling isoc packet in either of the cases? Of course we should. It could schedule the URB for the first time slot known to be available, even if that means skipping some time slots which the hardware might (or might not) be able to use. IMO, this approach is better. But, what should we call as first time slot known to be available. Current code calculates it as now (current time slot) + delta microframe, and delta is kept as fixed. However, different system works with different values of delta. IMO, This delta should be dynamic and software should update it on the basis of initial value and feedback (past failure/missed isoc experience). Clemens disagrees, and I accept his recommendation. It could try to schedule the URB for the next time slot after the last one used by the preceding URB, even if that time slot has already expired. There is no meaning of submitting an URB for expired time slot. In anyway, it is not going to be transferred and will further result in under-run/missed packet. That is not true. For example, suppose 2 time slots have already expired, but the submitted URB contains 5 packets. Even though it's too late for the first two packets, the last three can still be transferred. Alan Stern -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: How should we handle isochronous underruns?
On Mon, 1 Jul 2013, Laurent Pinchart wrote: That's correct. Unlike the UAC driver, the UVC driver doesn't care too much about exact timings. Avoiding packet loss is my main concern. As video frames are not delivered to userspace before they are fully transmitted, and given the large number of URBs required to transmit a video frame, I can submit more URBs (without reasonable limits) if that can help avoiding underruns. A couple of users have experienced underruns in the past due to a rogue driver disabling interrupts for tens of milliseconds. I don't think switching to tasklets will have a strong adverse effect, the 20ms buffer time should be large enough to avoid underruns in sane systems. What about error recovery for insane systems? If we do get a 50-ms gap in the data stream, what's the best way for the UVC driver to learn this has happens and attempt to carry on? Alan Stern -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: How should we handle isochronous underruns?
Hi Alan, On Monday 01 July 2013 11:08:33 Alan Stern wrote: On Mon, 1 Jul 2013, Laurent Pinchart wrote: That's correct. Unlike the UAC driver, the UVC driver doesn't care too much about exact timings. Avoiding packet loss is my main concern. As video frames are not delivered to userspace before they are fully transmitted, and given the large number of URBs required to transmit a video frame, I can submit more URBs (without reasonable limits) if that can help avoiding underruns. A couple of users have experienced underruns in the past due to a rogue driver disabling interrupts for tens of milliseconds. I don't think switching to tasklets will have a strong adverse effect, the 20ms buffer time should be large enough to avoid underruns in sane systems. What about error recovery for insane systems? If we do get a 50-ms gap in the data stream, what's the best way for the UVC driver to learn this has happens and attempt to carry on? When isochronous packets are lost video frames get corrupted. UVC doesn't provide sequence numbers for packets, so there's no way to know exactly how many packets have been lost. However, several synchronization methods are available for the driver to detect frame boundaries (namely an End-Of-Frame bit and a Frame ID bit that toggles for every frame in isochronous packet headers). The driver already handles those bits and marks video frames as complete when the EOF bit or an FID transition is detected. For uncompressed formats the driver then checks whether the received data size matches the frame size, and marks the frame as bad if it doesn't. That check can't be performed for compressed formats as the frame size is then variable. It would thus be helpful to receive a notification when a gap in the data stream is detected, through the URB status field for instance. The driver could then mark the frame being received as faulty and reset its state machine to resynchronize to the next frame boundary. -- Regards, Laurent Pinchart -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: How should we handle isochronous underruns?
On Mon, 1 Jul 2013, Laurent Pinchart wrote: What about error recovery for insane systems? If we do get a 50-ms gap in the data stream, what's the best way for the UVC driver to learn this has happens and attempt to carry on? When isochronous packets are lost video frames get corrupted. UVC doesn't provide sequence numbers for packets, so there's no way to know exactly how many packets have been lost. However, several synchronization methods are available for the driver to detect frame boundaries (namely an End-Of-Frame bit and a Frame ID bit that toggles for every frame in isochronous packet headers). The driver already handles those bits and marks video frames as complete when the EOF bit or an FID transition is detected. For uncompressed formats the driver then checks whether the received data size matches the frame size, and marks the frame as bad if it doesn't. That check can't be performed for compressed formats as the frame size is then variable. It would thus be helpful to receive a notification when a gap in the data stream is detected, through the URB status field for instance. The driver could then mark the frame being received as faulty and reset its state machine to resynchronize to the next frame boundary. One possibility is to set urb-status to -EXDEV if an underrun has caused the entire URB to be too late. Currently, under those conditions the HCD rejects the URB submission with a -EXDEV error code, which doesn't seem to be the right approach. Drivers don't expect isochronous _submissions_ to fail, although they are prepared to see failure statuses for isochronous _completions_. Of course, it has been true all along that the status fields in the URB's individual usb_iso_packet_descriptor structures indicate any errors. But HCDs tend to set urb-status to 0 always, for isochronous URBs. Is there any advantage to setting urb-status to -EXDEV, given that we already set urb-iso_frame_desc[i].status to -EXDEV for each i? (There's also an urb-error_count field, which reports how many of the isochronous packets got an error. It is hardly used; I think we could remove it.) This boils down to three possibilities for how to handle URBs that were submitted too late -- that is, so late that all the time slots for all the URB's packets are known to have expired already: (1) Reject the submission with -EXDEV -- this is what we do now. (2) Accept the submission, but have the URB complete immediately with urb-status and all the packet statuses set to -EXDEV. (3) Accept the submission, but have the URB complete immediately with urb-status set to 0 and all the packet statuses set to -EXDEV. For (1), the only way to recover is to submit an URB with URB_ISO_ASAP set. This is essentially the same as shutting down the stream and restarting it. For (2) and (3), the stream's next time slot value would be updated in the usual way. For example, if 10 slots had expired and the driver was submitting URBs containing 4 packets each, then the first and second URBs would complete right away with errors, and the first two packets of the third URB would get errors, but the last two packets of the third URB would be assigned to the two upcoming time slots and would be treated normally. Thus recovery would be automatic, at the cost of wasting two URBs. Since we expect underruns to be rare, maybe this is acceptable. With (2) or (3), the driver could also recover right away by setting the URB_ISO_ASAP flag on its next URB, but then synchronization would be lost. You wouldn't want to do this if synchronization matters. But if it doesn't, the driver could simply set URB_ISO_ASAP on every URB to avoid worrying about the problem -- the flag would have no effect in the absence of underruns. (The main reason the current code uses (1) is because (2) and (3) require completion to occur _during_ submission. Resubmissions would thus be nested, consuming excessive stack space, and there would be a deadlock if the completion handler tried to acquire a lock that was held during submission. When a tasklet is used for completions, none of these problems arise.) Any preference? Clemens, what do you think? Alan Stern -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: How should we handle isochronous underruns?
Alan Stern wrote: (1) Reject the submission with -EXDEV -- this is what we do now. (2) Accept the submission, but have the URB complete immediately with urb-status and all the packet statuses set to -EXDEV. (3) Accept the submission, but have the URB complete immediately with urb-status set to 0 and all the packet statuses set to -EXDEV. The audio driver currently ignores submission errors (because of a bug: the obvious way to stop the PCM stream would result in a deadlock; the tasklet completion might change this), and checks urb-status only to detect if the stream should be stopped (because of a 'normal' stop or unplugging.) So (2) or (3) (no matter which) is preferrable over (1). For (1), the only way to recover is to submit an URB with URB_ISO_ASAP set. This is essentially the same as shutting down the stream and restarting it. For (2) and (3), the stream's next time slot value would be updated in the usual way. For example, if 10 slots had expired and the driver was submitting URBs containing 4 packets each, then the first and second URBs would complete right away with errors, and the first two packets of the third URB would get errors, but the last two packets of the third URB would be assigned to the two upcoming time slots and would be treated normally. Thus recovery would be automatic, This is certainly preferrable over (1). at the cost of wasting two URBs. Since we expect underruns to be rare, maybe this is acceptable. And it's the only recovery mechanism that can preserves the timing/ synchronization of the overall stream. Regards, Clemens -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
How should we handle isochronous underruns?
Clement and Laurent: The two of you seem to be the people who make the most use of isochronous USB transfers. Since the ehci-hcd driver is being changed to use a tasklet for URB completion callbacks, it looks like I will need to reconsider how isochronous underruns get handled. The basic prolem is simple enough: What should the HCD do when an URB is submitted after an isochronous pipeline has emptied out? The problem will be more acute than in the past, because URB resubmissions will no longer be synchronous with URB completions, thanks to the tasklet. That is, in the current code, if the completion handler resubmits the URB, the resubmission occurs before the HCD finishes the completion callback. But in the new code, the HCD will simply hand the URB over to the tasklet, and the resubmission won't occur until some time later (when the tasklet wakes up and invokes the completion handler). As far as the HCD is concerned, the completion will already be finished. Thus, for example, even if the pipeline contains only a single URB, with the current code it will not become empty. But with the new code it will. If the load on the system is too high, the pipeline could empty out even if it normally contains two or more URBs. This means that the HCD will have trouble telling the difference between an underrun and a normal restart of a stopped I/O stream. In both cases it will see an URB being submitted to an empty queue. Here's where the URB_ISO_ASAP flag will be useful; if that flag is set then the URB is restarting a stopped stream, but if it isn't set then the pipeline experienced an underrun. Naturally, under normal circumstances this won't matter, because underruns shouldn't occur. But I know from experience that people try to push the latency as far down as they can, which increases the likelihood of underruns. There are several possible things the HCD could do when an underrun occurs: It could schedule the URB for the first time slot known to be available, even if that means skipping some time slots which the hardware might (or might not) be able to use. It could try to schedule the URB for the next time slot after the last one used by the preceding URB, even if that time slot has already expired. Something in between... What would be best for your purposes? Or do you have any different suggestions? Alan Stern -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: How should we handle isochronous underruns?
On Sun, Jun 30, 2013 at 11:02 PM, Alan Stern st...@rowland.harvard.edu wrote: Clement and Laurent: The two of you seem to be the people who make the most use of isochronous USB transfers. Since the ehci-hcd driver is being changed to use a tasklet for URB completion callbacks, it looks like I will need to reconsider how isochronous underruns get handled. Without using tasklet, the hard interrupt handler still can be delayed for some time, and switching to tasklet doesn't change the fact of the probable delay of URB handling. The basic prolem is simple enough: What should the HCD do when an URB is submitted after an isochronous pipeline has emptied out? I think some time-related data should be helpful for the discussion, for example, how long the isochronous pipeline may become empty in current audio/video driver(application) without any URB resubmit during the period? The problem will be more acute than in the past, because URB resubmissions will no longer be synchronous with URB completions, thanks to the tasklet. That is, in the current code, if the completion handler resubmits the URB, the resubmission occurs before the HCD finishes the completion callback. But in the new code, the HCD will simply hand the URB over to the tasklet, and the resubmission won't occur until some time later (when the tasklet wakes up and invokes the completion handler). As far as the HCD is concerned, the completion The delay should be very small(generally several microseconds) since isochronous URBs are completed in high priority tasklet. I don't think the introduced tasklet delay is a problem for EHCI since per EHCI spec the maximum rate at which the host controller will issue interrupts is one microframe(125us), which means isochronous transfer completion can be reported to CPU with about 125us delay in hardware level. will already be finished. Thus, for example, even if the pipeline contains only a single URB, with the current code it will not become empty. But with the new code it will. If the load on the system is too high, the pipeline could empty out even if it normally contains two or more URBs. Single URB may not work well too when running complete() in hard irq context. In UVC driver, looks it may submit at most 5 URBs which can include max 32 packets, so it will take about 5*32*125us(20ms) to make isoc pipeline empty suppose the interval is 1uF. But I don't know how USB audio driver uses URBs, and could it submit only one isoc URB to playback/record audio data? This means that the HCD will have trouble telling the difference between an underrun and a normal restart of a stopped I/O stream. In both cases it will see an URB being submitted to an empty queue. IMO, we should try to avoid the situation, and in UVC cases looks it is impossible to see an URB being submitted to an empty queue except for at the moment of starting streaming or interrupt disabled for extremely long. Here's where the URB_ISO_ASAP flag will be useful; if that flag is set then the URB is restarting a stopped stream, but if it isn't set then the pipeline experienced an underrun. Naturally, under normal circumstances this won't matter, because underruns shouldn't occur. But I know from experience that people try to push the latency as far down as they can, which increases the likelihood of underruns. There are several possible things the HCD could do when an underrun occurs: It could schedule the URB for the first time slot known to be available, even if that means skipping some time slots which the hardware might (or might not) be able to use. It could try to schedule the URB for the next time slot after the last one used by the preceding URB, even if that time slot has already expired. Something in between... What would be best for your purposes? Or do you have any different suggestions? Alan Stern -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Thanks, -- Ming Lei -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
