[alsa-devel] [PATCH alsa-lib 0/4] Add the support for explicit sync of appl_ptr
Hi,
this is a patchset for alsa-lib corresponding to the patchset I've sent for kernel. The first two patches do the actual fixes, while the last two are merely cleanups / optimizations.
thanks,
Takashi
===
Takashi Iwai (4): pcm: hw: Add the support for explicit sync of appl_ptr pcm: hw: Call USER_PVERSION ioctl at open pcm: hw: Remove superfluous call of snd_pcm_set_appl_ptr() pcm: hw: Reduce redundant sync_ptr() calls
include/sound/asound.h | 4 +++- src/pcm/pcm_hw.c | 47 ++++++++++++++++++++++++++++++++--------------- 2 files changed, 35 insertions(+), 16 deletions(-)
The recent kernel informs user-space when a driver requires the explicit notification of appl_ptr update via a new flag, SNDRV_PCM_INFO_SYNC_APPLPTR. When this flag is set, the user-space is supposed to report the appl_ptr via SNDRV_PCM_IOCTL_SYNC_PTR ioctl.
This patch implements the requested behavior in the PCM hw plugin.
Signed-off-by: Takashi Iwai tiwai@suse.de --- include/sound/asound.h | 1 + src/pcm/pcm_hw.c | 27 +++++++++++++++++++++------ 2 files changed, 22 insertions(+), 6 deletions(-)
diff --git a/include/sound/asound.h b/include/sound/asound.h index fb8d7d7ef8ad..346db40e5932 100644 --- a/include/sound/asound.h +++ b/include/sound/asound.h @@ -268,6 +268,7 @@ typedef int __bitwise snd_pcm_subformat_t; #define SNDRV_PCM_INFO_MMAP_VALID 0x00000002 /* period data are valid during transfer */ #define SNDRV_PCM_INFO_DOUBLE 0x00000004 /* Double buffering needed for PCM start/stop */ #define SNDRV_PCM_INFO_BATCH 0x00000010 /* double buffering */ +#define SNDRV_PCM_INFO_SYNC_APPLPTR 0x00000020 /* need the explicit sync of appl_ptr update */ #define SNDRV_PCM_INFO_INTERLEAVED 0x00000100 /* channels are interleaved */ #define SNDRV_PCM_INFO_NONINTERLEAVED 0x00000200 /* channels are not interleaved */ #define SNDRV_PCM_INFO_COMPLEX 0x00000400 /* complex frame organization (mmap only) */ diff --git a/src/pcm/pcm_hw.c b/src/pcm/pcm_hw.c index 30cd5d0f6503..8855868f5ea2 100644 --- a/src/pcm/pcm_hw.c +++ b/src/pcm/pcm_hw.c @@ -91,6 +91,7 @@ typedef struct { int fd; int card, device, subdevice; int sync_ptr_ioctl; + int sync_applptr; volatile struct snd_pcm_mmap_status * mmap_status; struct snd_pcm_mmap_control *mmap_control; struct snd_pcm_sync_ptr *sync_ptr; @@ -128,11 +129,13 @@ struct timespec snd_pcm_hw_fast_tstamp(snd_pcm_t *pcm) } #endif /* DOC_HIDDEN */
-static int sync_ptr1(snd_pcm_hw_t *hw, unsigned int flags) +static int sync_ptr1(snd_pcm_hw_t *hw, struct snd_pcm_sync_ptr *ptr, + unsigned int flags) { int err; - hw->sync_ptr->flags = flags; - err = ioctl((hw)->fd, SNDRV_PCM_IOCTL_SYNC_PTR, (hw)->sync_ptr); + + ptr->flags = flags; + err = ioctl(hw->fd, SNDRV_PCM_IOCTL_SYNC_PTR, ptr); if (err < 0) { err = -errno; SYSMSG("SNDRV_PCM_IOCTL_SYNC_PTR failed (%i)", err); @@ -143,7 +146,15 @@ static int sync_ptr1(snd_pcm_hw_t *hw, unsigned int flags)
static inline int sync_ptr(snd_pcm_hw_t *hw, unsigned int flags) { - return hw->sync_ptr ? sync_ptr1(hw, flags) : 0; + return hw->sync_ptr ? sync_ptr1(hw, hw->sync_ptr, flags) : 0; +} + +/* explicit notification of appl_ptr update to kernel */ +static int sync_applptr(snd_pcm_hw_t *hw) +{ + struct snd_pcm_sync_ptr ptr; + ptr.c.control = *hw->mmap_control; + return sync_ptr1(hw, &ptr, 0); }
static int snd_pcm_hw_clear_timer_queue(snd_pcm_hw_t *hw) @@ -327,6 +338,7 @@ static int snd_pcm_hw_hw_params(snd_pcm_t *pcm, snd_pcm_hw_params_t * params) SYSMSG("SNDRV_PCM_IOCTL_HW_PARAMS failed (%i)", err); return err; } + hw->sync_applptr = !!(params->info & SNDRV_PCM_INFO_SYNC_APPLPTR); params->info &= ~0xf0000000; if (pcm->tstamp_type != SND_PCM_TSTAMP_TYPE_GETTIMEOFDAY) params->info |= SND_PCM_INFO_MONOTONIC; @@ -568,7 +580,7 @@ static int snd_pcm_hw_hwsync(snd_pcm_t *pcm) int fd = hw->fd, err; if (SNDRV_PROTOCOL_VERSION(2, 0, 3) <= hw->version) { if (hw->sync_ptr) { - err = sync_ptr1(hw, SNDRV_PCM_SYNC_PTR_HWSYNC); + err = sync_ptr1(hw, hw->sync_ptr, SNDRV_PCM_SYNC_PTR_HWSYNC); if (err < 0) return err; } else { @@ -995,7 +1007,10 @@ static snd_pcm_sframes_t snd_pcm_hw_mmap_commit(snd_pcm_t *pcm, snd_pcm_hw_t *hw = pcm->private_data;
snd_pcm_mmap_appl_forward(pcm, size); - sync_ptr(hw, 0); + if (hw->sync_ptr) + sync_ptr(hw, 0); + else if (hw->sync_applptr) + sync_applptr(hw); #ifdef DEBUG_MMAP fprintf(stderr, "appl_forward: hw_ptr = %li, appl_ptr = %li, size = %li\n", *pcm->hw.ptr, *pcm->appl.ptr, size); #endif
Up from the new PCM protocol 2.0.14, user-space can inform the protocol version it supports to kernel, so that the kernel may switch its behavior depending on it. Add this ioctl call in the PCM hw plugin at opening.
Signed-off-by: Takashi Iwai tiwai@suse.de --- include/sound/asound.h | 3 ++- src/pcm/pcm_hw.c | 10 ++++++++++ 2 files changed, 12 insertions(+), 1 deletion(-)
diff --git a/include/sound/asound.h b/include/sound/asound.h index 346db40e5932..9ea2a638321c 100644 --- a/include/sound/asound.h +++ b/include/sound/asound.h @@ -152,7 +152,7 @@ struct snd_hwdep_dsp_image { * * *****************************************************************************/
-#define SNDRV_PCM_VERSION SNDRV_PROTOCOL_VERSION(2, 0, 13) +#define SNDRV_PCM_VERSION SNDRV_PROTOCOL_VERSION(2, 0, 14)
typedef unsigned long snd_pcm_uframes_t; typedef signed long snd_pcm_sframes_t; @@ -564,6 +564,7 @@ enum { #define SNDRV_PCM_IOCTL_INFO _IOR('A', 0x01, struct snd_pcm_info) #define SNDRV_PCM_IOCTL_TSTAMP _IOW('A', 0x02, int) #define SNDRV_PCM_IOCTL_TTSTAMP _IOW('A', 0x03, int) +#define SNDRV_PCM_IOCTL_USER_PVERSION _IOW('A', 0x04, int) #define SNDRV_PCM_IOCTL_HW_REFINE _IOWR('A', 0x10, struct snd_pcm_hw_params) #define SNDRV_PCM_IOCTL_HW_PARAMS _IOWR('A', 0x11, struct snd_pcm_hw_params) #define SNDRV_PCM_IOCTL_HW_FREE _IO('A', 0x12) diff --git a/src/pcm/pcm_hw.c b/src/pcm/pcm_hw.c index 8855868f5ea2..2775483f1933 100644 --- a/src/pcm/pcm_hw.c +++ b/src/pcm/pcm_hw.c @@ -1475,6 +1475,16 @@ int snd_pcm_hw_open_fd(snd_pcm_t **pcmp, const char *name, if (SNDRV_PROTOCOL_INCOMPATIBLE(ver, SNDRV_PCM_VERSION_MAX)) return -SND_ERROR_INCOMPATIBLE_VERSION;
+ if (SNDRV_PROTOCOL_VERSION(2, 0, 14) <= ver) { + /* inform the protocol version we're supporting */ + unsigned int user_ver = SNDRV_PCM_VERSION; + if (ioctl(fd, SNDRV_PCM_IOCTL_USER_PVERSION, &user_ver) < 0) { + ret = -errno; + SNDMSG("USER_PVERSION failed\n"); + return ret; + } + } + #if defined(HAVE_CLOCK_GETTIME) && defined(CLOCK_MONOTONIC) if (SNDRV_PROTOCOL_VERSION(2, 0, 9) <= ver) { struct timespec timespec;
There is a call of snd_pcm_set_appl_ptr() in snd_pcm_hw_hw_params() only for the capture direction. This must be a leftover from the ancient code. Although it's harmless for now, it's superfluous and confusing. Let's kill it.
Signed-off-by: Takashi Iwai tiwai@suse.de --- src/pcm/pcm_hw.c | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-)
diff --git a/src/pcm/pcm_hw.c b/src/pcm/pcm_hw.c index 2775483f1933..721b18d2bbbb 100644 --- a/src/pcm/pcm_hw.c +++ b/src/pcm/pcm_hw.c @@ -342,14 +342,7 @@ static int snd_pcm_hw_hw_params(snd_pcm_t *pcm, snd_pcm_hw_params_t * params) params->info &= ~0xf0000000; if (pcm->tstamp_type != SND_PCM_TSTAMP_TYPE_GETTIMEOFDAY) params->info |= SND_PCM_INFO_MONOTONIC; - err = sync_ptr(hw, 0); - if (err < 0) - return err; - if (pcm->stream == SND_PCM_STREAM_CAPTURE) { - snd_pcm_set_appl_ptr(pcm, &hw->mmap_control->appl_ptr, hw->fd, - SNDRV_PCM_MMAP_OFFSET_CONTROL); - } - return 0; + return sync_ptr(hw, 0); }
static void snd_pcm_hw_close_timer(snd_pcm_hw_t *hw)
There are lots of places calling sync_ptr() in the PCM hw plugin, but a few of them are redundant. For example, the sync_ptr() call after snd_pcm_hw_params() can be omitted, since the possible state change will be fetched again at snd_pcm_hw_state(), and there should be no hwptr/applptr changes by that.
This patch reduces such redundant sync_ptr() calls for the minor performance optimization. The other places calling sync_ptr(), especially the ones passing SNDRV_PCM_SYNC_PTR_APPL, are still likely necessary since they need to sync the status after the ioctl, so they are left as-is.
Signed-off-by: Takashi Iwai tiwai@suse.de --- src/pcm/pcm_hw.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/src/pcm/pcm_hw.c b/src/pcm/pcm_hw.c index 721b18d2bbbb..66658599a6a6 100644 --- a/src/pcm/pcm_hw.c +++ b/src/pcm/pcm_hw.c @@ -342,7 +342,7 @@ static int snd_pcm_hw_hw_params(snd_pcm_t *pcm, snd_pcm_hw_params_t * params) params->info &= ~0xf0000000; if (pcm->tstamp_type != SND_PCM_TSTAMP_TYPE_GETTIMEOFDAY) params->info |= SND_PCM_INFO_MONOTONIC; - return sync_ptr(hw, 0); + return 0; }
static void snd_pcm_hw_close_timer(snd_pcm_hw_t *hw) @@ -608,7 +608,7 @@ static int snd_pcm_hw_prepare(snd_pcm_t *pcm) SYSMSG("SNDRV_PCM_IOCTL_PREPARE failed (%i)", err); return err; } - return sync_ptr(hw, SNDRV_PCM_SYNC_PTR_APPL); + return 0; }
static int snd_pcm_hw_reset(snd_pcm_t *pcm) @@ -631,7 +631,6 @@ static int snd_pcm_hw_start(snd_pcm_t *pcm) assert(pcm->stream != SND_PCM_STREAM_PLAYBACK || snd_pcm_mmap_playback_hw_avail(pcm) > 0); #endif - sync_ptr(hw, 0); if (ioctl(hw->fd, SNDRV_PCM_IOCTL_START) < 0) { err = -errno; SYSMSG("SNDRV_PCM_IOCTL_START failed (%i)", err);
Hi,
On Jun 21 2017 00:35, Takashi Iwai wrote:
Hi,
this is a patchset for alsa-lib corresponding to the patchset I've sent for kernel. The first two patches do the actual fixes, while the last two are merely cleanups / optimizations.
thanks,
Takashi
===
Takashi Iwai (4): pcm: hw: Add the support for explicit sync of appl_ptr pcm: hw: Call USER_PVERSION ioctl at open pcm: hw: Remove superfluous call of snd_pcm_set_appl_ptr() pcm: hw: Reduce redundant sync_ptr() calls
include/sound/asound.h | 4 +++- src/pcm/pcm_hw.c | 47 ++++++++++++++++++++++++++++++++--------------- 2 files changed, 35 insertions(+), 16 deletions(-)
I support this patchset.
Reviewed-by: Takashi Sakamoto o-takashi@sakamocchi.jp Tested-by: Takashi Sakamoto o-takashi@sakamocchi.jp
I did quick test with my devices and patched driver, running with kernel in 'topic/pcm-sync-applptr' branch[1]. I prepare for below four cases with combinations between ALSA fireworks driver and alsa-lib:
- driver with/without the flag - alsa-lib with/without the patch
As long as checking logs from trace-cmd and strace, I got expected results in all of these cases.
[1] https://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound.git/log/?h=topic...
Thanks
Takashi Sakamoto
participants (2)
-
Takashi Iwai -
Takashi Sakamoto