ASoC: core: only convert non DPCM link to DPCM link

[bardliao]

We don't need to overwrite the dpcm_playback and dpcm_capture values
if the link is a DCPM link.

Fixes: 218fe9b ("ASoC: soc-core: Set dpcm_playback / dpcm_capture")

This PR will fix the unexpected error such as "CPU DAI iDisp1 for rtd iDisp1 does not support capture" when #2070 is applied.

[RanderWang]

@bardliao why we get BE here ? this function only deal with FE in topology.

[bardliao]

@RanderWang If soc_check_tplg_fes find a matched machine (like sof_sdw), it will overwrite all preset links. See for_each_card_prelinks(card, i, dai_link)

[RanderWang]

so soc_check_tplg_fes doesn't only check fes. why force dpcm_playback & dpcm_capture to 1 ? I don't get it.

[dbaluta]

@RanderWang @bardliao i think I'm at fault for this

commit 218fe9b7ec7f32c10a07539365488d80af7b0084
Author: Daniel Baluta <daniel.baluta@nxp.com>
Date:   Wed Dec 4 17:13:33 2019 +0200

    ASoC: soc-core: Set dpcm_playback / dpcm_capture
    
    When converting a normal link to a DPCM link we need
    to set dpcm_playback / dpcm_capture otherwise playback/capture
    streams will not be created resulting in errors like this:
    
    [   36.039111]  sai1-wm8960-hifi: ASoC: no backend playback stream
    
    Fixes: a655de808cbde ("ASoC: core: Allow topology to override machine driver FE DAI link config")

I think the proper fix would be to check if the original link has playback OR capture capability and update only that capability instead of updating both playback and capture.

[lyakh]

this is unrelated. But looking at the code I'd rather remove the goto by using

	if (strcmp(component->driver->ignore_machine, card->dev->driver->name) &&
	    strcmp(component->driver->ignore_machine, dev_name(card->dev)))
		continue;

in a separate PR of course

[lyakh]

not for this PR, but in line 1649/1650 above card->dai_link[i].name should be dai_link->name

[dbaluta]

At this moment in code we can reach if dai_link is a normal link (no_pcm = 0, dynamic=0) or a BE link (no_pcm=1).

If the link is already BE, we should do nothing.
if the link is normal link, we should convert it to BE.
-> no_pcm = 1
-> now here is the challenge as dai_link doesn't have a .capture or .playback field.

It has:

-> playback_only=1
   -> .dpcm_playback = 1
   -> dpcm_capture = 0
-> capture_only =1
   -> .dpcm_playback = 0
   -> .dpcm_capture =1
-> BUT what does capture_only = 0 and playback_only = 0 means, logically it could be either bot playback/capture supported or none.

[bardliao]

BUT what does capture_only = 0 and playback_only = 0 means, logically it could be either bot playback/capture supported or none.

I thought about the capture_only and playback_only flag, too. But it seems it is set by a few machine driver only. I am not sure if we can relay on the capture_only and playback_only flag or not. The other question is that is there any link supports none of playback and capture?

[dbaluta]

@bardliao If I would have to guess I would say that not-supporting playback/capture makes no sense. So, I think we can infer that by default if capture_only is not set and playback_only is not set then both playback/capture are supported.
I have sent an email to alsa mailing list to clarify this situation:

https://mailman.alsa-project.org/pipermail/alsa-devel/2020-May/168352.html

I think the best way to fix this is as follows:

diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
index 8321e75ff244..464004648bd0 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -1722,9 +1722,18 @@ static void soc_check_tplg_fes(struct snd_soc_card *card)
                        dai_link->platforms->name = component->name;
 
                        /* convert non BE into BE */
-                       dai_link->no_pcm = 1;
-                       dai_link->dpcm_playback = 1;
-                       dai_link->dpcm_capture = 1;
+                       if (!dai_link->no_pcm) {
+
+                               dai_link->no_pcm = 1;
+
+                               dai_link->dpcm_playback = 1;
+                               dai_link->dpcm_capture = 1;
+
+                               if (dai_link->playback_only)
+                                       dai_link->dpcm_capture = 0;
+                               if (dai_link->capture_only)
+                                       dai_link->dpcm_playback = 0;
+                       }

[plbossart]

@dbaluta I asked the question on alsa-devel on why we have separate flags for dpcm and non-dpcm solutions. I think it's a complete mess. We should have .pcm_playback and .pcm_capture flags only, you can infer playback_only and capture_only when only one of the two is set.

[plbossart]

@bardliao please check PR #2070