On Wed 27 Mar 2024 at 01:06, Kuninori Morimoto kuninori.morimoto.gx@renesas.com wrote:
Hi Jerome
Thank you for your feedback
I'm not quite sure what you mean by "should have validation" and what setting exactly we should validate ?
I know I should be able to able to understand that from the code below but, somehow I have trouble deciphering it.
Current ASoC have validation for ^^^ part
DPCM [CPU/xxxx]-[xxxx/Codec] ^^^^ (A) Normal [CPU/Codec] ^^^^^^^^^^^
(In many case, this "xxxx" is "dummy")
Yes for many DPCM user, you have:
DPCM [CPU/dummy]-[dummy/Codec]
FYI: on Amlogic it is mostly the following (only considering DCPM, omitting C2C ...)
DPCM [CPU-FE/dummy]-[CPU-BE/Codec]
With possibly several BE instances per FE, and several codecs per BE.
And there is even this for loopbacks:
DPCM [CPU-FE/dummy]-[CPU-BE/dummy]
By this patch-set, It will check all cases
DPCM [CPU/xxxx]-[xxxx/Codec] ^^^^^^^^^ ^^^^^^^^^^ (B) Normal [CPU/Codec] ^^^^^^^^^^^
At first, in [CPU/xxxx] case, "xxxx" part should be also checked (in many case, this "xxxx" is "dummy").
And, because it didn't check (A) part before, (B) part might be error on some board (at least Intel board). To avoid such case, temporally it uses "dummy" instead of "Codec" before [15/15]. This means (B) part checked as like below.
[xxxx/Codec] -> [xxxx/dummy]
Because "dummy" will pass all cases, (B) part is almost same as no check. Yes, it is no meaning, but the code will be simple.
Where you have a CPU supporting both direction and 2 codecs, each supporting 1 stream direction ? This is a valid i2s configuration.
(snip)
/*
* FIXME
* FIXME / CLEAN-UP-ME*/
- DPCM BE Codec has been no checked before.
- It should be checked, but it breaks compatibility.
- It ignores BE Codec here, so far.
if (dai_link->no_pcm)codec_dai = dummy_dai;
if ((dai_link->no_pcm) &&((cpu_play_t && !codec_play_t) ||(cpu_capture_t && !codec_capture_t))) {dev_warn_once(rtd->dev, "DCPM BE Codec has no stream settings (%s)\n",codec_dai->name);Taking one codec at a time, would you trigger a warning for the use case I described above ?
Oops, indeed it will indicate warning in your case. How about this ?
if ((dai_link->no_pcm) &&
^ Actually my comment applies to all links, DPCM backend or not
(!codec_play_t && !codec_capture_t)) {
A codec that does not support playback and does not support capture does not support much, does it ? ;)
I suppose you meant something like:
(!cpu_play_t && !codec_capture_t)) {
Then at first glance, maybe ... CPU and codec seem to exclude each other but that will only work as long as DCPM is limited to a single CPU per link.
dev_warn_once(...) ...}
Thank you for your help !!
Best regards
Renesas Electronics Ph.D. Kuninori Morimoto