ALSA dma_area not utilizing D-cahce

ALSA dma_area not utilizing D-cahce

[MASAO TAKAHASHI]

Hi everybody. 
Please advice me to resolve my problem.
1.  My environment
    Hardware	Alpha-project MS104-SH4AG with SH7764
	kernel		sh-linux-2.6.29
2. My problem
	I am ussing a proprietary ALSA driver for SH7764 SSI.
	I have found runtime->dma_area address refers DMA area
	via D-cache.
	So, playback is too late after executing snd_pcm_writei().
	Until D-cache writeback is done, sound data stays in D-cache.
	I want to read and write from/to main memory 
	without D-cache.

3. My idea to solve this problem
	add some codes in xxx_prepare() routine as follows
	
	save_playback_area = runtime->dma_area;
	runtime->dma_area = P2SEGADDR(runtime->dma_area);

Is my idea right?

Best regards.

Masao Takahashi in Japan

---------- 

MASAO TAKAHASHI <masao-takahashi@kanno.co.jp>
KANNO WORKS CO., LTD. JAPAN
TEL 093-436-2330

Re: ALSA dma_area not utilizing D-cahce

[Paul Mundt]

On Wed, May 16, 2012 at 09:51:23AM +0900, MASAO TAKAHASHI wrote:
> Hi everybody. 
> Please advice me to resolve my problem.
> 1.  My environment
>     Hardware	Alpha-project MS104-SH4AG with SH7764
> 	kernel		sh-linux-2.6.29
> 2. My problem
> 	I am ussing a proprietary ALSA driver for SH7764 SSI.
> 	I have found runtime->dma_area address refers DMA area
> 	via D-cache.
> 	So, playback is too late after executing snd_pcm_writei().
> 	Until D-cache writeback is done, sound data stays in D-cache.
> 	I want to read and write from/to main memory 
> 	without D-cache.
> 
> 3. My idea to solve this problem
> 	add some codes in xxx_prepare() routine as follows
> 	
> 	save_playback_area = runtime->dma_area;
> 	runtime->dma_area = P2SEGADDR(runtime->dma_area);
> 
> Is my idea right?
> 
There are a couple of places that need to be fixed. At a quick glance:

	- snd_pcm_default_page_ops()
	- snd_pcm_lib_default_mmap()

Both are in need of fixing up.

The snd_pcm_default_page_ops() case is easy, as we can use the same
approach that MIPS does (sh also provides a CAC_ADDR definition).

The snd_pcm_lib_default_mmap() case is a bit more involved. The last time
this topic came up a generic pgprot_noncached() solution was stalled as
pgprot_noncached() wasn't available on all of the platforms. That's been
addressed now, so it may be worth revisiting.

At present the biggest problem with fixing up the default mmap behaviour
is that we don't have any easy way for working out the coherence model on
a struct device level. ie, we may have snooping enabled on the PCI side
for device<->memory coherence while at the same time being non-coherent
for the L1 D-cache case at the platform device level.

There also exists a snd_pcm_lib_mmap_iomem() now that handles the
pgprot_noncached() case which you might be able to use with your driver.

That being said, P2SEGADDR is non-portable legacy garbage -- drivers
should never use it directly. Unfortunately we don't have any easy way to
use it in the headers while forcibly blowing up the build for anything
else.

Re: ALSA dma_area not utilizing D-cahce

[MASAO TAKAHASHI]

On Wed, 16 May 2012 14:38:57 +0900
Paul Mundt <lethal@linux-sh.org> wrote:

> On Wed, May 16, 2012 at 09:51:23AM +0900, MASAO TAKAHASHI wrote:
> > Hi everybody. 
> > Please advice me to resolve my problem.
> > 1.  My environment
> >     Hardware	Alpha-project MS104-SH4AG with SH7764
> > 	kernel		sh-linux-2.6.29
> > 2. My problem
> > 	I am ussing a proprietary ALSA driver for SH7764 SSI.
> > 	I have found runtime->dma_area address refers DMA area
> > 	via D-cache.
> > 	So, playback is too late after executing snd_pcm_writei().
> > 	Until D-cache writeback is done, sound data stays in D-cache.
> > 	I want to read and write from/to main memory 
> > 	without D-cache.
> > 
> > 3. My idea to solve this problem
> > 	add some codes in xxx_prepare() routine as follows
> > 	
> > 	save_playback_area = runtime->dma_area;
> > 	runtime->dma_area = P2SEGADDR(runtime->dma_area);
> > 
> > Is my idea right?
> > 
> There are a couple of places that need to be fixed. At a quick glance:
> 
> 	- snd_pcm_default_page_ops()
> 	- snd_pcm_lib_default_mmap()
> 
> Both are in need of fixing up.
> 
> The snd_pcm_default_page_ops() case is easy, as we can use the same
> approach that MIPS does (sh also provides a CAC_ADDR definition).
> 
> The snd_pcm_lib_default_mmap() case is a bit more involved. The last time
> this topic came up a generic pgprot_noncached() solution was stalled as
> pgprot_noncached() wasn't available on all of the platforms. That's been
> addressed now, so it may be worth revisiting.
> 
> At present the biggest problem with fixing up the default mmap behaviour
> is that we don't have any easy way for working out the coherence model on
> a struct device level. ie, we may have snooping enabled on the PCI side
> for device<->memory coherence while at the same time being non-coherent
> for the L1 D-cache case at the platform device level.
> 
> There also exists a snd_pcm_lib_mmap_iomem() now that handles the
> pgprot_noncached() case which you might be able to use with your driver.
> 
> That being said, P2SEGADDR is non-portable legacy garbage -- drivers
> should never use it directly. Unfortunately we don't have any easy way to
> use it in the headers while forcibly blowing up the build for anything
> else.

Do you advice to use snd_pcm_mmap_writei() call?
I think that the mmap method is used when dmix or snd_pcm_mmap_writei()
 or dsnoop/snd_pcm_mmap_readi().
I am using snd_pcm_writei() call.
Then I could not use these  functions as below.
  - snd_pcm_default_page_ops()
  - snd_pcm_lib_default_mmap()

Please advice me another one.

