lkml.org 
[lkml]   [2020]   [Jul]   [27]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH] ASoC: Intel: Atom: use hardware counter to update hw_ptr
From
Date


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;
>

\
 
 \ /
  Last update: 2020-07-27 16:40    [W:0.075 / U:1.204 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site