Re: [PATCH v3 01/23] ASoC: soc-pcm: cleanup soc_get_playback_capture()

[Kuninori Morimoto <kuninori.morimoto.gx@xxxxxxxxxxx>]

Hi Pierre-Louis, Mark

Because Japanese will dive into long vacation since next week,
I want to post mail before that. I will back at 7th May.

> > > (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?

	(A) : checked CPU capabilities
	(B) : uses dpcm_xxx flag
	(C) : checks both dpcm_xxx and capabilities
	...

In my understanding, in summary, this dpcm_xxx flag was added to rescue
dummy DAI which is used on DCPM BE as CPU at (B), because some of them
might not have channels_min (This "dummy DAI" is not same as soc-utils's
dummy DAI). Let's name it as "no_chan_DAI" here.
In this patch, it was added as "mandatory flag", not "option flag",
thus all DPCM needed to use this dpcm_xxx flag.

After that (C) was added, but it was contradiction, because
it checks both dpcm_xxx and channels_min.
If my understanding was correct, original "no_chan_DAI" was supposed to
stop working, because it doesn't have channels_min. But there is no
such report after (C), during this 4 years.
We don't know which DAI is the "no_chan_DAI" (?)

Possibilities are as follows
	- No one is using "no_chan_DAI"
	- "no_chan_DAI" is no longer exist : it was removed ?
	- "no_chan_DAI" is no longer exist : it has channels_min ?

If my expectation was correct, we don't need dpcm_xxx anymore.

But because we have been used dpcm_xxx for 10 years since (B),
I understand to feel anxious to suddenly remove dpcm_xxx.
I think it should be removed anyway, but want to have grace time ?
If so, the idea is that we can use it as "option flag" instead of
"mandatory flag" for a while, like below

	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;

* maybe we want to indicate message like "place re-check the flag and
  remove it" via dev_info() if dpcm_xxx flag was used ?

I think +2 kernel version or so is enough for grace time ?
After that, we can remove dpcm_xxx flag.

When we consider it very detail, above code can't 100% keep compatibility
if the user have been used this dpcm_xxx flag to limit availability,
for example in case of DAI can use both playback/capture, but it had
dpcm_playback flag only. But it can use playback_only flag, instead.
But it is very difficult to find such DAI. Each user need to check.

I personally think that remove dpcm_xxx directly is no ploblem, but what
do you think ? I'm happy to hear any opinion, and happy to create
grace time code if someone want, but if there was no comment during
Japanese long vacation, I will create patch to remove dpcm_xxx directly.


BTW, I would like to know detail things around this topic. I'm happy if
someone knows it.

* Why dummy DAI doesn't/can't have channels_min ?

* Why it checks CPU side channels_min only when DPCM ?
  I think it should check both CPU and Codec.
  I could understand if it checks FE:CPU and BE:Codec (it is assuming
  other side was dummy), but both (FE/BE) check CPU side only...

Thank you for your help !!

Best regards
---
Renesas Electronics
Ph.D. Kuninori Morimoto