FS#12452 - Reduce interrupt latency for AMS v1 and v2

Attached to Project: Rockbox
Opened by Michael Sevakis (MikeS) - Wednesday, 14 December 2011, 03:12 GMT
Last edited by Michael Sevakis (MikeS) - Sunday, 08 January 2012, 22:34 GMT
Task Type Patches
Category Operating System/Drivers
Status Closed
Assigned To Michael Sevakis (MikeS)
Operating System Sansa AMSv1
Severity Low
Priority Normal
Reported Version Release 3.9
Due in Version Undecided
Due Date Undecided
Percent Complete 100%
Votes 0
Private No


These devices can spend alot of time in ISR's waiting to perform operations like reading the DBOP or waiting for it to go idle. AMSv2 has PMU read and write functions that keep interrupts disabled for a full two I2C transfer. This can prevent audio interrupts from being serviced in a timely manner resulting in occasional dropouts for playback or small bits of lost data for recording. The solutions are 1) Enable stack interrupts on all targets 2) Rewrite ascodec_read/write_pmu to disable interrupts only to place two consecutive requests in the queue but wait with interrupts enabled for completion.

Interrupts callbacks are in SVC mode on its own stack. I'm not certain what size is reasonable. Theoretically all enabled interrupts could happen up to the highest level, potentially consuming quite a bit of stack in the worst case and 1/2 the current IRQ stack just to save context.

I also disabled semaphore_release context switch for the moment. A different scheme that explicitely checks for thread execution context will be needed as interrupts being disabled won't be a way to discern whether or not interrupt context is executing.

One interesting issue on Fuze v2 that I have not been able to figure out is that if the GPIOA ISR is interrupted by a higher-priority interrupt, execution ends up in all sorts of strange places, with the concomitant fault screen or freeze. Unless the cause of that is remedied, GPIOA's ISR will have to be the highest priority or the enabling of IRQ during ISR selected on a per-handler basis. GPIOA's ISR is very short so shouldn't hurt latency very much.

Surprisingly, no other stacking I checked by running recording and delaying for a few thousand microseconds inside the ISR in order to force audio interrupts on top of another produced any sort of fault.
This task depends upon

Closed by  Michael Sevakis (MikeS)
Sunday, 08 January 2012, 22:34 GMT
Reason for closing:  Accepted
Additional comments about closing:  r31642. No point in continuing with this in this venue. :-)
Comment by Michael Sevakis (MikeS) - Thursday, 15 December 2011, 03:37 GMT
Sync to r31264.
Comment by Mark Wilhelmsson (markanini) - Sunday, 18 December 2011, 22:00 GMT
In the latest builds I've experienced some tiny dropouts in recordings. I'm guessing this patch is related to that? Can't say I noticed anything during playback, codecs I use are relatively CPU friendly though.
Any specific conditions you'd want me to test this patch for Mike?
Comment by Michael Sevakis (MikeS) - Monday, 19 December 2011, 09:07 GMT
Yes, it is meant to address that sort of thing. It shouldn't have any trouble in terms of raw CPU power keeping up. There probably isn't much more to do than to apply the patch, make a build with it and record in the way you usually would that causes the occasional dropout.

Testing by recording a sine wave is most telling since missing samples are easily seen since the cycle is truncated and distinguishable from amplitude glitches. Some before and after WAV files would be good to link to on here.

If you need an updated patch because it won't apply to a newer revision, just give word.
Comment by Mark Wilhelmsson (markanini) - Monday, 19 December 2011, 22:26 GMT
I'll take you up on the resynced patch offer. "Hunk #1 FAILED at 161" even when I checked out r31264 using -r. Here's a sine recording using r31323: Seems the dropouts occur during the first 15 secs.
Comment by Michael Sevakis (MikeS) - Tuesday, 20 December 2011, 06:51 GMT
NP! Thanks for the recording too.
Comment by Michael Sevakis (MikeS) - Tuesday, 20 December 2011, 07:05 GMT
The waveform you provided was quite distorted and I couldn't see well what's going on. I have this one that I use. Make sure the recording is perfectly clean, not clipped or anything.
Comment by Michael Sevakis (MikeS) - Saturday, 07 January 2012, 15:56 GMT
Argh, ought I just commit this (after cleaning it up) and hope for the best? There are still areas that only this will guarantee the needed response for PCM. I did check ISR requirements on all targets and nothing stood out as dangerous to have be preempted.
Comment by Michael Sevakis (MikeS) - Saturday, 07 January 2012, 20:30 GMT
Update this b!07c#.

Only thing I'm not certain about would be the final SVC stack size, which I just left at 256 words for now.