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
