[alsa-devel] [PATCH 0/3] ALSA: pcm: add 'applptr' event of tracepoint
Hi,
This patchset is for a event at updating application-side position in kernel space. This also includes my previous patch:
[alsa-devel] [PATCH 2/2] ALSA: pcm: add arrangement for applying appl_ptr http://mailman.alsa-project.org/pipermail/alsa-devel/2017-May/121091.html
In design of ALSA PCM core, status and control data for runtime of ALSA PCM substream are shared between kernel/user spaces by page frame mapping with read-only attribute. The application-side position is a part of the status data and ALSA PCM applications can't update it.
When applications are written by read/write programming scenario, they can be unaware of update of application-side position. In this case, ALSA PCM core certainly performs it.
However, when applications are written by mmap programming scenario, if maintaining the application side position accurately in kernel space, applications should voluntarily execute ioctl(2) with some commands when they handles any PCM frames. If not voluntarily, the events are not probed.
There's a loophole, using architectures to which ALSA PCM core judges non-cache coherent. In this case, the status and control data is not mapped into processe's VMA for any applications. Userland library, alsa-lib, is programmed for this case. It executes ioctl(2) with SNDRV_PCM_IOCTL_SYNC_PTR command every time to requiring the status and control data.
ARM is such an architecture. Below is an example with serial sound interface (ssi) on i.mx6 quad core SoC. I use v4.1 kernel released by fsl-community with patches from VIA Tech. Inc. for VAB820, and my backport patches for relevant features for this patchset. I use Ubuntu 17.04 from ports.ubuntu.com as user land for armhf architecture.
$ aplay -v -M -D hw:imx6vab820sgtl5,0 /dev/urandom -f S16_LE -r 48000 --period-size=128 --buffer-size=256 Playing raw data '/dev/urandom' : Signed 16 bit Little Endian, Rate 48000 Hz, Mono Hardware PCM card 0 'imx6-vab820-sgtl5000' device 0 subdevice 0 Its setup is: stream : PLAYBACK access : MMAP_INTERLEAVED format : S16_LE subformat : STD channels : 1 rate : 48000 exact rate : 48000 (48000/1) msbits : 16 buffer_size : 256 period_size : 128 period_time : 2666 tstamp_mode : NONE tstamp_type : MONOTONIC period_step : 1 avail_min : 128 period_event : 0 start_threshold : 256 stop_threshold : 256 silence_threshold: 0 silence_size : 0 boundary : 1073741824 appl_ptr : 0 hw_ptr : 0 mmap_area[0] = 0x76f98000,0,16 (16)
$ trace-cmd record -e snd_pcm:hwptr -e snd_pcm:applptr $ trace-cmd report ... 60.208495: applptr: pcmC0D0p/sub0: prev=1792, curr=1792, avail=0, period=128, buf=256 60.208633: applptr: pcmC0D0p/sub0: prev=1792, curr=1792, avail=0, period=128, buf=256 60.210022: hwptr: pcmC0D0p/sub0: IRQ: pos=128, old=1536, base=1536, period=128, buf=256 60.210202: applptr: pcmC0D0p/sub0: prev=1792, curr=1792, avail=128, period=128, buf=256 60.210344: hwptr: pcmC0D0p/sub0: POS: pos=128, old=1664, base=1536, period=128, buf=256 60.210348: applptr: pcmC0D0p/sub0: prev=1792, curr=1792, avail=128, period=128, buf=256 60.210486: applptr: pcmC0D0p/sub0: prev=1792, curr=1792, avail=128, period=128, buf=256 60.210626: applptr: pcmC0D0p/sub0: prev=1792, curr=1920, avail=0, period=128, buf=256 60.211002: applptr: pcmC0D0p/sub0: prev=1920, curr=1920, avail=0, period=128, buf=256 60.211142: hwptr: pcmC0D0p/sub0: POS: pos=128, old=1664, base=1536, period=128, buf=256 60.211146: applptr: pcmC0D0p/sub0: prev=1920, curr=1920, avail=0, period=128, buf=256 60.211287: applptr: pcmC0D0p/sub0: prev=1920, curr=1920, avail=0, period=128, buf=256 60.212690: hwptr: pcmC0D0p/sub0: IRQ: pos=0, old=1664, base=1536, period=128, buf=256 60.212866: applptr: pcmC0D0p/sub0: prev=1920, curr=1920, avail=128, period=128, buf=256 60.212999: hwptr: pcmC0D0p/sub0: POS: pos=0, old=1792, base=1792, period=128, buf=256 60.213003: applptr: pcmC0D0p/sub0: prev=1920, curr=1920, avail=128, period=128, buf=256 60.213135: applptr: pcmC0D0p/sub0: prev=1920, curr=1920, avail=128, period=128, buf=256 60.213276: applptr: pcmC0D0p/sub0: prev=1920, curr=2048, avail=0, period=128, buf=256 60.213654: applptr: pcmC0D0p/sub0: prev=2048, curr=2048, avail=0, period=128, buf=256 60.213796: hwptr: pcmC0D0p/sub0: POS: pos=0, old=1792, base=1792, period=128, buf=256 60.213800: applptr: pcmC0D0p/sub0: prev=2048, curr=2048, avail=0, period=128, buf=256 60.213937: applptr: pcmC0D0p/sub0: prev=2048, curr=2048, avail=0, period=128, buf=256 60.215356: hwptr: pcmC0D0p/sub0: IRQ: pos=128, old=1792, base=1792, period=128, buf=256 60.215542: applptr: pcmC0D0p/sub0: prev=2048, curr=2048, avail=128, period=128, buf=256 60.215679: hwptr: pcmC0D0p/sub0: POS: pos=128, old=1920, base=1792, period=128, buf=256 60.215683: applptr: pcmC0D0p/sub0: prev=2048, curr=2048, avail=128, period=128, buf=256 60.215813: applptr: pcmC0D0p/sub0: prev=2048, curr=2048, avail=128, period=128, buf=256 60.215947: applptr: pcmC0D0p/sub0: prev=2048, curr=2176, avail=0, period=128, buf=256 ...
We can surely see 'applptr' event is probed even if the application run for mmap programming scenario ('-M' option for 'hw' plugin). Below is a result of strace:
02:44:15.886382 ioctl(4, SNDRV_PCM_IOCTL_SYNC_PTR, 0x56a32b30) = 0 02:44:15.887203 poll([{fd=4, events=POLLOUT|POLLERR|POLLNVAL}], 1, -1) = 1 ([{fd=4, revents=POLLOUT}]) 02:44:15.887471 ioctl(4, SNDRV_PCM_IOCTL_SYNC_PTR, 0x56a32b30) = 0 02:44:15.887637 ioctl(4, SNDRV_PCM_IOCTL_SYNC_PTR, 0x56a32b30) = 0 02:44:15.887805 ioctl(4, SNDRV_PCM_IOCTL_SYNC_PTR, 0x56a32b30) = 0 02:44:15.887969 ioctl(4, SNDRV_PCM_IOCTL_SYNC_PTR, 0x56a32b30) = 0 02:44:15.888132 read(3, "..."..., 256) = 256 02:44:15.889040 ioctl(4, SNDRV_PCM_IOCTL_SYNC_PTR, 0x56a32b30) = 0 02:44:15.889221 ioctl(4, SNDRV_PCM_IOCTL_SYNC_PTR, 0x56a32b30) = 0 02:44:15.889431 ioctl(4, SNDRV_PCM_IOCTL_SYNC_PTR, 0x56a32b30) = 0 02:44:15.889606 poll([{fd=4, events=POLLOUT|POLLERR|POLLNVAL}], 1, -1) = 1 ([{fd=4, revents=POLLOUT}]) 02:44:15.889833 ioctl(4, SNDRV_PCM_IOCTL_SYNC_PTR, 0x56a32b30) = 0 02:44:15.889998 ioctl(4, SNDRV_PCM_IOCTL_SYNC_PTR, 0x56a32b30) = 0 02:44:15.890164 ioctl(4, SNDRV_PCM_IOCTL_SYNC_PTR, 0x56a32b30) = 0 02:44:15.891048 ioctl(4, SNDRV_PCM_IOCTL_SYNC_PTR, 0x56a32b30) = 0 02:44:15.891228 read(3, "..."..., 256) = 256 02:44:15.891497 ioctl(4, SNDRV_PCM_IOCTL_SYNC_PTR, 0x56a32b30) = 0 02:44:15.891661 ioctl(4, SNDRV_PCM_IOCTL_SYNC_PTR, 0x56a32b30) = 0 02:44:15.891829 ioctl(4, SNDRV_PCM_IOCTL_SYNC_PTR, 0x56a32b30) = 0 02:44:15.891991 poll([{fd=4, events=POLLOUT|POLLERR|POLLNVAL}], 1, -1) = 1 ([{fd=4, revents=POLLOUT}]) 02:44:15.893007 ioctl(4, SNDRV_PCM_IOCTL_SYNC_PTR, 0x56a32b30) = 0
We can see 7 calls of ioctl(2) with SNDRV_PCM_IOCTL_SYNC_PTR per loop with call of poll(2). 128 PCM frames are transferred per loop of one poll(2), because the PCM substream is configured with S16_LE format and 1 channel (2 byte * 1 * 128 = 256 bytes). This equals to the size of period of PCM buffer. Comparing to the probed data, one of the 7 system calls is actually used to commit the number of copied PCM frames to kernel space.
The tracepoint event is useful to investigate this case.
I note that below modules are related to the above sample.
* snd-soc-dummy.ko * snd-soc-imx-sgtl5000.ko * snd-soc-fsl-ssi.ko * snd-soc-imx-pcm-dma.ko * snd-soc-sgtl5000.ko
Takashi Sakamoto (3): ALSA: pcm: unify codes to operate application-side position on PCM buffer ALSA: pcm: add 'applptr' event of tracepoint ALSA: pcm: use %s instead of %c for format of PCM buffer tracepoints
sound/core/pcm_lib.c | 31 +++++++++++++++++++++++++++--- sound/core/pcm_local.h | 2 ++ sound/core/pcm_native.c | 28 ++++----------------------- sound/core/pcm_trace.h | 50 +++++++++++++++++++++++++++++++++++++++++++------ 4 files changed, 78 insertions(+), 33 deletions(-)
In a series of recent work, ALSA PCM core got some arrangements to handle application-side position on PCM buffer. However, relevant codes still disperse to two translation units
This commit unifies these codes into a helper function.
Signed-off-by: Takashi Sakamoto o-takashi@sakamocchi.jp --- sound/core/pcm_lib.c | 27 ++++++++++++++++++++++++--- sound/core/pcm_local.h | 2 ++ sound/core/pcm_native.c | 28 ++++------------------------ 3 files changed, 30 insertions(+), 27 deletions(-)
diff --git a/sound/core/pcm_lib.c b/sound/core/pcm_lib.c index 9dc7bbfe8853..d82f1437667f 100644 --- a/sound/core/pcm_lib.c +++ b/sound/core/pcm_lib.c @@ -2101,6 +2101,27 @@ static int pcm_accessible_state(struct snd_pcm_runtime *runtime) } }
+/* update to the given appl_ptr and call ack callback if needed; + * when an error is returned, take back to the original value + */ +int pcm_lib_apply_appl_ptr(struct snd_pcm_substream *substream, + snd_pcm_uframes_t appl_ptr) +{ + struct snd_pcm_runtime *runtime = substream->runtime; + snd_pcm_uframes_t old_appl_ptr = runtime->control->appl_ptr; + int ret; + + runtime->control->appl_ptr = appl_ptr; + if (substream->ops->ack) { + ret = substream->ops->ack(substream); + if (ret < 0) { + runtime->control->appl_ptr = old_appl_ptr; + return ret; + } + } + return 0; +} + /* the common loop for read/write data */ snd_pcm_sframes_t __snd_pcm_lib_xfer(struct snd_pcm_substream *substream, void *data, bool interleaved, @@ -2220,9 +2241,9 @@ snd_pcm_sframes_t __snd_pcm_lib_xfer(struct snd_pcm_substream *substream, appl_ptr += frames; if (appl_ptr >= runtime->boundary) appl_ptr -= runtime->boundary; - runtime->control->appl_ptr = appl_ptr; - if (substream->ops->ack) - substream->ops->ack(substream); + err = pcm_lib_apply_appl_ptr(substream, appl_ptr); + if (err < 0) + goto _end_unlock;
offset += frames; size -= frames; diff --git a/sound/core/pcm_local.h b/sound/core/pcm_local.h index e4bf2af62b02..16f254732b2a 100644 --- a/sound/core/pcm_local.h +++ b/sound/core/pcm_local.h @@ -27,6 +27,8 @@ int snd_pcm_hw_constraints_complete(struct snd_pcm_substream *substream); int snd_pcm_hw_constraint_mask(struct snd_pcm_runtime *runtime, snd_pcm_hw_param_t var, u_int32_t mask);
+int pcm_lib_apply_appl_ptr(struct snd_pcm_substream *substream, + snd_pcm_uframes_t appl_ptr); int snd_pcm_update_state(struct snd_pcm_substream *substream, struct snd_pcm_runtime *runtime); int snd_pcm_update_hw_ptr(struct snd_pcm_substream *substream); diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c index 076187ae8859..a597aa7c4b98 100644 --- a/sound/core/pcm_native.c +++ b/sound/core/pcm_native.c @@ -2591,27 +2591,6 @@ static int do_pcm_hwsync(struct snd_pcm_substream *substream) } }
-/* update to the given appl_ptr and call ack callback if needed; - * when an error is returned, take back to the original value - */ -static int apply_appl_ptr(struct snd_pcm_substream *substream, - snd_pcm_uframes_t appl_ptr) -{ - struct snd_pcm_runtime *runtime = substream->runtime; - snd_pcm_uframes_t old_appl_ptr = runtime->control->appl_ptr; - int ret; - - runtime->control->appl_ptr = appl_ptr; - if (substream->ops->ack) { - ret = substream->ops->ack(substream); - if (ret < 0) { - runtime->control->appl_ptr = old_appl_ptr; - return ret; - } - } - return 0; -} - /* increase the appl_ptr; returns the processed frames or a negative error */ static snd_pcm_sframes_t forward_appl_ptr(struct snd_pcm_substream *substream, snd_pcm_uframes_t frames, @@ -2628,7 +2607,7 @@ static snd_pcm_sframes_t forward_appl_ptr(struct snd_pcm_substream *substream, appl_ptr = runtime->control->appl_ptr + frames; if (appl_ptr >= (snd_pcm_sframes_t)runtime->boundary) appl_ptr -= runtime->boundary; - ret = apply_appl_ptr(substream, appl_ptr); + ret = pcm_lib_apply_appl_ptr(substream, appl_ptr); return ret < 0 ? ret : frames; }
@@ -2648,7 +2627,7 @@ static snd_pcm_sframes_t rewind_appl_ptr(struct snd_pcm_substream *substream, appl_ptr = runtime->control->appl_ptr - frames; if (appl_ptr < 0) appl_ptr += runtime->boundary; - ret = apply_appl_ptr(substream, appl_ptr); + ret = pcm_lib_apply_appl_ptr(substream, appl_ptr); return ret < 0 ? ret : frames; }
@@ -2776,7 +2755,8 @@ static int snd_pcm_sync_ptr(struct snd_pcm_substream *substream, } snd_pcm_stream_lock_irq(substream); if (!(sync_ptr.flags & SNDRV_PCM_SYNC_PTR_APPL)) { - err = apply_appl_ptr(substream, sync_ptr.c.control.appl_ptr); + err = pcm_lib_apply_appl_ptr(substream, + sync_ptr.c.control.appl_ptr); if (err < 0) { snd_pcm_stream_unlock_irq(substream); return err;
In design of ALSA PCM core, both of hardware-side and application-side position on PCM buffer are maintained as a part of information for runtime of PCM substream. In a view of ALSA PCM application, these two positions can be updated by executing ioctl(2) with some commands.
There's an event of tracepoint for hardware-side position; 'hwptr'. On the other hand, no events for application-side position.
This commit adds a new event for this purpose; 'applptr'. When the application-side position is changed in kernel space, this event is probed with useful information for developers.
I note that the event is not probed for all of ALSA PCM applications, because the application-side position is passively maintained in mmap programming scenario. In this case, applications should be programmed with ioctl(2) with SNDRV_PCM_IOCTL_SYNC_PTR to commit the application-side position to kernel land when it operates any PCM frames. In read/write programming scenario, this is needless. They're OK just to transfer any PCM frames by ioctl(2) with SNDRV_PCM_IOCTL_[READ|WRITE][N/I]_FRAMES, then stuffs in kernel land updates the application-side position and the event is probed.
My additional note is lock acquisition. The event is probed under acquiring PCM stream lock. This means that calculation in the event is free from any hardware events.
Signed-off-by: Takashi Sakamoto o-takashi@sakamocchi.jp --- sound/core/pcm_lib.c | 4 ++++ sound/core/pcm_trace.h | 38 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 42 insertions(+)
diff --git a/sound/core/pcm_lib.c b/sound/core/pcm_lib.c index d82f1437667f..e73b6e4135f6 100644 --- a/sound/core/pcm_lib.c +++ b/sound/core/pcm_lib.c @@ -42,6 +42,7 @@ #define trace_hwptr(substream, pos, in_interrupt) #define trace_xrun(substream) #define trace_hw_ptr_error(substream, reason) +#define trace_applptr(substream, prev, curr) #endif
static int fill_silence_frames(struct snd_pcm_substream *substream, @@ -2119,6 +2120,9 @@ int pcm_lib_apply_appl_ptr(struct snd_pcm_substream *substream, return ret; } } + + trace_applptr(substream, old_appl_ptr, appl_ptr); + return 0; }
diff --git a/sound/core/pcm_trace.h b/sound/core/pcm_trace.h index b63b654da5ff..e672368ab878 100644 --- a/sound/core/pcm_trace.h +++ b/sound/core/pcm_trace.h @@ -102,6 +102,44 @@ TRACE_EVENT(hw_ptr_error, __entry->number, __entry->reason) );
+TRACE_EVENT(applptr, + TP_PROTO(struct snd_pcm_substream *substream, snd_pcm_uframes_t prev, snd_pcm_uframes_t curr), + TP_ARGS(substream, prev, curr), + TP_STRUCT__entry( + __field( unsigned int, card ) + __field( unsigned int, device ) + __field( unsigned int, number ) + __field( unsigned int, stream ) + __field( snd_pcm_uframes_t, prev ) + __field( snd_pcm_uframes_t, curr ) + __field( snd_pcm_uframes_t, avail ) + __field( snd_pcm_uframes_t, period_size ) + __field( snd_pcm_uframes_t, buffer_size ) + ), + TP_fast_assign( + __entry->card = (substream)->pcm->card->number; + __entry->device = (substream)->pcm->device; + __entry->number = (substream)->number; + __entry->stream = (substream)->stream; + __entry->prev = (prev); + __entry->curr = (curr); + __entry->avail = (substream)->stream ? snd_pcm_capture_avail(substream->runtime) : snd_pcm_playback_avail(substream->runtime); + __entry->period_size = (substream)->runtime->period_size; + __entry->buffer_size = (substream)->runtime->buffer_size; + ), + TP_printk("pcmC%dD%d%s/sub%d: prev=%lu, curr=%lu, avail=%lu, period=%lu, buf=%lu", + __entry->card, + __entry->device, + __entry->stream ? "c" : "p", + __entry->number, + __entry->prev, + __entry->curr, + __entry->avail, + __entry->period_size, + __entry->buffer_size + ) +); + #endif /* _PCM_TRACE_H */
/* This part must be outside protection */
On Sun, 11 Jun 2017 08:48:07 +0200, Takashi Sakamoto wrote:
In design of ALSA PCM core, both of hardware-side and application-side position on PCM buffer are maintained as a part of information for runtime of PCM substream. In a view of ALSA PCM application, these two positions can be updated by executing ioctl(2) with some commands.
There's an event of tracepoint for hardware-side position; 'hwptr'. On the other hand, no events for application-side position.
This commit adds a new event for this purpose; 'applptr'. When the application-side position is changed in kernel space, this event is probed with useful information for developers.
I note that the event is not probed for all of ALSA PCM applications, because the application-side position is passively maintained in mmap programming scenario. In this case, applications should be programmed with ioctl(2) with SNDRV_PCM_IOCTL_SYNC_PTR to commit the application-side position to kernel land when it operates any PCM frames.
Here "should" is no correct choice of word. As you already noted in the cover letter, the notification of appl_ptr in the mmap mode with the status/control mmap is voluntary. So, it's not "should be programmed" but "may be programmed".
thanks,
Takashi
On Jun 12 2017 01:53, Takashi Iwai wrote:
On Sun, 11 Jun 2017 08:48:07 +0200, Takashi Sakamoto wrote:
In design of ALSA PCM core, both of hardware-side and application-side position on PCM buffer are maintained as a part of information for runtime of PCM substream. In a view of ALSA PCM application, these two positions can be updated by executing ioctl(2) with some commands.
There's an event of tracepoint for hardware-side position; 'hwptr'. On the other hand, no events for application-side position.
This commit adds a new event for this purpose; 'applptr'. When the application-side position is changed in kernel space, this event is probed with useful information for developers.
I note that the event is not probed for all of ALSA PCM applications, because the application-side position is passively maintained in mmap programming scenario. In this case, applications should be programmed with ioctl(2) with SNDRV_PCM_IOCTL_SYNC_PTR to commit the application-side position to kernel land when it operates any PCM frames.
Here "should" is no correct choice of word. As you already noted in the cover letter, the notification of appl_ptr in the mmap mode with the status/control mmap is voluntary. So, it's not "should be programmed" but "may be programmed".
Indeed. It's better.
Regards
Takashi Sakamoto
As long as I know, in userspace, '%c' format on printing format for tracepoint is replaced with '>c<' by existent tracing program; i.g. 'perf-trace' and 'trace-cmd'. This is inconvenient.
This commit replaces the format with '%s'. The length of letters in the format string is not changed, thus this commit doesn't increase object size.
In theory, I should work for improvements of these tracing programs, but here I'd like to save my time to work for the other projects.
Signed-off-by: Takashi Sakamoto o-takashi@sakamocchi.jp --- sound/core/pcm_trace.h | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/sound/core/pcm_trace.h b/sound/core/pcm_trace.h index e672368ab878..3ddec1b8ae46 100644 --- a/sound/core/pcm_trace.h +++ b/sound/core/pcm_trace.h @@ -34,9 +34,9 @@ TRACE_EVENT(hwptr, __entry->old_hw_ptr = (substream)->runtime->status->hw_ptr; __entry->hw_ptr_base = (substream)->runtime->hw_ptr_base; ), - TP_printk("pcmC%dD%d%c/sub%d: %s: pos=%lu, old=%lu, base=%lu, period=%lu, buf=%lu", + TP_printk("pcmC%dD%d%s/sub%d: %s: pos=%lu, old=%lu, base=%lu, period=%lu, buf=%lu", __entry->card, __entry->device, - __entry->stream == SNDRV_PCM_STREAM_PLAYBACK ? 'p' : 'c', + __entry->stream == SNDRV_PCM_STREAM_PLAYBACK ? "p" : "c", __entry->number, __entry->in_interrupt ? "IRQ" : "POS", (unsigned long)__entry->pos, @@ -69,9 +69,9 @@ TRACE_EVENT(xrun, __entry->old_hw_ptr = (substream)->runtime->status->hw_ptr; __entry->hw_ptr_base = (substream)->runtime->hw_ptr_base; ), - TP_printk("pcmC%dD%d%c/sub%d: XRUN: old=%lu, base=%lu, period=%lu, buf=%lu", + TP_printk("pcmC%dD%d%s/sub%d: XRUN: old=%lu, base=%lu, period=%lu, buf=%lu", __entry->card, __entry->device, - __entry->stream == SNDRV_PCM_STREAM_PLAYBACK ? 'p' : 'c', + __entry->stream == SNDRV_PCM_STREAM_PLAYBACK ? "p" : "c", __entry->number, (unsigned long)__entry->old_hw_ptr, (unsigned long)__entry->hw_ptr_base, @@ -96,9 +96,9 @@ TRACE_EVENT(hw_ptr_error, __entry->stream = (substream)->stream; __entry->reason = (why); ), - TP_printk("pcmC%dD%d%c/sub%d: ERROR: %s", + TP_printk("pcmC%dD%d%s/sub%d: ERROR: %s", __entry->card, __entry->device, - __entry->stream == SNDRV_PCM_STREAM_PLAYBACK ? 'p' : 'c', + __entry->stream == SNDRV_PCM_STREAM_PLAYBACK ? "p" : "c", __entry->number, __entry->reason) );
On Sun, 11 Jun 2017 08:48:05 +0200, Takashi Sakamoto wrote:
Hi,
This patchset is for a event at updating application-side position in kernel space. This also includes my previous patch:
[alsa-devel] [PATCH 2/2] ALSA: pcm: add arrangement for applying appl_ptr http://mailman.alsa-project.org/pipermail/alsa-devel/2017-May/121091.html
In design of ALSA PCM core, status and control data for runtime of ALSA PCM substream are shared between kernel/user spaces by page frame mapping with read-only attribute. The application-side position is a part of the status data and ALSA PCM applications can't update it.
When applications are written by read/write programming scenario, they can be unaware of update of application-side position. In this case, ALSA PCM core certainly performs it.
However, when applications are written by mmap programming scenario, if maintaining the application side position accurately in kernel space, applications should voluntarily execute ioctl(2) with some commands when they handles any PCM frames. If not voluntarily, the events are not probed.
There's a loophole, using architectures to which ALSA PCM core judges non-cache coherent. In this case, the status and control data is not mapped into processe's VMA for any applications. Userland library, alsa-lib, is programmed for this case. It executes ioctl(2) with SNDRV_PCM_IOCTL_SYNC_PTR command every time to requiring the status and control data.
ARM is such an architecture. Below is an example with serial sound interface (ssi) on i.mx6 quad core SoC. I use v4.1 kernel released by fsl-community with patches from VIA Tech. Inc. for VAB820, and my backport patches for relevant features for this patchset. I use Ubuntu 17.04 from ports.ubuntu.com as user land for armhf architecture.
$ aplay -v -M -D hw:imx6vab820sgtl5,0 /dev/urandom -f S16_LE -r 48000 --period-size=128 --buffer-size=256 Playing raw data '/dev/urandom' : Signed 16 bit Little Endian, Rate 48000 Hz, Mono Hardware PCM card 0 'imx6-vab820-sgtl5000' device 0 subdevice 0 Its setup is: stream : PLAYBACK access : MMAP_INTERLEAVED format : S16_LE subformat : STD channels : 1 rate : 48000 exact rate : 48000 (48000/1) msbits : 16 buffer_size : 256 period_size : 128 period_time : 2666 tstamp_mode : NONE tstamp_type : MONOTONIC period_step : 1 avail_min : 128 period_event : 0 start_threshold : 256 stop_threshold : 256 silence_threshold: 0 silence_size : 0 boundary : 1073741824 appl_ptr : 0 hw_ptr : 0 mmap_area[0] = 0x76f98000,0,16 (16)
$ trace-cmd record -e snd_pcm:hwptr -e snd_pcm:applptr $ trace-cmd report ... 60.208495: applptr: pcmC0D0p/sub0: prev=1792, curr=1792, avail=0, period=128, buf=256 60.208633: applptr: pcmC0D0p/sub0: prev=1792, curr=1792, avail=0, period=128, buf=256 60.210022: hwptr: pcmC0D0p/sub0: IRQ: pos=128, old=1536, base=1536, period=128, buf=256 60.210202: applptr: pcmC0D0p/sub0: prev=1792, curr=1792, avail=128, period=128, buf=256 60.210344: hwptr: pcmC0D0p/sub0: POS: pos=128, old=1664, base=1536, period=128, buf=256 60.210348: applptr: pcmC0D0p/sub0: prev=1792, curr=1792, avail=128, period=128, buf=256 60.210486: applptr: pcmC0D0p/sub0: prev=1792, curr=1792, avail=128, period=128, buf=256 60.210626: applptr: pcmC0D0p/sub0: prev=1792, curr=1920, avail=0, period=128, buf=256 60.211002: applptr: pcmC0D0p/sub0: prev=1920, curr=1920, avail=0, period=128, buf=256 60.211142: hwptr: pcmC0D0p/sub0: POS: pos=128, old=1664, base=1536, period=128, buf=256 60.211146: applptr: pcmC0D0p/sub0: prev=1920, curr=1920, avail=0, period=128, buf=256 60.211287: applptr: pcmC0D0p/sub0: prev=1920, curr=1920, avail=0, period=128, buf=256 60.212690: hwptr: pcmC0D0p/sub0: IRQ: pos=0, old=1664, base=1536, period=128, buf=256 60.212866: applptr: pcmC0D0p/sub0: prev=1920, curr=1920, avail=128, period=128, buf=256 60.212999: hwptr: pcmC0D0p/sub0: POS: pos=0, old=1792, base=1792, period=128, buf=256 60.213003: applptr: pcmC0D0p/sub0: prev=1920, curr=1920, avail=128, period=128, buf=256 60.213135: applptr: pcmC0D0p/sub0: prev=1920, curr=1920, avail=128, period=128, buf=256 60.213276: applptr: pcmC0D0p/sub0: prev=1920, curr=2048, avail=0, period=128, buf=256 60.213654: applptr: pcmC0D0p/sub0: prev=2048, curr=2048, avail=0, period=128, buf=256 60.213796: hwptr: pcmC0D0p/sub0: POS: pos=0, old=1792, base=1792, period=128, buf=256 60.213800: applptr: pcmC0D0p/sub0: prev=2048, curr=2048, avail=0, period=128, buf=256 60.213937: applptr: pcmC0D0p/sub0: prev=2048, curr=2048, avail=0, period=128, buf=256 60.215356: hwptr: pcmC0D0p/sub0: IRQ: pos=128, old=1792, base=1792, period=128, buf=256 60.215542: applptr: pcmC0D0p/sub0: prev=2048, curr=2048, avail=128, period=128, buf=256 60.215679: hwptr: pcmC0D0p/sub0: POS: pos=128, old=1920, base=1792, period=128, buf=256 60.215683: applptr: pcmC0D0p/sub0: prev=2048, curr=2048, avail=128, period=128, buf=256 60.215813: applptr: pcmC0D0p/sub0: prev=2048, curr=2048, avail=128, period=128, buf=256 60.215947: applptr: pcmC0D0p/sub0: prev=2048, curr=2176, avail=0, period=128, buf=256 ...
We can surely see 'applptr' event is probed even if the application run for mmap programming scenario ('-M' option for 'hw' plugin). Below is a result of strace:
02:44:15.886382 ioctl(4, SNDRV_PCM_IOCTL_SYNC_PTR, 0x56a32b30) = 0 02:44:15.887203 poll([{fd=4, events=POLLOUT|POLLERR|POLLNVAL}], 1, -1) = 1 ([{fd=4, revents=POLLOUT}]) 02:44:15.887471 ioctl(4, SNDRV_PCM_IOCTL_SYNC_PTR, 0x56a32b30) = 0 02:44:15.887637 ioctl(4, SNDRV_PCM_IOCTL_SYNC_PTR, 0x56a32b30) = 0 02:44:15.887805 ioctl(4, SNDRV_PCM_IOCTL_SYNC_PTR, 0x56a32b30) = 0 02:44:15.887969 ioctl(4, SNDRV_PCM_IOCTL_SYNC_PTR, 0x56a32b30) = 0 02:44:15.888132 read(3, "..."..., 256) = 256 02:44:15.889040 ioctl(4, SNDRV_PCM_IOCTL_SYNC_PTR, 0x56a32b30) = 0 02:44:15.889221 ioctl(4, SNDRV_PCM_IOCTL_SYNC_PTR, 0x56a32b30) = 0 02:44:15.889431 ioctl(4, SNDRV_PCM_IOCTL_SYNC_PTR, 0x56a32b30) = 0 02:44:15.889606 poll([{fd=4, events=POLLOUT|POLLERR|POLLNVAL}], 1, -1) = 1 ([{fd=4, revents=POLLOUT}]) 02:44:15.889833 ioctl(4, SNDRV_PCM_IOCTL_SYNC_PTR, 0x56a32b30) = 0 02:44:15.889998 ioctl(4, SNDRV_PCM_IOCTL_SYNC_PTR, 0x56a32b30) = 0 02:44:15.890164 ioctl(4, SNDRV_PCM_IOCTL_SYNC_PTR, 0x56a32b30) = 0 02:44:15.891048 ioctl(4, SNDRV_PCM_IOCTL_SYNC_PTR, 0x56a32b30) = 0 02:44:15.891228 read(3, "..."..., 256) = 256 02:44:15.891497 ioctl(4, SNDRV_PCM_IOCTL_SYNC_PTR, 0x56a32b30) = 0 02:44:15.891661 ioctl(4, SNDRV_PCM_IOCTL_SYNC_PTR, 0x56a32b30) = 0 02:44:15.891829 ioctl(4, SNDRV_PCM_IOCTL_SYNC_PTR, 0x56a32b30) = 0 02:44:15.891991 poll([{fd=4, events=POLLOUT|POLLERR|POLLNVAL}], 1, -1) = 1 ([{fd=4, revents=POLLOUT}]) 02:44:15.893007 ioctl(4, SNDRV_PCM_IOCTL_SYNC_PTR, 0x56a32b30) = 0
We can see 7 calls of ioctl(2) with SNDRV_PCM_IOCTL_SYNC_PTR per loop with call of poll(2). 128 PCM frames are transferred per loop of one poll(2), because the PCM substream is configured with S16_LE format and 1 channel (2 byte * 1 * 128 = 256 bytes). This equals to the size of period of PCM buffer. Comparing to the probed data, one of the 7 system calls is actually used to commit the number of copied PCM frames to kernel space.
The tracepoint event is useful to investigate this case.
I note that below modules are related to the above sample.
- snd-soc-dummy.ko
- snd-soc-imx-sgtl5000.ko
- snd-soc-fsl-ssi.ko
- snd-soc-imx-pcm-dma.ko
- snd-soc-sgtl5000.ko
This cover letter texts have much better explanation than the description in your patch itself. Why not putting such an excellent example in the change log?
thanks,
Takashi
On Jun 12 2017 02:03, Takashi Iwai wrote:
This cover letter texts have much better explanation than the description in your patch itself. Why not putting such an excellent example in the change log?
This report depends on my original research somehow. Source for this embedded board is from VIA Tech. Inc. and not mainlined yet. Furthermore, I prepared for backport patches by my self. Totally, it might not be reproductive as is entirely. This is a reason that I wrote the report in the cover letter.
However, I believe that the others can get the similar result on the other boards based on ARM architecture, because ALSA stuffs are written as they work what I report. In this meaning, it's worth to put the report into the change log.
I'm OK to post revised version.
Thanks
Takashi Sakamoto
participants (2)
-
Takashi Iwai -
Takashi Sakamoto