I think that runtime->dma_area address accesses into D-cache.
Is it right?
Is the runtime->dma_area address is a paging object ?
 
Regards
 
   

---------- 
MASAO TAKAHASHI <masao-takahashi@kanno.co.jp>
KANNO WORKS CO., LTD. JAPAN
TEL 093-436-2330

Re: ALSA dma_area not utilizing D-cahce

[Paul Mundt]

On Thu, May 17, 2012 at 02:16:21PM +0900, MASAO TAKAHASHI wrote:
> On Wed, 16 May 2012 14:38:57 +0900
> Paul Mundt <lethal@linux-sh.org> wrote:
> > There are a couple of places that need to be fixed. At a quick glance:
> > 
> > 	- snd_pcm_default_page_ops()
> > 	- snd_pcm_lib_default_mmap()
> > 
> > Both are in need of fixing up.
> > 
> > The snd_pcm_default_page_ops() case is easy, as we can use the same
> > approach that MIPS does (sh also provides a CAC_ADDR definition).
> > 
> > The snd_pcm_lib_default_mmap() case is a bit more involved. The last time
> > this topic came up a generic pgprot_noncached() solution was stalled as
> > pgprot_noncached() wasn't available on all of the platforms. That's been
> > addressed now, so it may be worth revisiting.
> > 
> > At present the biggest problem with fixing up the default mmap behaviour
> > is that we don't have any easy way for working out the coherence model on
> > a struct device level. ie, we may have snooping enabled on the PCI side
> > for device<->memory coherence while at the same time being non-coherent
> > for the L1 D-cache case at the platform device level.
> > 
> > There also exists a snd_pcm_lib_mmap_iomem() now that handles the
> > pgprot_noncached() case which you might be able to use with your driver.
> > 
> > That being said, P2SEGADDR is non-portable legacy garbage -- drivers
> > should never use it directly. Unfortunately we don't have any easy way to
> > use it in the headers while forcibly blowing up the build for anything
> > else.
> 
> Do you advice to use snd_pcm_mmap_writei() call?
> I think that the mmap method is used when dmix or snd_pcm_mmap_writei()
>  or dsnoop/snd_pcm_mmap_readi().
> I am using snd_pcm_writei() call.
> Then I could not use these  functions as below.
>   - snd_pcm_default_page_ops()
>   - snd_pcm_lib_default_mmap()
> 
I have no idea how the userspace library works, so you'll need to ask
ALSA folks about that.

> Please advice me another one.
> 
> I think that runtime->dma_area address accesses into D-cache.
> Is it right?
> Is the runtime->dma_area address is a paging object ?
>  
This is going to depend entirely on your driver. If your driver is
deciding to use a vmalloc backed area, then yes, it will be cached. If
you are obtaining your memory from vmalloc then that's going to reside in
P3SEG on legacy paltforms anyways, so P2SEGADDR isn't going to help you
regardless.

If you need a consistent DMA mapping in your driver make sure you are
using SNDRV_DMA_TYPE_DEV. I notice the fsi driver has this comment:

        /*
         * dont use SNDRV_DMA_TYPE_DEV, since it will oops the SH kernel
         * in MMAP mode (i.e. aplay -M)
         */

which suggests that someone was lazy and trying to avoid fixing up the
aforementioned snd_pcm_default_page_ops/snd_pcm_lib_default_mmap cases.

I've added Morimoto-san to the Cc, as I'm definitely the wrong person to ask
about anything involving alsa drivers.

Re: ALSA dma_area not utilizing D-cahce

[MASAO TAKAHASHI]

On Thu, 17 May 2012 16:22:41 +0900
Paul Mundt <lethal@linux-sh.org> wrote:

> On Thu, May 17, 2012 at 02:16:21PM +0900, MASAO TAKAHASHI wrote:
> > On Wed, 16 May 2012 14:38:57 +0900
> > Paul Mundt <lethal@linux-sh.org> wrote:
> > > There are a couple of places that need to be fixed. At a quick glance:
> > > 
> > > 	- snd_pcm_default_page_ops()
> > > 	- snd_pcm_lib_default_mmap()
> > > 
> > > Both are in need of fixing up.
> > > 
> > > The snd_pcm_default_page_ops() case is easy, as we can use the same
> > > approach that MIPS does (sh also provides a CAC_ADDR definition).
> > > 
> > > The snd_pcm_lib_default_mmap() case is a bit more involved. The last time
> > > this topic came up a generic pgprot_noncached() solution was stalled as
> > > pgprot_noncached() wasn't available on all of the platforms. That's been
> > > addressed now, so it may be worth revisiting.
> > > 
> > > At present the biggest problem with fixing up the default mmap behaviour
> > > is that we don't have any easy way for working out the coherence model on
> > > a struct device level. ie, we may have snooping enabled on the PCI side
> > > for device<->memory coherence while at the same time being non-coherent
> > > for the L1 D-cache case at the platform device level.
> > > 
> > > There also exists a snd_pcm_lib_mmap_iomem() now that handles the
> > > pgprot_noncached() case which you might be able to use with your driver.
> > > 
> > > That being said, P2SEGADDR is non-portable legacy garbage -- drivers
> > > should never use it directly. Unfortunately we don't have any easy way to
> > > use it in the headers while forcibly blowing up the build for anything
> > > else.
> > 
> > Do you advice to use snd_pcm_mmap_writei() call?
> > I think that the mmap method is used when dmix or snd_pcm_mmap_writei()
> >  or dsnoop/snd_pcm_mmap_readi().
> > I am using snd_pcm_writei() call.
> > Then I could not use these  functions as below.
> >   - snd_pcm_default_page_ops()
> >   - snd_pcm_lib_default_mmap()
> > 
> I have no idea how the userspace library works, so you'll need to ask
> ALSA folks about that.
> 
> > Please advice me another one.
> > 
> > I think that runtime->dma_area address accesses into D-cache.
> > Is it right?
> > Is the runtime->dma_area address is a paging object ?
> >  
> This is going to depend entirely on your driver. If your driver is
> deciding to use a vmalloc backed area, then yes, it will be cached. If
> you are obtaining your memory from vmalloc then that's going to reside in
> P3SEG on legacy paltforms anyways, so P2SEGADDR isn't going to help you
> regardless.
> 
> If you need a consistent DMA mapping in your driver make sure you are
> using SNDRV_DMA_TYPE_DEV. I notice the fsi driver has this comment:
> 
>         /*
>          * dont use SNDRV_DMA_TYPE_DEV, since it will oops the SH kernel
>          * in MMAP mode (i.e. aplay -M)
>          */
> 
> which suggests that someone was lazy and trying to avoid fixing up the
> aforementioned snd_pcm_default_page_ops/snd_pcm_lib_default_mmap cases.
> 
> I've added Morimoto-san to the Cc, as I'm definitely the wrong person to ask
> about anything involving alsa drivers.
My driver assigns DMA area as follows.

 snd_pcm_lib_preallocate_pages_for_all(
    pcm,
    SNDRV_DMA_TYPE_CONTINUOUS,
    snd_dma_continuous_data(GFP_KERNEL),
    64*1024, 64*1024);
