On Wed, May 12, 2021 at 09:45:33AM +0900, Kuninori Morimoto wrote: Hi Morimoto-san, First up sorry it's taken me a while to respond to this - I wanted to get it clear in my head what I thought the best approach is here. I think your code here is good with a couple of documentation tweaks: > For example, some driver want to select format "automatically", > but want to select other fields "manually", because of complex limitation. > Or other example, in case of both CPU and Codec are possible to be > clock provider, but the quality was different. > In these case, user need/want to *manually* select each fields > from Sound Card driver. > > It uses Sound Card specified fields preferentially, and try to select > non-specific fields from CPU and Codec driver settings if driver had > callbacks. > In other words, we can select all dai_fmt via Sound Card driver > same as before. I mentioned last time around the idea of having first and second preferences for the DAIs, for things like "this will work but only if you get the configuration exactly right". These patches don't support that, they just treat everything the DAI reports as being equally valid. I've actually come round to thinking that might be OK for now but... > +/** > + * snd_soc_dai_get_fmt - get enable DAI hardware audio format. s/get enable DAI hardware/get supported/ > + * @dai: DAI > + * @fmt: SND_SOC_POSSIBLE_DAIFMT_* format value. > + * > + * Configures the DAI hardware format and clocking. ...I think here we should say something like This should return only formats implemented with high quality by the DAI so that the core can configure a format which will work well with other devices. For example devices which don't support both edges of the LRCLK signal in I2S style formats should only list DSP modes. This will mean that sometimes fewer formats are reported here than are supported by set_fmt(). so that instead of worrying about formats that aren't quite supported properly we just tell drivers not to list them for now so if the system is relying on the framework to pick the DAI format then we know it's one that's robust, we're not going to choose one that the hardware in one of the devices isn't very good at. Does that make sense to you? If we did do first and second choices we'd get an algorithm like: 1. Try to match both DAI's 1st choice, if match use that. 2. Pick one DAI A, try it's 2nd choice with the other DAI B's 1st choice. 3. Swap and try 1st choice of A and 2nd choice of B. 4. Try 2nd choices of both DAIs. but I think that might get too complicated and we'd end up stuck if we ever wanted to change the algorithm since DTs that don't specify a format may break if a different format is picked. If we go with ranking every format individually it gets even more complicated for both driver authors and the core. I think with the documentation tweak above your idea is a better one for now, we can always go back later with more experience.
Attachment:
signature.asc
Description: PGP signature