Separate dma driver for cpu_dais

Separate dma driver for cpu_dais

[jassi brar]

Hi,
  The query concerns ASOC.

Currently we have one platform per card.
What if my card has two dai_links with each cpu_dai fed data
with different mechanism(one with system dma controller and
the other with a dma dedicated to the Audio block). The cpu_dai's
are tightly coupled and I don't wanna have separate socdev's for them.

Presently, I define a dma_id flag in snd_soc_dai, make the snd_soc_card
point to a 'wrapper/muxed' platform driver that checks the dma_id flag and
calls the relevant platform's callbacks. Of course, each cpu_dai initializes
the dma_id to a unique value that corresponds to it's real dma mechanism.

What approach/changes would you suggest I take, so that someday I could
submit the implementation for mainline inclusion?

(For those curious to have a reference - I am talking about Samsung's SoCs
<http://dev.odroid.com/wiki/odroid/pds/HardwareInformation/S5PC100_UM_REV101.pdf
refer to figures on page-1635 & 1637>
that have I2S controllers with h/w mixing capability and where
'secondary fifo' can be
fed with I2S internal DMA controller with dedicated h/w ring buffer)

Thanks.

Re: Separate dma driver for cpu_dais

[Joonyoung Shim]

On 2/17/2010 11:39 AM, jassi brar wrote:
> Hi,
>   The query concerns ASOC.
> 
> Currently we have one platform per card.
> What if my card has two dai_links with each cpu_dai fed data
> with different mechanism(one with system dma controller and
> the other with a dma dedicated to the Audio block). The cpu_dai's
> are tightly coupled and I don't wanna have separate socdev's for them.
> 

Hi, all.

Additionally, i have one more question.

At the ASoC, is there a situation to share the dai(cpu or codec)?
If the dai is shared, i just think the ASoC core cannot support it.

