On Thu 23 Jul 2020 at 20:05, Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com wrote:
Previous updates to set dailink capabilities and check dailink capabilities were based on a flawed assumption that all dais support the same capabilities as the dailink. This is true for TDM configurations
Actually it is not true for TDM either, having codecs with different playback/capture caps is valid with TDM as well
but existing configurations use an amplifier and a capture device on the same dailink, and the tests would prevent the card from probing.
This patch modifies the snd_soc_dai_link_set_capabilities() helper so that the dpcm_playback (resp. dpcm_capture) dailink capabilities are set if at least one dai supports playback (resp. capture).
Likewise the checks are modified so that an error is reported only when dpcm_playback (resp. dpcm_capture) is set but none of the CPU DAIs support playback (resp. capture).
This was not is the original proposal you sent ...
Fixes: 25612477d20b5 ('ASoC: soc-dai: set dai_link dpcm_ flags with a helper') Fixes: b73287f0b0745 ('ASoC: soc-pcm: dpcm: fix playback/capture checks') Suggested-by: Jerome Brunet jbrunet@baylibre.com Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com
sound/soc/soc-dai.c | 16 +++++++++------- sound/soc/soc-pcm.c | 42 ++++++++++++++++++++++++------------------ 2 files changed, 33 insertions(+), 25 deletions(-)
diff --git a/sound/soc/soc-dai.c b/sound/soc/soc-dai.c index 98f0c98b06bb..ef1700e5e1bf 100644 --- a/sound/soc/soc-dai.c +++ b/sound/soc/soc-dai.c @@ -402,28 +402,30 @@ void snd_soc_dai_link_set_capabilities(struct snd_soc_dai_link *dai_link) struct snd_soc_dai_link_component *codec; struct snd_soc_dai *dai; bool supported[SNDRV_PCM_STREAM_LAST + 1];
bool supported_cpu;
bool supported_codec; int direction; int i;
for_each_pcm_streams(direction) {
supported[direction] = true;
supported_cpu = false;supported_codec = false;for_each_link_cpus(dai_link, i, cpu) { dai = snd_soc_find_dai(cpu);
if (!dai || !snd_soc_dai_stream_valid(dai, direction)) {supported[direction] = false;
if (dai && snd_soc_dai_stream_valid(dai, direction)) {supported_cpu = true; break; }
if (!supported[direction])continue;if (!dai || !snd_soc_dai_stream_valid(dai, direction)) {supported[direction] = false;
if (dai && snd_soc_dai_stream_valid(dai, direction)) {supported_codec = true; break; }}
supported[direction] = supported_cpu && supported_codec;}
dai_link->dpcm_playback = supported[SNDRV_PCM_STREAM_PLAYBACK];
diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c index f2c7c85ad40c..aac61df0c50e 100644 --- a/sound/soc/soc-pcm.c +++ b/sound/soc/soc-pcm.c @@ -2743,30 +2743,36 @@ int soc_new_pcm(struct snd_soc_pcm_runtime *rtd, int num) if (rtd->dai_link->dpcm_playback) { stream = SNDRV_PCM_STREAM_PLAYBACK;
for_each_rtd_cpu_dais(rtd, i, cpu_dai)if (!snd_soc_dai_stream_valid(cpu_dai,stream)) {dev_err(rtd->card->dev,"CPU DAI %s for rtd %s does not support playback\n",cpu_dai->name,rtd->dai_link->stream_name);return -EINVAL;
for_each_rtd_cpu_dais(rtd, i, cpu_dai) {if (snd_soc_dai_stream_valid(cpu_dai, stream)) {playback = 1;break; }
playback = 1;
}if (!playback) {dev_err(rtd->card->dev,"No CPU DAIs support playback for stream %s\n",rtd->dai_link->stream_name);return -EINVAL;}
Again, this is changing the original meaning of the flag from "playback allowed" to "playback required".
This patch (or the orignal) does not explain why this change of meaning is necessary ? The point I was making here [0] still stands.
If your evil plan is to get rid of 2 of the 4 flags, why go through the trouble of the changing the meaning and effect of one them ?
[0]: https://lore.kernel.org/r/1j1rla9s22.fsf@starbuckisacylon.baylibre.com
} if (rtd->dai_link->dpcm_capture) { stream = SNDRV_PCM_STREAM_CAPTURE;
for_each_rtd_cpu_dais(rtd, i, cpu_dai)if (!snd_soc_dai_stream_valid(cpu_dai,stream)) {dev_err(rtd->card->dev,"CPU DAI %s for rtd %s does not support capture\n",cpu_dai->name,rtd->dai_link->stream_name);return -EINVAL;
for_each_rtd_cpu_dais(rtd, i, cpu_dai) {if (snd_soc_dai_stream_valid(cpu_dai, stream)) {capture = 1;break; }
capture = 1;
}if (!capture) {dev_err(rtd->card->dev,"No CPU DAIs support capture for stream %s\n",rtd->dai_link->stream_name);return -EINVAL;}