Re: [PATCH 2/2] ehci: Respect IST when scheduling new iTDs.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Sun, Oct 11, 2009 at 7:31 AM, Alan Stern <stern@xxxxxxxxxxxxxxxxxxx> wrote:
> On Fri, 9 Oct 2009, Sarah Sharp wrote:
>
>> @@ -1354,6 +1354,7 @@ iso_stream_schedule (
>>               period <<= 3;
>>
>>       now = ehci_readl(ehci, &ehci->regs->frame_index) % mod;
>> +     next = now + ehci->i_thresh;
>>
>>       /* when's the last uframe this urb could start? */
>>       max = now + mod;
>> @@ -1365,13 +1366,15 @@ iso_stream_schedule (
>>        */
>>       if (likely (!list_empty (&stream->td_list))) {
>>               start = stream->next_uframe;
>> -             if (start < now)
>> +             if (start < next)
>>                       start += mod;
>>
>>               /* Fell behind (by up to twice the slop amount)? */
>>               if (start >= max - 2 * 8 * SCHEDULE_SLOP)
>>                       start += period * DIV_ROUND_UP(
>> -                                     max - start, period) - mod;
>> +                                     max - start + ehci->i_thresh,
>> +                                     period)
>> +                             - mod;
>>
>>               /* Tried to schedule too far into the future? */
>>               if (unlikely((start + sched->span) >= max)) {
>
> Sorry, what I wrote before wasn't quite right.  This computation is
> tricky.  To check whether start is too small, you have to fit it into
> the interval from next to (next + mod - 1).  But then to check whether
> it is too big, you have to fit it into the interval from now to (now +
> mod - 1).  And then in the end, you need to assign stream->next_uframe
> a value between 0 and (mod - 1).
>
> Since mod is always a power of two, the easiest approach may be
> something like this:
>
>        next = now + ehci->i_thread;
>                ...
>                start = stream->next_uframe;
>
>                /* Fell behind (by up to twice the slop amount)? */
>                if ((start - next) & (mod - 1) >=
>                                mod - 2 * 8 * SCHEDULE_SLOP)
>                        start += period * DIV_ROUND_UP(
>                                        (next - start) & (mod - 1),
>                                        period);
>
>                /* Tried to schedule too far into the future? */
>                if (unlikely(((start - now) & (mod - 1)) + sched->span
>                                >= max)) { ...
>                }
>                stream->next_uframe = start % mod;
>                goto ready;
>
> Alan Stern
>
Hi, I hate to throw another issue into this, but I have done userspace
audio drivers via usb devices in the past and I think Linux does it
wrong. Since you are dealing with scheduling (although not with ohci
and uhci which have similar issues), I think it is germane.

If you try a test and slow a host down that is feeding audio, Linux
will put in a gap (large pop) and then continue with the data. But a
persons ears are very sensitive to the music timing and it seems like
the system stutters. This means that the music is going along and then
there is a pop, a pause and then it picks up again with the original
music and misses the beat. I believe it should be music, pop, gap, and
then the music continues as if there was no gap.

I have looked at the USB 2.0 spec and while somewhat vague, section
5.12 it says:
<quote>
In an isochronous communication system, the transmitter and receiver
must remain time- and datasynchronized
to deliver data robustly. The USB does not support transmission retry
of isochronous data so
that minimal bandwidth can be allocated to isochronous transfers and
time synchronization is not lost due to
a retry delay. However, it is critical that a USB isochronous
transmitter/receiver pair still remain
synchronized both in normal data transmission cases and in cases where
errors occur on the bus. </quote>

What I think this says and what I think matches the definition of
isochronous transfers (constant time, not guaranteed delivery) is that
1) for an iso transfer with either startframe = nextframe
2) or with the iso_asap option set,
Linux should discard whatever frames were "late" and could not be
scheduled ie mark them done. People who do iso gadgets know that the
data is not guaranteed, but they DO want the constant timing. Delaying
a transfer by 10+ (u)frames because the host is slow for whatever
reason violates the synchronization desired by human ears, eyes and I
believe the USB 2.0 spec.

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

[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux