Re: [PATCH 3/3] imx-ssi: Use a hrtimer in FIQ mode

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxxxxxxxx>

Liam

-- 
Freelance Developer, SlimLogic Ltd
ASoC and Voltage Regulator Maintainer.
http://www.slimlogic.co.uk

_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

[Index of Archives]     [ALSA User]     [Linux Audio Users]     [Kernel Archive]     [Asterisk PBX]     [Photo Sharing]     [Linux Sound]     [Video 4 Linux]     [Gimp]     [Yosemite News]

  Powered by Linux