So , it does not have SNDRV_DMA_TYPE_DEV type as you said.
I have an another idea to use ioremap_nocache as follows.
 In xxx_prepare() function in the driver
 to add the next codes.
      runtime->dma_area = ioremap_nocache
			(virt_to_phys(runtime->dma_area), 64*1024);
 Is it good?

Regards

Masao Takahashi

---------- 
MASAO TAKAHASHI <masao-takahashi@kanno.co.jp>
KANNO WORKS CO., LTD. JAPAN
TEL 093-436-2330

Re: ALSA dma_area not utilizing D-cahce

[Paul Mundt]

On Thu, May 17, 2012 at 04:51:06PM +0900, MASAO TAKAHASHI wrote:
> On Thu, 17 May 2012 16:22:41 +0900
> Paul Mundt <lethal@linux-sh.org> wrote:
> > On Thu, May 17, 2012 at 02:16:21PM +0900, MASAO TAKAHASHI wrote:
> > > Is the runtime->dma_area address is a paging object ?
> > >  
> > This is going to depend entirely on your driver. If your driver is
> > deciding to use a vmalloc backed area, then yes, it will be cached. If
> > you are obtaining your memory from vmalloc then that's going to reside in
> > P3SEG on legacy paltforms anyways, so P2SEGADDR isn't going to help you
> > regardless.
> > 
> > If you need a consistent DMA mapping in your driver make sure you are
> > using SNDRV_DMA_TYPE_DEV. I notice the fsi driver has this comment:
> > 
> >         /*
> >          * dont use SNDRV_DMA_TYPE_DEV, since it will oops the SH kernel
> >          * in MMAP mode (i.e. aplay -M)
> >          */
> > 
> > which suggests that someone was lazy and trying to avoid fixing up the
> > aforementioned snd_pcm_default_page_ops/snd_pcm_lib_default_mmap cases.
> > 
> > I've added Morimoto-san to the Cc, as I'm definitely the wrong person to ask
> > about anything involving alsa drivers.
> My driver assigns DMA area as follows.
> 
>  snd_pcm_lib_preallocate_pages_for_all(
>     pcm,
>     SNDRV_DMA_TYPE_CONTINUOUS,
>     snd_dma_continuous_data(GFP_KERNEL),
>     64*1024, 64*1024);
> So , it does not have SNDRV_DMA_TYPE_DEV type as you said.
> I have an another idea to use ioremap_nocache as follows.
>  In xxx_prepare() function in the driver
>  to add the next codes.
>       runtime->dma_area = ioremap_nocache
> 			(virt_to_phys(runtime->dma_area), 64*1024);
>  Is it good?
> 
No, you are basically just duplicating the behaviour of SNDRV_DMA_TYPE_DEV.
You really want dma_alloc_coherent(), which is what SNDRV_DMA_TYPE_DEV
gets you. This will map to dma_generic_alloc_coherent() in
arch/sh/mm/consistent.c, which does what you are trying to do, only
correctly.

Your implementation fails to take in to account extant cachelines for
pages obtained from the page allocator prior to DMA, which is about as
fun to debug as it sounds.

Any time you decide to start fighting the API and roll your own thing it
is going to end badly. The FSI driver gets away with it because it does
streaming DMA and uses the dma-mapping API correctly.

Rather than trying to hack around SNDRV_DMA_TYPE_DEV and inventing your
own interface, your cycles would be better spent trying to figure out
where SNDRV_DMA_TYPE_DEV goes wrong in the mmap case and sending patches.
You already have several places to look at to get you started.

Re: ALSA dma_area not utilizing D-cahce

[MASAO TAKAHASHI]

On Thu, 17 May 2012 17:03:09 +0900
Paul Mundt <lethal@linux-sh.org> wrote:

