On Mon, May 25, 2009 at 9:15 AM, Jon Smirl <jonsmirl@xxxxxxxxx> wrote:
> On Mon, May 25, 2009 at 2:16 AM, Grant Likely <grant.likely@xxxxxxxxxxxx> wrote:
>> On Sun, May 24, 2009 at 7:38 PM, Jon Smirl <jonsmirl@xxxxxxxxx> wrote:
>>> +static unsigned short psc_ac97_read(struct snd_ac97 *ac97, unsigned short reg)
>>> +{
>>> + int timeout;
>>> + unsigned int val;
>>> +
>>> + spin_lock(&psc_dma->lock);
>>> +
>>> + /* Wait for it to be ready */
>>> + timeout = 1000;
>>> + while ((--timeout) && (in_be16(&psc_dma->psc_regs->sr_csr.status) &
>>> + MPC52xx_PSC_SR_CMDSEND))
>>> + udelay(10);
>>
>> Holy unbounded latency Batman! This code waits up to 10ms for a register read!
>>
>> I hate spinning, but if it must be done; I'd like to see it small.
>> What is the worst case latency? 125us for 8000Hz bus speed? If you
>> must spin; can a cpu_relax() be used instead of the udelay() while
>> watch the timebase? Timur recently posted a patch which makes this
>> easier.
>>
>> http://patchwork.ozlabs.org/patch/27414/
>>
>> They *should* be appearing in Ben's -next branch soon.
>
>
> The link always runs at 12.288Mhz. Each frame is 256 bits. Worst case
> you wait for two frames, 42us. If it doesn't respond in 42us the codec
> clock is not ticking ( a recurring problem I am running into). These
> codecs may be going into a sleep mode I don't understand, but this is
> not the right place to try and wake them up. I'll lower the retry
> counts to 10 instead of 1000.
That still leaves the problem of unecessarily burning time. udelay
shouldn't be passed any value larger than 1. In fact, I think udelay
itself is too coarse grained. Plus, I'd rather see the timebase used
as the exit condition (as mentioned in previous email).
> I played around with implementing this on a kernel thread with
> interrupts. It can be done but the code is a lot more complex.
A kernel thread is definitely the wrong approach. However, if this is
non-atomic context and IRQs are available, then a wait queue can be
used. 42us is about 16k processor clocks. I'm not sure what the IRQ
and scheduling overhead is so I don't know whether it would be a net
gain or loss in performance. However, it would be a net gain in worst
case latency.
> BTW, 8000Hz is implemented by slot stuffing. The link always runs at
> 12.288Mhz. The DACs are double buffered. When a sample is transfered
> between buffers it sets a bit on the link back to the host, and the
> host sends the next sample in the appropriate slot.
ok.
g.
--
Grant Likely, B.Sc., P.Eng.
Secret Lab Technologies Ltd.
_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
