[alsa-devel] About ASoC DAIs cleanup

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

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?

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

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..

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

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.

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 :)

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

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.

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

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.

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
participants (3)
-
Kuninori Morimoto
-
Mark Brown
-
Vinod Koul