There is the case at the S5PC1XX, it supports the hardware mixing by
using two tx fifo. First Jassi implemented codes using each other cpu
dai as Jassi says at the above for the hardware mixing, but it should
share codec_dai and need some modification of ASoC core.
(I can see codes at the below url
http://git.kernel.org/?p=linux/kernel/git/kki_ap/linux-2.6-samsung.git)

Jassi, how do you think about this?

Re: Separate dma driver for cpu_dais

[jassi brar]

On Wed, Feb 17, 2010 at 3:58 PM, Joonyoung Shim <jy0922.shim@samsung.com> wrote:
> Additionally, i have one more question.
>
> At the ASoC, is there a situation to share the dai(cpu or codec)?
Currently, ASOC doesn't allow sharing cpu or codec dai.

> If the dai is shared, i just think the ASoC core cannot support it.
Correct.

> There is the case at the S5PC1XX, it supports the hardware mixing by
> using two tx fifo. First Jassi implemented codes using each other cpu
> dai as Jassi says at the above for the hardware mixing, but it should
> share codec_dai and need some modification of ASoC core.
> (I can see codes at the below url
> http://git.kernel.org/?p=linux/kernel/git/kki_ap/linux-2.6-samsung.git)
> Jassi, how do you think about this?
Though outside this topic, please see latest code from samsung GIT.
I already share wm8580 dai with I2S primary_fifo dai as well
as secondary_fifo dai. I implemented temporary workaround to share
dais(luckily codec_dai wasn't used in a way to make it impossible,
though cpu_dai still can't be shared).
Also, there is some reason I didn't point to our repository:- the code is
not meant for mainline. It's just a 'workaround' for urgent requirement.

Now, I wud like to get back to getting suggestions about how to
approach the solution from mainline's POV rather than discussing
Samsung's internal code.

Re: Separate dma driver for cpu_dais

[Joonyoung Shim]

On 2/17/2010 4:24 PM, jassi brar wrote:
> On Wed, Feb 17, 2010 at 3:58 PM, Joonyoung Shim <jy0922.shim@samsung.com> wrote:
>> Additionally, i have one more question.
>>
>> At the ASoC, is there a situation to share the dai(cpu or codec)?
> Currently, ASOC doesn't allow sharing cpu or codec dai.
> 
>> If the dai is shared, i just think the ASoC core cannot support it.
> Correct.
> 
>> There is the case at the S5PC1XX, it supports the hardware mixing by
>> using two tx fifo. First Jassi implemented codes using each other cpu
>> dai as Jassi says at the above for the hardware mixing, but it should
>> share codec_dai and need some modification of ASoC core.
>> (I can see codes at the below url
>> http://git.kernel.org/?p=linux/kernel/git/kki_ap/linux-2.6-samsung.git)
>> Jassi, how do you think about this?
> Though outside this topic, please see latest code from samsung GIT.
> I already share wm8580 dai with I2S primary_fifo dai as well
> as secondary_fifo dai. I implemented temporary workaround to share
> dais(luckily codec_dai wasn't used in a way to make it impossible,
> though cpu_dai still can't be shared).
> Also, there is some reason I didn't point to our repository:- the code is
> not meant for mainline. It's just a 'workaround' for urgent requirement.
> 

I think even if it is workaround, the codes orient should go to the 
right side. Anyway, i just wonder about sharing the dai and supporting 
it at the ASoC.

Re: Separate dma driver for cpu_dais

[Mark Brown]

On Wed, Feb 17, 2010 at 03:58:15PM +0900, Joonyoung Shim wrote:

> At the ASoC, is there a situation to share the dai(cpu or codec)?
> If the dai is shared, i just think the ASoC core cannot support it.

I'm not sure what you mean by "sharing" the DAI - sharing between what?
I think you mean using the same DAI in multiple dai_links but I'm not
100% sure?

> There is the case at the S5PC1XX, it supports the hardware mixing by
> using two tx fifo. First Jassi implemented codes using each other cpu
> dai as Jassi says at the above for the hardware mixing, but it should
> share codec_dai and need some modification of ASoC core.

What modifications are you looking for here?  ASoC doesn't make any
effort to avoid reuse of DAIs in multiple links - it doesn't actively do
anything to help here but equally well it's not supposed to get in the
way.

The only thing I can think of off the top of my head that would cause
actual problems is that the DAPM events for stream stop/start in the
CODEC might not be doing reference counting, I'd need to check.  Other
than that there's a bunch of things that could make use cases like this
nicer like providing a way of bundling links that share signals together
and providing callbacks when things start and stop (so shared clocking
can be turned on and off, for example) but these should be things that
could be open coded in individual drivers.

Re: Separate dma driver for cpu_dais

[jassi brar]

On Wed, Feb 17, 2010 at 7:52 PM, Mark Brown
<broonie@opensource.wolfsonmicro.com> wrote:
> On Wed, Feb 17, 2010 at 03:58:15PM +0900, Joonyoung Shim wrote:
>> There is the case at the S5PC1XX, it supports the hardware mixing by
>> using two tx fifo. First Jassi implemented codes using each other cpu
>> dai as Jassi says at the above for the hardware mixing, but it should
>> share codec_dai and need some modification of ASoC core.
>
> What modifications are you looking for here?  ASoC doesn't make any
> effort to avoid reuse of DAIs in multiple links - it doesn't actively do
> anything to help here but equally well it's not supposed to get in the
> way.
The workaround to enable a cpu_dai to be used in multiple dai_links isn't
going to work as such because of cpu_dai->runtime = runtime done
in soc_pcm_open.
Luckily for codec_dai, we can use the same in more than one dai_link.

> The only thing I can think of off the top of my head that would cause
> actual problems is that the DAPM events for stream stop/start in the
> CODEC might not be doing reference counting, I'd need to check.  Other
> than that there's a bunch of things that could make use cases like this
> nicer like providing a way of bundling links that share signals together
> and providing callbacks when things start and stop (so shared clocking
> can be turned on and off, for example) but these should be things that
> could be open coded in individual drivers.
Also, how cleanly could we avoid startup/shutdown/mute etc callbacks
when a codec_dai is used in multiple dai_links, is to be seen.
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: Separate dma driver for cpu_dais

[Mark Brown]

On Wed, Feb 17, 2010 at 09:15:56PM +0900, jassi brar wrote:
> On Wed, Feb 17, 2010 at 7:52 PM, Mark Brown

> > What modifications are you looking for here?  ASoC doesn't make any
> > effort to avoid reuse of DAIs in multiple links - it doesn't actively do
> > anything to help here but equally well it's not supposed to get in the
> > way.

> The workaround to enable a cpu_dai to be used in multiple dai_links isn't

What workaround?

> going to work as such because of cpu_dai->runtime = runtime done
> in soc_pcm_open.
> Luckily for codec_dai, we can use the same in more than one dai_link.

Right, OK - CPU DAIs don't come up so much so I've not noticed that.

> > The only thing I can think of off the top of my head that would cause
> > actual problems is that the DAPM events for stream stop/start in the
> > CODEC might not be doing reference counting, I'd need to check.  Other
> > than that there's a bunch of things that could make use cases like this
> > nicer like providing a way of bundling links that share signals together
> > and providing callbacks when things start and stop (so shared clocking
> > can be turned on and off, for example) but these should be things that
> > could be open coded in individual drivers.

> Also, how cleanly could we avoid startup/shutdown/mute etc callbacks
> when a codec_dai is used in multiple dai_links, is to be seen.

The mute is just part of the reference counting I was mentioning, that's
fairly straightforward to resolve I'd expect.

Startup and shutdown will just work for a lot of drivers, either through
not having relevant callbacks in the first place or winding up being a
noop due to rewriting an identical configuration - as a first
approximation we can probably get away with open coding in any drivers
that do have a problem.

Re: Separate dma driver for cpu_dais

[jassi brar]

On Wed, Feb 17, 2010 at 10:14 PM, Mark Brown
<broonie@opensource.wolfsonmicro.com> wrote:
> On Wed, Feb 17, 2010 at 09:15:56PM +0900, jassi brar wrote:
>> On Wed, Feb 17, 2010 at 7:52 PM, Mark Brown
>
>> > What modifications are you looking for here?  ASoC doesn't make any
>> > effort to avoid reuse of DAIs in multiple links - it doesn't actively do
>> > anything to help here but equally well it's not supposed to get in the
>> > way.
>
>> The workaround to enable a cpu_dai to be used in multiple dai_links isn't
>
> What workaround?
The same as that for codec_dai.
cpu_dai is used exact the same way as codec_dai except for runtime
pointer assignment in soc_pcm_open as i mentioned already.

>> going to work as such because of cpu_dai->runtime = runtime done
>> in soc_pcm_open.
>> Luckily for codec_dai, we can use the same in more than one dai_link.
>
> Right, OK - CPU DAIs don't come up so much so I've not noticed that.
Though, from what I have seen, except atmel-pcm.c, no driver reads
snd_pcm_runtime from the cpu_dai. And atmel-pcm.c can be easily
modified to accommodate any change.
So, it seems, the member 'struct snd_pcm_runtime *'  of 'struct snd_soc_dai'
is mostly unused. We can easily move it to 'struct snd_soc_dai_link' or
even drop it altogether to reduce redundancy(I prefer).
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: Separate dma driver for cpu_dais

[Mark Brown]

On Thu, Feb 18, 2010 at 11:14:57AM +0900, jassi brar wrote:
> On Wed, Feb 17, 2010 at 10:14 PM, Mark Brown
> > On Wed, Feb 17, 2010 at 09:15:56PM +0900, jassi brar wrote:

> >> The workaround to enable a cpu_dai to be used in multiple dai_links isn't

> > What workaround?

> The same as that for codec_dai.
> cpu_dai is used exact the same way as codec_dai except for runtime
> pointer assignment in soc_pcm_open as i mentioned already.

Could you please be more specific about what the workaround you are
referring to is?  I'm genuinely not clear what you mean here.

> So, it seems, the member 'struct snd_pcm_runtime *'  of 'struct snd_soc_dai'
> is mostly unused. We can easily move it to 'struct snd_soc_dai_link' or
> even drop it altogether to reduce redundancy(I prefer).

May as well drop it once the Atmel drivers no longer need it.

Re: Separate dma driver for cpu_dais

[jassi brar]

On Thu, Feb 18, 2010 at 6:35 PM, Mark Brown
<broonie@opensource.wolfsonmicro.com> wrote:
> On Thu, Feb 18, 2010 at 11:14:57AM +0900, jassi brar wrote:
>> On Wed, Feb 17, 2010 at 10:14 PM, Mark Brown
>> > On Wed, Feb 17, 2010 at 09:15:56PM +0900, jassi brar wrote:
>
>> >> The workaround to enable a cpu_dai to be used in multiple dai_links isn't
>
>> > What workaround?
>
>> The same as that for codec_dai.
>> cpu_dai is used exact the same way as codec_dai except for runtime
>> pointer assignment in soc_pcm_open as i mentioned already.
>
> Could you please be more specific about what the workaround you are
> referring to is?  I'm genuinely not clear what you mean here.
In order for a codec_dai to be used in more than one dai_link
I check for its 'active' counter and call shutdown, startup, mute
and schedule_delayed_work only if no other dai_link is
actively using that codec_dai.

>> So, it seems, the member 'struct snd_pcm_runtime *'  of 'struct snd_soc_dai'
>> is mostly unused. We can easily move it to 'struct snd_soc_dai_link' or
>> even drop it altogether to reduce redundancy(I prefer).
> May as well drop it once the Atmel drivers no longer need it.
Ok, will send a patch tomorrow for just that.
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: Separate dma driver for cpu_dais

[Mark Brown]

On Thu, Feb 18, 2010 at 06:42:26PM +0900, jassi brar wrote:
> On Thu, Feb 18, 2010 at 6:35 PM, Mark Brown

> > Could you please be more specific about what the workaround you are
> > referring to is?  I'm genuinely not clear what you mean here.

> In order for a codec_dai to be used in more than one dai_link
> I check for its 'active' counter and call shutdown, startup, mute
> and schedule_delayed_work only if no other dai_link is
> actively using that codec_dai.

Where is the code you are talking about here?  This sounds like driver
code...

Re: Separate dma driver for cpu_dais

[jassi brar]

On Thu, Feb 18, 2010 at 6:52 PM, Mark Brown
<broonie@opensource.wolfsonmicro.com> wrote:
> On Thu, Feb 18, 2010 at 06:42:26PM +0900, jassi brar wrote:
>> On Thu, Feb 18, 2010 at 6:35 PM, Mark Brown
>
>> > Could you please be more specific about what the workaround you are
>> > referring to is?  I'm genuinely not clear what you mean here.
>
>> In order for a codec_dai to be used in more than one dai_link
>> I check for its 'active' counter and call shutdown, startup, mute
>> and schedule_delayed_work only if no other dai_link is
>> actively using that codec_dai.
>
> Where is the code you are talking about here?  This sounds like driver
> code...
Ahh... it's all messed up by that samsung.git link that Joonyoung Shim shared.
I assumed you had a look at that. The code just a quick workaround, so
I didn't put it up here
http://git.kernel.org/?p=linux/kernel/git/kki_ap/linux-2.6-samsung.git;a=commitdiff;h=e5858948bb2c534251b65a850caf197ae880ce1f
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: Separate dma driver for cpu_dais

[Mark Brown]

On Thu, Feb 18, 2010 at 07:32:22PM +0900, jassi brar wrote:
> On Thu, Feb 18, 2010 at 6:52 PM, Mark Brown

> > Where is the code you are talking about here?  This sounds like driver
> > code...

> Ahh... it's all messed up by that samsung.git link that Joonyoung Shim shared.
> I assumed you had a look at that. The code just a quick workaround, so
> I didn't put it up here
> http://git.kernel.org/?p=linux/kernel/git/kki_ap/linux-2.6-samsung.git;a=commitdiff;h=e5858948bb2c534251b65a850caf197ae880ce1f

His link was to the entire tree rather than a particular bit of code, a
bit of a needle in the haystack thing going on there (especially since
you have actually modified the core and for looking at stuff like that
I'd generally drill down into the platform directory before looking at
logs or code).

The bits of that patch that make active a reference counter look good at
first glance, could you please pull them out and submit them?  Like I
said in reply to Joonyoung it's not immediately clear to me that the
startup and shutdown calls should be suppressed since I'd expect that at
least some drivers are going to want to know about multiple uses (for
example, returning -EBUSY if someone tries to have too many things
active at once).

In general for a vendor BSP I'd strongly recommend against any changes
in the core that don't get submitted to mainline - it's more of a
maintinance burden and makes it harder for people to take the drivers
and use them with other kernel versions if they don't notice the change.
Obviously the ideal is to merge the drivers as well, but changes in the
core are more risky to carry out of mainline than drivers.

Re: Separate dma driver for cpu_dais

[jassi brar]

On Thu, Feb 18, 2010 at 7:57 PM, Mark Brown
<broonie@opensource.wolfsonmicro.com> wrote:
> On Thu, Feb 18, 2010 at 07:32:22PM +0900, jassi brar wrote:
>> On Thu, Feb 18, 2010 at 6:52 PM, Mark Brown
>
>> > Where is the code you are talking about here?  This sounds like driver
>> > code...
>
>> Ahh... it's all messed up by that samsung.git link that Joonyoung Shim shared.
>> I assumed you had a look at that. The code just a quick workaround, so
>> I didn't put it up here
>> http://git.kernel.org/?p=linux/kernel/git/kki_ap/linux-2.6-samsung.git;a=commitdiff;h=e5858948bb2c534251b65a850caf197ae880ce1f
>
> His link was to the entire tree rather than a particular bit of code, a
> bit of a needle in the haystack thing going on there (especially since
> you have actually modified the core and for looking at stuff like that
> I'd generally drill down into the platform directory before looking at
> logs or code).
Actually my patches are at the top of 2.6.29 branch(the only one that
has relevant code).
 I wanted to start separate threads for each independent topics towards
implementing a) platform per dai_link, b) dai sharing among dai_links
and c) multi-device per card, but the thread veered off. Anyways...

> The bits of that patch that make active a reference counter look good at
> first glance, could you please pull them out and submit them?
Sure I can, though I think of a few more checks to place in the code.
Will submit tomorrow from workplace.

>  Like I
> said in reply to Joonyoung it's not immediately clear to me that the
> startup and shutdown calls should be suppressed since I'd expect that at
> least some drivers are going to want to know about multiple uses (for
> example, returning -EBUSY if someone tries to have too many things
> active at once).
IMO codecs should simply do as directed by the ASOC.
The multi-instance logic has better be at one place(soc-core.c) rather than
in each codec's driver.
For that reason I modified soc-core.c rather than my device's codec
and cpu driver.

> In general for a vendor BSP I'd strongly recommend against any changes
> in the core that don't get submitted to mainline - it's more of a
> maintinance burden and makes it harder for people to take the drivers
> and use them with other kernel versions if they don't notice the change.
As explained above, I thought I had a reason to do so.
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: Separate dma driver for cpu_dais

[Mark Brown]

On Thu, Feb 18, 2010 at 08:59:38PM +0900, jassi brar wrote:
> On Thu, Feb 18, 2010 at 7:57 PM, Mark Brown

> >  Like I
> > said in reply to Joonyoung it's not immediately clear to me that the
> > startup and shutdown calls should be suppressed since I'd expect that at
> > least some drivers are going to want to know about multiple uses (for
> > example, returning -EBUSY if someone tries to have too many things
> > active at once).

> IMO codecs should simply do as directed by the ASOC.

Right, but that doesn't mean that the device drivers can't do anything
useful here - in terms of restrictions they can't do anything the
hardware can't implmenent.  Another example here is that startup
function can set constraints based on the configuration of other running
links if the hardware has any limitations in that regard (some will,
some won't, and the limitations may not even be anything to do with
audio in some designs).

> The multi-instance logic has better be at one place(soc-core.c) rather than
> in each codec's driver.
> For that reason I modified soc-core.c rather than my device's codec
> and cpu driver.

I'm not saying don't put anything in the core, I'm saying the change you
have implemented goes too far by completely removing the notification to
drivers.  It's not going to be possible for the core to anticipate every
possible thing that a driver might need, and unless a requirement is
common between a number of drivers it's not going to be a benefit to put
it into the core.

> > In general for a vendor BSP I'd strongly recommend against any changes
> > in the core that don't get submitted to mainline - it's more of a
> > maintinance burden and makes it harder for people to take the drivers
> > and use them with other kernel versions if they don't notice the change.

> As explained above, I thought I had a reason to do so.

If you're carrying changes that are already upstream that's fine, it's
carrying changes that are not upstream that causes problems - if the
change has gone upstream then when you rebase or merge with the upstream
version things will work themselves out.  It's changes that haven't gone
upstream that cause trouble, both in terms of maintaining them and for
users trying to do things like merge drivers from several different
sources.

Obviously there are cases where changes in generic code are a good idea,
and I'd encourage you to make such changes, all I'm saying is that I
recommend against shipping them in a vendor BSP without making sure that
they're going to be OK upstream (and ideally are actually upstream).

Re: Separate dma driver for cpu_dais

[jassi brar]

On Thu, Feb 18, 2010 at 10:10 PM, Mark Brown
<broonie@opensource.wolfsonmicro.com> wrote:
> On Thu, Feb 18, 2010 at 08:59:38PM +0900, jassi brar wrote:
>> On Thu, Feb 18, 2010 at 7:57 PM, Mark Brown
>> IMO codecs should simply do as directed by the ASOC.
>
> Right, but that doesn't mean that the device drivers can't do anything
> useful here - in terms of restrictions they can't do anything the
> hardware can't implmenent.  Another example here is that startup
> function can set constraints based on the configuration of other running
> links if the hardware has any limitations in that regard (some will,
> some won't, and the limitations may not even be anything to do with
> audio in some designs).
Okay. Here's another perspective ...
If a dai can have further parallel sub-configurations, perhaps it should
be further divided into more dais.
So, I see a snd_soc_dai as the simplest h/w unit in the system and
presumably  already fully exploited by the active dai_link i.e, none of
its bits can be changed without disturbing the active stream. The
number of shares should be transparent to a dai.
If this assumption is valid please read on, otherwise correct me.

The set of {rate, sample size & format, channels} defines one active
dai. The second stream sharing this dai can not change any of these
parameters without spoiling the active playback/capture - we can allow
this configuration overriding as a feature or prevent as a bug. I
prefer latter. Since there isn't anything new to do now, the ASoC core
can
simply avoid calling hooks in drivers rather than having 50 drivers
implement a check.
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: Separate dma driver for cpu_dais

[Mark Brown]

On Tue, Feb 23, 2010 at 02:45:47PM +0900, jassi brar wrote:

> Okay. Here's another perspective ...
> If a dai can have further parallel sub-configurations, perhaps it should
> be further divided into more dais.
> So, I see a snd_soc_dai as the simplest h/w unit in the system and
> presumably  already fully exploited by the active dai_link i.e, none of
> its bits can be changed without disturbing the active stream. The
> number of shares should be transparent to a dai.
> If this assumption is valid please read on, otherwise correct me.

That doesn't map too well to hardware - it's relatively rare for devices
to require complete symmetry between the playback and capture portions
of the device, though often there are some constraints.  Usually there
is sufficient overlap in the hardware and documentation to describe them
as a single interface but not so much as to force the same configuration
for playback and record.

> The set of {rate, sample size & format, channels} defines one active
> dai. The second stream sharing this dai can not change any of these
> parameters without spoiling the active playback/capture - we can allow
> this configuration overriding as a feature or prevent as a bug. I
> prefer latter. Since there isn't anything new to do now, the ASoC core
> can
> simply avoid calling hooks in drivers rather than having 50 drivers
> implement a check.

If we were to go down this route it'd mean changing the existing drivers
to split their DAIs, which seems like a lot of work and doesn't save us
from having to implement the constraints that do exist.  This would be
pretty invasive and I can't clearly see if either approach gives much of
a win in itself.

It seems easier to approach this by providing ways for drivers to
communicate common constraints to the core with things like flags.
That'd at least mean less change for existing code.

Re: Separate dma driver for cpu_dais

[jassi brar]

On Thu, Feb 18, 2010 at 8:59 PM, jassi brar
> On Thu, Feb 18, 2010 at 7:57 PM, Mark Brown
>
>> The bits of that patch that make active a reference counter look good at
>> first glance, could you please pull them out and submit them?
> Sure I can, though I think of a few more checks to place in the code.
> Will submit tomorrow from workplace.
I was stuck with some other tasks today, I will send it next week.
Today later I will submit patches for discarding 'runtime' parameter
from snd_soc_dai, though.

Re: Separate dma driver for cpu_dais

[Mark Brown]

On Fri, Feb 19, 2010 at 06:48:58PM +0900, jassi brar wrote:

> I was stuck with some other tasks today, I will send it next week.
> Today later I will submit patches for discarding 'runtime' parameter
> from snd_soc_dai, though.

No problem - take your time.  Given that the merge window is likely to
open in the next few days I'd probably hold a change like this off for
2.6.35 anyway.  Thanks for doing this work.

Re: Separate dma driver for cpu_dais

[Joonyoung Shim]

On 2/17/2010 7:52 PM, Mark Brown wrote:
> On Wed, Feb 17, 2010 at 03:58:15PM +0900, Joonyoung Shim wrote:
> 
>> At the ASoC, is there a situation to share the dai(cpu or codec)?
>> If the dai is shared, i just think the ASoC core cannot support it.
> 
> I'm not sure what you mean by "sharing" the DAI - sharing between what?
> I think you mean using the same DAI in multiple dai_links but I'm not
> 100% sure?
> 

Yes, it is what i mean.

>> There is the case at the S5PC1XX, it supports the hardware mixing by
>> using two tx fifo. First Jassi implemented codes using each other cpu
>> dai as Jassi says at the above for the hardware mixing, but it should
>> share codec_dai and need some modification of ASoC core.
> 
> What modifications are you looking for here?  ASoC doesn't make any

Thing modified is the active counting of DAI(struct snd_soc_dai) and pcm
stream(struct snd_soc_pcm_stream). Also, startup() in ops functions of 
same DAI shouldn't be called several times when the device using same 
DAI is opened.

> effort to avoid reuse of DAIs in multiple links - it doesn't actively do
> anything to help here but equally well it's not supposed to get in the
> way.
> 
> The only thing I can think of off the top of my head that would cause
> actual problems is that the DAPM events for stream stop/start in the
> CODEC might not be doing reference counting, I'd need to check.  Other

Ah, i didn't think about this. Right, this should be considered too.

> than that there's a bunch of things that could make use cases like this
> nicer like providing a way of bundling links that share signals together
> and providing callbacks when things start and stop (so shared clocking
> can be turned on and off, for example) but these should be things that
> could be open coded in individual drivers.
> 

Sorry, what does "be open coded" mean?

Re: Separate dma driver for cpu_dais

[Mark Brown]

On Thu, Feb 18, 2010 at 03:12:53PM +0900, Joonyoung Shim wrote:
> On 2/17/2010 7:52 PM, Mark Brown wrote:

> > What modifications are you looking for here?  ASoC doesn't make any

> Thing modified is the active counting of DAI(struct snd_soc_dai) and pcm
> stream(struct snd_soc_pcm_stream). Also, startup() in ops functions of 
> same DAI shouldn't be called several times when the device using same 
> DAI is opened.

Yeah, though it's not 100% clear to me that the multiple calls to
startup() are a bad thing - if the driver does need to coordinate
between multiple possible links then this gives it an opportunity to
handle that.  On the other hand it's likely to be easier for drivers
that do allocations on startup() if there are separate callbacks for
first reference and for each use.

> > can be turned on and off, for example) but these should be things that
> > could be open coded in individual drivers.

> Sorry, what does "be open coded" mean?

Explicitly written in individual drivers rather than being factored out
or otherwise using common code.

Re: Separate dma driver for cpu_dais

[Mark Brown]

On Wed, Feb 17, 2010 at 11:39:47AM +0900, jassi brar wrote:

>   The query concerns ASOC.

Please do remember to CC maintainers on things.  Mail that is sent to
the mailing list only is very likely to get missed.

> Currently we have one platform per card.
> What if my card has two dai_links with each cpu_dai fed data
> with different mechanism(one with system dma controller and
> the other with a dma dedicated to the Audio block). The cpu_dai's
> are tightly coupled and I don't wanna have separate socdev's for them.

Your platform driver would need to handle this.  

> Presently, I define a dma_id flag in snd_soc_dai, make the snd_soc_card
> point to a 'wrapper/muxed' platform driver that checks the dma_id flag and
> calls the relevant platform's callbacks. Of course, each cpu_dai initializes
> the dma_id to a unique value that corresponds to it's real dma mechanism.

Yes, that's fine - you could also set some function pointers for in an
ops structure in the driver private DMA data that gets passed through,
avoiding the need for switch statements but either way works.  This is
not really much different to supplying any other information about how
to set up the DMA for a given front end, it's just that the information
includes going down different code paths for different controllers.

> (For those curious to have a reference - I am talking about Samsung's SoCs
> <http://dev.odroid.com/wiki/odroid/pds/HardwareInformation/S5PC100_UM_REV101.pdf
> refer to figures on page-1635 & 1637>
> that have I2S controllers with h/w mixing capability and where
> 'secondary fifo' can be
> fed with I2S internal DMA controller with dedicated h/w ring buffer)

So.  Ideally for hardware mixing you'd represent the hardware mixing as
a device within ASoC but that's not currently supported - the multi
CODEC work should take care of it but it's not there yet.

However, the hardware mixing you've got there looks relatively simple so
it should work fine to just represent it as two external DAIs - it looks
like there's no sample rate or format conversion so the benefits of
having the mixing visible to ASoC are relatively minor, the main benefit
would be the ability to run the two DAIs inside the CPU at different
rates from the external DAI but the lack of conversion means that's not
possible anyway and the driver will have to constrain the second channel
to what the first is doing anyway.

Overall I don't see any immediate problem with supporting this in
mainline as-is from what I've looked at so far.

Re: Separate dma driver for cpu_dais

[jassi brar]

On Wed, Feb 17, 2010 at 7:42 PM, Mark Brown
<broonie@opensource.wolfsonmicro.com> wrote:
> On Wed, Feb 17, 2010 at 11:39:47AM +0900, jassi brar wrote:
>>   The query concerns ASOC.
> Please do remember to CC maintainers on things.  Mail that is sent to
> the mailing list only is very likely to get missed.
Ok.
>> that have I2S controllers with h/w mixing capability and where
>> 'secondary fifo' can be
>> fed with I2S internal DMA controller with dedicated h/w ring buffer)
>
> So.  Ideally for hardware mixing you'd represent the hardware mixing as
> a device within ASoC but that's not currently supported - the multi
> CODEC work should take care of it but it's not there yet.
is that work visible on some branch in ur tree?

> Overall I don't see any immediate problem with supporting this in
> mainline as-is from what I've looked at so far.
I assume this is about dma_id flag?
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: Separate dma driver for cpu_dais

[Mark Brown]

On Wed, Feb 17, 2010 at 09:13:05PM +0900, jassi brar wrote:
> On Wed, Feb 17, 2010 at 7:42 PM, Mark Brown

> > So.  Ideally for hardware mixing you'd represent the hardware mixing as
> > a device within ASoC but that's not currently supported - the multi
> > CODEC work should take care of it but it's not there yet.

> is that work visible on some branch in ur tree?

I'm not doing it, it's on Liam's list.  I generally don't maintian
branches for anything.

> > Overall I don't see any immediate problem with supporting this in
> > mainline as-is from what I've looked at so far.

> I assume this is about dma_id flag?

The hardware mixing too, like I say it's the most trivial case possible
so doesn't run into many problems.