Re: [PATCH] ASoC: Intel: Atom: use hardware counter to update hw_ptr

From: Pierre-Louis Bossart
Date: Mon Jul 27 2020 - 10:09:49 EST

On 7/26/20 11:08 AM, Brent Lu wrote:
The ring buffer counter runs faster than hardware counter if the
period size in hw_param is larger than 240. Although the differce is
not much (around 2k frames), it causes false underrun in CRAS
sometimes because it's using 256 frames as period size in hw_param.

All the Atom firmware assumes data chunks in multiples of 1ms (typically 5, 10 or 20ms). I have never seen anyone use 256 frames, that's asking for trouble really.

it's actually the same with Skylake and SOF in most cases.

Is this a 'real' problem or a problem detected by the Chrome ALSA compliance tests, in the latter case that would hint at a too generic value of min_period.

Using the hardware counter could provide precise hw_ptr to user space
and avoid the false underrun in CRAS.

Signed-off-by: Brent Lu <>
sound/soc/intel/atom/sst/sst_drv_interface.c | 15 +++------------
1 file changed, 3 insertions(+), 12 deletions(-)

diff --git a/sound/soc/intel/atom/sst/sst_drv_interface.c b/sound/soc/intel/atom/sst/sst_drv_interface.c
index 7624953..1949ad9 100644
--- a/sound/soc/intel/atom/sst/sst_drv_interface.c
+++ b/sound/soc/intel/atom/sst/sst_drv_interface.c
@@ -485,7 +485,6 @@ static inline int sst_calc_tstamp(struct intel_sst_drv *ctx,
struct snd_pcm_substream *substream,
struct snd_sst_tstamp *fw_tstamp)
- size_t delay_bytes, delay_frames;
size_t buffer_sz;
u32 pointer_bytes, pointer_samples;
@@ -493,22 +492,14 @@ static inline int sst_calc_tstamp(struct intel_sst_drv *ctx,
dev_dbg(ctx->dev, "mrfld hardware_counter %llu in bytes\n",
- if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
- delay_bytes = (size_t) (fw_tstamp->ring_buffer_counter -
- fw_tstamp->hardware_counter);
- else
- delay_bytes = (size_t) (fw_tstamp->hardware_counter -
- fw_tstamp->ring_buffer_counter);
- delay_frames = bytes_to_frames(substream->runtime, delay_bytes);
buffer_sz = snd_pcm_lib_buffer_bytes(substream);
- div_u64_rem(fw_tstamp->ring_buffer_counter, buffer_sz, &pointer_bytes);
+ div_u64_rem(fw_tstamp->hardware_counter, buffer_sz, &pointer_bytes);
pointer_samples = bytes_to_samples(substream->runtime, pointer_bytes);
- dev_dbg(ctx->dev, "pcm delay %zu in bytes\n", delay_bytes);
info->buffer_ptr = pointer_samples / substream->runtime->channels;
+ info->pcm_delay = 0;

and that seems also wrong? Why would the delay be zero?

- info->pcm_delay = delay_frames;
dev_dbg(ctx->dev, "buffer ptr %llu pcm_delay rep: %llu\n",
info->buffer_ptr, info->pcm_delay);
return 0;