> On Thu, May 17, 2012 at 04:51:06PM +0900, MASAO TAKAHASHI wrote:
> > On Thu, 17 May 2012 16:22:41 +0900
> > Paul Mundt <lethal@linux-sh.org> wrote:
> > > On Thu, May 17, 2012 at 02:16:21PM +0900, MASAO TAKAHASHI wrote:
> > > > Is the runtime->dma_area address is a paging object ?
> > > >  
> > > This is going to depend entirely on your driver. If your driver is
> > > deciding to use a vmalloc backed area, then yes, it will be cached. If
> > > you are obtaining your memory from vmalloc then that's going to reside in
> > > P3SEG on legacy paltforms anyways, so P2SEGADDR isn't going to help you
> > > regardless.
> > > 
> > > If you need a consistent DMA mapping in your driver make sure you are
> > > using SNDRV_DMA_TYPE_DEV. I notice the fsi driver has this comment:
> > > 
> > >         /*
> > >          * dont use SNDRV_DMA_TYPE_DEV, since it will oops the SH kernel
> > >          * in MMAP mode (i.e. aplay -M)
> > >          */
> > > 
> > > which suggests that someone was lazy and trying to avoid fixing up the
> > > aforementioned snd_pcm_default_page_ops/snd_pcm_lib_default_mmap cases.
> > > 
> > > I've added Morimoto-san to the Cc, as I'm definitely the wrong person to ask
> > > about anything involving alsa drivers.
> > My driver assigns DMA area as follows.
> > 
> >  snd_pcm_lib_preallocate_pages_for_all(
> >     pcm,
> >     SNDRV_DMA_TYPE_CONTINUOUS,
> >     snd_dma_continuous_data(GFP_KERNEL),
> >     64*1024, 64*1024);
> > So , it does not have SNDRV_DMA_TYPE_DEV type as you said.
> > I have an another idea to use ioremap_nocache as follows.
> >  In xxx_prepare() function in the driver
> >  to add the next codes.
> >       runtime->dma_area = ioremap_nocache
> > 			(virt_to_phys(runtime->dma_area), 64*1024);
> >  Is it good?
> > 
> No, you are basically just duplicating the behaviour of SNDRV_DMA_TYPE_DEV.
> You really want dma_alloc_coherent(), which is what SNDRV_DMA_TYPE_DEV
> gets you. This will map to dma_generic_alloc_coherent() in
> arch/sh/mm/consistent.c, which does what you are trying to do, only
> correctly.
> 
> Your implementation fails to take in to account extant cachelines for
> pages obtained from the page allocator prior to DMA, which is about as
> fun to debug as it sounds.
> 
> Any time you decide to start fighting the API and roll your own thing it
> is going to end badly. The FSI driver gets away with it because it does
> streaming DMA and uses the dma-mapping API correctly.
> 
> Rather than trying to hack around SNDRV_DMA_TYPE_DEV and inventing your
> own interface, your cycles would be better spent trying to figure out
> where SNDRV_DMA_TYPE_DEV goes wrong in the mmap case and sending patches.
> You already have several places to look at to get you started.
I am afraid to misunderstand your advice cause my poor english.
I will wait for an answer of Morimoto-san. Is it OK? 


---------- 

MASAO TAKAHASHI <masao-takahashi@kanno.co.jp>
KANNO WORKS CO., LTD. JAPAN
TEL 093-436-2330

Re: ALSA dma_area not utilizing D-cahce

[MASAO TAKAHASHI]

On Thu, 17 May 2012 17:03:09 +0900
Paul Mundt <lethal@linux-sh.org> wrote:

> On Thu, May 17, 2012 at 04:51:06PM +0900, MASAO TAKAHASHI wrote:
> > On Thu, 17 May 2012 16:22:41 +0900
> > Paul Mundt <lethal@linux-sh.org> wrote:
> > > On Thu, May 17, 2012 at 02:16:21PM +0900, MASAO TAKAHASHI wrote:
> > > > Is the runtime->dma_area address is a paging object ?
> > > >  
> > > This is going to depend entirely on your driver. If your driver is
> > > deciding to use a vmalloc backed area, then yes, it will be cached. If
> > > you are obtaining your memory from vmalloc then that's going to reside in
> > > P3SEG on legacy paltforms anyways, so P2SEGADDR isn't going to help you
> > > regardless.
> > > 
> > > If you need a consistent DMA mapping in your driver make sure you are
> > > using SNDRV_DMA_TYPE_DEV. I notice the fsi driver has this comment:
> > > 
> > >         /*
> > >          * dont use SNDRV_DMA_TYPE_DEV, since it will oops the SH kernel
> > >          * in MMAP mode (i.e. aplay -M)
> > >          */
> > > 
> > > which suggests that someone was lazy and trying to avoid fixing up the
> > > aforementioned snd_pcm_default_page_ops/snd_pcm_lib_default_mmap cases.
> > > 
> > > I've added Morimoto-san to the Cc, as I'm definitely the wrong person to ask
> > > about anything involving alsa drivers.
> > My driver assigns DMA area as follows.
> > 
> >  snd_pcm_lib_preallocate_pages_for_all(
> >     pcm,
> >     SNDRV_DMA_TYPE_CONTINUOUS,
> >     snd_dma_continuous_data(GFP_KERNEL),
> >     64*1024, 64*1024);
> > So , it does not have SNDRV_DMA_TYPE_DEV type as you said.
> > I have an another idea to use ioremap_nocache as follows.
> >  In xxx_prepare() function in the driver
> >  to add the next codes.
> >       runtime->dma_area = ioremap_nocache
> > 			(virt_to_phys(runtime->dma_area), 64*1024);
> >  Is it good?
> > 
> No, you are basically just duplicating the behaviour of SNDRV_DMA_TYPE_DEV.
> You really want dma_alloc_coherent(), which is what SNDRV_DMA_TYPE_DEV
> gets you. This will map to dma_generic_alloc_coherent() in
> arch/sh/mm/consistent.c, which does what you are trying to do, only
> correctly.
> 
> Your implementation fails to take in to account extant cachelines for
> pages obtained from the page allocator prior to DMA, which is about as
> fun to debug as it sounds.
> 
> Any time you decide to start fighting the API and roll your own thing it
> is going to end badly. The FSI driver gets away with it because it does
> streaming DMA and uses the dma-mapping API correctly.
> 
> Rather than trying to hack around SNDRV_DMA_TYPE_DEV and inventing your
> own interface, your cycles would be better spent trying to figure out
> where SNDRV_DMA_TYPE_DEV goes wrong in the mmap case and sending patches.
> You already have several places to look at to get you started.
I made some changes from the original ALSA driver.
And made a patch as follows.
Is it right ?
Please help me.

diff -Nru a/arch/sh/include/asm/page.h b/arch/sh/include/asm/page.h
--- a/arch/sh/include/asm/page.h	2012-05-21 09:05:58.556556493 +0900
+++ b/arch/sh/include/asm/page.h	2012-05-21 09:14:44.649922912 +0900
@@ -136,6 +136,11 @@
 #define __pa(x)	((unsigned long)(x)-PAGE_OFFSET)
 #define __va(x)	((void *)((unsigned long)(x)+PAGE_OFFSET))
 #endif
+/* add 2012/5/21 */
+#if defined(CONFIG_29BIT)
+#define UNCAC_ADDR(addr)	P2SEGADDR(addr)
+#define CAC_ADDR(addr)		P1SEGADDR(addr)
+#endif
 
 #define pfn_to_kaddr(pfn)	__va((pfn) << PAGE_SHIFT)
 #define page_to_phys(page)	(page_to_pfn(page) << PAGE_SHIFT)
