On 11/27/2013 11:31 AM, Takashi Iwai wrote:
At Wed, 27 Nov 2013 09:58:18 +0100, Lars-Peter Clausen wrote:
snd_pcm_limit_hw_rates() will initialize the minimum and maximum sample rate for the PCM stream based on the rates specified in the rates field. Since we call snd_pcm_limit_hw_rates() after soc_pcm_init_runtime_hw() it will essentially overwrite the min and max rate set in soc_pcm_init_runtime_hw(). This may cause the minimum or maximum rate to be set to a value outside the range of one of the components if one of the components sets either SNDRV_PCM_RATE_CONTINUOUS or SNDRV_PCM_RATE_KNOT and the other component specified a discrete rate via SNDRV_PCM_RATE_[0-9]* that is outside of the first component's rate range. To fix this first calculate the minimum and maximum rates using snd_pcm_limit_hw_rates() and then on top of that apply the contraints specified in the snd_soc_pcm_stream structs.
Signed-off-by: Lars-Peter Clausen lars@metafoo.de
sound/soc/soc-pcm.c | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-)
diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c index c4ef880..22af038 100644 --- a/sound/soc/soc-pcm.c +++ b/sound/soc/soc-pcm.c @@ -215,12 +215,12 @@ static void soc_pcm_apply_msb(struct snd_pcm_substream *substream, } }
-static void soc_pcm_init_runtime_hw(struct snd_pcm_hardware *hw, +static void soc_pcm_init_runtime_hw(struct snd_pcm_runtime *runtime, struct snd_soc_pcm_stream *codec_stream, struct snd_soc_pcm_stream *cpu_stream) {
- hw->rate_min = max(codec_stream->rate_min, cpu_stream->rate_min);
- hw->rate_max = min_not_zero(codec_stream->rate_max, cpu_stream->rate_max);
- struct snd_pcm_hardware *hw = &runtime->hw;
- hw->channels_min = max(codec_stream->channels_min, cpu_stream->channels_min); hw->channels_max = min(codec_stream->channels_max,
@@ -233,6 +233,13 @@ static void soc_pcm_init_runtime_hw(struct snd_pcm_hardware *hw, if (cpu_stream->rates & (SNDRV_PCM_RATE_KNOT | SNDRV_PCM_RATE_CONTINUOUS)) hw->rates |= codec_stream->rates;
- snd_pcm_limit_hw_rates(runtime);
Strictly speaking, snd_pcm_limit_hw_rates() should be applied only in case without KNOT nor CONTINUOUS flag. But, I guess this would work better as is since there might be drivers that don't give proper rate_min and rate_max but rely on the rate bits, and min_not_zero() does the right thing in the code below.
My thinking as well. There are some driver which do set CONTINUOUS or KNOT, but don't specify a minimum and maximum rate and probably rely on the fact that we always call snd_pcm_limit_hw_rates() (E.g. the kirkwood drivers do this). In my opinion those drivers should be fixed and once that is done we can add a check that skips snd_pcm_limit_hw_rates() if KNOT or CONTINUOUS is set. The current behavior isn't optimal e.g. if a driver sets SNDRV_PCM_RATE_CONTINUOUS and SNDRV_PCM_RATE_8000_192000 and rate_max to 384000, we'd still end up with a maximum rate of 192000 because of snd_pcm_limit_hw_rates().
The only missing thing is the conflict between CONTINUOUS and KNOT. Could you put the fix I suggested into this series, too?
ok.
- Lars