Re: [PATCH 1/4] ASoC: soc-pcm: dpcm: fix playback/capture checks

[Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>]

>>> simple-card.c and audio-graph-card do hard-code but that's done with 
>>> C in
>>> the driver:
>>>
>>>     ret = asoc_simple_parse_daifmt(dev, cpu_ep, codec_ep,
>>>                        NULL, &dai_link->dai_fmt);
>>>     if (ret < 0)
>>>         goto out_put_node;
>>>
>>>     dai_link->dpcm_playback        = 1;
>>>     dai_link->dpcm_capture        = 1;
>>>
>>>
>>> that that should be fixed based on the DAI format used in that 
>>> dai_link - in
>>> other words we can make sure the capabilities of the dailink are aligned
>>> with the dais while parsing the DT blobs.
>>
>> But how do you know which capabilities to set? The device tree doesn't
>> tells us that. We could add some code to look up the snd_soc_dai_driver
>> early, based on the references in the device tree (basically something
>> like snd_soc_of_get_dai_name(), see
>>    
>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/sound/soc/soc-core.c#n2988) 
>>
>>
>> At least to me that function doesn't exactly look trivial though,
>> and that's just to properly fill in the dpcm_playback/capture
>> parameters. Essentially those parameters only complicate the current
>> device tree use case,  where you want the DAI link to be for both
>> playback/capture, but restricted to the capabilities of the DAI.
>>
>> Just wondering if setting up dpcm_playback/capture properly is worth it
>> at all in this case. This isn't necessary for the non-DPCM case either,
>> there we automatically set it based on the DAI capabilities.
> 
> We can add a simple loop for each direction that relies on
> snd_soc_dai_stream_valid() to identify if each DAI is capable of doing 
> playback/capture.

see below completely untested diff to show what I had in mind: we 
already make use of snd_soc_dai_stream_valid() in other parts of the 
core so we should be able to determine dpcm_playback/capture based on 
the same information already used.

diff --git a/sound/soc/generic/audio-graph-card.c 
b/sound/soc/generic/audio-graph-card.c
index 9ad35d9940fe..4c67f1f65eb4 100644
--- a/sound/soc/generic/audio-graph-card.c
+++ b/sound/soc/generic/audio-graph-card.c
@@ -215,7 +215,9 @@ static int graph_dai_link_of_dpcm(struct 
asoc_simple_priv *priv,
         struct asoc_simple_dai *dai;
         struct snd_soc_dai_link_component *cpus = dai_link->cpus;
         struct snd_soc_dai_link_component *codecs = dai_link->codecs;
+       int stream;
         int ret;
+       int i;

         /* Do it all CPU endpoint, and 1st Codec endpoint */
         if (!li->cpu && dup_codec)
@@ -317,8 +319,34 @@ static int graph_dai_link_of_dpcm(struct 
asoc_simple_priv *priv,
         if (ret < 0)
                 goto out_put_node;

-       dai_link->dpcm_playback         = 1;
-       dai_link->dpcm_capture          = 1;
+       for_each_pcm_streams(stream) {
+               struct snd_soc_dai_link_component *cpu;
+               struct snd_soc_dai_link_component *codec;
+               struct snd_soc_dai *d;
+               bool dpcm_direction = true;
+
+               for_each_link_cpus(dai_link, i, cpu) {
+                       d = snd_soc_find_dai(cpu);
+                       if (!d || !snd_soc_dai_stream_valid(d, stream)) {
+                               dpcm_direction = false;
+                               break;
+                       }
+               }
+               for_each_link_codecs(dai_link, i, codec) {
+                       d = snd_soc_find_dai(codec);
+                       if (!d || !snd_soc_dai_stream_valid(d, stream)) {
+                               dpcm_direction = false;
+                               break;
+                       }
+               }
+               if (dpcm_direction) {
+                       if (stream == SNDRV_PCM_STREAM_PLAYBACK)
+                               dai_link->dpcm_playback = 1;
+                       if (stream == SNDRV_PCM_STREAM_CAPTURE)
+                               dai_link->dpcm_capture = 1;
+               }
+       }
+
         dai_link->ops                   = &graph_ops;
         dai_link->init                  = asoc_simple_dai_init;