About ASoC DAIs cleanup

About ASoC DAIs cleanup

[Kuninori Morimoto]

Hi Mark, Lars-Peter
Cc Vinod

About current CPU/Codec DAIs, it is unbalance now I guess.
Codec supports multi DAIs, but CPU is only single DAI.
Because of this unbalanced DAIs support,
Each DAIs code are not readable.

In perfect world, I guess no categorized DAI is nice,
but in reality, it is difficult.
Because many drivers are directly using rtd->codec_dai / rtd->cpu_dai,
and it is difficult to lookup CPU/Codec DAI from non categorize DAIs.
Thus, we can't support non-categorized DAI at this point.

Now Vinod want to use Multi CPU DAI.
If we can support Multi CPU DAI / Multi Codec DAI,
we can cleanup current DAI code more.
But what do you think ?

Best regards
---
Kuninori Morimoto

Re: About ASoC DAIs cleanup

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 719 bytes --]

On Wed, Dec 13, 2017 at 07:19:39AM +0000, Kuninori Morimoto wrote:

> Because many drivers are directly using rtd->codec_dai / rtd->cpu_dai,
> and it is difficult to lookup CPU/Codec DAI from non categorize DAIs.
> Thus, we can't support non-categorized DAI at this point.

> Now Vinod want to use Multi CPU DAI.
> If we can support Multi CPU DAI / Multi Codec DAI,
> we can cleanup current DAI code more.
> But what do you think ?

Cleaning things up so we don't need to use rtd->cpu_dai and rtd->codec_dai
would definitely be nice, it's also useful for CODEC<->CODEC links.  Off
the top of my head wrapping the accesses with macros/functions then
implementing a way of getting the DAI behind them would be tractable?

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: About ASoC DAIs cleanup

[Kuninori Morimoto]

Hi Mark

Thank you for your feedback

> > Because many drivers are directly using rtd->codec_dai / rtd->cpu_dai,
> > and it is difficult to lookup CPU/Codec DAI from non categorize DAIs.
> > Thus, we can't support non-categorized DAI at this point.
> 
> > Now Vinod want to use Multi CPU DAI.
> > If we can support Multi CPU DAI / Multi Codec DAI,
> > we can cleanup current DAI code more.
> > But what do you think ?
> 
> Cleaning things up so we don't need to use rtd->cpu_dai and rtd->codec_dai
> would definitely be nice, it's also useful for CODEC<->CODEC links.  Off
> the top of my head wrapping the accesses with macros/functions then
> implementing a way of getting the DAI behind them would be tractable?

About cleanup rtd->cpu_dai / rtd->codec_dai topic,
I don't know we can do it at this point.
But, I'm happy to think about it.
I'm thinking like this
	1st step: add Multi CPU support
	2nd step: Share code between CPU/Codec DAI
	3rd step: think about No-Categorized DAI (if possible)

Because many drivers are directly using rtd->cpu_dai, rtd->codec_dai

Best regards
---
Kuninori Morimoto

Re: About ASoC DAIs cleanup

[Vinod Koul]

On Thu, Dec 14, 2017 at 03:07:30AM +0000, Kuninori Morimoto wrote:
> 
> Hi Mark
> 
> Thank you for your feedback
> 
> > > Because many drivers are directly using rtd->codec_dai / rtd->cpu_dai,
> > > and it is difficult to lookup CPU/Codec DAI from non categorize DAIs.
> > > Thus, we can't support non-categorized DAI at this point.
> > 
> > > Now Vinod want to use Multi CPU DAI.
> > > If we can support Multi CPU DAI / Multi Codec DAI,
> > > we can cleanup current DAI code more.
> > > But what do you think ?

Thanks Morimoto San for initiating the thread,

> > 
> > Cleaning things up so we don't need to use rtd->cpu_dai and rtd->codec_dai
> > would definitely be nice, it's also useful for CODEC<->CODEC links.  Off
> > the top of my head wrapping the accesses with macros/functions then
> > implementing a way of getting the DAI behind them would be tractable?

Yes but one of the problems I see that we have specific ordering on the
DAI ops between various components, is that a specific requirement?

> About cleanup rtd->cpu_dai / rtd->codec_dai topic,
> I don't know we can do it at this point.
> But, I'm happy to think about it.
> I'm thinking like this
> 	1st step: add Multi CPU support

Okay we will send the patches for this by next week or so...

> 	2nd step: Share code between CPU/Codec DAI

And then discuss how to make it common

> 	3rd step: think about No-Categorized DAI (if possible)
> Because many drivers are directly using rtd->cpu_dai, rtd->codec_dai

We should fix that part while at it. I guess drivers needs own dai,
possibly for name or some data, so if they have helpers for that, we
should be able to remove those..

-- 
~Vinod

Re: About ASoC DAIs cleanup

[Kuninori Morimoto]

Hi Vinod

> Thanks Morimoto San for initiating the thread,

I'm happy about that

> 	1st step: add Multi CPU support
>
> Okay we will send the patches for this by next week or so...

Sounds very nice for me