diff -Nru a/sound/soc/sh/sh7764-pcm.c b/sound/soc/sh/sh7764-pcm.c
--- a/sound/soc/sh/sh7764-pcm.c	2012-05-21 09:35:51.720808534 +0900
+++ b/sound/soc/sh/sh7764-pcm.c	2012-05-21 09:45:36.027357354 +0900
@@ -639,6 +639,89 @@
 
 	return frames;
 }
+/*
+ * fault callback for mmapping a RAM page
+ */
+static int sh7764_mmap_data_fault(struct vm_area_struct *area,
+						struct vm_fault *vmf)
+{
+	struct snd_pcm_substream *substream = area->vm_private_data;
+	struct snd_pcm_runtime *runtime;
+	unsigned long offset;
+	struct page * page;
+	void *vaddr;
+	size_t dma_bytes;
+	
+	pr_debug("mmap_data_fault\n");
+	if (substream = NULL)
+		return VM_FAULT_SIGBUS;
+	runtime = substream->runtime;
+	offset = vmf->pgoff << PAGE_SHIFT;
+	dma_bytes = PAGE_ALIGN(runtime->dma_bytes);
+	if (offset > dma_bytes - PAGE_SIZE)
+		return VM_FAULT_SIGBUS;
+	if (substream->ops->page) {
+		page = substream->ops->page(substream, offset);
+		if (!page)
+			return VM_FAULT_SIGBUS;
+	} else {
+		vaddr = CAC_ADDR(runtime->dma_area + offset);
+		page = virt_to_page(vaddr);
+	}
+	get_page(page);
+	vmf->page = page;
+	pr_debug("mmap_data_fault:page = %x\n", (unsigned int)page);
+	return 0;
+}
+
+static struct vm_operations_struct sh7764_pcm_vm_ops_data +{
+	.fault =	sh7764_mmap_data_fault,
+};
+
+/****************************************************
+ * 	Title :sh7764_pcm_mmap			*
+ ****************************************************/
+static int	sh7764_pcm_mmap(struct snd_pcm_substream *substream,
+				struct vm_area_struct *area)
+{
+	unsigned long	offset;
+	unsigned long	mp;
+	unsigned long	virt;
+	int				map_size;
+	struct page *map, *mapend, *ip;
+	struct snd_pcm_runtime *runtime;
+	int	ret;
+
+	
+#ifdef pgprot_noncached
+	/*
+ 	 * disable non-chahe attribute cause
+ 	 */ 
+	area->vm_page_prot = pgprot_noncached(area->vm_page_prot);
+#endif
+	offset = area->vm_pgoff << PAGE_SHIFT;
+	runtime = substream->runtime;
+	map_size = runtime->dma_bytes;
+	mp = (unsigned long)runtime->dma_addr;
+	map = virt_to_page(mp);
+	mapend = virt_to_page(mp+map_size - 1);
+	for (ip = map; ip <= mapend; ip++){
+		SetPageReserved(ip);
+	}
+	area->vm_ops = &sh7764_pcm_vm_ops_data;
+	area->vm_private_data = substream;
+	area->vm_flags |= VM_IO;
+	ret = io_remap_pfn_range(area, area->vm_start, mp>>PAGE_SHIFT, 
+						area->vm_end - area->vm_start, area->vm_page_prot);
+	if (ret ){
+		pr_debug("sh7764-pcm.c:remap_pfn_range returned EAGAIN\n");
+		return -EAGAIN;
+	}
+	atomic_inc(&substream->mmap_count);
+	return 0;
+}
+
 static struct snd_pcm_ops sh7764_ssidma_ops = {
 	.open		= sh7764_ssidma_open,
 	.close		= sh7764_ssidma_close,
@@ -648,6 +731,7 @@
 	.prepare	= sh7764_prepare,
 	.trigger	= sh7764_trigger,
 	.pointer	= sh7764_pos,
+	.mmap		= sh7764_pcm_mmap,	
 };
 
 static void sh7764_ssidma_free(struct snd_pcm *pcm)


Thanks in advance
---------- 

MASAO TAKAHASHI <masao-takahashi@kanno.co.jp>
KANNO WORKS CO., LTD. JAPAN
TEL 093-436-2330

Re: ALSA dma_area not utilizing D-cahce

[MASAO TAKAHASHI]

On Thu, 17 May 2012 17:03:09 +0900
Paul Mundt <lethal@linux-sh.org> wrote:

