Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:18 as permitted sender) client-ip=2620:137:e000::1:18;
Message-ID: <51c8d2541ccff2689b9164ab9b671b0b2514e65f.camel@linux.intel.com>
Subject: Re: [RFC PATCH v2 4/6] ASoC: soc-pcm: tweak DPCM BE hw_param() call
 order
From:   Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
To:     Sameer Pujar <spujar@nvidia.com>, broonie@kernel.org,
        lgirdwood@gmail.com, robh+dt@kernel.org, krzk+dt@kernel.org,
        perex@perex.cz, tiwai@suse.com, peter.ujfalusi@linux.intel.com,
        pierre-louis.bossart@linux.intel.com
Cc:     oder_chiou@realtek.com, devicetree@vger.kernel.org,
        alsa-devel@alsa-project.org, linux-kernel@vger.kernel.org,
        jonathanh@nvidia.com, thierry.reding@gmail.com,
        linux-tegra@vger.kernel.org
Date:   Mon, 28 Mar 2022 08:29:45 -0700
In-Reply-To: <1648448050-15237-5-git-send-email-spujar@nvidia.com>
References: <1648448050-15237-1-git-send-email-spujar@nvidia.com>
         <1648448050-15237-5-git-send-email-spujar@nvidia.com>
Content-Type: text/plain; charset="UTF-8"
User-Agent: Evolution 3.36.3-0ubuntu1 
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Precedence: bulk

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