> > 	3rd step: think about No-Categorized DAI (if possible)
> > Because many drivers are directly using rtd->cpu_dai, rtd->codec_dai
> 
> We should fix that part while at it. I guess drivers needs own dai,
> possibly for name or some data, so if they have helpers for that, we
> should be able to remove those..

I'm happy to investigate how to do it

Best regards
---
Kuninori Morimoto

Re: About ASoC DAIs cleanup

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 1212 bytes --]

On Thu, Dec 14, 2017 at 10:36:48AM +0530, Vinod Koul wrote:
> On Thu, Dec 14, 2017 at 03:07:30AM +0000, Kuninori Morimoto wrote:

> > > Cleaning things up so we don't need to use rtd->cpu_dai and rtd->codec_dai
> > > would definitely be nice, it's also useful for CODEC<->CODEC links.  Off
> > > the top of my head wrapping the accesses with macros/functions then
> > > implementing a way of getting the DAI behind them would be tractable?

> Yes but one of the problems I see that we have specific ordering on the
> DAI ops between various components, is that a specific requirement?

It's designed to minimize pops, it's not a hard requirement but changing
it might break things for existing systems.

> > 	3rd step: think about No-Categorized DAI (if possible)
> > Because many drivers are directly using rtd->cpu_dai, rtd->codec_dai

> We should fix that part while at it. I guess drivers needs own dai,
> possibly for name or some data, so if they have helpers for that, we
> should be able to remove those..

The drivers need some way to get their driver data back and to know
which DAI to address on multi device hardware.  Most of the callbacks
get the DAI passed in directly, that's the simplest thing.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: About ASoC DAIs cleanup

[Vinod Koul]

On Thu, Dec 14, 2017 at 11:37:18AM +0000, Mark Brown wrote:
> On Thu, Dec 14, 2017 at 10:36:48AM +0530, Vinod Koul wrote:
> > On Thu, Dec 14, 2017 at 03:07:30AM +0000, Kuninori Morimoto wrote:
> 
> > > > Cleaning things up so we don't need to use rtd->cpu_dai and rtd->codec_dai
> > > > would definitely be nice, it's also useful for CODEC<->CODEC links.  Off
> > > > the top of my head wrapping the accesses with macros/functions then
> > > > implementing a way of getting the DAI behind them would be tractable?
> 
> > Yes but one of the problems I see that we have specific ordering on the
> > DAI ops between various components, is that a specific requirement?
> 
> It's designed to minimize pops, it's not a hard requirement but changing
> it might break things for existing systems.

Yes that was my hunch too, so lets keep the order unchanged.

> > > 	3rd step: think about No-Categorized DAI (if possible)
> > > Because many drivers are directly using rtd->cpu_dai, rtd->codec_dai
> 
> > We should fix that part while at it. I guess drivers needs own dai,
> > possibly for name or some data, so if they have helpers for that, we
> > should be able to remove those..
> 
> The drivers need some way to get their driver data back and to know
> which DAI to address on multi device hardware.  Most of the callbacks
> get the DAI passed in directly, that's the simplest thing.

Precisely :)

-- 
~Vinod

Re: About ASoC DAIs cleanup

[Kuninori Morimoto]

Hi Mark

> > > > > Cleaning things up so we don't need to use rtd->cpu_dai and rtd->codec_dai
> > > > > would definitely be nice, it's also useful for CODEC<->CODEC links.  Off
> > > > > the top of my head wrapping the accesses with macros/functions then
> > > > > implementing a way of getting the DAI behind them would be tractable?
> > 
> > > Yes but one of the problems I see that we have specific ordering on the
> > > DAI ops between various components, is that a specific requirement?
> > 
> > It's designed to minimize pops, it's not a hard requirement but changing
> > it might break things for existing systems.
> 
> Yes that was my hunch too, so lets keep the order unchanged.

Current DAI has CPU/Codec categorize, and sometimes we want to use
Codec <-> Codec connection.
Thus, we want to remove current cpu_xx/codec_xx naming.
But, because of above reason, we need to know which DAI is which portion,
thus, we need flags anyway, I think.

My quick idea is using "peripheral" flag.
I think we can set it on soc_bind_dai_link() ?
And we can use it like below

	for_each_dais(dai, xxx) {
		if(!dai->peripheral)
			/* non Peripheral == CPU portion */

		if(dai->peripheral)
			/* Peripheral == Codec portion */
	}

But what do you think about this idea/naming etc ?
I want to investigate more about Codec <-> Codec connection,
which driver is using it ? Especially CPU portion

Best regards
---
Kuninori Morimoto

Re: About ASoC DAIs cleanup

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 906 bytes --]

On Mon, Dec 18, 2017 at 02:16:07AM +0000, Kuninori Morimoto wrote:

> My quick idea is using "peripheral" flag.
> I think we can set it on soc_bind_dai_link() ?
> And we can use it like below

> 	for_each_dais(dai, xxx) {
> 		if(!dai->peripheral)
> 			/* non Peripheral == CPU portion */
> 
> 		if(dai->peripheral)
> 			/* Peripheral == Codec portion */
> 	}