> On Thu, May 17, 2012 at 04:51:06PM +0900, MASAO TAKAHASHI wrote:
> > On Thu, 17 May 2012 16:22:41 +0900
> > Paul Mundt <lethal@linux-sh.org> wrote:
> > > On Thu, May 17, 2012 at 02:16:21PM +0900, MASAO TAKAHASHI wrote:
> > > > Is the runtime->dma_area address is a paging object ?
> > > >  
> > > This is going to depend entirely on your driver. If your driver is
> > > deciding to use a vmalloc backed area, then yes, it will be cached. If
> > > you are obtaining your memory from vmalloc then that's going to reside in
> > > P3SEG on legacy paltforms anyways, so P2SEGADDR isn't going to help you
> > > regardless.
> > > 
> > > If you need a consistent DMA mapping in your driver make sure you are
> > > using SNDRV_DMA_TYPE_DEV. I notice the fsi driver has this comment:
> > > 
> > >         /*
> > >          * dont use SNDRV_DMA_TYPE_DEV, since it will oops the SH kernel
> > >          * in MMAP mode (i.e. aplay -M)
> > >          */
> > > 
> > > which suggests that someone was lazy and trying to avoid fixing up the
> > > aforementioned snd_pcm_default_page_ops/snd_pcm_lib_default_mmap cases.
> > > 
> > > I've added Morimoto-san to the Cc, as I'm definitely the wrong person to ask
> > > about anything involving alsa drivers.
> > My driver assigns DMA area as follows.
> > 
> >  snd_pcm_lib_preallocate_pages_for_all(
> >     pcm,
> >     SNDRV_DMA_TYPE_CONTINUOUS,
> >     snd_dma_continuous_data(GFP_KERNEL),
> >     64*1024, 64*1024);
> > So , it does not have SNDRV_DMA_TYPE_DEV type as you said.
> > I have an another idea to use ioremap_nocache as follows.
> >  In xxx_prepare() function in the driver
> >  to add the next codes.
> >       runtime->dma_area = ioremap_nocache
> > 			(virt_to_phys(runtime->dma_area), 64*1024);
> >  Is it good?
> > 
> No, you are basically just duplicating the behaviour of SNDRV_DMA_TYPE_DEV.
> You really want dma_alloc_coherent(), which is what SNDRV_DMA_TYPE_DEV
> gets you. This will map to dma_generic_alloc_coherent() in
> arch/sh/mm/consistent.c, which does what you are trying to do, only
> correctly.
> 
> Your implementation fails to take in to account extant cachelines for
> pages obtained from the page allocator prior to DMA, which is about as
> fun to debug as it sounds.
> 
> Any time you decide to start fighting the API and roll your own thing it
> is going to end badly. The FSI driver gets away with it because it does
> streaming DMA and uses the dma-mapping API correctly.
> 
> Rather than trying to hack around SNDRV_DMA_TYPE_DEV and inventing your
> own interface, your cycles would be better spent trying to figure out
> where SNDRV_DMA_TYPE_DEV goes wrong in the mmap case and sending patches.
> You already have several places to look at to get you started.
I do miss.
Please discards previous mail.
The corret patch as follows.
diff -Nru a/arch/sh/include/asm/page.h b/arch/sh/include/asm/page.h
--- a/arch/sh/include/asm/page.h	2012-05-21 09:05:58.556556493 +0900
+++ b/arch/sh/include/asm/page.h	2012-05-21 09:14:44.649922912 +0900
@@ -136,6 +136,11 @@
 #define __pa(x)	((unsigned long)(x)-PAGE_OFFSET)
 #define __va(x)	((void *)((unsigned long)(x)+PAGE_OFFSET))
 #endif
+/* add 2012/5/21 */
+#if defined(CONFIG_29BIT)
+#define UNCAC_ADDR(addr)	P2SEGADDR(addr)
+#define CAC_ADDR(addr)		P1SEGADDR(addr)
+#endif
 
 #define pfn_to_kaddr(pfn)	__va((pfn) << PAGE_SHIFT)
 #define page_to_phys(page)	(page_to_pfn(page) << PAGE_SHIFT)
diff -Nru a/sound/soc/sh/sh7764-pcm.c b/sound/soc/sh/sh7764-pcm.c
--- a/sound/soc/sh/sh7764-pcm.c	2012-05-21 09:35:51.720808534 +0900
+++ b/sound/soc/sh/sh7764-pcm.c	2012-05-21 10:38:45.319271348 +0900
@@ -496,7 +496,7 @@
 	sd->vperiod = sd->buffer_size / sd->period_size;
 
 	if (substream->stream = SNDRV_PCM_STREAM_PLAYBACK) {
-		ssidma_outl(sd, (u32)runtime->dma_area, SSIRDMADR);
+		ssidma_outl(sd, (u32)runtime->dma_addr, SSIRDMADR);
 		ssidma_outl(sd, sd->buffer_size, SSIRDMCNTR);
 		ssidma_outl(sd, blcnt, SSIBLCNTSR);
 		ssidma_outl(sd, sd->period_size / blcnt, SSIBLNCNTSR);
@@ -639,6 +639,89 @@
 
 	return frames;
 }
+/*
+ * fault callback for mmapping a RAM page
+ */
+static int sh7764_mmap_data_fault(struct vm_area_struct *area,
+						struct vm_fault *vmf)
+{
+	struct snd_pcm_substream *substream = area->vm_private_data;
+	struct snd_pcm_runtime *runtime;
+	unsigned long offset;
+	struct page * page;
+	void *vaddr;
+	size_t dma_bytes;
+	
+	pr_debug("mmap_data_fault\n");
+	if (substream = NULL)
+		return VM_FAULT_SIGBUS;
+	runtime = substream->runtime;
+	offset = vmf->pgoff << PAGE_SHIFT;
+	dma_bytes = PAGE_ALIGN(runtime->dma_bytes);
+	if (offset > dma_bytes - PAGE_SIZE)
+		return VM_FAULT_SIGBUS;
+	if (substream->ops->page) {
+		page = substream->ops->page(substream, offset);
+		if (!page)
+			return VM_FAULT_SIGBUS;
+	} else {
+		vaddr = CAC_ADDR(runtime->dma_area + offset);
+		page = virt_to_page(vaddr);
+	}
+	get_page(page);
+	vmf->page = page;
+	pr_debug("mmap_data_fault:page = %x\n", (unsigned int)page);
+	return 0;
+}
+
+static struct vm_operations_struct sh7764_pcm_vm_ops_data +{
+	.fault =	sh7764_mmap_data_fault,
+};
+
+/****************************************************
+ * Title :sh7764_pcm_mmap			*
+ ****************************************************/
+static int	sh7764_pcm_mmap(struct snd_pcm_substream *substream,
+				struct vm_area_struct *area)
+{
+	unsigned long	offset;
+	unsigned long	mp;
+	unsigned long	virt;
+	int				map_size;
+	struct page *map, *mapend, *ip;
+	struct snd_pcm_runtime *runtime;
+	int	ret;
+
+	
+#ifdef pgprot_noncached
+	/*
+ 	 * disable non-chahe attribute cause
+ 	 */ 
+	area->vm_page_prot = pgprot_noncached(area->vm_page_prot);
+#endif
+	offset = area->vm_pgoff << PAGE_SHIFT;
+	runtime = substream->runtime;
+	map_size = runtime->dma_bytes;
+	mp = (unsigned long)runtime->dma_addr;
+	map = virt_to_page(mp);
+	mapend = virt_to_page(mp+map_size - 1);
+	for (ip = map; ip <= mapend; ip++){
+		SetPageReserved(ip);
+	}
+	area->vm_ops = &sh7764_pcm_vm_ops_data;
+	area->vm_private_data = substream;
+	area->vm_flags |= VM_IO;
+	ret = io_remap_pfn_range(area, area->vm_start, mp>>PAGE_SHIFT, 
+						area->vm_end - area->vm_start, area->vm_page_prot);
+	if (ret ){
+		pr_debug("sh7764-pcm.c:remap_pfn_range returned EAGAIN\n");
+		return -EAGAIN;
+	}
+	atomic_inc(&substream->mmap_count);
+	return 0;
+}
+
 static struct snd_pcm_ops sh7764_ssidma_ops = {
 	.open		= sh7764_ssidma_open,
 	.close		= sh7764_ssidma_close,
@@ -648,6 +731,7 @@
 	.prepare	= sh7764_prepare,
 	.trigger	= sh7764_trigger,
 	.pointer	= sh7764_pos,
+	.mmap		= sh7764_pcm_mmap,	
 };
 
 static void sh7764_ssidma_free(struct snd_pcm *pcm)
