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

[Liam Girdwood <lrg@slimlogic.co.uk>]

On Thu, 2010-04-08 at 15:03 +0100, Mark Brown wrote:
> On Thu, Apr 08, 2010 at 03:13:53PM +0200, Sascha Hauer wrote:
> > On Thu, Apr 08, 2010 at 10:53:25AM +0100, Mark Brown wrote:
> 
> > > 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.
> 
> The risk here is fragility caused by delaying the notification.
> 
> The issue is that if the period is long enough and/or the application is
> running too close to where the hardware is then the delay of what's
> likely to be almost an entire period is likely to glitch - you can end
> up with a situation where you're essentially notifying immediately
> before the end of the next period rather than immediately after the end
> of the current period.
> 
> Consider, for example what happens if the hrtimer runs slightly faster
> than the audio clock.  Eventually you'll get:
> 
>  - The timer runs, period A has a frame or two to go still.
>  - Period A completes, period A' begins.
>  - The timer fires again - period A is notified, period A' is almost
>    complete.
> 
> In this situation the completion notifications lag the actual completion
> of the frame by almost a period (though this lag will go down over time).
> Most of the time this will still be fine and everything will work OK but
> I would expect this to cause issues with some applications, especially
> if they're trying to be latency sensitive.

I agree this will drift but I _think_ we are probably workable here for
most latency sensitive apps as long as the worst case notification delay
is 1 period, pointer() is accurate to a few frames and there is a least
> 3 or 4 period buffers available for use in our driver buffer (for each
direction). 

Currently the min periods for this driver is 2, so I would probably
change to 4 to reduce drift related glitches.

Liam

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