On 11/16/21 1:45 AM, Kuninori Morimoto wrote:
> From: Kuninori Morimoto <kuninori.morimoto.gx@xxxxxxxxxxx>
>
> Current soc_pcm_pointer() is manually calculating
> both CPU-DAI's max delay (= A)
> and Codec-DAI's max delay (= B).
>
> static snd_pcm_uframes_t soc_pcm_pointer(...)
> {
> ...
> ^ for_each_rtd_cpu_dais(rtd, i, cpu_dai)
> (A) cpu_delay = max(cpu_delay, ...);
> v delay += cpu_delay;
>
> ^ for_each_rtd_codec_dais(rtd, i, codec_dai)
> (B) codec_delay = max(codec_delay, ...);
> v delay += codec_delay;
>
> runtime->delay = delay;
> ...
> }
>
> Current soc_pcm_pointer() and the total delay calculating
> is not readable / difficult to understand.
>
> This patch update snd_soc_dai_delay() to snd_soc_pcm_dai_delay(),
> and calcule both CPU/Codec delay in one function.
When the hw_ptr is updated, the avail/delay value are also modified.
see the diagram in
https://www.kernel.org/doc/html/latest/sound/designs/timestamping.html?highlight=pcm%20timestamping
I would think it's more accurate to update the delay information while
dealing with the hw_ptr update, no?
>
> Link: https://lore.kernel.org/r/87fszl4yrq.wl-kuninori.morimoto.gx@xxxxxxxxxxx
> Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx@xxxxxxxxxxx>
> ---
> include/sound/soc-dai.h | 4 ++--
> sound/soc/soc-dai.c | 40 ++++++++++++++++++++++++++++------------
> sound/soc/soc-pcm.c | 18 ++----------------
> 3 files changed, 32 insertions(+), 30 deletions(-)
>
> diff --git a/include/sound/soc-dai.h b/include/sound/soc-dai.h
> index 0dcb361a98bb..5d4dd7c5450b 100644
> --- a/include/sound/soc-dai.h
> +++ b/include/sound/soc-dai.h
> @@ -208,8 +208,6 @@ int snd_soc_dai_startup(struct snd_soc_dai *dai,
> struct snd_pcm_substream *substream);
> void snd_soc_dai_shutdown(struct snd_soc_dai *dai,
> struct snd_pcm_substream *substream, int rollback);
> -snd_pcm_sframes_t snd_soc_dai_delay(struct snd_soc_dai *dai,
> - struct snd_pcm_substream *substream);
> void snd_soc_dai_suspend(struct snd_soc_dai *dai);
> void snd_soc_dai_resume(struct snd_soc_dai *dai);
> int snd_soc_dai_compress_new(struct snd_soc_dai *dai,
> @@ -238,6 +236,8 @@ int snd_soc_pcm_dai_trigger(struct snd_pcm_substream *substream, int cmd,
> int rollback);
> int snd_soc_pcm_dai_bespoke_trigger(struct snd_pcm_substream *substream,
> int cmd);
> +void snd_soc_pcm_dai_delay(struct snd_pcm_substream *substream,
> + snd_pcm_sframes_t *cpu_delay, snd_pcm_sframes_t *codec_delay);
>
> int snd_soc_dai_compr_startup(struct snd_soc_dai *dai,
> struct snd_compr_stream *cstream);
> diff --git a/sound/soc/soc-dai.c b/sound/soc/soc-dai.c
> index 3db0fcf24385..6078afe335f8 100644
> --- a/sound/soc/soc-dai.c
> +++ b/sound/soc/soc-dai.c
> @@ -453,18 +453,6 @@ void snd_soc_dai_shutdown(struct snd_soc_dai *dai,
> soc_dai_mark_pop(dai, substream, startup);
> }
>
> -snd_pcm_sframes_t snd_soc_dai_delay(struct snd_soc_dai *dai,
> - struct snd_pcm_substream *substream)
> -{
> - int delay = 0;
> -
> - if (dai->driver->ops &&
> - dai->driver->ops->delay)
> - delay = dai->driver->ops->delay(substream, dai);
> -
> - return delay;
> -}
> -
> int snd_soc_dai_compress_new(struct snd_soc_dai *dai,
> struct snd_soc_pcm_runtime *rtd, int num)
> {
> @@ -693,6 +681,34 @@ int snd_soc_pcm_dai_bespoke_trigger(struct snd_pcm_substream *substream,
> return 0;
> }
>
> +void snd_soc_pcm_dai_delay(struct snd_pcm_substream *substream,
> + snd_pcm_sframes_t *cpu_delay,
> + snd_pcm_sframes_t *codec_delay)
> +{
> + struct snd_soc_pcm_runtime *rtd = asoc_substream_to_rtd(substream);
> + struct snd_soc_dai *dai;
> + int i;
> +
> + /*
> + * We're looking for the delay through the full audio path so it needs to
> + * be the maximum of the DAIs doing transmit and the maximum of the DAIs
> + * doing receive (ie, all CPUs and all CODECs) rather than just the maximum
> + * of all DAIs.
> + */
> +
> + /* for CPU */
> + for_each_rtd_cpu_dais(rtd, i, dai)
> + if (dai->driver->ops &&
> + dai->driver->ops->delay)
> + *cpu_delay = max(*cpu_delay, dai->driver->ops->delay(substream, dai));
> +
> + /* for Codec */
> + for_each_rtd_codec_dais(rtd, i, dai)
> + if (dai->driver->ops &&
> + dai->driver->ops->delay)
> + *codec_delay = max(*codec_delay, dai->driver->ops->delay(substream, dai));
> +}
> +
> int snd_soc_dai_compr_startup(struct snd_soc_dai *dai,
> struct snd_compr_stream *cstream)
> {
> diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c
> index 4309e6131c40..b1ef4d02511f 100644
> --- a/sound/soc/soc-pcm.c
> +++ b/sound/soc/soc-pcm.c
> @@ -1084,15 +1084,11 @@ static int soc_pcm_trigger(struct snd_pcm_substream *substream, int cmd)
> */
> static snd_pcm_uframes_t soc_pcm_pointer(struct snd_pcm_substream *substream)
> {
> - struct snd_soc_pcm_runtime *rtd = asoc_substream_to_rtd(substream);
> - struct snd_soc_dai *cpu_dai;
> - struct snd_soc_dai *codec_dai;
> struct snd_pcm_runtime *runtime = substream->runtime;
> snd_pcm_uframes_t offset = 0;
> snd_pcm_sframes_t delay = 0;
> snd_pcm_sframes_t codec_delay = 0;
> snd_pcm_sframes_t cpu_delay = 0;
> - int i;
>
> /* clearing the previous total delay */
> runtime->delay = 0;
> @@ -1102,19 +1098,9 @@ static snd_pcm_uframes_t soc_pcm_pointer(struct snd_pcm_substream *substream)
> /* base delay if assigned in pointer callback */
> delay = runtime->delay;
>
> - for_each_rtd_cpu_dais(rtd, i, cpu_dai) {
> - cpu_delay = max(cpu_delay,
> - snd_soc_dai_delay(cpu_dai, substream));
> - }
> - delay += cpu_delay;
> -
> - for_each_rtd_codec_dais(rtd, i, codec_dai) {
> - codec_delay = max(codec_delay,
> - snd_soc_dai_delay(codec_dai, substream));
> - }
> - delay += codec_delay;
> + snd_soc_pcm_dai_delay(substream, &cpu_delay, &codec_delay);
>
> - runtime->delay = delay;
> + runtime->delay = delay + cpu_delay + codec_delay;
>
> return offset;
> }
>