@@ -663,7 +747,7 @@
 	 * in MMAP mode (i.e. aplay -M)
 	 */
 	snd_pcm_lib_preallocate_pages_for_all(pcm,
-					      SNDRV_DMA_TYPE_CONTINUOUS,
+					      SNDRV_DMA_TYPE_DEV,
 					      snd_dma_continuous_data(GFP_KERNEL),
 					      64*1024, 64*1024);
 
---------- 

MASAO TAKAHASHI <masao-takahashi@kanno.co.jp>
KANNO WORKS CO., LTD. JAPAN
TEL 093-436-2330

Re: ALSA dma_area not utilizing D-cahce

[Paul Mundt]

On Mon, May 21, 2012 at 10:44:14AM +0900, MASAO TAKAHASHI wrote:
> diff -Nru a/arch/sh/include/asm/page.h b/arch/sh/include/asm/page.h
> --- a/arch/sh/include/asm/page.h	2012-05-21 09:05:58.556556493 +0900
> +++ b/arch/sh/include/asm/page.h	2012-05-21 09:14:44.649922912 +0900
> @@ -136,6 +136,11 @@
>  #define __pa(x)	((unsigned long)(x)-PAGE_OFFSET)
>  #define __va(x)	((void *)((unsigned long)(x)+PAGE_OFFSET))
>  #endif
> +/* add 2012/5/21 */
> +#if defined(CONFIG_29BIT)
> +#define UNCAC_ADDR(addr)	P2SEGADDR(addr)
> +#define CAC_ADDR(addr)		P1SEGADDR(addr)
> +#endif
>  
>  #define pfn_to_kaddr(pfn)	__va((pfn) << PAGE_SHIFT)
>  #define page_to_phys(page)	(page_to_pfn(page) << PAGE_SHIFT)

Are you using an old kernel? UNCAC_ADDR/CAC_ADDR should already be
defined in page.h, and do the P2/P1 wrapping for the 29-bit phys case.

> +static int	sh7764_pcm_mmap(struct snd_pcm_substream *substream,
> +				struct vm_area_struct *area)
> +{
..
> +#ifdef pgprot_noncached
> +	/*
> + 	 * disable non-chahe attribute cause
> + 	 */ 
> +	area->vm_page_prot = pgprot_noncached(area->vm_page_prot);
> +#endif

You don't want the ifdef here. pgprot_noncached() is now part of the
generic API, so you can always rely on it. It's worse if you don't have
it defined and suddenly end up with a cached attribute for the VMA that
you weren't expecting.

> +	offset = area->vm_pgoff << PAGE_SHIFT;
> +	runtime = substream->runtime;
> +	map_size = runtime->dma_bytes;
> +	mp = (unsigned long)runtime->dma_addr;
> +	map = virt_to_page(mp);
> +	mapend = virt_to_page(mp+map_size - 1);
> +	for (ip = map; ip <= mapend; ip++){
> +		SetPageReserved(ip);
> +	}
> +	area->vm_ops = &sh7764_pcm_vm_ops_data;
> +	area->vm_private_data = substream;
> +	area->vm_flags |= VM_IO;
> +	ret = io_remap_pfn_range(area, area->vm_start, mp>>PAGE_SHIFT, 
> +						area->vm_end - area->vm_start, area->vm_page_prot);
> +	if (ret ){
> +		pr_debug("sh7764-pcm.c:remap_pfn_range returned EAGAIN\n");
> +		return -EAGAIN;
> +	}
> +	atomic_inc(&substream->mmap_count);
> +	return 0;
> +}
> +
Do you really need this? It looks like you could just use
snd_pcm_lib_mmap_iomem() directly.

Re: ALSA dma_area not utilizing D-cahce

[MASAO TAKAHASHI]

On Mon, 21 May 2012 11:10:04 +0900
Paul Mundt <lethal@linux-sh.org> wrote:

