[PATCH] ASoC: snd_dmaengine: add common api for pcm_mmap

[PATCH] ASoC: snd_dmaengine: add common api for pcm_mmap

[Laxman Dewangan]

The pcm mmap for the substream dma buffer is done using the
dma_mmap_writecombine().
Some of the chip sound driver like Spear, Tegra, Davinci,
ep93xx,snd_imx, snd_mxs, NUC900, OMAP, Samsung are doing this
mapping locally.
Add api in the snd dmaengine driver so that different client of
snd dmaengine can use this rather than implementing locally.

Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
---
 include/sound/dmaengine_pcm.h |    2 ++
 sound/soc/soc-dmaengine-pcm.c |   17 +++++++++++++++++
 2 files changed, 19 insertions(+), 0 deletions(-)

diff --git a/include/sound/dmaengine_pcm.h b/include/sound/dmaengine_pcm.h
index b877334..15c0d3c 100644
--- a/include/sound/dmaengine_pcm.h
+++ b/include/sound/dmaengine_pcm.h
@@ -47,4 +47,6 @@ int snd_dmaengine_pcm_close(struct snd_pcm_substream *substream);
 
 struct dma_chan *snd_dmaengine_pcm_get_chan(struct snd_pcm_substream *substream);
 
+int snd_dmaengine_pcm_mmap_writecombine(struct snd_pcm_substream *substream,
+		struct vm_area_struct *vma);
 #endif
diff --git a/sound/soc/soc-dmaengine-pcm.c b/sound/soc/soc-dmaengine-pcm.c
index 5df529e..6535eb3 100644
--- a/sound/soc/soc-dmaengine-pcm.c
+++ b/sound/soc/soc-dmaengine-pcm.c
@@ -21,6 +21,7 @@
 #include <linux/module.h>
 #include <linux/init.h>
 #include <linux/dmaengine.h>
+#include <linux/dma-mapping.h>
 #include <linux/slab.h>
 #include <sound/pcm.h>
 #include <sound/pcm_params.h>
@@ -313,3 +314,19 @@ int snd_dmaengine_pcm_close(struct snd_pcm_substream *substream)
 	return 0;
 }
 EXPORT_SYMBOL_GPL(snd_dmaengine_pcm_close);
+
+/**
+ * snd_dmaengine_pcm_mmap_writecombine - mmap stream dma buffer using
+ * dma_mmap_writecombine().
+ * @substream: PCM substream
+ * @vma: VM area.
+ */
+int snd_dmaengine_pcm_mmap_writecombine(struct snd_pcm_substream *substream,
+			struct vm_area_struct *vma)
+{
+	struct snd_pcm_runtime *runtime = substream->runtime;
+
+	return dma_mmap_writecombine(substream->pcm->card->dev, vma,
+		runtime->dma_area, runtime->dma_addr, runtime->dma_bytes);
+}
+EXPORT_SYMBOL_GPL(snd_dmaengine_pcm_mmap_writecombine);
-- 
1.7.1.1

Re: [PATCH] ASoC: snd_dmaengine: add common api for pcm_mmap

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 546 bytes --]

On Thu, Jun 28, 2012 at 12:31:26PM +0530, Laxman Dewangan wrote:
> The pcm mmap for the substream dma buffer is done using the
> dma_mmap_writecombine().
> Some of the chip sound driver like Spear, Tegra, Davinci,
> ep93xx,snd_imx, snd_mxs, NUC900, OMAP, Samsung are doing this
> mapping locally.
> Add api in the snd dmaengine driver so that different client of
> snd dmaengine can use this rather than implementing locally.

OK, but I'd expect this to come along with one or more patches
converting at least some of the users to the new API...

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

Re: [PATCH] ASoC: snd_dmaengine: add common api for pcm_mmap

[Lars-Peter Clausen]

On 06/28/2012 09:01 AM, Laxman Dewangan wrote:
> The pcm mmap for the substream dma buffer is done using the
> dma_mmap_writecombine().
> Some of the chip sound driver like Spear, Tegra, Davinci,
> ep93xx,snd_imx, snd_mxs, NUC900, OMAP, Samsung are doing this
> mapping locally.
> Add api in the snd dmaengine driver so that different client of
> snd dmaengine can use this rather than implementing locally.
> 

This is not really related to the dmaengine pcm driver. It's more of a
coincidence that all upstream drivers which use the dmaengine pcm driver
also use write-combined memory. In my opinion it would be better to add this
to the ALSA core. Now that there is a generic dma_mmap_writecombine it may
make sense to integrate this with snd_pcm_lib_default_mmap.

- Lars

> Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
> ---
>  include/sound/dmaengine_pcm.h |    2 ++
>  sound/soc/soc-dmaengine-pcm.c |   17 +++++++++++++++++
>  2 files changed, 19 insertions(+), 0 deletions(-)
> 
> diff --git a/include/sound/dmaengine_pcm.h b/include/sound/dmaengine_pcm.h
> index b877334..15c0d3c 100644
> --- a/include/sound/dmaengine_pcm.h
> +++ b/include/sound/dmaengine_pcm.h
> @@ -47,4 +47,6 @@ int snd_dmaengine_pcm_close(struct snd_pcm_substream *substream);
>  
>  struct dma_chan *snd_dmaengine_pcm_get_chan(struct snd_pcm_substream *substream);
>  
> +int snd_dmaengine_pcm_mmap_writecombine(struct snd_pcm_substream *substream,
> +		struct vm_area_struct *vma);
>  #endif
> diff --git a/sound/soc/soc-dmaengine-pcm.c b/sound/soc/soc-dmaengine-pcm.c
> index 5df529e..6535eb3 100644
> --- a/sound/soc/soc-dmaengine-pcm.c
> +++ b/sound/soc/soc-dmaengine-pcm.c
> @@ -21,6 +21,7 @@
>  #include <linux/module.h>
>  #include <linux/init.h>
>  #include <linux/dmaengine.h>
> +#include <linux/dma-mapping.h>
>  #include <linux/slab.h>
>  #include <sound/pcm.h>
>  #include <sound/pcm_params.h>
> @@ -313,3 +314,19 @@ int snd_dmaengine_pcm_close(struct snd_pcm_substream *substream)
>  	return 0;
>  }
>  EXPORT_SYMBOL_GPL(snd_dmaengine_pcm_close);
> +
> +/**
> + * snd_dmaengine_pcm_mmap_writecombine - mmap stream dma buffer using
> + * dma_mmap_writecombine().
> + * @substream: PCM substream
> + * @vma: VM area.
> + */
> +int snd_dmaengine_pcm_mmap_writecombine(struct snd_pcm_substream *substream,
> +			struct vm_area_struct *vma)
> +{
> +	struct snd_pcm_runtime *runtime = substream->runtime;
> +
> +	return dma_mmap_writecombine(substream->pcm->card->dev, vma,
> +		runtime->dma_area, runtime->dma_addr, runtime->dma_bytes);
> +}
> +EXPORT_SYMBOL_GPL(snd_dmaengine_pcm_mmap_writecombine);

Re: [PATCH] ASoC: snd_dmaengine: add common api for pcm_mmap

[Laxman Dewangan]

On Thursday 28 June 2012 04:21 PM, Mark Brown wrote:
> * PGP Signed by an unknown key
>
> On Thu, Jun 28, 2012 at 12:31:26PM +0530, Laxman Dewangan wrote:
>> The pcm mmap for the substream dma buffer is done using the
>> dma_mmap_writecombine().
>> Some of the chip sound driver like Spear, Tegra, Davinci,
>> ep93xx,snd_imx, snd_mxs, NUC900, OMAP, Samsung are doing this
>> mapping locally.
>> Add api in the snd dmaengine driver so that different client of
>> snd dmaengine can use this rather than implementing locally.
> OK, but I'd expect this to come along with one or more patches
> converting at least some of the users to the new API...

This is effort towards moving the Tegra pcm driver for dmangine based 
dma driver inplace of legacy dma driver for Tegra which have private API.

Currently, following 4 pcm driver can use this API directly keeping that 
they already using the generic snd dmaengine pcm driver.
ep93xx,
spear,
fsl,
mxs.
And Tegra will be next.

Other driver which can use this API are davinci, OMAP, samsung but 
because they are not using the snd_dmaengine_pcm driver's api as of now 
so not be very much useful for them.


Should I send one patch per driver change or squash all 4 file changes 
in one patch? Whatever will be easy or recommended?

Re: [PATCH] ASoC: snd_dmaengine: add common api for pcm_mmap

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 571 bytes --]

On Thu, Jun 28, 2012 at 05:26:58PM +0530, Laxman Dewangan wrote:

> Other driver which can use this API are davinci, OMAP, samsung but
> because they are not using the snd_dmaengine_pcm driver's api as of
> now so not be very much useful for them.

There's no reason why we can't factor out more common code - this is
essentially what Lars-Peter was saying.

> Should I send one patch per driver change or squash all 4 file
> changes in one patch? Whatever will be easy or recommended?

It doesn't really matter that much, but please take on board
Lars-Peter's comments.

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

Re: [PATCH] ASoC: snd_dmaengine: add common api for pcm_mmap

[Laxman Dewangan]

On Thursday 28 June 2012 05:32 PM, Mark Brown wrote:
> * PGP Signed by an unknown key
>
> On Thu, Jun 28, 2012 at 05:26:58PM +0530, Laxman Dewangan wrote:
>
>> Other driver which can use this API are davinci, OMAP, samsung but
>> because they are not using the snd_dmaengine_pcm driver's api as of
>> now so not be very much useful for them.
> There's no reason why we can't factor out more common code - this is
> essentially what Lars-Peter was saying.
>

Ok, then let me start on this.

>> Should I send one patch per driver change or squash all 4 file
>> changes in one patch? Whatever will be easy or recommended?
> It doesn't really matter that much, but please take on board
> Lars-Peter's comments.

Ok then I will go for one patch per file.
I will send the patches in series.

Re: [PATCH] ASoC: snd_dmaengine: add common api for pcm_mmap

[Takashi Iwai]

At Thu, 28 Jun 2012 13:09:36 +0200,
Lars-Peter Clausen wrote:
> 
> On 06/28/2012 09:01 AM, Laxman Dewangan wrote:
> > The pcm mmap for the substream dma buffer is done using the
> > dma_mmap_writecombine().
> > Some of the chip sound driver like Spear, Tegra, Davinci,
> > ep93xx,snd_imx, snd_mxs, NUC900, OMAP, Samsung are doing this
> > mapping locally.
> > Add api in the snd dmaengine driver so that different client of
> > snd dmaengine can use this rather than implementing locally.
> > 
> 
> This is not really related to the dmaengine pcm driver. It's more of a
> coincidence that all upstream drivers which use the dmaengine pcm driver
> also use write-combined memory. In my opinion it would be better to add this
> to the ALSA core. Now that there is a generic dma_mmap_writecombine it may
> make sense to integrate this with snd_pcm_lib_default_mmap.

