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 <[email protected]>
---
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
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...
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 <[email protected]>
> ---
> 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);
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?
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.
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.
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 <[email protected]>
> > ---
> > 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);
>
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?
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
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.
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 <[email protected]>
| Reviewed-by: Kyungmin Park <[email protected]>
| ---
| 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
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?
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
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
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.
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
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.
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
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
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.
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
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