Hi Pierre-Louis
Thank you for your feedback
(B) commit 1e9de42f4324b91ce2e9da0939cab8fc6ae93893 ("Explicitly set BE DAI link supported stream directions") force use to dpcm_xxx flag
if (rtd->dai_link->dynamic || rtd->dai_link->no_pcm) { playback = rtd->dai_link->dpcm_playback; capture = rtd->dai_link->dpcm_capture;
The reason for this (B) addition is very clear from the commit message
" Some BE DAIs can be "dummy" (when the DSP is controlling the DAI) and as such wont have set a minimum number of playback or capture channels required for BE DAI registration (to establish supported stream directions). "
I'm still not yet 100% understand around this "dummy" DAI, but is it *not* soc-utils.c :: dummy_dai, but some original dummy DAI is used somewhere ?
I know ${LINUX}/sound/soc/codecs/hda.c :: card_binder_dai is one of the DAI which is used but doesn't have channels_min. I think it is used as BE "Codec", but code is checking "CPU" side.
Do you know what does this "BE dummy DAI" specifically means here?
Or if (B) added dpcm_xxx as "option flag", it was understandable for me. like this
if (rtd->dai_link->dynamic || rtd->dai_link->no_pcm) { playback = (cpu_dai->driver->playback.channels_min > 0) || rtd->dai_link->dpcm_playback; capture = (cpu_dai->driver->capture.channels_min > 0) || rtd->dai_link->dpcm_capture;
That would essentially revert (C), since the direction would ignore the actual capabilities of the DAI.
IMHO, we should really try with this revert and go back to the initial definition of (A), where dpcm_xxx is an optional escape mechanism to allow machine drivers to set the direction. I would guess that would impact mostly DT/simple-card users and Qualcomm, if the commit message of (C) is still relevant.
I can agree that above code makes dpcm_xxx flag option, and allow to machine drivers to set direction without thinking actual DAI capabilities. I think it is effective if it was around (C) timing.
(A) : checked CPU capabilities (B) : uses dpcm_xxx flag (C) : checks both dpcm_xxx and capabilities ...
But *current* ASoC is checking both "actual capabilities" and "dpcm_xxx" flag in the same time, and have no problems today.
Around (A)-(B) timing, some DAI had issue (= channels_min was not set). Let's name it as "no_chan_DAI". It should be CPU DAI and used as BE if my understanding was correct.
Because "no_chan_DAI" exist, (B) was added.
But after that (C) was added, and it checks channels_min again. It continues today. This means "no_chan_DAI" today have channels_min, otherwise it can't work today.
If my above understanding were all correct, do we still need dpcm_xxx ? because "no_chan_DAI" is no longer exist. Just remove dpcm_xxx seems no problem, I guess...
Then we can discuss about merging code and removing flags. For now we have different directions/opinions on something that's 10 years old, last modified 4 years ago. We will break lots of eggs if we don't first agree on what works and what doesn't.
This 23-patch code merge/simplification is premature at this point IMHO, we should first land in a state where everyone is happy with how dpcm_xxx flags need to be handled. We can merge dpcm_xxx and xxx_only flags later when we understand what they are supposed to do...
Thank you for your help. The problem becoming more clear. Let's focus to "revert C code" or "remove dpcm_xxx" first.
And now I need a coffee refill :-)
Enjoy :)
Thank you for your help !!
Best regards --- Renesas Electronics Ph.D. Kuninori Morimoto