About snd_dmaengine_pcm_trigger()

About snd_dmaengine_pcm_trigger()

[Kuninori Morimoto]

Hi Mark, Lars and all

I need your opinion

Now, I'm working for ${LINUX}/sound/soc/sh/fsi and ${LINUX}/sound/soc/sh/rcar drirvers.
These drivers are supporting DMAEngine transfer,
but it is using original DMAEngine method,
not using snd_dmaengine_pcm_trigger() or snd_dmaengine_pcm_register().
I was requested to use it from Mark.

The reason why I couldn't use it was our DMAEngine
didn't have "cyclic" tranfer support which is used on
dmaengine_pcm_prepare_and_submit()
But now, I created cyclic support on our DMA driver (on my local PC at this point)

Then, I noticed our drivers still can't use snd_dmaengine_pcm_xxx() method.

1st, our device needs PIO tranfer support too.
Current PIO/DMA transfer are sharing many methods/functions.
Code will become difficult to read if it is separated forcibly.
(to using snd_dmaengine_pcm_xxx())

2nd, our device DMA ON/OFF timing has relation ship to other register settings.
Unfortunately, our device is picky, it needs like this

               HW init setting -> DMA ON -> HW start setting

It can be solved if snd_dmaengine_pcm_xxx() has callbacks,
but many callbacks are needed...

3rd, our device needs special method on snd_pcm_ops
int the future.

>From my point of view,
our driver can replace to use "cyclic" DMA transfer
instead of current original DMAEngine method.
I guess, it is not difficult.
But, using snd_dmaengine_pcm_xxx() is difficult.

But, what do you think ?

Re: About snd_dmaengine_pcm_trigger()

[Lars-Peter Clausen]

On 03/05/2014 06:20 AM, Kuninori Morimoto wrote:
> Hi Mark, Lars and all
>
> I need your opinion
>
> Now, I'm working for ${LINUX}/sound/soc/sh/fsi and ${LINUX}/sound/soc/sh/rcar drirvers.
> These drivers are supporting DMAEngine transfer,
> but it is using original DMAEngine method,
> not using snd_dmaengine_pcm_trigger() or snd_dmaengine_pcm_register().
> I was requested to use it from Mark.
>
> The reason why I couldn't use it was our DMAEngine
> didn't have "cyclic" tranfer support which is used on
> dmaengine_pcm_prepare_and_submit()
> But now, I created cyclic support on our DMA driver (on my local PC at this point)
>
> Then, I noticed our drivers still can't use snd_dmaengine_pcm_xxx() method.
>
> 1st, our device needs PIO tranfer support too.
> Current PIO/DMA transfer are sharing many methods/functions.
> Code will become difficult to read if it is separated forcibly.
> (to using snd_dmaengine_pcm_xxx())

Ideally from a high level abstraction point of few PIO and DMA would be two 
completely separate components that can be used interchangeably. There might 
be hardware restrictions though that do not allow this.

>
> 2nd, our device DMA ON/OFF timing has relation ship to other register settings.
> Unfortunately, our device is picky, it needs like this
>
>                 HW init setting -> DMA ON -> HW start setting

What kind of HW init is this?

>
> It can be solved if snd_dmaengine_pcm_xxx() has callbacks,
> but many callbacks are needed...

You can wrap the snd_dmaengine_pcm functions in your pcm_ops callbacks, like 
this:

int sh_pcm_trigger(struct snd_pcm_substream *substream, int cmd)
{
	init hw ...
	
	snd_dmaengine_pcm_trigger(substream, cmd);

	start hw ...
}

Each of the snd_dmaengine_pcm functions does pretty much one specific task. 
There is not much room for inserting callbacks.

>
> 3rd, our device needs special method on snd_pcm_ops
> int the future.
>

What kind of special ops?

>  From my point of view,
> our driver can replace to use "cyclic" DMA transfer
> instead of current original DMAEngine method.
> I guess, it is not difficult.
> But, using snd_dmaengine_pcm_xxx() is difficult.
>

I don't think using the snd_dmaengine_pcm helpers should be difficult, 
since, as I said, each of them perform one very specific task. If you'd 
directly call the dmaengine API you'd end up with pretty much the same code 
as the dmaengine PCM helpers. What's more of a challenge would be to use the 
generic dmaengine PCM driver.

I think the issue with the sound/soc/sh/ code is that there never was a 
clean separation between the DMA/PIO and the DAI configuration code. And 
especially with rcar you seem to be reimplementing a ASoC-like framework 
on-top of ASoC. Which makes it rather hard to reused generic code, since the 
generic code doesn't know about all the custom hooks and callbacks. The ASoC 
framework is not set in stone, if there is something missing to properly 
support your hardware you should try to extend the framework to support this 
rather than working around it in your drivers.

- Lars

Re: About snd_dmaengine_pcm_trigger()

