About Cleanup ASoC

[Cezary Rojewski]

On 2022-06-03 1:57 AM, Kuninori Morimoto wrote:
> Thanks.
> "my Sound" mean "my sound driver".
> The Device image is like this
> 
> 	+-- DeviceA --+
> 	|+-----------+|
> 	||Component  ||
> 	||         [DAI]
> 	||         [DAI]
> 	...
> 	||         [DAI]
> 	||         [DAI]
> 	|+-----------+|
> 	+-------------+
> 
> It calls devm_snd_soc_register_component() here.
> Number of DAIs = rsnd_rdai_nr(priv) is based on the SoC (= DT settings),
> but these are same DAI.
> 	<https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/sound/soc/sh/rcar/core.c?h=v5.18#n1923>
> 
> The DAIs are setuped here.
> 	<https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/sound/soc/sh/rcar/core.c?h=v5.18#n1350>
> 
> Component driver is here
> 	<https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/sound/soc/sh/rcar/core.c?h=v5.18#n1810>
> 
> DAI ops is here
> 	<https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/sound/soc/sh/rcar/core.c?h=v5.18#n1067>


Hello Kuninori,

Thank you for the links - helped me speed up the learning process.

So, together with Amadeo we spent some time analyzing your audio stack. 
The equivalent of avs's pci_driver (sound/soc/intel/avs/core.c) seems to 
be rcar_sound platform_driver defined in sound/soc/sh/rcar/core.c.
There is no firmware or topology involved. Instead, we have hardware 
components, each representing some processing function e.g.: sample rate 
conversion. There is also a component which does not represent any of 
such functions - represents ASoC side of things instead. It's called 
*DAI*. And that's the component we were looking at mostly.

I believe each *DAI* exposes up to four FEs depending on the board, and 
during PCM operations, every FE enlists one or more hw components 
(rsnd_mod instances) to do the actual job.

Earlier in the thread you specified the following example as being 
problematic:

	+-- basic board --------+
	|+--------+             |
	|| CPU ch0| <--> CodecA |
	||     ch1| <-+         |
	|+--------+   |         |
	+-------------|---------+

	+-- expansion board ----+
	|             |         |
	|             +-> CodecB|
	+-----------------------+

with suggestion to solve it with 1xComponent:NxCard relation which is 
not supported by ASoC. However, we believe ASoC already presents 
solutions to such problem, either through 1) compound card or less 
commonly, by 2) splitting one big CPU-Card pair into several smaller pairs.

Relation that ASoC does support is NxComponent:1xCard. As both CPU and 
Codec are components, this roughly translates to: NxCPU:1xCard:NxCodec. 
Your case is an example of 1xCPU:1xCard:2xCodec. Such 
1xCPU:1xCard:NxCodec combinations are arguably the most common ones and 
developers majority of time are picking option 1). Less hassle. See 
sound/soc/intel/boards for examples.

As I believe the expansion board has some identifier, we will be able to 
fetch whether we are dealing with basic board or basic+expansion board. 
Or at least the board name should differ between the two devices. If so, 
two machine board drivers are needed - one servicing just the basic 
board (basic sound card) and the other for combo board (combo sound card).


Another option is registration of many components allowing developer to 
represent each logical block with a single component. This comes down to 
invoking snd_soc_register_component() multiple times.

This is especially useful if one or more of your four FEs available on 
the R-Car board has nothing to do with CodecA. Let's say FEs 1,2,3 are 
actually routed through CodecA whereas FE 4 through CodecB. It is 
logical to split the CPU functionality so that FE 4 is being serviced by 
the CPU component that is responsible for just that one part. This can 
be modified further if we want to expose entire set of FEs for both 
CodecA and CodecB. In such case the essence of PCM operations should be 
moved to some common code so that functions exposed via dai->ops become 
wrappers - eliminates code duplication. Then just create two instances 
of your CPU component, assign dai_driver(s) and register them both.

Currently, you're calling snd_soc_register_component() just once in your 
driver so option 2) is not possible without altering 
rsnd_dai_probe()/rsnd_probe().


Perhaps there is something I'm missing but considering that ASoC does 
have solutions to the raised problem, I do not see why either 1) or 2) 
cannot be made use of to deal with the problem. I and Amadeo can 
definitely help with 2) should you select it :)


Regards,
Czarek