Re: [PATCH 1/5] ASoC: soc-dai: update snd_soc_dai_delay() to snd_soc_pcm_dai_delay()

On 11/16/21 1:45 AM, Kuninori Morimoto wrote:

From: Kuninori Morimoto kuninori.morimoto.gx@renesas.com

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?highl...

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@renesas.com Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com

---

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;

}