On Mon, May 25, 2009 at 9:15 AM, Jon Smirl jonsmirl@gmail.com wrote:
On Mon, May 25, 2009 at 2:16 AM, Grant Likely grant.likely@secretlab.ca wrote:
On Sun, May 24, 2009 at 7:38 PM, Jon Smirl jonsmirl@gmail.com 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.