Agreed.

Also, it must be portable.  So far, only ARM has
dma_mmap_writecombine(), thus the build on other arch would fail as
is.


thanks,

Takashi

> 
> - Lars
> 
> > Signed-off-by: Laxman Dewangan <ldewangan@nvidia.com>
> > ---
> >  include/sound/dmaengine_pcm.h |    2 ++
> >  sound/soc/soc-dmaengine-pcm.c |   17 +++++++++++++++++
> >  2 files changed, 19 insertions(+), 0 deletions(-)
> > 
> > diff --git a/include/sound/dmaengine_pcm.h b/include/sound/dmaengine_pcm.h
> > index b877334..15c0d3c 100644
> > --- a/include/sound/dmaengine_pcm.h
> > +++ b/include/sound/dmaengine_pcm.h
> > @@ -47,4 +47,6 @@ int snd_dmaengine_pcm_close(struct snd_pcm_substream *substream);
> >  
> >  struct dma_chan *snd_dmaengine_pcm_get_chan(struct snd_pcm_substream *substream);
> >  
> > +int snd_dmaengine_pcm_mmap_writecombine(struct snd_pcm_substream *substream,
> > +		struct vm_area_struct *vma);
> >  #endif
> > diff --git a/sound/soc/soc-dmaengine-pcm.c b/sound/soc/soc-dmaengine-pcm.c
> > index 5df529e..6535eb3 100644
> > --- a/sound/soc/soc-dmaengine-pcm.c
> > +++ b/sound/soc/soc-dmaengine-pcm.c
> > @@ -21,6 +21,7 @@
> >  #include <linux/module.h>
> >  #include <linux/init.h>
> >  #include <linux/dmaengine.h>
> > +#include <linux/dma-mapping.h>
> >  #include <linux/slab.h>
> >  #include <sound/pcm.h>
> >  #include <sound/pcm_params.h>
> > @@ -313,3 +314,19 @@ int snd_dmaengine_pcm_close(struct snd_pcm_substream *substream)
> >  	return 0;
> >  }
> >  EXPORT_SYMBOL_GPL(snd_dmaengine_pcm_close);
> > +
> > +/**
> > + * snd_dmaengine_pcm_mmap_writecombine - mmap stream dma buffer using
> > + * dma_mmap_writecombine().
> > + * @substream: PCM substream
> > + * @vma: VM area.
> > + */
> > +int snd_dmaengine_pcm_mmap_writecombine(struct snd_pcm_substream *substream,
> > +			struct vm_area_struct *vma)
> > +{
> > +	struct snd_pcm_runtime *runtime = substream->runtime;
> > +
> > +	return dma_mmap_writecombine(substream->pcm->card->dev, vma,
> > +		runtime->dma_area, runtime->dma_addr, runtime->dma_bytes);
> > +}
> > +EXPORT_SYMBOL_GPL(snd_dmaengine_pcm_mmap_writecombine);
> 

Re: [alsa-devel] [PATCH] ASoC: snd_dmaengine: add common api for pcm_mmap

[Mark Brown]

On Thu, Jun 28, 2012 at 02:15:38PM +0200, Takashi Iwai wrote:
> Lars-Peter Clausen wrote:

> > to the ALSA core. Now that there is a generic dma_mmap_writecombine it may
> > make sense to integrate this with snd_pcm_lib_default_mmap.

> Agreed.

> Also, it must be portable.  So far, only ARM has
> dma_mmap_writecombine(), thus the build on other arch would fail as
> is.

Oh, dear - this means we don't actually have a generic API at all.  Is
there any effort being made to make this generally available?

Re: [alsa-devel] [PATCH] ASoC: snd_dmaengine: add common api for pcm_mmap

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 655 bytes --]

On Thu, Jun 28, 2012 at 02:30:26PM +0200, Lars-Peter Clausen wrote:
> On 06/28/2012 02:18 PM, Mark Brown wrote:

> > Oh, dear - this means we don't actually have a generic API at all.  Is
> > there any effort being made to make this generally available?

> Yes there is, and in fact in next/master there is generic
> dma_mmap_writecombine. I'm not quite sure how it behaves on non ARM archs
> though. But in theory it should fallback to a normal coherent mapping. Marek
> did a lot of work on this, I've put him on Cc.

OK, that's not so bad.  Where is this code, can we merge it into ALSA?
If not it's probably best to punt until after the merge window.

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

Re: [alsa-devel] [PATCH] ASoC: snd_dmaengine: add common api for pcm_mmap

[Lars-Peter Clausen]

On 06/28/2012 02:18 PM, Mark Brown wrote:
> On Thu, Jun 28, 2012 at 02:15:38PM +0200, Takashi Iwai wrote:
>> Lars-Peter Clausen wrote:
> 
>>> to the ALSA core. Now that there is a generic dma_mmap_writecombine it may
>>> make sense to integrate this with snd_pcm_lib_default_mmap.
> 
>> Agreed.
> 
>> Also, it must be portable.  So far, only ARM has
>> dma_mmap_writecombine(), thus the build on other arch would fail as
>> is.
> 
> Oh, dear - this means we don't actually have a generic API at all.  Is
> there any effort being made to make this generally available?

