On Thu, 2010-04-08 at 15:13 +0200, Sascha Hauer wrote:
On Thu, Apr 08, 2010 at 10:53:25AM +0100, Mark Brown wrote:
On Thu, Apr 08, 2010 at 11:31:26AM +0200, Sascha Hauer wrote:
- /* Restart the timer; if we didn't report we'll run on the next tick */
- add_timer(&iprtd->timer);
- hrtimer_forward_now(hrt, ns_to_ktime(iprtd->poll_time_ns));
Hrm, this looks like it's going to have an issue with clock drift - we're now unconditionally advancing the timer every period, even if the data transfer hasn't pushed through a period of data. This will cause problems on lengthy playbacks (and shorter ones if the clocks are sufficiently out of sync).
We are calling snd_pcm_period_elapsed when at least one period is over. As I see it the worst thing that could happen is that we have not transfered enough data for one period in the timer callback and thus we call snd_pcm_period_elapsed in the next timer callback, so about one period too late, but the comment in sound/core/pcm_lib.c says:
Even if more than one periods have elapsed since the last call, you have to call this only once.
So I think this should be save.
Yeah agreed, as long as it's called at least once then we should be safe here.
Btw, how does this driver behave under moderate -> heavy IO load. I do remember the SDMA based driver occasionally starved the SSI FIFO under moderate IO load (i.e. aplay reading wav file over NFS).
All Acked-by: Liam Girdwood lrg@slimlogic.co.uk
Liam