Messages in this thread | | | Subject | Re: [PATCH] ASoC: Intel: Atom: use hardware counter to update hw_ptr | From | Pierre-Louis Bossart <> | Date | Mon, 27 Jul 2020 09:09:33 -0500 |
| |
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 <brent.lu@intel.com> > --- > 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, > fw_tstamp->ring_buffer_counter); > dev_dbg(ctx->dev, "mrfld hardware_counter %llu in bytes\n", > fw_tstamp->hardware_counter); > - 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; >
| |