Rockbox mail archiveSubject: Re: Classic holdswitch polling
Re: Classic holdswitch polling
From: Michael Sparmann <theseven_at_gmx.net>
Date: Sat, 10 Mar 2012 22:55:36 +0100
>> Sorry, i didn't explain it properly, was referring to protect I2C read
>> and write functions using a critical section instead of the actual
>> mutex, then a thread accessing I2C cannot be interrupted by the tick
> That's a very bad thing to do. I2C shouldn't keep interrupts masked that
> long or latency is horrible for audio playback. Latency should be
> effectively non-existent if possible. Doing that stuff on AS3525 was
> causing recording glitches, especially when not using DMA (due to
> interrupt frequency making it more likely).
>> This is a small modification affecting only target code but not valid
>> for a definitive solution, everybody agrees slow polling must not be
>> done inside critical sections. By the way, some info about I2C timing
>> using default clocks:
>> time to transfer 1 byte + ACK (9 bits total) = 43-45 usecs
>> i2c_read(N registers) = (3.3 + N) * 45 usecs approx.
>> holdswitch polling = 200 usecs approx.
>> get date = 500 usecs approx.
> Like I said: horrible thing to do. I think we agree, yay?
> A queueable driver can make problems like that disappear.
That isn't going to get any better as long as the I2C request is being
handled in the ISR, which it needs to be if the hold switch tick task
waits for the I2C access to finish, even if I2C itself is async. We are
in IRQ mode while the tick task is running, so other, more important,
IRQs can't preempt this.
But this also means that we *are* currently doing IRQ lockouts of that
duration regularly, so they apparently aren't a problem (yet). DMA is
double buffered these days, so this could only cause issues for very
very small playback buffers.
On the other hand this might be responsible for quite a bit of CPU load.
Let's say 300us every 10ms, that's a whopping 3% of CPU load for the
hold switch polling alone! Does it really need to be done that often?
Might cause quite some battery drain.