Re: [RFC PATCH v2 4/6] ASoC: soc-pcm: tweak DPCM BE hw_param() call order

[Ranjani Sridharan <ranjani.sridharan@linux.intel.com>]

On Mon, 2022-03-28 at 11:44 +0530, Sameer Pujar wrote:
> For DPCM links, the order of hw_param() call depends on the sequence
> of
> BE connection to FE. It is possible that one BE link can provide
> clock
> to another BE link. In such cases consumer BE DAI, to get the rate
> set
> by provider BE DAI, can use the standard clock functions only if
> provider
> has already set the appropriate rate during its hw_param() stage.
> 
> Presently the order is fixed and does not depend on the provider and
> consumer relationships. So the clock rates need to be known ahead of
> hw_param() stage.
> 
> This patch tweaks the hw_param() order by connecting the provider BEs
> late to a FE. With this hw_param() calls for provider BEs happen
> first
> and then followed by consumer BEs. The consumers can use the standard
> clk_get_rate() function to get the rate of the clock they depend on.
> 
> Signed-off-by: Sameer Pujar <spujar@nvidia.com>
> ---
>  TODO:
>   * The FE link is not considered in this. For Tegra it is fine to
>     call hw_params() for FE at the end. But systems, which want to
> apply
>     this tweak for FE as well, have to extend this tweak to FE.
>   * Also only DPCM is considered here. If normal links require such
>     tweak, it needs to be extended.
> 
>  sound/soc/soc-pcm.c | 60
> ++++++++++++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 59 insertions(+), 1 deletion(-)
> 
> diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c
> index 9a95468..5829514 100644
> --- a/sound/soc/soc-pcm.c
> +++ b/sound/soc/soc-pcm.c
> @@ -1442,6 +1442,29 @@ static int dpcm_prune_paths(struct
> snd_soc_pcm_runtime *fe, int stream,
>  	return prune;
>  }
>  
> +static bool defer_dpcm_be_connect(struct snd_soc_pcm_runtime *rtd)
> +{
> +	struct snd_soc_dai *dai;
> +	int i;
> +
> +	if (!(rtd->dai_link->dai_fmt & SND_SOC_DAIFMT_FORMAT_MASK))
> +		return false;
Is this check necessary?
> +
> +	if ((rtd->dai_link->dai_fmt &
> SND_SOC_DAIFMT_CLOCK_PROVIDER_MASK) ==
> +	    SND_SOC_DAIFMT_CBC_CFC) {
> +
> +		for_each_rtd_cpu_dais(rtd, i, dai) {
> +
> +			if (!snd_soc_dai_is_dummy(dai))
> +				return true;
> +		}
> +	}
> +
> +	return false;
> +}
> +
> +#define MAX_CLK_PROVIDER_BE 10
> +
>  static int dpcm_add_paths(struct snd_soc_pcm_runtime *fe, int
> stream,
>  	struct snd_soc_dapm_widget_list **list_)
>  {
> @@ -1449,7 +1472,8 @@ static int dpcm_add_paths(struct
> snd_soc_pcm_runtime *fe, int stream,
>  	struct snd_soc_dapm_widget_list *list = *list_;
>  	struct snd_soc_pcm_runtime *be;
>  	struct snd_soc_dapm_widget *widget;
> -	int i, new = 0, err;
> +	struct snd_soc_pcm_runtime *prov[MAX_CLK_PROVIDER_BE];
> +	int i, new = 0, err, count = 0;
>  
>  	/* Create any new FE <--> BE connections */
>  	for_each_dapm_widgets(list, i, widget) {
> @@ -1489,6 +1513,40 @@ static int dpcm_add_paths(struct
> snd_soc_pcm_runtime *fe, int stream,
>  		    (be->dpcm[stream].state !=
> SND_SOC_DPCM_STATE_CLOSE))
>  			continue;
>  
> +		/* Connect clock provider BEs at the end */
> +		if (defer_dpcm_be_connect(be)) {
> +			if (count >= MAX_CLK_PROVIDER_BE) {
What determines MAX_CLK_PROVIDER_BE? why 10? Can you use rtd->num_cpus
instead? 
Thanks,
Ranjani