> But what do you think about this idea/naming etc ?
> I want to investigate more about Codec <-> Codec connection,
> which driver is using it ? Especially CPU portion

I'm thinking it might be better to keep the list ordered in the DAI link
- that will scale up better with multi-drop links.  What's going to be a
bit more tricky sometimes is working out which end of the link is a CPU
DAI but we can probably take a good guess easily enough on order neutral
bindings and things liken simple-card already know explicitly.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: About ASoC DAIs cleanup

[Kuninori Morimoto]

Hi Mark

Thank you for sharing ideas

> > 	for_each_dais(dai, xxx) {
> > 		if(!dai->peripheral)
> > 			/* non Peripheral == CPU portion */
> > 
> > 		if(dai->peripheral)
> > 			/* Peripheral == Codec portion */
> > 	}
> 
> > But what do you think about this idea/naming etc ?
> > I want to investigate more about Codec <-> Codec connection,
> > which driver is using it ? Especially CPU portion
> 
> I'm thinking it might be better to keep the list ordered in the DAI link
> - that will scale up better with multi-drop links.  What's going to be a
> bit more tricky sometimes is working out which end of the link is a CPU
> DAI but we can probably take a good guess easily enough on order neutral
> bindings and things liken simple-card already know explicitly.

If my understanding was correct, we can call all DAIs
by one for_each loop with controllable order on your idea.
This is nice.
But, callback order will be exchanged ?
For example soc_pcm_trigger() case, .trigger callback order currently is

	Codec DAI -> Component(Platform) -> CPU DAI -> RTD

it will be

	all ordered DAIs -> Component(Platform) -> RTD

Codec / CPU callback order are OK, but DAI / Component order is exchanged.
If this is not a big problem, we can do it.

And one issue I noticed.
If we merged all Codec/CPU DAI into one DAI list, and without
flags (like .peripheral flag), current DAI master/slave direction will be problem.
At least snd_soc_runtime_set_dai_fmt() is switching it for Codec <-> Codec case.
If we can change current SND_SOC_DAIFMT_CBx_CFx style to
xx_MASTER / xx_SLAVE style on each DAIs, this can be no problem I think.
I guess Lars is thinking about it ?

Best regards
---
Kuninori Morimoto

Re: About ASoC DAIs cleanup

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 1235 bytes --]

On Thu, Dec 21, 2017 at 01:28:09AM +0000, Kuninori Morimoto wrote:

> If my understanding was correct, we can call all DAIs
> by one for_each loop with controllable order on your idea.
> This is nice.
> But, callback order will be exchanged ?
> For example soc_pcm_trigger() case, .trigger callback order currently is

> 	Codec DAI -> Component(Platform) -> CPU DAI -> RTD

> it will be

> 	all ordered DAIs -> Component(Platform) -> RTD

> Codec / CPU callback order are OK, but DAI / Component order is exchanged.
> If this is not a big problem, we can do it.

Ah, yes - we'd need to mix in the platform :/

> And one issue I noticed.
> If we merged all Codec/CPU DAI into one DAI list, and without
> flags (like .peripheral flag), current DAI master/slave direction will be problem.

Yes, there's other things need to be fixed - I'm not saying it'd be a
simple transition.

> At least snd_soc_runtime_set_dai_fmt() is switching it for Codec <-> Codec case.
> If we can change current SND_SOC_DAIFMT_CBx_CFx style to
> xx_MASTER / xx_SLAVE style on each DAIs, this can be no problem I think.
> I guess Lars is thinking about it ?

It's been talked about for years but it's another of these things that's
a lot of work to transition.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: About ASoC DAIs cleanup

[Kuninori Morimoto]

Hi Mark

Thank you for your feedback

> > Codec / CPU callback order are OK, but DAI / Component order is exchanged.
> > If this is not a big problem, we can do it.
> 
> Ah, yes - we'd need to mix in the platform :/
(snip)
> Yes, there's other things need to be fixed - I'm not saying it'd be a
> simple transition.
(snip)
> It's been talked about for years but it's another of these things that's
> a lot of work to transition.

Hmm... OK...
So, we have 2 choices ?

	choice 1) Use ordered DAI
		- We need to select all DAI / Component callback order somehow
	          (I think every callback can use same order ?).
		- SND_SOC_DAIFMT_CBx_CFx exchanged is mandatory
		- DAI categorize is no longer needed.
	choice 2) Use categorized DAI
		- DAI categorize is still needed.
		- SND_SOC_DAIFMT_CBx_CFx exchanged is not mandatory

I think choice 2) is good for step-by-step approach ;)

I think, we can use "1 dai_link approach style" on choice 2) anyway.
And it can use "ordered DAI" approach style too.
It doesn't include "component order", but good step for "choice 1)" ?

If I use "CPU/Codec" naming here,
we will have multi CPU DAI, and multi Codec DAI.
Then, we can know num_cpu_dai, num_codec_dai.
1 ordered dai_link will be

	dai_link = CPU0, CPU1, ... Codec0, Codec1, ...

we can know which one is CPU/Codec DAI by using num_cpu/codec_dai.

Best regards
---
Kuninori Morimoto