> On Mon, May 21, 2012 at 10:44:14AM +0900, MASAO TAKAHASHI wrote:
> > diff -Nru a/arch/sh/include/asm/page.h b/arch/sh/include/asm/page.h
> > --- a/arch/sh/include/asm/page.h	2012-05-21 09:05:58.556556493 +0900
> > +++ b/arch/sh/include/asm/page.h	2012-05-21 09:14:44.649922912 +0900
> > @@ -136,6 +136,11 @@
> >  #define __pa(x)	((unsigned long)(x)-PAGE_OFFSET)
> >  #define __va(x)	((void *)((unsigned long)(x)+PAGE_OFFSET))
> >  #endif
> > +/* add 2012/5/21 */
> > +#if defined(CONFIG_29BIT)
> > +#define UNCAC_ADDR(addr)	P2SEGADDR(addr)
> > +#define CAC_ADDR(addr)		P1SEGADDR(addr)
> > +#endif
> >  
> >  #define pfn_to_kaddr(pfn)	__va((pfn) << PAGE_SHIFT)
> >  #define page_to_phys(page)	(page_to_pfn(page) << PAGE_SHIFT)
> 
> Are you using an old kernel? UNCAC_ADDR/CAC_ADDR should already be
> defined in page.h, and do the P2/P1 wrapping for the 29-bit phys case.
Yes, I am using sh-linux-2.6.29.
UNCAC_ADDR/CAC_ADDR is not defined yet.
> 
> > +static int	sh7764_pcm_mmap(struct snd_pcm_substream *substream,
> > +				struct vm_area_struct *area)
> > +{
> ..
> > +#ifdef pgprot_noncached
> > +	/*
> > + 	 * disable non-chahe attribute cause
> > + 	 */ 
> > +	area->vm_page_prot = pgprot_noncached(area->vm_page_prot);
> > +#endif
> 
> You don't want the ifdef here. pgprot_noncached() is now part of the
> generic API, so you can always rely on it. It's worse if you don't have
> it defined and suddenly end up with a cached attribute for the VMA that
> you weren't expecting.
I understand. I remove ifdef.
> 
> > +	offset = area->vm_pgoff << PAGE_SHIFT;
> > +	runtime = substream->runtime;
> > +	map_size = runtime->dma_bytes;
> > +	mp = (unsigned long)runtime->dma_addr;
> > +	map = virt_to_page(mp);
> > +	mapend = virt_to_page(mp+map_size - 1);
> > +	for (ip = map; ip <= mapend; ip++){
> > +		SetPageReserved(ip);
> > +	}
> > +	area->vm_ops = &sh7764_pcm_vm_ops_data;
> > +	area->vm_private_data = substream;
> > +	area->vm_flags |= VM_IO;
> > +	ret = io_remap_pfn_range(area, area->vm_start, mp>>PAGE_SHIFT, 
> > +						area->vm_end - area->vm_start,
> > area->vm_page_prot);
> > +	if (ret ){
> > +		pr_debug("sh7764-pcm.c:remap_pfn_range returned EAGAIN\n");
> > +		return -EAGAIN;
> > +	}
> > +	atomic_inc(&substream->mmap_count);
> > +	return 0;
> > +}
> > +
> Do you really need this? It looks like you could just use
> snd_pcm_lib_mmap_iomem() directly.
Is SetPageReserved() not needed ?
 


---------- 
高橋 正雄
MASAO TAKAHASHI <masao-takahashi@kanno.co.jp>
KANNO WORKS CO., LTD. JAPAN
TEL 093-436-2330

Re: ALSA dma_area not utilizing D-cahce

[Paul Mundt]

On Mon, May 21, 2012 at 11:40:05AM +0900, MASAO TAKAHASHI wrote:
> 
> On Mon, 21 May 2012 11:10:04 +0900
> Paul Mundt <lethal@linux-sh.org> wrote:
> > > +	offset = area->vm_pgoff << PAGE_SHIFT;
> > > +	runtime = substream->runtime;
> > > +	map_size = runtime->dma_bytes;
> > > +	mp = (unsigned long)runtime->dma_addr;
> > > +	map = virt_to_page(mp);
> > > +	mapend = virt_to_page(mp+map_size - 1);
> > > +	for (ip = map; ip <= mapend; ip++){
> > > +		SetPageReserved(ip);
> > > +	}
> > > +	area->vm_ops = &sh7764_pcm_vm_ops_data;
> > > +	area->vm_private_data = substream;
> > > +	area->vm_flags |= VM_IO;
> > > +	ret = io_remap_pfn_range(area, area->vm_start, mp>>PAGE_SHIFT, 
> > > +						area->vm_end - area->vm_start,
> > > area->vm_page_prot);
> > > +	if (ret ){
> > > +		pr_debug("sh7764-pcm.c:remap_pfn_range returned EAGAIN\n");
> > > +		return -EAGAIN;
> > > +	}
> > > +	atomic_inc(&substream->mmap_count);
> > > +	return 0;
> > > +}
> > > +
> > Do you really need this? It looks like you could just use
> > snd_pcm_lib_mmap_iomem() directly.
> Is SetPageReserved() not needed ?
>  
No, VM_IO on the VMA ensures that no one's going to touch the pages. If
we switch to using our own VMA for consistent mapping the consistent DMA
allocator then we would need to take care of it, but this is nothing your
mmap routine needs to worry about.

Re: ALSA dma_area not utilizing D-cahce

[MASAO TAKAHASHI]

On Mon, 21 May 2012 12:21:22 +0900
Paul Mundt <lethal@linux-sh.org> wrote:

> On Mon, May 21, 2012 at 11:40:05AM +0900, MASAO TAKAHASHI wrote:
> > 
> > On Mon, 21 May 2012 11:10:04 +0900
> > Paul Mundt <lethal@linux-sh.org> wrote:
> > > > +	offset = area->vm_pgoff << PAGE_SHIFT;
> > > > +	runtime = substream->runtime;
> > > > +	map_size = runtime->dma_bytes;
> > > > +	mp = (unsigned long)runtime->dma_addr;
> > > > +	map = virt_to_page(mp);
> > > > +	mapend = virt_to_page(mp+map_size - 1);
> > > > +	for (ip = map; ip <= mapend; ip++){
> > > > +		SetPageReserved(ip);
> > > > +	}
> > > > +	area->vm_ops = &sh7764_pcm_vm_ops_data;
> > > > +	area->vm_private_data = substream;
> > > > +	area->vm_flags |= VM_IO;
> > > > +	ret = io_remap_pfn_range(area, area->vm_start, mp>>PAGE_SHIFT, 
> > > > +						area->vm_end - area->vm_start,
> > > > area->vm_page_prot);
> > > > +	if (ret ){
> > > > +		pr_debug("sh7764-pcm.c:remap_pfn_range returned EAGAIN\n");
> > > > +		return -EAGAIN;
> > > > +	}
> > > > +	atomic_inc(&substream->mmap_count);
> > > > +	return 0;
> > > > +}
> > > > +
> > > Do you really need this? It looks like you could just use
> > > snd_pcm_lib_mmap_iomem() directly.
> > Is SetPageReserved() not needed ?
> >  
> No, VM_IO on the VMA ensures that no one's going to touch the pages. If
> we switch to using our own VMA for consistent mapping the consistent DMA
> allocator then we would need to take care of it, but this is nothing your
> mmap routine needs to worry about.
OK, I understand
Thanks a lot for you advice.
I'll test my patch.



---------- 

MASAO TAKAHASHI <masao-takahashi@kanno.co.jp>
KANNO WORKS CO., LTD. JAPAN
TEL 093-436-2330