On 6/16/20 4:02 AM, Stephan Gerhold wrote:
On Tue, Jun 16, 2020 at 10:54:17AM +0200, Stephan Gerhold wrote:
Hi Pierre,
On Mon, Jun 08, 2020 at 02:44:12PM -0500, Pierre-Louis Bossart wrote:
Recent changes in the ASoC core prevent multi-cpu BE dailinks from being used. DPCM does support multi-cpu DAIs for BE Dailinks, but not for FE.
First I want to apologize for introducing this regression. Actually when I made the "Only allow playback/capture if supported" patch I did not realize it would also be used for BE DAIs. :)
No need to apologize, it helped identify configuration issues none of us identified. That's progress for me.
Handle the FE checks first, and make sure all DAIs support the same capabilities within the same dailink.
BugLink: https://github.com/thesofproject/linux/issues/2031 Fixes: 9b5db059366ae2 ("ASoC: soc-pcm: dpcm: Only allow playback/capture if supported") Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Reviewed-by: Bard Liao yung-chuan.liao@linux.intel.com Reviewed-by: Guennadi Liakhovetski guennadi.liakhovetski@linux.intel.com Reviewed-by: Ranjani Sridharan ranjani.sridharan@linux.intel.com Reviewed-by: Daniel Baluta daniel.baluta@gmail.com
sound/soc/soc-pcm.c | 44 ++++++++++++++++++++++++++++++++++---------- 1 file changed, 34 insertions(+), 10 deletions(-)
diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c index 276505fb9d50..2c114b4542ce 100644 --- a/sound/soc/soc-pcm.c +++ b/sound/soc/soc-pcm.c @@ -2789,20 +2789,44 @@ int soc_new_pcm(struct snd_soc_pcm_runtime *rtd, int num) struct snd_pcm *pcm; char new_name[64]; int ret = 0, playback = 0, capture = 0;
int stream; int i;
if (rtd->dai_link->dynamic && rtd->num_cpus > 1) {
dev_err(rtd->dev,"DPCM doesn't support Multi CPU for Front-Ends yet\n");return -EINVAL;}
if (rtd->dai_link->dynamic || rtd->dai_link->no_pcm) {
cpu_dai = asoc_rtd_to_cpu(rtd, 0);if (rtd->num_cpus > 1) {dev_err(rtd->dev,"DPCM doesn't support Multi CPU yet\n");return -EINVAL;
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;Unfortunately the "return -EINVAL" here and below break the case where dpcm_playback/dpcm_capture does not exactly match the capabilities of the referenced CPU DAIs. To quote from my commit message:
At the moment, PCM devices for DPCM are only created based on the dpcm_playback/capture parameters of the DAI link, without considering if the CPU/FE DAI is actually capable of playback/capture. Normally the dpcm_playback/capture parameter should match the capabilities of the CPU DAI. However, there is no way to set that parameter from the device tree (e.g. with simple-audio-card or qcom sound cards). dpcm_playback/capture are always both set to 1.The basic idea for my commit was to basically stop using dpcm_playback/capture for the device tree case and infer the capabilities solely based on referenced DAIs. The DAIs expose if they are capable of playback/capture, so I see no reason to be required to duplicate that into the DAI link setup (unless you want to specifically restrict a DAI link to one direction for some reason...)
With your patch probe now fails with:
7702000.sound: CPU DAI MultiMedia1 for rtd MultiMedia1 does not support capturebecause sound/soc/qcom/common.c sets dpcm_playback = dpcm_capture = 1 even though that FE DAI is currently configured to be playback-only.
I believe Srinivas fixed that problem for the BE DAIs in commit a2120089251f ("ASoC: qcom: common: set correct directions for dailinks") (https://lore.kernel.org/alsa-devel/20200612123711.29130-2-srinivas.kandagatl...)
For the QCOM case it may be feasible to set dpcm_playback/dpcm_capture appropriately because it is basically only used with one particular DAI driver. But simple-audio-card is generic and used with many different drivers so hard-coding a call into some other driver like Srinivas did above won't work in that case.
Doesn't simple-card rely on DT blobs that can also be updated?
I wonder if we should downgrade your dev_err(...) to a dev_dbg(...), and then simply avoid setting playback/capture = 0.
Hmm, I wanted to write "avoid setting playback/capture = 1" here of course. If dpcm_playback/capture is set but not actually supported don't error out but just ignore it. That would essentially make dpcm_playback/capture just a restriction of the CPU DAI capabilities.
Not sure if there is even a usecase for such a restriction, maybe dpcm_playback/capture could even be removed entirely...
I'd rather keep the error and fix those bad configurations, and in a second step unify dpcm_capture/dpcm_playback/playback_only/capture_only. We have too many flags to identify directions and when given too many choices it's likely we are going to have corner cases for a while.