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@xxxxxxxxxxxxxxx>
Reviewed-by: Bard Liao <yung-chuan.liao@xxxxxxxxxxxxxxx>
Reviewed-by: Guennadi Liakhovetski <guennadi.liakhovetski@xxxxxxxxxxxxxxx>
Reviewed-by: Ranjani Sridharan <ranjani.sridharan@xxxxxxxxxxxxxxx>
Reviewed-by: Daniel Baluta <daniel.baluta@xxxxxxxxx>
---
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 capture
because 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.kandagatla@xxxxxxxxxx/)
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.
