Skip to contentSkip to conversation

mpc8610: Add mmap support

PATCH In review Timur Tabi 4 participants 7 messages Jan 17, 2008

Subsystems (3)
lore
Share & tools
lore

Timur Tabi

author
#0 lore raw

PATCH 1/1 mpc8610: Add mmap support

Enable mmap support in the MPC8610 ASoC driver. The driver can use ALSA's default mmap functionality, it was just not enabled previously.

---
 sound/soc/fsl/fsl_dma.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)
sound/soc/fsl/fsl_dma.c +3 -1
index 2173203..652514f 100644
@@ -136,7 +136,9 @@ struct fsl_dma_private {
  */
 static const struct snd_pcm_hardware fsl_dma_hardware = {
 
-	.info   		= SNDRV_PCM_INFO_INTERLEAVED,
+	.info   		= SNDRV_PCM_INFO_INTERLEAVED |
+				  SNDRV_PCM_INFO_MMAP |
+				  SNDRV_PCM_INFO_MMAP_VALID,
 	.formats		= FSLDMA_PCM_FORMATS,
 	.rates  		= FSLDMA_PCM_RATES,
 	.rate_min       	= 5512,
Signature · 1 line
1.5.2.4
#1 lore raw
Quoted 4 lines · Timur Tabi

Enable mmap support in the MPC8610 ASoC driver. The driver can use ALSA's default mmap functionality, it was just not enabled previously.

[sorry, I may be completely wrong below about the .page callback purpose - in this case, just confirm that the testcase works]

Hm, the "struct snd_pcm_ops fsl_dma_ops" doesn't contain the .page callback. With saa7134-alsa, this led to the "device claims to support mmap but actually doesn't work" bug, see the following subthread:

http://mailman.alsa-project.org/pipermail/alsa-devel/2007-October/003773.html

IOW: have you actually tested that the following commands work (X,Y refer to the FSL chip)?

aplay -M -D hw:X,Y sample.wav
arecord -f cd -M -D:X,Y capture.wav

Quoted 19 lines
---
 sound/soc/fsl/fsl_dma.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)
sound/soc/fsl/fsl_dma.c +3 -1
index 2173203..652514f 100644
@@ -136,7 +136,9 @@ struct fsl_dma_private {
  */
 static const struct snd_pcm_hardware fsl_dma_hardware = {
 
-	.info   		= SNDRV_PCM_INFO_INTERLEAVED,
+	.info   		= SNDRV_PCM_INFO_INTERLEAVED |
+				  SNDRV_PCM_INFO_MMAP |
+				  SNDRV_PCM_INFO_MMAP_VALID,
 	.formats		= FSLDMA_PCM_FORMATS,
 	.rates  		= FSLDMA_PCM_RATES,
 	.rate_min       	= 5512,
Signature · 1 line
Alexander E. Patrakov
author
#2 lore raw
Quoted 5 lines · Alexander E. Patrakov
Quoted 4 lines · Timur Tabi

Enable mmap support in the MPC8610 ASoC driver. The driver can use ALSA's
default mmap functionality, it was just not enabled previously.

Quoted 8 lines
Quoted 1 line

[sorry, I may be completely wrong below about the .page callback purpose
- in this case, just confirm that the testcase works]

Hm, the "struct snd_pcm_ops fsl_dma_ops" doesn't contain the .page callback. With saa7134-alsa, this led to the "device claims to support mmap but actually doesn't work" bug, see the following subthread:

Isn't that an ARM chip? The default MMAP handler in ALSA doesn't support ARM, but it works for PowerPC.

Quoted 8 lines

http://mailman.alsa-project.org/pipermail/alsa-devel/2007-October/003773.html

IOW: have you actually tested that the following commands work (X,Y refer to the FSL chip)?

aplay -M -D hw:X,Y sample.wav
arecord -f cd -M -D:X,Y capture.wav

I haven't tried record, but I did try playback, and it worked. Without this patch, adding -M gives me this error:

aplay: set_params:852: Access type not available

After applying this patch, playback works fine.

Signature · 2 lines
Timur Tabi
Linux kernel developer at Freescale
#3 lore raw

At Thu, 17 Jan 2008 10:31:37 -0600,

Quoted 28 lines · Timur Tabi
Quoted 5 lines · Alexander E. Patrakov
Quoted 4 lines · Timur Tabi

Enable mmap support in the MPC8610 ASoC driver. The driver can use ALSA's
default mmap functionality, it was just not enabled previously.

Quoted 8 lines
Quoted 1 line

[sorry, I may be completely wrong below about the .page callback purpose
- in this case, just confirm that the testcase works]

Hm, the "struct snd_pcm_ops fsl_dma_ops" doesn't contain the .page callback. With saa7134-alsa, this led to the "device claims to support mmap but actually doesn't work" bug, see the following subthread:

Isn't that an ARM chip? The default MMAP handler in ALSA doesn't support ARM, but it works for PowerPC.

Quoted 8 lines

http://mailman.alsa-project.org/pipermail/alsa-devel/2007-October/003773.html

IOW: have you actually tested that the following commands work (X,Y refer to the FSL chip)?

aplay -M -D hw:X,Y sample.wav
arecord -f cd -M -D:X,Y capture.wav

Quoted 6 lines

I haven't tried record, but I did try playback, and it worked. Without this patch, adding -M gives me this error:

aplay: set_params:852: Access type not available

After applying this patch, playback works fine.

Great, applied to ALSA tree now.

Thanks.

Takashi

#4 lore raw
In reply to Timur Tabi
Quoted 7 lines · Timur Tabi
Quoted 3 lines · Alexander E. Patrakov

Hm, the "struct snd_pcm_ops fsl_dma_ops" doesn't contain the .page callback. With saa7134-alsa, this led to the "device claims to support mmap but actually doesn't work" bug, see the following subthread:

Isn't that an ARM chip? The default MMAP handler in ALSA doesn't support ARM, but it works for PowerPC.

saa7134 is a Philips chip that you'll find in many TV tuners.

As for the .page logic, my question was indeed stupid. I should have read the snd_pcm_mmap_data_nopage() function and notice that, by default, it wants to do this:

vaddr = runtime->dma_area + offset;
page = virt_to_page(vaddr);

and saa7134 wants to use vmalloc_to_page() instead of virt_to_page() due to the way it allocates the DMA buffer.

Quoted 6 lines

I haven't tried record, but I did try playback, and it worked. Without this patch, adding -M gives me this error:

aplay: set_params:852: Access type not available

After applying this patch, playback works fine.

Good!

Signature · 1 line
Alexander E. Patrakov
#5 lore raw
In reply to Timur Tabi
Quoted 11 lines · Timur Tabi
Quoted 10 lines · Alexander E. Patrakov
Quoted 5 lines · Timur Tabi

Enable mmap support in the MPC8610 ASoC driver. The driver can use ALSA's
default mmap functionality, it was just not enabled previously.

[sorry, I may be completely wrong below about the .page callback purpose
- in this case, just confirm that the testcase works]

Quoted 6 lines
Quoted 3 lines

Hm, the "struct snd_pcm_ops fsl_dma_ops" doesn't contain the .page callback. With saa7134-alsa, this led to the "device claims to support mmap but actually doesn't work" bug, see the following subthread:

Isn't that an ARM chip? The default MMAP handler in ALSA doesn't support ARM, but it works for PowerPC.

Alexander is talking about the same thing I was in my previous message to you. saa7134, like cx88-alsa, uses vmalloc instead of ALSA to allocate the DMA buffer, and so a special page callback is needed.

Quoted 6 lines

I haven't tried record, but I did try playback, and it worked. Without this patch, adding -M gives me this error:

aplay: set_params:852: Access type not available

After applying this patch, playback works fine.

The key thing to know is that you will not get an error message if your page callback is wrong! Everything will appear to work. You just won't hear any sound.

author
#6 lore raw
Quoted 3 lines · Trent Piepho

The key thing to know is that you will not get an error message if your page callback is wrong! Everything will appear to work. You just won't hear any sound.

When I said playback works fine, I was wearing my headphones. :-)

Signature · 2 lines
Timur Tabi
Linux kernel developer at Freescale