Hi Sarah,
On Mon, Oct 19, 2009 at 2:57 PM, Sarah Sharp
<sarah.a.sharp@xxxxxxxxxxxxxxx> wrote:
> Most drivers will never cause this behavior, because they use multiple
> URBs with multiple packets to keep the bus busy. If you limit the number
> of URBs to one, you may be able to hit this bug.
>
Or if you run a heavily loaded system. (embedded?) Remember Linux is
not real time, just best effort. Periodic transfers are inherently a
race between cpu and the ehci controller. However the ?hci controller
will maintain a constant clock out to peripherals -ie SOF.
> Make sure the EHCI driver does not schedule full-speed transfers within
> the IST. Make sure that when we fall behind the current microframe plus
> IST, we schedule the new transfer at the next periodic interval after the
> IST.
>
Good.
<snip>
> Don't change the scheduling for new transfers, since the schedule slop will
> always be greater than the IST. Allow high speed isochronous transfers to
> be scheduled within the IST, since this doesn't trigger the Intel chipset
> bug.
>
> Make sure that if the host caches the full frame, the EHCI driver's
> internal isochronous threshold (ehci->i_thresh) is set to
> 8 microframes + 2 microframes wiggle room. This is similar to what is done in
> the case where the host caches less than the full frame.
>
> Signed-off-by: Sarah Sharp <sarah.a.sharp@xxxxxxxxxxxxxxx>
> Cc: Alan Stern <stern@xxxxxxxxxxxxxxxxxxx>
> ---
>
>
> if (HCC_ISOC_CACHE(hcc_params)) // full frame cache
> - ehci->i_thresh = 8;
> + ehci->i_thresh = 2 + 8;
> else // N microframes cached
> ehci->i_thresh = 2 + HCC_ISOC_THRES(hcc_params);
>
I have no problem with having a minimum "lead" required on a ehci race.
> /* when's the last uframe this urb could start? */
> max = now + mod;
> @@ -1365,16 +1366,29 @@ iso_stream_schedule (
> */
> if (likely (!list_empty (&stream->td_list))) {
> start = stream->next_uframe;
> - if (start < now)
> - start += mod;
>
> /* Fell behind (by up to twice the slop amount)? */
> - if (start >= max - 2 * SCHEDULE_SLOP)
> - start += period * DIV_ROUND_UP(
> - max - start, period) - mod;
> + /* For HS devices, allow scheduling within the IST.
> + * For FS/LS devices, don't. (Work around for ICH9 bug.)
> + */
> + if (stream->highspeed) {
> + if (start < now)
> + start += mod;
> +
This is the problem. It completely violates the definition of
isochronous. Periodic endpoints are guaranteed bandwidth and
timeliness. There is no guarantee about delivery - in fact iso is not
guaranteed to be delivered safely at all. Inserting big gaps in a
transfer because of unrelated processor loads or chip bugs is wrong.
If an app needs guaranteed good data, it should not be using iso
transfers. An app that needs delivery guarantees should use bulk or
maybe high bandwidth interrupt transfers.
I think you should mark the missed transfers as failed and just skip
them. This would maintain the "chronous" aspect of the transfers.
Delaying 80 uframes means delaying up to 80*1024*3=245760 bytes that
were supposed to be timely on isochronous transfers. FS audio delays
of 10 msec is very irritating and noticable.
Alan made a comment about excepting the ASAP scheduling, but since
that is the only practical scheduling method for applications, I think
all missed schedule windows should just be returned as "done" with an
error per iso packet transfer within the urb.
Regards, Steve
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
