On Mon 13 May 2024 at 00:11, Kuninori Morimoto kuninori.morimoto.gx@renesas.com wrote:
Hi Jerome
Thank you for your feedback and analysis !
- 1st problem: I see that following your removal of snd_soc_dai_link_set_capabilities(), the dpcm_playback/capture flags are no longer properly initialised in the amlogic card drivers. That will need fixing.
(snip)
This codec is not meant to have capture channels. I think DT description and the card driver settings (before the removal of snd_soc_dai_link_set_capabilities()) are correct.
OK, I see. Thank you for your analysis.
The problem was my patch checks CPU direction vs Codec direction only, thus, it will indicates unexpected warnings, like this case.
Thank you for finding it, I hope v2 patch should be OK for you.
I'll check
IMO, those check become too restrictive. We are adding a lot of code I'm not sure I understand what we stand by going so far in the consistency checks.
Initially those dpcm_playback/capture flag could be used to just forcefully disable a link direction, regardless of the CPUs or codecs present on the link. It was just another setting and it was not required to be consistent with anything. It just declared whether the direction was allowed on the link, or not.
It changed this commit, and the flags suddenly needed to be consistent with whatever was on link: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/so...
I complained back then and I still don't think this change was good.
If the flags needs to consistent with what is on the link, and we have the ability to check it, why let card drivers set it and then complain about it in ASoC checks ? Why not deal with it in the framework directly ?
I think the simplest solution would to just go back to the initial meaning of these flags which was just the card driver declaring the direction allowed/disallowed on a link. Then ASoC can mix that information with whatever it finds with DAI drivers and figure out what is actually possible.
It would give a clear and separate semantic meaning to those flags instead something redundant with the DAI driver info.
What we have currently is not straight forward to set and check. It makes the code unnecessarily complicated and difficult to maintain. I think the difficulties with this patchset are a good illustration of that, unfortunately.
By this patch-set, it will be handled automatically via CPU and Codec driver settings, Card driver will no longer need to consider about direction (like non-DPCM), and I hope people (including you) will be happy about that.
If it makes things simpler, I am happy for sure :)
However, I'm a bit confused. If it is handled automatically by the CPUs and Codecs settings, does it mean dpcm_playback/capture flags are no-ops from now on ?
Should I update my card drivers to ditch those flags completely ?
May I still disable a direction on a link from the card driver, like in the case I described above, when a TDM link has no slots for a direction ?
Thank you for your help !!
Best regards
Renesas Electronics Ph.D. Kuninori Morimoto