[PATCH 7/7] ASoC: soc-pcm: Merge CPU/Codec at soc_pcm_pointer()

[Pierre-Louis Bossart]

On 3/10/20 8:07 PM, Kuninori Morimoto wrote:
> 
> From: Kuninori Morimoto <kuninori.morimoto.gx at renesas.com>
> 
> soc_pcm_pointer() is getting eash DAI's delay via snd_soc_dai_delay(),
> but, it is adding CPU delay and Codec delay.
> We need is not "added delay", but "max delay" of CPU/Codec.
> This patch finds maximum delay from CPU/Codec.

The max() used between all cpu or codec dais for the same dailink is 
largely defensive programming, in practice it's not clear to me if we 
have different delays reported by differents codec or cpu_dais. I would 
expect all cpu dais in the same dailink to report the same delay, and 
likewise all codecs dais in the same dailink should provide the same value.

Now doing a max between cpu and codec dais does not seem right to me. 
You may have a delay in a DSP and a delay in a codec, and worst case the 
delay is the total of the two. It wouldn't matter too much with a 
'simple' codec with limited buffering, but the moment the codec itself 
has a DSP and internal buffering this change in accounting would 
introduce a real offset.


> Signed-off-by: Kuninori Morimoto <kuninori.morimoto.gx at renesas.com>
> ---
>   sound/soc/soc-pcm.c | 27 +++++++--------------------
>   1 file changed, 7 insertions(+), 20 deletions(-)
> 
> diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c
> index c0f318699fe4..675de7c0eaa4 100644
> --- a/sound/soc/soc-pcm.c
> +++ b/sound/soc/soc-pcm.c
> @@ -1274,13 +1274,10 @@ static int soc_pcm_bespoke_trigger(struct snd_pcm_substream *substream,
>   static snd_pcm_uframes_t soc_pcm_pointer(struct snd_pcm_substream *substream)
>   {
>   	struct snd_soc_pcm_runtime *rtd = substream->private_data;
> -	struct snd_soc_dai *cpu_dai;
> -	struct snd_soc_dai *codec_dai;
> +	struct snd_soc_dai *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;
> +	snd_pcm_sframes_t add_delay = 0;
>   	int i;
>   
>   	/* clearing the previous total delay */
> @@ -1288,22 +1285,12 @@ static snd_pcm_uframes_t soc_pcm_pointer(struct snd_pcm_substream *substream)
>   
>   	offset = snd_soc_pcm_component_pointer(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;
> +	for_each_rtd_dais(rtd, i, dai)
> +		add_delay = max(add_delay,
> +				snd_soc_dai_delay(dai, substream));
>   
> -	runtime->delay = delay;
> +	/* base delay if assigned in pointer callback */
> +	runtime->delay += add_delay;
>   
>   	return offset;
>   }
>