On Thu, Apr 08, 2010 at 02:30:13PM +0100, Liam Girdwood wrote:
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).
aplay works fine with files over nfs and survives moderate hackbench attacks. I do not check the underrun bit though so there might be glitches I did not hear.
Sascha