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.