Hi Pierre-Louis
Cc Mark
> Your explanation seems to contradict the sentence above "This
> availability check should be available not only for DPCM, but for all
> connections."
>
> Can we actually do this 'availability check' for non-DPCM connections.
>
> > How about this ?
> >
> > If either playback or capture assertion flag was presented,
> > not presented direction will be disabled by ASoC even if
> > it was available.
>
> Did you mean
>
> "
> The playback (resp. capture) direction will be disabled by ASoC if the
> playback_assertion (resp. capture) flag is false - even if this
> direction was available at the DAI level
> "
> > (0, 0) : Both are not must item. available direction is used as-is.
> > But it will be error if nothing was available.
>
> That new wording makes me even more confused. What does 'available'
> refer to and at which level is this?
>
> This seems also to contradict the definitions above, "available
> direction is used as-is" is not aligned with "not presented direction
> will be disabled by ASoC even if it was available".
It is complicated by the attempt to merge dpcm_xxx and xxx_only flags.
And I noticed that my one of other attemption was not indicated.
Let's cleanup what does this patch-set want to do
I still wondering why dpcm_xxx flag itself is needed.
(A) Before, it checks channels_min for DPCM, same as current non-DPCM.
This is very clear for me. Let's name this as "validation check"
if (rtd->dai_link->dynamic || rtd->dai_link->no_pcm) {
if (cpu_dai->driver->playback.channels_min)
playback = 1;
if (cpu_dai->driver->capture.channels_min)
capture = 1;
(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;
(C) 9b5db059366ae2087e07892b5fc108f81f4ec189
("ASoC: soc-pcm: dpcm: Only allow playback/capture if supported")
checks channels_min (= validation check) again
if (rtd->dai_link->dynamic || rtd->dai_link->no_pcm) {
cpu_dai = asoc_rtd_to_cpu(rtd, 0);
...
playback = rtd->dai_link->dpcm_playback &&
snd_soc_dai_stream_valid(cpu_dai, SNDRV_PCM_STREAM_PLAYBACK);
capture = rtd->dai_link->dpcm_capture &&
snd_soc_dai_stream_valid(cpu_dai, SNDRV_PCM_STREAM_CAPTURE);
(D) b73287f0b0745961b14e5ebcce92cc8ed24d4d52
("ASoC: soc-pcm: dpcm: fix playback/capture checks") expanded it to
multi connection.
So, I would say nothing has changed, but become more complicated.
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;
So my opinion is this dpcm_xxx is unnecessary flag that only complicate
matters. I guess almost all Card don't need this flag, this means
"validation check" only is veryenough, same as current non-DPCM.
But because of these history, dpcm_xxx flag have been used as
"passage permit" or "gate way". It doesn't try to "validation check" if
dpcm_xxx flag was not set. This is the reason why I try to merge
dpcm_xxx and xxx_only flag. These are doing the same things with
dirrerent flags, IMO.
OTOH, some Card want to detect error if expected direction
(playback/capture) was not valid. I guess this is your commitment (?).
So, let's keep xxx_only flag as-is, and use dpcm_xxx as "available_check".
I'm not sure what is the good naming, but for example
"playback_available_check" flag means "owner is expecting playback is
valid/available, and want to receive error if not".
I'm not sure how many owner want this flag, thus I think "option flag"
is very enough (= not mandatory, as I mentioned in the patch-set).
If we makes these checks generalize,
For DPCM, (for example new DPCM) it can remove/ignore "available_check"
flag if it don't need, same as current non-DPCM.
And for non-DPCM, it can use "available_check" if needed,
same as current DPCM.
What do you think ? what is your opinion ?
Thank you for your help !!
Best regards
---
Renesas Electronics
Ph.D. Kuninori Morimoto