[Kuninori Morimoto]

Hi Lars

Thank you for your feedback

> > 3rd, our device needs special method on snd_pcm_ops
> > int the future.
> >
> 
> What kind of special ops?

It would like to use snd_soc_dapm_widget,
but we can update snd_soc_component instead of platform ?

> I don't think using the snd_dmaengine_pcm helpers should be difficult, 
> since, as I said, each of them perform one very specific task. If you'd 
> directly call the dmaengine API you'd end up with pretty much the same code 
> as the dmaengine PCM helpers. What's more of a challenge would be to use the 
> generic dmaengine PCM driver.
> 
> I think the issue with the sound/soc/sh/ code is that there never was a 
> clean separation between the DMA/PIO and the DAI configuration code. And 
> especially with rcar you seem to be reimplementing a ASoC-like framework 
> on-top of ASoC. Which makes it rather hard to reused generic code, since the 
> generic code doesn't know about all the custom hooks and callbacks. The ASoC 
> framework is not set in stone, if there is something missing to properly 
> support your hardware you should try to extend the framework to support this 
> rather than working around it in your drivers.

I see...
I need investigate more about this,
but, before that, can I confirm ? 

basically...

platform  : for PIO or DMA transfer method
component : for DAI configuration = CPU side HW settings etc

Is this correct ?

And about ASoC-like framework, rcar driver needs to control many kind of devices,
and it depends on platform which device is used.
(each devices have different feature)
So, I used ASoC-like framework, but in this case, what should I do ?

Re: About snd_dmaengine_pcm_trigger()

[Lars-Peter Clausen]

On 03/05/2014 09:32 AM, Kuninori Morimoto wrote:
>
> Hi Lars
>
> Thank you for your feedback
>
>>> 3rd, our device needs special method on snd_pcm_ops
>>> int the future.
>>>
>>
>> What kind of special ops?
>
> It would like to use snd_soc_dapm_widget,
> but we can update snd_soc_component instead of platform ?

I'm not sure I fully understand what you mean, by using snd_soc_dapm_widget 
in the pcm ops.

>
>> I don't think using the snd_dmaengine_pcm helpers should be difficult,
>> since, as I said, each of them perform one very specific task. If you'd
>> directly call the dmaengine API you'd end up with pretty much the same code
>> as the dmaengine PCM helpers. What's more of a challenge would be to use the
>> generic dmaengine PCM driver.
>>
>> I think the issue with the sound/soc/sh/ code is that there never was a
>> clean separation between the DMA/PIO and the DAI configuration code. And
>> especially with rcar you seem to be reimplementing a ASoC-like framework
>> on-top of ASoC. Which makes it rather hard to reused generic code, since the
>> generic code doesn't know about all the custom hooks and callbacks. The ASoC
>> framework is not set in stone, if there is something missing to properly
>> support your hardware you should try to extend the framework to support this
>> rather than working around it in your drivers.
>
> I see...
> I need investigate more about this,
> but, before that, can I confirm ?
>
> basically...
>
> platform  : for PIO or DMA transfer method
> component : for DAI configuration = CPU side HW settings etc
>
> Is this correct ?
>

Yes.

> And about ASoC-like framework, rcar driver needs to control many kind of devices,
> and it depends on platform which device is used.
> (each devices have different feature)
> So, I used ASoC-like framework, but in this case, what should I do ?

Make them ASoC components ;) I don't know the hardware, but looking at the 
code it doesn't seem to be that different from other modern host side audio 
processing units. But one step at a time, first maybe try and see if you can 
switch over to using the dmaengine PCM helper functions. Then later thing 
about how this all can be better integrated into the ASoC framework.

- Lars

Re: About snd_dmaengine_pcm_trigger()

[Kuninori Morimoto]

Hi Lars, Mark

> > platform  : for PIO or DMA transfer method
> > component : for DAI configuration = CPU side HW settings etc
> >
> > Is this correct ?
> >
> 
> Yes.

Thank you

> > And about ASoC-like framework, rcar driver needs to control many kind of devices,
> > and it depends on platform which device is used.
> > (each devices have different feature)
> > So, I used ASoC-like framework, but in this case, what should I do ?
> 
> Make them ASoC components ;) I don't know the hardware, but looking at the 
> code it doesn't seem to be that different from other modern host side audio 
> processing units. But one step at a time, first maybe try and see if you can 
> switch over to using the dmaengine PCM helper functions. Then later thing 
> about how this all can be better integrated into the ASoC framework.

OK, I see. will do.
I guess it needs many steps.
OTOH, in reality, I need to work for my business task too.
Thus, actually, I need to add new feature on current rcar driver.
So, My plain is...

 - send new feature patch to ML
 - work for switching to above ASoC framework

Is this acceptable ?

Best regards
---
Kuninori Morimoto

Re: About snd_dmaengine_pcm_trigger()

