[alsa-devel] [PATCH] ASoC: Intel: Bsw: Read sst DSP DMA pointer after period elapse occurs

[Fang, Yang A]

> -----Original Message-----
> From: Fang, Yang A
> Sent: Wednesday, August 19, 2015 6:30 PM
> To: 'Mark Brown'
> Cc: 'lgirdwood at gmail.com'; 'alsa-devel at alsa-project.org';
> 'dgreid at chromium.org'; Nujella, Sathyanarayana;
> 'kevin.strasser at linux.intel.com'; Sripathi, Srinivas; Iriawan, Denny; Jain,
> Praveen K; Koul, Vinod; Mirche, Dinesh
> Subject: RE: [PATCH] ASoC: Intel: Bsw: Read sst DSP DMA pointer after
> period elapse occurs
> 
> 
> 
> > -----Original Message-----
> > From: Fang, Yang A
> > Sent: Wednesday, August 19, 2015 6:25 PM
> > To: 'Mark Brown'
> > Cc: lgirdwood at gmail.com; alsa-devel at alsa-project.org;
> > dgreid at chromium.org; Nujella, Sathyanarayana;
> > kevin.strasser at linux.intel.com; Sripathi, Srinivas; Iriawan, Denny;
> > Jain, Praveen K; Koul, Vinod; Mirche, Dinesh
> > Subject: RE: [PATCH] ASoC: Intel: Bsw: Read sst DSP DMA pointer after
> > period elapse occurs
> >
> >
> >
> > > -----Original Message-----
> > > From: Mark Brown [mailto:broonie at kernel.org]
> > > Sent: Tuesday, August 18, 2015 5:35 PM
> > > To: Fang, Yang A
> > > Cc: lgirdwood at gmail.com; alsa-devel at alsa-project.org;
> > > dgreid at chromium.org; Nujella, Sathyanarayana;
> > > kevin.strasser at linux.intel.com; Sripathi, Srinivas; Iriawan, Denny;
> > > Jain, Praveen K; Koul, Vinod; Mirche, Dinesh
> > > Subject: Re: [PATCH] ASoC: Intel: Bsw: Read sst DSP DMA pointer
> > > after period elapse occurs
> > >
> > > On Tue, Aug 18, 2015 at 05:33:33PM +0000, Fang, Yang A wrote:
> > >
> > > > > > We should not read sst hw_ptr and pcm_delay in
> > > > > > sst_platform_pcm_pointer Since it will be ricky if
> > > > > > sst_platform_pcm_pointer is called while dsp is updating the
> > > > > > timestamp.Now read sst hw_ptr after period elapse interrupt
> occurs.
> > >
> > > > > So I guess there's still some risk that we could run into
> > > > > problems here if we take too long to service the interrupt
> > > > > and/or the period is very short?  Is there anything we can do to
> improve that?
> > >
> > > >  I think before and after the patch. There is no difference in
> > > > term of handling the period elapse interrupt . because before the
> > > > patch sst_period_elapsed calls the snd_pcm_period_elapsed which in
> > > > turn calling the sst_platform_pcm_pointer which calls the
> > stream_read_tstamp.
> > >
> > > I'm not sure I understand what the patch fixes then?  My concern is
> > > that we're just moving the timing around which changes but doesn't
> > > address the race condition.
> >
> > We are seeing the issue during the long time playback.
> > The ring_buffer_counter (part of snd_sst_tstamp  struct )supposes only
> > increasing by the firmware  once firmware fetches  one period size of data.
> >  It will update the ring_buffer_counter in the mailbox and trigger a
> > period elapse interrupt.  We have found the ring_buffer_counter got
> > decreased during long time playback if we  call stream_read_tstamp to
> > read mailbox  in sst_platform_pcm_pointer function.
> >
> > There are two cases sst_platform_pcm_pointer will be getting called.
> > One is called by sst_period_elapsed function.
> >  The second case is triggered by the userspace ioctl . The second case
> > supposes reading the same case  ring_buffer_counter  value as last
> > read when period elapse interrupt  occurs . However we saw it somehow
> > decreases.This causes audio stutter.
> 
> So I moved  stream_read_tstamp  to read the mailbox sst_period_elapsed to
> make sure read after receving the interrupt and return a cached value if
> other place tried to read. you are right this is indeed a workaround patch for
> the time being
Hi Mark,
Could you please let me know your further comment? 
Thanks,
Yang