Re: out of sync capture samples with MMAP on MIPS

[Takashi Iwai <tiwai@suse.de>]

On Wed, 23 May 2018 20:36:37 +0200,
Daniel Danzberger wrote:
> 
> I am using alsa's mmap() interface to capture audio samples on some AR71xx MIPS
> devices via USB and sometimes the sampled audio sounds very disordered.
> 
> After debugging deeper into this issue I found that samples are fine in the
> kernel's vmalloc'ed buffer (runtime->dma_area). But once they get accessed from
> userspace via mmaped memory, the samples differ.
> 
> I wrote a module + userspace program to demonstrate the issue:
> git@github.com:dddaniel/mmaptest.git
> 
> The userspace tool reads the data a couple of times with a small delay.
> As you see below, after a couple of reads the data gets closer to look like in
> the kernel buffer:
> ---
> root@OpenWrt:/# mmaptest-user
> mmap addr: 0x77a04000
> [  760.464968] mmap page 7573000 at va 87573000
> check memory ...FAIL (at byte 0)
> check memory ...FAIL (at byte 96)
> check memory ...FAIL (at byte 96)
> check memory ...FAIL (at byte 96)
> check memory ...FAIL (at byte 128)
> ---
> 
> Does someone here experienced similar issues ?

It's likely because of the cache coherency issue on ARM.
Currently USB-audio driver uses the buffer via vmalloc() for the
intermediate buffer, and this may go out of sync on non-coherent arch
like ARM.

Does the patch below help?  Not sure whether it works on ARM at all,
as I've tested only on x86.


thanks,

Takashi

---
diff --git a/sound/usb/pcm.c b/sound/usb/pcm.c
index 20bed1c7a312..ec798517d1ee 100644
--- a/sound/usb/pcm.c
+++ b/sound/usb/pcm.c
@@ -727,8 +727,13 @@ static int snd_usb_hw_params(struct snd_pcm_substream *substream,
 	struct audioformat *fmt;
 	int ret;
 
+#ifdef SND_USB_USE_DEV_ALLOC
+	ret = snd_pcm_lib_malloc_pages(substream,
+				       params_buffer_bytes(hw_params));
+#else
 	ret = snd_pcm_lib_alloc_vmalloc_buffer(substream,
 					       params_buffer_bytes(hw_params));
+#endif
 	if (ret < 0)
 		return ret;
 
@@ -780,7 +785,11 @@ static int snd_usb_hw_free(struct snd_pcm_substream *substream)
 		snd_usb_endpoint_deactivate(subs->data_endpoint);
 		snd_usb_unlock_shutdown(subs->stream->chip);
 	}
+#ifdef SND_USB_USE_DEV_ALLOC
+	return snd_pcm_lib_free_pages(substream);
+#else
 	return snd_pcm_lib_free_vmalloc_buffer(substream);
+#endif
 }
 
 /*
@@ -1700,8 +1709,12 @@ static const struct snd_pcm_ops snd_usb_playback_ops = {
 	.prepare =	snd_usb_pcm_prepare,
 	.trigger =	snd_usb_substream_playback_trigger,
 	.pointer =	snd_usb_pcm_pointer,
+#ifdef SND_USB_USE_DEV_ALLOC
+	.page =		snd_pcm_sgbuf_ops_page,
+#else
 	.page =		snd_pcm_lib_get_vmalloc_page,
 	.mmap =		snd_pcm_lib_mmap_vmalloc,
+#endif
 };
 
 static const struct snd_pcm_ops snd_usb_capture_ops = {
@@ -1713,8 +1726,12 @@ static const struct snd_pcm_ops snd_usb_capture_ops = {
 	.prepare =	snd_usb_pcm_prepare,
 	.trigger =	snd_usb_substream_capture_trigger,
 	.pointer =	snd_usb_pcm_pointer,
+#ifdef SND_USB_USE_DEV_ALLOC
+	.page =		snd_pcm_sgbuf_ops_page,
+#else
 	.page =		snd_pcm_lib_get_vmalloc_page,
 	.mmap =		snd_pcm_lib_mmap_vmalloc,
+#endif
 };
 
 void snd_usb_set_pcm_ops(struct snd_pcm *pcm, int stream)
@@ -1723,3 +1740,15 @@ void snd_usb_set_pcm_ops(struct snd_pcm *pcm, int stream)
 			stream == SNDRV_PCM_STREAM_PLAYBACK ?
 			&snd_usb_playback_ops : &snd_usb_capture_ops);
 }
+
+#ifdef SND_USB_USE_DEV_ALLOC
+void snd_usb_preallocate_buffer(struct snd_usb_substream *subs)
+{
+	struct snd_pcm *pcm = subs->stream->pcm;
+	struct snd_pcm_substream *s = pcm->streams[subs->direction].substream;
+	struct device *dev = subs->dev->bus->controller;
+
+	snd_pcm_lib_preallocate_pages(s, SNDRV_DMA_TYPE_DEV_SG,
+				      dev, 64*1024, 512*1024);
+}
+#endif
diff --git a/sound/usb/pcm.h b/sound/usb/pcm.h
index 35740d5ef268..7caa76201212 100644
--- a/sound/usb/pcm.h
+++ b/sound/usb/pcm.h
@@ -2,6 +2,9 @@
 #ifndef __USBAUDIO_PCM_H
 #define __USBAUDIO_PCM_H
 
+/* use the standard dev alloc */
+#define SND_USB_USE_DEV_ALLOC
+
 snd_pcm_uframes_t snd_usb_pcm_delay(struct snd_usb_substream *subs,
 				    unsigned int rate);
 
@@ -11,5 +14,10 @@ int snd_usb_init_pitch(struct snd_usb_audio *chip, int iface,
 		       struct usb_host_interface *alts,
 		       struct audioformat *fmt);
 
+#ifdef SND_USB_USE_DEV_ALLOC
+void snd_usb_preallocate_buffer(struct snd_usb_substream *subs);
+#else
+#define snd_usb_preallocate_buffer(subs)	/* NOP */
+#endif
 
 #endif /* __USBAUDIO_PCM_H */
diff --git a/sound/usb/stream.c b/sound/usb/stream.c
index d16e1c23f4e9..729afd808cc4 100644
--- a/sound/usb/stream.c
+++ b/sound/usb/stream.c
@@ -106,6 +106,8 @@ static void snd_usb_init_substream(struct snd_usb_stream *as,
 	subs->ep_num = fp->endpoint;
 	if (fp->channels > subs->channels_max)
 		subs->channels_max = fp->channels;
+
+	snd_usb_preallocate_buffer(subs);
 }
 
 /* kctl callbacks for usb-audio channel maps */