[Mark Brown]

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

On Wed, Mar 05, 2014 at 04:22:38PM -0800, Kuninori Morimoto wrote:

> Thus, actually, I need to add new feature on current rcar driver.
> So, My plain is...

>  - send new feature patch to ML
>  - work for switching to above ASoC framework

> Is this acceptable ?

Yes, it's fine.

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

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

Re: About snd_dmaengine_pcm_trigger()

[Kuninori Morimoto]

Hi Mark

> > Thus, actually, I need to add new feature on current rcar driver.
> > So, My plain is...
> 
> >  - send new feature patch to ML
> >  - work for switching to above ASoC framework
> 
> > Is this acceptable ?
> 
> Yes, it's fine.

Thank you

Re: About snd_dmaengine_pcm_trigger()

[Kuninori Morimoto]

Hi Lars, Mark

> Make them ASoC components ;) I don't know the hardware, but looking at the 
> code it doesn't seem to be that different from other modern host side audio 
> processing units. But one step at a time, first maybe try and see if you can 
> switch over to using the dmaengine PCM helper functions. Then later thing 
> about how this all can be better integrated into the ASoC framework.

In my quick check,
I guess, my drivers (FSI/rcar) can use 
${LINUX}/sound/core/pcm_dmaengine.c methods as 1st step.

OTOH, these drivers has PIO transfer too.
So, if ALSA has PIO method like pcm_pio.c (?) which has similar method,
it seems easy to switch over.
But what do you think ?

Best regards
---
Kuninori Morimoto

Re: About snd_dmaengine_pcm_trigger()

[Lars-Peter Clausen]

On 03/11/2014 05:47 AM, Kuninori Morimoto wrote:
>
> Hi Lars, Mark
>
>> Make them ASoC components ;) I don't know the hardware, but looking at the
>> code it doesn't seem to be that different from other modern host side audio
>> processing units. But one step at a time, first maybe try and see if you can
>> switch over to using the dmaengine PCM helper functions. Then later thing
>> about how this all can be better integrated into the ASoC framework.
>
> In my quick check,
> I guess, my drivers (FSI/rcar) can use
> ${LINUX}/sound/core/pcm_dmaengine.c methods as 1st step.
>
> OTOH, these drivers has PIO transfer too.
> So, if ALSA has PIO method like pcm_pio.c (?) which has similar method,
> it seems easy to switch over.
> But what do you think ?

If there are other platforms that also do PIO in a very similar way it might 
make sense to factor this out.

The reason why we factored out the dmaengine DMA stuff is because there is a 
standard API that hides the implementation details and is supposed to behave 
the same for each system. For PIO you do not have such a abstraction layer 
that hides the details, so I'm not sure how similar PIO between different 
platforms will actually be.

- Lars

Re: About snd_dmaengine_pcm_trigger()

[Mark Brown]

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

On Tue, Mar 11, 2014 at 10:47:32AM +0100, Lars-Peter Clausen wrote:

> The reason why we factored out the dmaengine DMA stuff is because
> there is a standard API that hides the implementation details and is
> supposed to behave the same for each system. For PIO you do not have
> such a abstraction layer that hides the details, so I'm not sure how
> similar PIO between different platforms will actually be.

I keep thinking that there ought to be *some* commonality between the
platforms that use FIQs if only in the timer stuff at least.  Part of
the reason that doesn't get factored out is that it's normally just a
temporary bodge until DMA is working of course, though Qualcomm do it
because their DMA for the DSP doesn't fit well with dmaengine and the
i.MX AC'97 needs it since the hardware doesn't fully understand AC'97.

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

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

Re: About snd_dmaengine_pcm_trigger()

[Kuninori Morimoto]

Hi Lars, Mark

> > The reason why we factored out the dmaengine DMA stuff is because
> > there is a standard API that hides the implementation details and is
> > supposed to behave the same for each system. For PIO you do not have
> > such a abstraction layer that hides the details, so I'm not sure how
> > similar PIO between different platforms will actually be.
> 
> I keep thinking that there ought to be *some* commonality between the
> platforms that use FIQs if only in the timer stuff at least.  Part of
> the reason that doesn't get factored out is that it's normally just a
> temporary bodge until DMA is working of course, though Qualcomm do it
> because their DMA for the DSP doesn't fit well with dmaengine and the
> i.MX AC'97 needs it since the hardware doesn't fully understand AC'97.

Yes, there is no pioengine (never created :)
I'm not sure detail of non-Renesas platform driver whether it is supporting PIO.
But at least my driver (= FSI/rcar) has PIO support,
and I think these can share same method/function.
So, I create pcm_pio.c under ${LINUX}/sound/soc/sh for Renesas chip
with pcm_dmaengine similar style (as much as possible).
And, use common ${LINUX}/sound/core/pcm_dmaengine.c for DMA.
How about this ?