Yes there is, and in fact in next/master there is generic
dma_mmap_writecombine. I'm not quite sure how it behaves on non ARM archs
though. But in theory it should fallback to a normal coherent mapping. Marek
did a lot of work on this, I've put him on Cc.

- Lars

Re: [alsa-devel] [PATCH] ASoC: snd_dmaengine: add common api for pcm_mmap

[Clemens Ladisch]

Mark Brown wrote:
> On Thu, Jun 28, 2012 at 02:15:38PM +0200, Takashi Iwai wrote:
>> So far, only ARM has dma_mmap_writecombine(), thus the build on other
>> arch would fail as is.
>
> Oh, dear - this means we don't actually have a generic API at all.  Is
> there any effort being made to make this generally available?

http://lkml.org/lkml/2012/6/15/27:
| Commit 9adc5374 ('common: dma-mapping: introduce mmap method') added a
| generic method for implementing mmap user call to dma_map_ops structure.
|
| This patch converts ARM and PowerPC architectures (the only providers of
| dma_mmap_coherent/dma_mmap_writecombine calls) to use this generic
| dma_map_ops based call and adds a generic cross architecture
| definition for dma_mmap_attrs, dma_mmap_coherent, dma_mmap_writecombine
| functions.
|
| The generic mmap virt_to_page-based fallback implementation is provided for
| architectures which don't provide their own implementation for mmap method.
|
| Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
| Reviewed-by: Kyungmin Park <kyungmin.park@samsung.com>
| ---
| Hello,
| This patch is a continuation of my works on dma-mapping cleanup and
| unification. Previous works (commit 58bca4a8fa ('Merge branch
| 'for-linus' of git://git.linaro.org/people/mszyprowski/linux-dma-mapping')
| has been merged to v3.4-rc2. Now I've focuses on providing implementation
| for all architectures so the drivers and some cross-architecture common
| helpers (like for example videobuf2) can start using this new api.
|
| I'm not 100% sure if the PowerPC changes are correct. The cases of
| dma_iommu_ops and vio_dma_mapping_ops are a bit suspicious for me, but I
| have no way to test and check if my changes works for that hardware.
|
| Best regards
| Marek Szyprowski
| Samsung Poland R&D Center


Regards,
Clemens

Re: [alsa-devel] [PATCH] ASoC: snd_dmaengine: add common api for pcm_mmap

[Laxman Dewangan]

On Thursday 28 June 2012 05:58 PM, Mark Brown wrote:
> * PGP Signed by an unknown key
>
> On Thu, Jun 28, 2012 at 02:30:26PM +0200, Lars-Peter Clausen wrote:
>> On 06/28/2012 02:18 PM, Mark Brown wrote:
>>> Oh, dear - this means we don't actually have a generic API at all.  Is
>>> there any effort being made to make this generally available?
>> Yes there is, and in fact in next/master there is generic
>> dma_mmap_writecombine. I'm not quite sure how it behaves on non ARM archs
>> though. But in theory it should fallback to a normal coherent mapping. Marek
>> did a lot of work on this, I've put him on Cc.

So can we put the function snd_pcm_lib_writecombine_mmap() in the 
pcm_native.c and only export this api for ARM i.e.  under macro  #ifdef 
CONFIG_ARM so that ARM based SOCs like Tegra/epa3xx/mxs/spear can 
directly use that one?
Or, wait for this common API until all ARCH support it?

Re: [PATCH] ASoC: snd_dmaengine: add common api for pcm_mmap

[Lars-Peter Clausen]

On 06/28/2012 01:56 PM, Laxman Dewangan wrote:
> On Thursday 28 June 2012 04:21 PM, Mark Brown wrote:
>> * PGP Signed by an unknown key
>>
>> On Thu, Jun 28, 2012 at 12:31:26PM +0530, Laxman Dewangan wrote:
>>> The pcm mmap for the substream dma buffer is done using the
>>> dma_mmap_writecombine().
>>> Some of the chip sound driver like Spear, Tegra, Davinci,
>>> ep93xx,snd_imx, snd_mxs, NUC900, OMAP, Samsung are doing this
>>> mapping locally.
>>> Add api in the snd dmaengine driver so that different client of
>>> snd dmaengine can use this rather than implementing locally.
>> OK, but I'd expect this to come along with one or more patches
>> converting at least some of the users to the new API...
> 
> This is effort towards moving the Tegra pcm driver for dmangine based dma
> driver inplace of legacy dma driver for Tegra which have private API.
> 
> Currently, following 4 pcm driver can use this API directly keeping that
> they already using the generic snd dmaengine pcm driver.
> ep93xx,
> spear,
> fsl,
> mxs.
> And Tegra will be next.
> 
> Other driver which can use this API are davinci, OMAP, samsung but because
> they are not using the snd_dmaengine_pcm driver's api as of now so not be
> very much useful for them.

Btw. all these drivers basically use the same code for buffer preallocation.
Since how the buffer is mmaped is related to how it is allocated it might be
worth to factor this out at the same time as well.

- Lars

Re: [alsa-devel] [PATCH] ASoC: snd_dmaengine: add common api for pcm_mmap

[Takashi Iwai]

At Thu, 28 Jun 2012 18:05:53 +0530,
Laxman Dewangan wrote:
> 
> On Thursday 28 June 2012 05:58 PM, Mark Brown wrote:
> > * PGP Signed by an unknown key
> >
> > On Thu, Jun 28, 2012 at 02:30:26PM +0200, Lars-Peter Clausen wrote:
> >> On 06/28/2012 02:18 PM, Mark Brown wrote:
> >>> Oh, dear - this means we don't actually have a generic API at all.  Is
> >>> there any effort being made to make this generally available?
> >> Yes there is, and in fact in next/master there is generic
> >> dma_mmap_writecombine. I'm not quite sure how it behaves on non ARM archs
> >> though. But in theory it should fallback to a normal coherent mapping. Marek
> >> did a lot of work on this, I've put him on Cc.
> 
> So can we put the function snd_pcm_lib_writecombine_mmap() in the 
> pcm_native.c and only export this api for ARM i.e.  under macro  #ifdef 
> CONFIG_ARM so that ARM based SOCs like Tegra/epa3xx/mxs/spear can 
> directly use that one?
> Or, wait for this common API until all ARCH support it?

I think it's fine to put it first in ALSA side with some ifdef.
A similar trick is already found for snd_pcm_lib_mmap_iomem.
See include/sound/pcm.h.

But, actually it's a still question what if an architecture doesn't
support the mmap of writecombine at all.  The proposed patch doesn't
allow you to know whether writecombine-mmap is possible or not on the
running architecture until you really try to call it and fail.
It's a missing piece, IMO.


Takashi

Re: [alsa-devel] [PATCH] ASoC: snd_dmaengine: add common api for pcm_mmap

[Laxman Dewangan]

On Thursday 28 June 2012 06:27 PM, Takashi Iwai wrote:
> At Thu, 28 Jun 2012 18:05:53 +0530,
> Laxman Dewangan wrote:
>> On Thursday 28 June 2012 05:58 PM, Mark Brown wrote:
>>> * PGP Signed by an unknown key
>>>
>>> On Thu, Jun 28, 2012 at 02:30:26PM +0200, Lars-Peter Clausen wrote:
>>>> On 06/28/2012 02:18 PM, Mark Brown wrote:
>>>>> Oh, dear - this means we don't actually have a generic API at all.  Is
>>>>> there any effort being made to make this generally available?
>>>> Yes there is, and in fact in next/master there is generic
>>>> dma_mmap_writecombine. I'm not quite sure how it behaves on non ARM archs
>>>> though. But in theory it should fallback to a normal coherent mapping. Marek
>>>> did a lot of work on this, I've put him on Cc.
>> So can we put the function snd_pcm_lib_writecombine_mmap() in the
>> pcm_native.c and only export this api for ARM i.e.  under macro  #ifdef
>> CONFIG_ARM so that ARM based SOCs like Tegra/epa3xx/mxs/spear can
>> directly use that one?
>> Or, wait for this common API until all ARCH support it?
> I think it's fine to put it first in ALSA side with some ifdef.
> A similar trick is already found for snd_pcm_lib_mmap_iomem.
> See include/sound/pcm.h.
>
> But, actually it's a still question what if an architecture doesn't
> support the mmap of writecombine at all.  The proposed patch doesn't
> allow you to know whether writecombine-mmap is possible or not on the
> running architecture until you really try to call it and fail.
> It's a missing piece, IMO.

Lars wanted to move the buffer allocation also to common place.
Then how about this?
create new file and header for snd-pcm-writecombine-buffer.c/.h and put 
this in sound/core.
Select this file compilation through config SND_PCM_WRITECOMBINE_BUFFER 
which will be select this config through sound/soc/xxx/Kconfig if they 
want to use.
This will provide three apis: new, free and mmap.

Re: [alsa-devel] [PATCH] ASoC: snd_dmaengine: add common api for pcm_mmap

[Takashi Iwai]

At Thu, 28 Jun 2012 18:34:02 +0530,
Laxman Dewangan wrote:
> 
> On Thursday 28 June 2012 06:27 PM, Takashi Iwai wrote:
> > At Thu, 28 Jun 2012 18:05:53 +0530,
> > Laxman Dewangan wrote:
> >> On Thursday 28 June 2012 05:58 PM, Mark Brown wrote:
> >>> * PGP Signed by an unknown key
> >>>
> >>> On Thu, Jun 28, 2012 at 02:30:26PM +0200, Lars-Peter Clausen wrote:
> >>>> On 06/28/2012 02:18 PM, Mark Brown wrote:
> >>>>> Oh, dear - this means we don't actually have a generic API at all.  Is
> >>>>> there any effort being made to make this generally available?
> >>>> Yes there is, and in fact in next/master there is generic
> >>>> dma_mmap_writecombine. I'm not quite sure how it behaves on non ARM archs
> >>>> though. But in theory it should fallback to a normal coherent mapping. Marek
> >>>> did a lot of work on this, I've put him on Cc.
> >> So can we put the function snd_pcm_lib_writecombine_mmap() in the
> >> pcm_native.c and only export this api for ARM i.e.  under macro  #ifdef
> >> CONFIG_ARM so that ARM based SOCs like Tegra/epa3xx/mxs/spear can
> >> directly use that one?
> >> Or, wait for this common API until all ARCH support it?
> > I think it's fine to put it first in ALSA side with some ifdef.
> > A similar trick is already found for snd_pcm_lib_mmap_iomem.
> > See include/sound/pcm.h.
> >
> > But, actually it's a still question what if an architecture doesn't
> > support the mmap of writecombine at all.  The proposed patch doesn't
> > allow you to know whether writecombine-mmap is possible or not on the
> > running architecture until you really try to call it and fail.
> > It's a missing piece, IMO.
> 
> Lars wanted to move the buffer allocation also to common place.

Which common place?

> Then how about this?
> create new file and header for snd-pcm-writecombine-buffer.c/.h and put 
> this in sound/core.
> Select this file compilation through config SND_PCM_WRITECOMBINE_BUFFER 
> which will be select this config through sound/soc/xxx/Kconfig if they 
> want to use.
> This will provide three apis: new, free and mmap.

Way too much hustles than necessary...


Takashi

Re: [alsa-devel] [PATCH] ASoC: snd_dmaengine: add common api for pcm_mmap

[Laxman Dewangan]

On Thursday 28 June 2012 06:58 PM, Takashi Iwai wrote:
> At Thu, 28 Jun 2012 18:34:02 +0530,
> Laxman Dewangan wrote:
>> On Thursday 28 June 2012 06:27 PM, Takashi Iwai wrote:
>>> At Thu, 28 Jun 2012 18:05:53 +0530,
>>> Laxman Dewangan wrote:
>>>> On Thursday 28 June 2012 05:58 PM, Mark Brown wrote:
>>>>> * PGP Signed by an unknown key
>>>>>
>>>>> On Thu, Jun 28, 2012 at 02:30:26PM +0200, Lars-Peter Clausen wrote:
>>>>>> On 06/28/2012 02:18 PM, Mark Brown wrote:
>>>>>>> Oh, dear - this means we don't actually have a generic API at all.  Is
>>>>>>> there any effort being made to make this generally available?
>>>>>> Yes there is, and in fact in next/master there is generic
>>>>>> dma_mmap_writecombine. I'm not quite sure how it behaves on non ARM archs
>>>>>> though. But in theory it should fallback to a normal coherent mapping. Marek
>>>>>> did a lot of work on this, I've put him on Cc.
>>>> So can we put the function snd_pcm_lib_writecombine_mmap() in the
>>>> pcm_native.c and only export this api for ARM i.e.  under macro  #ifdef
>>>> CONFIG_ARM so that ARM based SOCs like Tegra/epa3xx/mxs/spear can
>>>> directly use that one?
>>>> Or, wait for this common API until all ARCH support it?
>>> I think it's fine to put it first in ALSA side with some ifdef.
>>> A similar trick is already found for snd_pcm_lib_mmap_iomem.
>>> See include/sound/pcm.h.
>>>
>>> But, actually it's a still question what if an architecture doesn't
>>> support the mmap of writecombine at all.  The proposed patch doesn't
>>> allow you to know whether writecombine-mmap is possible or not on the
>>> running architecture until you really try to call it and fail.
>>> It's a missing piece, IMO.
>> Lars wanted to move the buffer allocation also to common place.
> Which common place?

He wanted to refactor allocation part also at same time and so I guessed 
that it will be in some common place and though about the sound/core.

>> Then how about this?
>> create new file and header for snd-pcm-writecombine-buffer.c/.h and put
>> this in sound/core.
>> Select this file compilation through config SND_PCM_WRITECOMBINE_BUFFER
>> which will be select this config through sound/soc/xxx/Kconfig if they
>> want to use.
>> This will provide three apis: new, free and mmap.
> Way too much hustles than necessary...
Ooh no. :-(  I had taken the idea from snd_dmaengine_pcm driver.
Bit I like to hear simple way which solves the purpose.

Re: [alsa-devel] [PATCH] ASoC: snd_dmaengine: add common api for pcm_mmap

[Takashi Iwai]

At Thu, 28 Jun 2012 19:06:07 +0530,
Laxman Dewangan wrote:
> 
> On Thursday 28 June 2012 06:58 PM, Takashi Iwai wrote:
> > At Thu, 28 Jun 2012 18:34:02 +0530,
> > Laxman Dewangan wrote:
> >> On Thursday 28 June 2012 06:27 PM, Takashi Iwai wrote:
> >>> At Thu, 28 Jun 2012 18:05:53 +0530,
> >>> Laxman Dewangan wrote:
> >>>> On Thursday 28 June 2012 05:58 PM, Mark Brown wrote:
> >>>>> * PGP Signed by an unknown key
> >>>>>
> >>>>> On Thu, Jun 28, 2012 at 02:30:26PM +0200, Lars-Peter Clausen wrote:
> >>>>>> On 06/28/2012 02:18 PM, Mark Brown wrote:
> >>>>>>> Oh, dear - this means we don't actually have a generic API at all.  Is
> >>>>>>> there any effort being made to make this generally available?
> >>>>>> Yes there is, and in fact in next/master there is generic
> >>>>>> dma_mmap_writecombine. I'm not quite sure how it behaves on non ARM archs
> >>>>>> though. But in theory it should fallback to a normal coherent mapping. Marek
> >>>>>> did a lot of work on this, I've put him on Cc.
> >>>> So can we put the function snd_pcm_lib_writecombine_mmap() in the
> >>>> pcm_native.c and only export this api for ARM i.e.  under macro  #ifdef
> >>>> CONFIG_ARM so that ARM based SOCs like Tegra/epa3xx/mxs/spear can
> >>>> directly use that one?
> >>>> Or, wait for this common API until all ARCH support it?
> >>> I think it's fine to put it first in ALSA side with some ifdef.
> >>> A similar trick is already found for snd_pcm_lib_mmap_iomem.
> >>> See include/sound/pcm.h.
> >>>
> >>> But, actually it's a still question what if an architecture doesn't
> >>> support the mmap of writecombine at all.  The proposed patch doesn't
> >>> allow you to know whether writecombine-mmap is possible or not on the
> >>> running architecture until you really try to call it and fail.
> >>> It's a missing piece, IMO.
> >> Lars wanted to move the buffer allocation also to common place.
> > Which common place?
> 
> He wanted to refactor allocation part also at same time and so I guessed 
> that it will be in some common place and though about the sound/core.

But the memory allocation rework is basically irrelevant from what's
needed for pcm_mmap_writecombine() now.  It'd need a lot more reworks
than the simple addition of dma_mmap_writecombine() wrapper.

> >> Then how about this?
> >> create new file and header for snd-pcm-writecombine-buffer.c/.h and put
> >> this in sound/core.
> >> Select this file compilation through config SND_PCM_WRITECOMBINE_BUFFER
> >> which will be select this config through sound/soc/xxx/Kconfig if they
> >> want to use.
> >> This will provide three apis: new, free and mmap.
> > Way too much hustles than necessary...
> Ooh no. :-(  I had taken the idea from snd_dmaengine_pcm driver.
> Bit I like to hear simple way which solves the purpose.

As Lars suggested, a simple ifdef should suffice for now.
With the upcoming generic dma_mmap_writecombine() stuff, it'd be even
simpler in future.

But, still we need to be careful about this.  As mentioned, there is
no flag to know the possibility of writecombine mmap beforehand.
It'd be nice if we have either a compile-time or a run-time flag /
function to check that.  Then the driver can also expose the mmap
capability to user-space depending on the flag.


Takashi

Re: [alsa-devel] [PATCH] ASoC: snd_dmaengine: add common api for pcm_mmap

[Laxman Dewangan]

On Thursday 28 June 2012 07:21 PM, Takashi Iwai wrote:
> At Thu, 28 Jun 2012 19:06:07 +0530,
> Laxman Dewangan wrote:
>
>>>> Then how about this?
>>>> create new file and header for snd-pcm-writecombine-buffer.c/.h and put
>>>> this in sound/core.
>>>> Select this file compilation through config SND_PCM_WRITECOMBINE_BUFFER
>>>> which will be select this config through sound/soc/xxx/Kconfig if they
>>>> want to use.
>>>> This will provide three apis: new, free and mmap.
>>> Way too much hustles than necessary...
>> Ooh no. :-(  I had taken the idea from snd_dmaengine_pcm driver.
>> Bit I like to hear simple way which solves the purpose.
> As Lars suggested, a simple ifdef should suffice for now.
> With the upcoming generic dma_mmap_writecombine() stuff, it'd be even
> simpler in future.
>
> But, still we need to be careful about this.  As mentioned, there is
> no flag to know the possibility of writecombine mmap beforehand.
> It'd be nice if we have either a compile-time or a run-time flag /
> function to check that.  Then the driver can also expose the mmap
> capability to user-space depending on the flag.

Yaah, this seems really simple. Thanks for suggestion.
If I understand it fully, the new apis will be declare in sound/pcm.h, 
implement it in pcm_native.c and use the config variable
CONFIG_SND_PCM_WRITECOMBINE_BUFFER for ifdef.

This will be selected in required sound/soc driver so they can use it 
like powerpc (for fsl) and ARM arch based soc.

Re: [alsa-devel] [PATCH] ASoC: snd_dmaengine: add common api for pcm_mmap

[Takashi Iwai]

At Thu, 28 Jun 2012 14:44:43 +0200,
Lars-Peter Clausen wrote:
> 
> On 06/28/2012 01:56 PM, Laxman Dewangan wrote:
> > On Thursday 28 June 2012 04:21 PM, Mark Brown wrote:
> >> * PGP Signed by an unknown key
> >>
> >> On Thu, Jun 28, 2012 at 12:31:26PM +0530, Laxman Dewangan wrote:
> >>> The pcm mmap for the substream dma buffer is done using the
> >>> dma_mmap_writecombine().
> >>> Some of the chip sound driver like Spear, Tegra, Davinci,
> >>> ep93xx,snd_imx, snd_mxs, NUC900, OMAP, Samsung are doing this
> >>> mapping locally.
> >>> Add api in the snd dmaengine driver so that different client of
> >>> snd dmaengine can use this rather than implementing locally.
> >> OK, but I'd expect this to come along with one or more patches
> >> converting at least some of the users to the new API...
> > 
> > This is effort towards moving the Tegra pcm driver for dmangine based dma
> > driver inplace of legacy dma driver for Tegra which have private API.
> > 
> > Currently, following 4 pcm driver can use this API directly keeping that
> > they already using the generic snd dmaengine pcm driver.
> > ep93xx,
> > spear,
> > fsl,
> > mxs.
> > And Tegra will be next.
> > 
> > Other driver which can use this API are davinci, OMAP, samsung but because
> > they are not using the snd_dmaengine_pcm driver's api as of now so not be
> > very much useful for them.
> 
> Btw. all these drivers basically use the same code for buffer preallocation.
> Since how the buffer is mmaped is related to how it is allocated it might be
> worth to factor this out at the same time as well.

Hrm.  Do you mean hacking more on sound/core/memalloc.c, or sanitize
all these stuff?  Adding dma_alloc_writecombine() variant is easy, but
at the same time, I also would like to rip off this ugly thing.


Takashi

Re: [alsa-devel] [PATCH] ASoC: snd_dmaengine: add common api for pcm_mmap

[Takashi Iwai]

At Thu, 28 Jun 2012 19:30:57 +0530,
Laxman Dewangan wrote:
> 
> On Thursday 28 June 2012 07:21 PM, Takashi Iwai wrote:
> > At Thu, 28 Jun 2012 19:06:07 +0530,
> > Laxman Dewangan wrote:
> >
> >>>> Then how about this?
> >>>> create new file and header for snd-pcm-writecombine-buffer.c/.h and put
> >>>> this in sound/core.
> >>>> Select this file compilation through config SND_PCM_WRITECOMBINE_BUFFER
> >>>> which will be select this config through sound/soc/xxx/Kconfig if they
> >>>> want to use.
> >>>> This will provide three apis: new, free and mmap.
> >>> Way too much hustles than necessary...
> >> Ooh no. :-(  I had taken the idea from snd_dmaengine_pcm driver.
> >> Bit I like to hear simple way which solves the purpose.
> > As Lars suggested, a simple ifdef should suffice for now.
> > With the upcoming generic dma_mmap_writecombine() stuff, it'd be even
> > simpler in future.
> >
> > But, still we need to be careful about this.  As mentioned, there is
> > no flag to know the possibility of writecombine mmap beforehand.
> > It'd be nice if we have either a compile-time or a run-time flag /
> > function to check that.  Then the driver can also expose the mmap
> > capability to user-space depending on the flag.
> 
> Yaah, this seems really simple. Thanks for suggestion.
> If I understand it fully, the new apis will be declare in sound/pcm.h, 
> implement it in pcm_native.c and use the config variable
> CONFIG_SND_PCM_WRITECOMBINE_BUFFER for ifdef.

Or this could be simply ifdef CONFIG_ARM.
Since this ifdef will be unnecessary soon later, I don't think we need
to give yet another kconfig.  Of course, we should put a good comment
around there for not keeping it for long time.


Takashi

Re: [alsa-devel] [PATCH] ASoC: snd_dmaengine: add common api for pcm_mmap

[Lars-Peter Clausen]

On 06/28/2012 03:51 PM, Takashi Iwai wrote:
> At Thu, 28 Jun 2012 19:06:07 +0530,
> Laxman Dewangan wrote:
> [...]
> 
> But, still we need to be careful about this.  As mentioned, there is
> no flag to know the possibility of writecombine mmap beforehand.
> It'd be nice if we have either a compile-time or a run-time flag /
> function to check that.  Then the driver can also expose the mmap
> capability to user-space depending on the flag.
> 

If writecombine is not supported it will fallback to a "normal" mmap.
Here's an excerpt from the DMA mapping documentation:

DMA_ATTR_WRITE_COMBINE
----------------------

DMA_ATTR_WRITE_COMBINE specifies that writes to the mapping may be
buffered to improve performance.

Since it is optional for platforms to implement DMA_ATTR_WRITE_COMBINE,
those that do not will simply ignore the attribute and exhibit default
behavior.

- Lars

RE: [alsa-devel] [PATCH] ASoC: snd_dmaengine: add common api for pcm_mmap

[Marek Szyprowski]

Hello,

On Thursday, June 28, 2012 2:30 PM Lars-Peter Clausen wrote:

> On 06/28/2012 02:18 PM, Mark Brown wrote:
> > On Thu, Jun 28, 2012 at 02:15:38PM +0200, Takashi Iwai wrote:
> >> Lars-Peter Clausen wrote:
> >
> >>> to the ALSA core. Now that there is a generic dma_mmap_writecombine it may
> >>> make sense to integrate this with snd_pcm_lib_default_mmap.
> >
> >> Agreed.
> >
> >> Also, it must be portable.  So far, only ARM has
> >> dma_mmap_writecombine(), thus the build on other arch would fail as
> >> is.
> >
> > Oh, dear - this means we don't actually have a generic API at all.  Is
> > there any effort being made to make this generally available?
> 
> Yes there is, and in fact in next/master there is generic
> dma_mmap_writecombine. I'm not quite sure how it behaves on non ARM archs
> though. But in theory it should fallback to a normal coherent mapping. Marek
> did a lot of work on this, I've put him on Cc.

On architectures which don't support DMA_ATTR_WRITECOMBINE attribute it falls
back to standard 'coherent' implementation, so it is equivalent to 
dma_mmap_coherent(). If architecture doesn't provide mmap callback, my patch
provides a so called 'generic' implementation based on virt_to_phys() macro.
Once this patch gets merged, I hope that each architecture will be reviewed 
if such implementation is correct. If not, some architectures might need to
provide their own implementations, but at least we can get the buffer-mmap
related hacks from the device drivers.

Best regards
-- 
Marek Szyprowski
Samsung Poland R&D Center