* About MIPS specific dma_mmap_coherent() @ 2010-04-13 4:58 Wu Zhangjin 2010-04-13 16:49 ` Takashi Iwai 0 siblings, 1 reply; 9+ messages in thread From: Wu Zhangjin @ 2010-04-13 4:58 UTC (permalink / raw) To: Ralf Baechle, Takashi Iwai; +Cc: linux-mips [-- Attachment #1: Type: text/plain, Size: 836 bytes --] Hi, Ralf and Takashi Seems Takashi have sent the MIPS specific dma_mmap_coherent() at 18 Aug 2008: http://www.linux-mips.org/archives/linux-mips/2008-08/msg00178.html But that patch have not been accepted since it was not suitable to all of the MIPS variants. Without that patch, the ALSA output will be broken in some of the MIPS variants, can we make the implementation in the above url be weak, then the particular MIPS variants can override it with their own versions but the common MIPS variants can share it to fix the ALSA problem? I have attached a change of the above patch, which is applicable to the linux-2.6.33 and linux-2.6.34-rcX and I have tested it on my YeeLoong netbook, the following command function well with it. $ mplayer -ao alsa file.mp3 but without it, the ALSA output is broken. Regards, Wu Zhangjin ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: About MIPS specific dma_mmap_coherent() 2010-04-13 4:58 About MIPS specific dma_mmap_coherent() Wu Zhangjin @ 2010-04-13 16:49 ` Takashi Iwai 2010-04-14 4:21 ` Wu Zhangjin 0 siblings, 1 reply; 9+ messages in thread From: Takashi Iwai @ 2010-04-13 16:49 UTC (permalink / raw) To: wuzhangjin; +Cc: Ralf Baechle, linux-mips At Tue, 13 Apr 2010 12:58:54 +0800, Wu Zhangjin wrote: > > Hi, Ralf and Takashi > > Seems Takashi have sent the MIPS specific dma_mmap_coherent() at 18 Aug > 2008: > > http://www.linux-mips.org/archives/linux-mips/2008-08/msg00178.html > > But that patch have not been accepted since it was not suitable to all > of the MIPS variants. > > Without that patch, the ALSA output will be broken in some of the MIPS > variants, can we make the implementation in the above url be weak, then > the particular MIPS variants can override it with their own versions but > the common MIPS variants can share it to fix the ALSA problem? > > I have attached a change of the above patch, which is applicable to the > linux-2.6.33 and linux-2.6.34-rcX and I have tested it on my YeeLoong > netbook, the following command function well with it. > > $ mplayer -ao alsa file.mp3 > > but without it, the ALSA output is broken. Hm, which driver/device are you using? Also, how is it broken? There is already a low-level hack in sound/core/pcm_native.c for MIPS, so I thought the kernel oops should have been avoided, at least. Maybe still pgprot_noncached() is missing, though. thanks, Takashi > > Regards, > Wu Zhangjin > [2 0001-MIPS-Implement-dma_mmap_coherent-for-ALSA-audio-outp.patch <text/x-patch; UTF-8 (7bit)>] > >From a6ee304febbd609d2936dd5b33a16482ef224c97 Mon Sep 17 00:00:00 2001 > From: Wu Zhangjin <wuzhangjin@gmail.com> > Date: Sun, 11 Apr 2010 03:58:13 +0800 > Subject: [PATCH] MIPS: Implement dma_mmap_coherent() for ALSA audio output > > A lazy version of dma_mmap_coherent() implementation for MIPS. > > Without this patch, the ALSA sound output of MIPS is broken: > > $ mplayer -ao alsa file.mp3 > > (This patch was sent out by Takashi Iwai at '18 Aug 2008' but not have > been applied yet for it is not suitable for all MIPS variants. If you > need more info, please access: > http://www.linux-mips.org/archives/linux-mips/2008-08/msg00178.html) > > Signed-off-by: Takashi Iwai <tiwai@suse.de> > --- > arch/mips/include/asm/dma-mapping.h | 4 ++++ > arch/mips/mm/dma-default.c | 13 +++++++++++++ > 2 files changed, 17 insertions(+), 0 deletions(-) > > diff --git a/arch/mips/include/asm/dma-mapping.h b/arch/mips/include/asm/dma-mapping.h > index 664ba53..c39bfdf 100644 > --- a/arch/mips/include/asm/dma-mapping.h > +++ b/arch/mips/include/asm/dma-mapping.h > @@ -74,4 +74,8 @@ extern int dma_is_consistent(struct device *dev, dma_addr_t dma_addr); > extern void dma_cache_sync(struct device *dev, void *vaddr, size_t size, > enum dma_data_direction direction); > > +#define ARCH_HAS_DMA_MMAP_COHERENT > +extern int dma_mmap_coherent(struct device *dev, struct vm_area_struct *vma, > + void *cpu_addr, dma_addr_t handle, size_t size); > + > #endif /* _ASM_DMA_MAPPING_H */ > diff --git a/arch/mips/mm/dma-default.c b/arch/mips/mm/dma-default.c > index 9547bc0..8388428 100644 > --- a/arch/mips/mm/dma-default.c > +++ b/arch/mips/mm/dma-default.c > @@ -375,3 +375,16 @@ void dma_cache_sync(struct device *dev, void *vaddr, size_t size, > } > > EXPORT_SYMBOL(dma_cache_sync); > + > +int __weak dma_mmap_coherent(struct device *dev, struct vm_area_struct *vma, > + void *cpu_addr, dma_addr_t handle, size_t size) > +{ > + struct page *pg; > + vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot); > + cpu_addr = (void *)dma_addr_to_virt(dev, handle); > + pg = virt_to_page(cpu_addr); > + return remap_pfn_range(vma, vma->vm_start, > + page_to_pfn(pg) + vma->vm_pgoff, > + size, vma->vm_page_prot); > +} > +EXPORT_SYMBOL(dma_mmap_coherent); > -- > 1.7.0 > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: About MIPS specific dma_mmap_coherent() 2010-04-13 16:49 ` Takashi Iwai @ 2010-04-14 4:21 ` Wu Zhangjin 2010-04-14 6:31 ` Takashi Iwai 0 siblings, 1 reply; 9+ messages in thread From: Wu Zhangjin @ 2010-04-14 4:21 UTC (permalink / raw) To: Takashi Iwai; +Cc: Ralf Baechle, linux-mips Hi, > > $ mplayer -ao alsa file.mp3 > > > > but without it, the ALSA output is broken. > > Hm, which driver/device are you using? Also, how is it broken? I'm using the cs5535 audio in my Yeeloong laptop with the config "CONFIG_SND_CS5535AUDIO=m" the above 'broken' means there is audio output with ALSA, but the output is not normal for it has lots of noises: sha sha sha... > > There is already a low-level hack in sound/core/pcm_native.c for MIPS, > so I thought the kernel oops should have been avoided, at least. > Maybe still pgprot_noncached() is missing, though. You mean this part for MIPS: static inline struct page * snd_pcm_default_page_ops(struct snd_pcm_substream *substream, unsigned long ofs) { void *vaddr = substream->runtime->dma_area + ofs; #if defined(CONFIG_MIPS) && defined(CONFIG_DMA_NONCOHERENT) if (substream->dma_buffer.dev.type == SNDRV_DMA_TYPE_DEV) return virt_to_page(CAC_ADDR(vaddr)); #endif ... } Yes, it was already there from one of your patches to linux-2.6.33, but the pgprot_noncached() was missing as you mentioned above. And I have found you have added the ARM specific dma_mmap_coherent() to snd_pcm_default_mmap() to mmap the DMA buffer on RAM: /* * mmap the DMA buffer on RAM */ static int snd_pcm_default_mmap(struct snd_pcm_substream *substream, struct vm_area_struct *area) { area->vm_flags |= VM_RESERVED; #ifdef ARCH_HAS_DMA_MMAP_COHERENT if (!substream->ops->page && substream->dma_buffer.dev.type == SNDRV_DMA_TYPE_DEV) return dma_mmap_coherent(substream->dma_buffer.dev.dev, area, substream->runtime->dma_area, substream->runtime->dma_addr, area->vm_end - area->vm_start); #endif /* ARCH_HAS_DMA_MMAP_COHERENT */ /* mmap with fault handler */ area->vm_ops = &snd_pcm_vm_ops_data_fault; return 0; } Before, we have added the low-level handling with pgprot_noncached() in snd_pcm_mmap() to fix it, but now, can we add MIPS specific dma_mmap_coherent() as ARM did? Regards, Wu Zhangjin > > [2 0001-MIPS-Implement-dma_mmap_coherent-for-ALSA-audio-outp.patch <text/x-patch; UTF-8 (7bit)>] > > >From a6ee304febbd609d2936dd5b33a16482ef224c97 Mon Sep 17 00:00:00 2001 > > From: Wu Zhangjin <wuzhangjin@gmail.com> > > Date: Sun, 11 Apr 2010 03:58:13 +0800 > > Subject: [PATCH] MIPS: Implement dma_mmap_coherent() for ALSA audio output > > > > A lazy version of dma_mmap_coherent() implementation for MIPS. > > > > Without this patch, the ALSA sound output of MIPS is broken: > > > > $ mplayer -ao alsa file.mp3 > > > > (This patch was sent out by Takashi Iwai at '18 Aug 2008' but not have > > been applied yet for it is not suitable for all MIPS variants. If you > > need more info, please access: > > http://www.linux-mips.org/archives/linux-mips/2008-08/msg00178.html) > > > > Signed-off-by: Takashi Iwai <tiwai@suse.de> > > --- > > arch/mips/include/asm/dma-mapping.h | 4 ++++ > > arch/mips/mm/dma-default.c | 13 +++++++++++++ > > 2 files changed, 17 insertions(+), 0 deletions(-) > > > > diff --git a/arch/mips/include/asm/dma-mapping.h b/arch/mips/include/asm/dma-mapping.h > > index 664ba53..c39bfdf 100644 > > --- a/arch/mips/include/asm/dma-mapping.h > > +++ b/arch/mips/include/asm/dma-mapping.h > > @@ -74,4 +74,8 @@ extern int dma_is_consistent(struct device *dev, dma_addr_t dma_addr); > > extern void dma_cache_sync(struct device *dev, void *vaddr, size_t size, > > enum dma_data_direction direction); > > > > +#define ARCH_HAS_DMA_MMAP_COHERENT > > +extern int dma_mmap_coherent(struct device *dev, struct vm_area_struct *vma, > > + void *cpu_addr, dma_addr_t handle, size_t size); > > + > > #endif /* _ASM_DMA_MAPPING_H */ > > diff --git a/arch/mips/mm/dma-default.c b/arch/mips/mm/dma-default.c > > index 9547bc0..8388428 100644 > > --- a/arch/mips/mm/dma-default.c > > +++ b/arch/mips/mm/dma-default.c > > @@ -375,3 +375,16 @@ void dma_cache_sync(struct device *dev, void *vaddr, size_t size, > > } > > > > EXPORT_SYMBOL(dma_cache_sync); > > + > > +int __weak dma_mmap_coherent(struct device *dev, struct vm_area_struct *vma, > > + void *cpu_addr, dma_addr_t handle, size_t size) > > +{ > > + struct page *pg; > > + vma->vm_page_prot = pgprot_noncached(vma->vm_page_prot); > > + cpu_addr = (void *)dma_addr_to_virt(dev, handle); > > + pg = virt_to_page(cpu_addr); > > + return remap_pfn_range(vma, vma->vm_start, > > + page_to_pfn(pg) + vma->vm_pgoff, > > + size, vma->vm_page_prot); > > +} > > +EXPORT_SYMBOL(dma_mmap_coherent); > > -- > > 1.7.0 > > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: About MIPS specific dma_mmap_coherent() 2010-04-14 4:21 ` Wu Zhangjin @ 2010-04-14 6:31 ` Takashi Iwai 2010-04-14 9:00 ` Wu Zhangjin 0 siblings, 1 reply; 9+ messages in thread From: Takashi Iwai @ 2010-04-14 6:31 UTC (permalink / raw) To: wuzhangjin; +Cc: Ralf Baechle, linux-mips At Wed, 14 Apr 2010 12:21:29 +0800, Wu Zhangjin wrote: > > Hi, > > > > $ mplayer -ao alsa file.mp3 > > > > > > but without it, the ALSA output is broken. > > > > Hm, which driver/device are you using? Also, how is it broken? > > I'm using the cs5535 audio in my Yeeloong laptop with the config > "CONFIG_SND_CS5535AUDIO=m" > > the above 'broken' means there is audio output with ALSA, but the output > is not normal for it has lots of noises: sha sha sha... > > > > > There is already a low-level hack in sound/core/pcm_native.c for MIPS, > > so I thought the kernel oops should have been avoided, at least. > > Maybe still pgprot_noncached() is missing, though. > > You mean this part for MIPS: > > static inline struct page * > snd_pcm_default_page_ops(struct snd_pcm_substream *substream, unsigned > long ofs) > { > void *vaddr = substream->runtime->dma_area + ofs; > #if defined(CONFIG_MIPS) && defined(CONFIG_DMA_NONCOHERENT) > if (substream->dma_buffer.dev.type == SNDRV_DMA_TYPE_DEV) > return virt_to_page(CAC_ADDR(vaddr)); > #endif > > ... > } > > Yes, it was already there from one of your patches to linux-2.6.33, but > the pgprot_noncached() was missing as you mentioned above. > > And I have found you have added the ARM specific dma_mmap_coherent() to > snd_pcm_default_mmap() to mmap the DMA buffer on RAM: > > /* > * mmap the DMA buffer on RAM > */ > static int snd_pcm_default_mmap(struct snd_pcm_substream *substream, > struct vm_area_struct *area) > { > area->vm_flags |= VM_RESERVED; > #ifdef ARCH_HAS_DMA_MMAP_COHERENT > if (!substream->ops->page && > substream->dma_buffer.dev.type == SNDRV_DMA_TYPE_DEV) > return dma_mmap_coherent(substream->dma_buffer.dev.dev, > area, > substream->runtime->dma_area, > substream->runtime->dma_addr, > area->vm_end - area->vm_start); > #endif /* ARCH_HAS_DMA_MMAP_COHERENT */ > /* mmap with fault handler */ > area->vm_ops = &snd_pcm_vm_ops_data_fault; > return 0; > } > > Before, we have added the low-level handling with pgprot_noncached() in > snd_pcm_mmap() to fix it, but now, can we add MIPS specific > dma_mmap_coherent() as ARM did? That would be a certainly fix, yes. I have no objection, of course ;) A quicky, less-intrusive one would be the patch below. thanks, Takashi --- diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c index c22ebb0..fd6703e 100644 --- a/sound/core/pcm_native.c +++ b/sound/core/pcm_native.c @@ -3188,6 +3188,8 @@ static int snd_pcm_default_mmap(struct snd_pcm_substream *substream, substream->runtime->dma_area, substream->runtime->dma_addr, area->vm_end - area->vm_start); +#elif defined(CONFIG_MIPS) && defined(CONFIG_DMA_NONCOHERENT) + area->vm_page_prot = pgprot_noncached(area->vm_page_prot); #endif /* ARCH_HAS_DMA_MMAP_COHERENT */ /* mmap with fault handler */ area->vm_ops = &snd_pcm_vm_ops_data_fault; ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: About MIPS specific dma_mmap_coherent() 2010-04-14 6:31 ` Takashi Iwai @ 2010-04-14 9:00 ` Wu Zhangjin 2010-04-14 15:46 ` Takashi Iwai 0 siblings, 1 reply; 9+ messages in thread From: Wu Zhangjin @ 2010-04-14 9:00 UTC (permalink / raw) To: Takashi Iwai; +Cc: Ralf Baechle, linux-mips On Wed, 2010-04-14 at 08:31 +0200, Takashi Iwai wrote: [...] > > > > Before, we have added the low-level handling with pgprot_noncached() in > > snd_pcm_mmap() to fix it, but now, can we add MIPS specific > > dma_mmap_coherent() as ARM did? > > That would be a certainly fix, yes. I have no objection, of course ;) > A quicky, less-intrusive one would be the patch below. The below patch works well, just tested it. > > > thanks, > > Takashi > > --- > diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c > index c22ebb0..fd6703e 100644 > --- a/sound/core/pcm_native.c > +++ b/sound/core/pcm_native.c > @@ -3188,6 +3188,8 @@ static int snd_pcm_default_mmap(struct snd_pcm_substream *substream, > substream->runtime->dma_area, > substream->runtime->dma_addr, > area->vm_end - area->vm_start); > +#elif defined(CONFIG_MIPS) && defined(CONFIG_DMA_NONCOHERENT) > + area->vm_page_prot = pgprot_noncached(area->vm_page_prot); > #endif /* ARCH_HAS_DMA_MMAP_COHERENT */ > /* mmap with fault handler */ > area->vm_ops = &snd_pcm_vm_ops_data_fault; ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: About MIPS specific dma_mmap_coherent() 2010-04-14 9:00 ` Wu Zhangjin @ 2010-04-14 15:46 ` Takashi Iwai 2010-05-11 18:48 ` Ralf Baechle 0 siblings, 1 reply; 9+ messages in thread From: Takashi Iwai @ 2010-04-14 15:46 UTC (permalink / raw) To: wuzhangjin; +Cc: Ralf Baechle, linux-mips At Wed, 14 Apr 2010 17:00:19 +0800, Wu Zhangjin wrote: > > On Wed, 2010-04-14 at 08:31 +0200, Takashi Iwai wrote: > [...] > > > > > > Before, we have added the low-level handling with pgprot_noncached() in > > > snd_pcm_mmap() to fix it, but now, can we add MIPS specific > > > dma_mmap_coherent() as ARM did? > > > > That would be a certainly fix, yes. I have no objection, of course ;) > > A quicky, less-intrusive one would be the patch below. > > The below patch works well, just tested it. Thanks. But, I remember vaguely that calling pgprot_noncached() unconditionally is dangerous. It should be checked somehow, e.g. via platform_device_is_coherent(). And, this found only in dma-coherence.h, and adding it to pcm_native.c would become messy like below... So, it'd be really better to add dma_mmap_coherent(), indeed. Takashi --- diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c index c22ebb0..4c3edc1 100644 --- a/sound/core/pcm_native.c +++ b/sound/core/pcm_native.c @@ -36,6 +36,9 @@ #include <sound/timer.h> #include <sound/minors.h> #include <asm/io.h> +#if defined(CONFIG_MIPS) && defined(CONFIG_DMA_NONCOHERENT) +#include <dma-coherence.h> +#endif /* * Compatibility @@ -3188,6 +3191,10 @@ static int snd_pcm_default_mmap(struct snd_pcm_substream *substream, substream->runtime->dma_area, substream->runtime->dma_addr, area->vm_end - area->vm_start); +#elif defined(CONFIG_MIPS) && defined(CONFIG_DMA_NONCOHERENT) + if (substream->dma_buffer.dev.type == SNDRV_DMA_TYPE_DEV && + !plat_device_is_coherent(substream->dma_buffer.dev.dev)) + area->vm_page_prot = pgprot_noncached(area->vm_page_prot); #endif /* ARCH_HAS_DMA_MMAP_COHERENT */ /* mmap with fault handler */ area->vm_ops = &snd_pcm_vm_ops_data_fault; ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: About MIPS specific dma_mmap_coherent() 2010-04-14 15:46 ` Takashi Iwai @ 2010-05-11 18:48 ` Ralf Baechle 2010-05-12 6:42 ` Wu Zhangjin 2010-05-12 8:41 ` Takashi Iwai 0 siblings, 2 replies; 9+ messages in thread From: Ralf Baechle @ 2010-05-11 18:48 UTC (permalink / raw) To: Takashi Iwai; +Cc: wuzhangjin, linux-mips On Wed, Apr 14, 2010 at 05:46:13PM +0200, Takashi Iwai wrote: > But, I remember vaguely that calling pgprot_noncached() > unconditionally is dangerous. It should be checked somehow, e.g. via > platform_device_is_coherent(). And, this found only in > dma-coherence.h, and adding it to pcm_native.c would become messy like > below... > > So, it'd be really better to add dma_mmap_coherent(), indeed. We agreed that this was only meant as a stop gap meassure. As such I do agree with either of http://patchwork.linux-mips.org/patch/1117/ http://patchwork.linux-mips.org/patch/1118/ Wu has tested the 1117 patch so that might make it preferable especially for 2.6.34 if we should go for that. Ralf ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: About MIPS specific dma_mmap_coherent() 2010-05-11 18:48 ` Ralf Baechle @ 2010-05-12 6:42 ` Wu Zhangjin 2010-05-12 8:41 ` Takashi Iwai 1 sibling, 0 replies; 9+ messages in thread From: Wu Zhangjin @ 2010-05-12 6:42 UTC (permalink / raw) To: Ralf Baechle; +Cc: Takashi Iwai, linux-mips Hi, Ralf & Takashi On Tue, 2010-05-11 at 19:48 +0100, Ralf Baechle wrote: > On Wed, Apr 14, 2010 at 05:46:13PM +0200, Takashi Iwai wrote: > > > But, I remember vaguely that calling pgprot_noncached() > > unconditionally is dangerous. It should be checked somehow, e.g. via > > platform_device_is_coherent(). And, this found only in > > dma-coherence.h, and adding it to pcm_native.c would become messy like > > below... > > > > So, it'd be really better to add dma_mmap_coherent(), indeed. > > We agreed that this was only meant as a stop gap meassure. As such I do > agree with either of > > http://patchwork.linux-mips.org/patch/1117/ > http://patchwork.linux-mips.org/patch/1118/ > > Wu has tested the 1117 patch so that might make it preferable especially > for 2.6.34 if we should go for that. > Just tested 1118, it works too ;) Regards, Wu Zhangjin ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: About MIPS specific dma_mmap_coherent() 2010-05-11 18:48 ` Ralf Baechle 2010-05-12 6:42 ` Wu Zhangjin @ 2010-05-12 8:41 ` Takashi Iwai 1 sibling, 0 replies; 9+ messages in thread From: Takashi Iwai @ 2010-05-12 8:41 UTC (permalink / raw) To: Ralf Baechle; +Cc: wuzhangjin, linux-mips At Tue, 11 May 2010 19:48:44 +0100, Ralf Baechle wrote: > > On Wed, Apr 14, 2010 at 05:46:13PM +0200, Takashi Iwai wrote: > > > But, I remember vaguely that calling pgprot_noncached() > > unconditionally is dangerous. It should be checked somehow, e.g. via > > platform_device_is_coherent(). And, this found only in > > dma-coherence.h, and adding it to pcm_native.c would become messy like > > below... > > > > So, it'd be really better to add dma_mmap_coherent(), indeed. > > We agreed that this was only meant as a stop gap meassure. As such I do > agree with either of > > http://patchwork.linux-mips.org/patch/1117/ > http://patchwork.linux-mips.org/patch/1118/ > > Wu has tested the 1117 patch so that might make it preferable especially > for 2.6.34 if we should go for that. OK, I merged the patch now as Wu confirmed it's working. Will be included in the next pull request. thanks, Takashi ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2010-05-12 8:41 UTC | newest] Thread overview: 9+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2010-04-13 4:58 About MIPS specific dma_mmap_coherent() Wu Zhangjin 2010-04-13 16:49 ` Takashi Iwai 2010-04-14 4:21 ` Wu Zhangjin 2010-04-14 6:31 ` Takashi Iwai 2010-04-14 9:00 ` Wu Zhangjin 2010-04-14 15:46 ` Takashi Iwai 2010-05-11 18:48 ` Ralf Baechle 2010-05-12 6:42 ` Wu Zhangjin 2010-05-12 8:41 ` Takashi Iwai
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.