-----Original Message----- From: Fang, Yang A Sent: Wednesday, August 19, 2015 6:30 PM To: 'Mark Brown' Cc: 'lgirdwood@gmail.com'; 'alsa-devel@alsa-project.org'; 'dgreid@chromium.org'; Nujella, Sathyanarayana; 'kevin.strasser@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@gmail.com; alsa-devel@alsa-project.org; dgreid@chromium.org; Nujella, Sathyanarayana; kevin.strasser@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@kernel.org] Sent: Tuesday, August 18, 2015 5:35 PM To: Fang, Yang A Cc: lgirdwood@gmail.com; alsa-devel@alsa-project.org; dgreid@chromium.org; Nujella, Sathyanarayana; kevin.strasser@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