Re: [alsa-devel] Question about snd_pcm_limit_hw_rates() call timing

On Tue, 21 Jan 2020 02:54:26 +0100, Kuninori Morimoto wrote:

Hi ALSA ML

soc-pcm has snd_pcm_limit_hw_rates() which determine rate_min/rate_max fields. It updates runtime->hw.rate_min/max (A) based on hw->rates (B).

int snd_pcm_limit_hw_rates(struct snd_pcm_runtime *runtime) { int i; for (i = 0; i < (int)snd_pcm_known_rates.count; i++) { (B) if (runtime->hw.rates & (1 << i)) { (A) runtime->hw.rate_min = snd_pcm_known_rates.list[i]; break; } } for (i = (int)snd_pcm_known_rates.count - 1; i >= 0; i--) { (B) if (runtime->hw.rates & (1 << i)) { (A) runtime->hw.rate_max = snd_pcm_known_rates.list[i]; break; } } return 0; }

I guess the calling timing is

1. set hw->rates
2. call snd_pcm_limit_hw_rates()
3. update hw->rate_min/max

soc_pcm_init_runtime_hw() is calling it as this order

static void soc_pcm_init_runtime_hw(xxx) { ...

1. hw->rates = snd_pcm_rate_mask_intersect(rates, cpu_stream->rates);
   
   

2. snd_pcm_limit_hw_rates(runtime);
   
   

3. hw->rate_min = max(hw->rate_min, cpu_stream->rate_min);
   

   hw->rate_min = max(hw->rate_min, rate_min); hw->rate_max = min_not_zero(hw->rate_max, cpu_stream->rate_max); hw->rate_max = min_not_zero(hw->rate_max, rate_max); }

But, dpcm_fe_dai_startup() are different.

static int dpcm_fe_dai_startup(xxx) { ... /* * dpcm_set_fe_runtime() updates runtime->hw.xxx */

1. 3. dpcm_set_fe_runtime(fe_substream);
      

2. snd_pcm_limit_hw_rates(runtime);
   

   ... }

I guess we need fixup dpcm_fe_dai_startup() ?

A good catch.

Actually the question is whether we need snd_pcm_limit_hw_rates() call or not. The current code in soc_pcm_init_runtime_hw() assumes that each cpu and codec dais already set the proper rate_min and rate_max, and narrow the range accordingly. So basically the call there is superfluous. OTOH, in dpcm_fe_dai_startup(), the call overrides the existing rate_min/max setup as you mentioned, so it may be wrong.

Or, better to ask -- is there any case that snd_pcm_limit_hw_rates() is mandatory in ASoC? The snd_pcm_limit_hw_rates() is for setting up rates_min and rates_max from rates bits. It's a function to be called only when we know that rates bits contain the full information and rates_min and rates_max are bogus. That is, its overriding behavior is designed.

OTOH, if the driver sets up already valid rates_min and rates_max values, there is no need to call this function at all.

thanks,

Takashi