On Mon 13 May 2024 at 00:11, Kuninori Morimoto <kuninori.morimoto.gx@xxxxxxxxxxx> 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/sound/soc?h=v6.9-rc7&id=b73287f0b074 >> >> 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 -- Jerome
