From 8b674308a26a774896652dafe0dac7ee297eed9b Mon Sep 17 00:00:00 2001 From: Takashi Sakamoto Date: Wed, 17 May 2017 08:48:17 +0900 Subject: ALSA: pcm: add const qualifier for read-only table for sampling rate There's a read-only table for each sampling rate, while it doesn't have const qualifier and can be modified. This commit add the qualifier. As a result, a symbol for the table moves from .data section to .rodata. Signed-off-by: Takashi Sakamoto Signed-off-by: Takashi Iwai --- sound/core/pcm_native.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) (limited to 'sound/core/pcm_native.c') diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c index 13dec5ec93f2..d5a967b57bb4 100644 --- a/sound/core/pcm_native.c +++ b/sound/core/pcm_native.c @@ -1986,8 +1986,10 @@ static int snd_pcm_hw_rule_sample_bits(struct snd_pcm_hw_params *params, #error "Change this table" #endif -static unsigned int rates[] = { 5512, 8000, 11025, 16000, 22050, 32000, 44100, - 48000, 64000, 88200, 96000, 176400, 192000 }; +static const unsigned int rates[] = { + 5512, 8000, 11025, 16000, 22050, 32000, 44100, + 48000, 64000, 88200, 96000, 176400, 192000 +}; const struct snd_pcm_hw_constraint_list snd_pcm_known_rates = { .count = ARRAY_SIZE(rates), -- cgit From b55f9fdcd3f0b3da7c9d4b6c67d75a1878653221 Mon Sep 17 00:00:00 2001 From: Takashi Sakamoto Date: Wed, 17 May 2017 08:48:20 +0900 Subject: ALSA: pcm: use helper function to refer parameter as read-only ALSA pcm core has hw_param_interval_c() to pick up parameter with const qualifier for safe programming. This commit applies it to the cases. Signed-off-by: Takashi Sakamoto Signed-off-by: Takashi Iwai --- sound/core/pcm_native.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'sound/core/pcm_native.c') diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c index d5a967b57bb4..f3a3580eb44c 100644 --- a/sound/core/pcm_native.c +++ b/sound/core/pcm_native.c @@ -1940,7 +1940,8 @@ static int snd_pcm_hw_rule_format(struct snd_pcm_hw_params *params, struct snd_pcm_hw_rule *rule) { unsigned int k; - struct snd_interval *i = hw_param_interval(params, rule->deps[0]); + const struct snd_interval *i = + hw_param_interval_c(params, rule->deps[0]); struct snd_mask m; struct snd_mask *mask = hw_param_mask(params, SNDRV_PCM_HW_PARAM_FORMAT); snd_mask_any(&m); -- cgit From f839cc1cbd29f2160ef4e621b920e254cc84133a Mon Sep 17 00:00:00 2001 From: Takashi Iwai Date: Fri, 19 May 2017 20:04:23 +0200 Subject: ALSA: pcm: Use a common helper for PCM state check and hwsync The mostly same codes for checking the current PCM state and calling hwsync are found in a few places. This patch simplifies them by creating a common helper function. It also fixes a couple of cases where we missed the proper state check (e.g. PAUSED state wasn't handled in rewind and snd_pcm_hwsync()), too. Reviewed-by: Takashi Sakamoto Signed-off-by: Takashi Iwai --- sound/core/pcm_native.c | 153 +++++++++++------------------------------------- 1 file changed, 35 insertions(+), 118 deletions(-) (limited to 'sound/core/pcm_native.c') diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c index f3a3580eb44c..93bd2c662c1d 100644 --- a/sound/core/pcm_native.c +++ b/sound/core/pcm_native.c @@ -2431,6 +2431,30 @@ static int snd_pcm_release(struct inode *inode, struct file *file) return 0; } +/* check and update PCM state; return 0 or a negative error + * call this inside PCM lock + */ +static int do_pcm_hwsync(struct snd_pcm_substream *substream) +{ + switch (substream->runtime->status->state) { + case SNDRV_PCM_STATE_DRAINING: + if (substream->stream == SNDRV_PCM_STREAM_CAPTURE) + return -EBADFD; + /* Fall through */ + case SNDRV_PCM_STATE_RUNNING: + return snd_pcm_update_hw_ptr(substream); + case SNDRV_PCM_STATE_PREPARED: + case SNDRV_PCM_STATE_PAUSED: + return 0; + case SNDRV_PCM_STATE_SUSPENDED: + return -ESTRPIPE; + case SNDRV_PCM_STATE_XRUN: + return -EPIPE; + default: + return -EBADFD; + } +} + static snd_pcm_sframes_t snd_pcm_playback_rewind(struct snd_pcm_substream *substream, snd_pcm_uframes_t frames) { @@ -2443,25 +2467,9 @@ static snd_pcm_sframes_t snd_pcm_playback_rewind(struct snd_pcm_substream *subst return 0; snd_pcm_stream_lock_irq(substream); - switch (runtime->status->state) { - case SNDRV_PCM_STATE_PREPARED: - break; - case SNDRV_PCM_STATE_DRAINING: - case SNDRV_PCM_STATE_RUNNING: - if (snd_pcm_update_hw_ptr(substream) >= 0) - break; - /* Fall through */ - case SNDRV_PCM_STATE_XRUN: - ret = -EPIPE; - goto __end; - case SNDRV_PCM_STATE_SUSPENDED: - ret = -ESTRPIPE; - goto __end; - default: - ret = -EBADFD; + ret = do_pcm_hwsync(substream); + if (ret < 0) goto __end; - } - hw_avail = snd_pcm_playback_hw_avail(runtime); if (hw_avail <= 0) { ret = 0; @@ -2491,25 +2499,9 @@ static snd_pcm_sframes_t snd_pcm_capture_rewind(struct snd_pcm_substream *substr return 0; snd_pcm_stream_lock_irq(substream); - switch (runtime->status->state) { - case SNDRV_PCM_STATE_PREPARED: - case SNDRV_PCM_STATE_DRAINING: - break; - case SNDRV_PCM_STATE_RUNNING: - if (snd_pcm_update_hw_ptr(substream) >= 0) - break; - /* Fall through */ - case SNDRV_PCM_STATE_XRUN: - ret = -EPIPE; + ret = do_pcm_hwsync(substream); + if (ret < 0) goto __end; - case SNDRV_PCM_STATE_SUSPENDED: - ret = -ESTRPIPE; - goto __end; - default: - ret = -EBADFD; - goto __end; - } - hw_avail = snd_pcm_capture_hw_avail(runtime); if (hw_avail <= 0) { ret = 0; @@ -2539,26 +2531,9 @@ static snd_pcm_sframes_t snd_pcm_playback_forward(struct snd_pcm_substream *subs return 0; snd_pcm_stream_lock_irq(substream); - switch (runtime->status->state) { - case SNDRV_PCM_STATE_PREPARED: - case SNDRV_PCM_STATE_PAUSED: - break; - case SNDRV_PCM_STATE_DRAINING: - case SNDRV_PCM_STATE_RUNNING: - if (snd_pcm_update_hw_ptr(substream) >= 0) - break; - /* Fall through */ - case SNDRV_PCM_STATE_XRUN: - ret = -EPIPE; - goto __end; - case SNDRV_PCM_STATE_SUSPENDED: - ret = -ESTRPIPE; + ret = do_pcm_hwsync(substream); + if (ret < 0) goto __end; - default: - ret = -EBADFD; - goto __end; - } - avail = snd_pcm_playback_avail(runtime); if (avail <= 0) { ret = 0; @@ -2588,26 +2563,9 @@ static snd_pcm_sframes_t snd_pcm_capture_forward(struct snd_pcm_substream *subst return 0; snd_pcm_stream_lock_irq(substream); - switch (runtime->status->state) { - case SNDRV_PCM_STATE_PREPARED: - case SNDRV_PCM_STATE_DRAINING: - case SNDRV_PCM_STATE_PAUSED: - break; - case SNDRV_PCM_STATE_RUNNING: - if (snd_pcm_update_hw_ptr(substream) >= 0) - break; - /* Fall through */ - case SNDRV_PCM_STATE_XRUN: - ret = -EPIPE; - goto __end; - case SNDRV_PCM_STATE_SUSPENDED: - ret = -ESTRPIPE; + ret = do_pcm_hwsync(substream); + if (ret < 0) goto __end; - default: - ret = -EBADFD; - goto __end; - } - avail = snd_pcm_capture_avail(runtime); if (avail <= 0) { ret = 0; @@ -2627,33 +2585,10 @@ static snd_pcm_sframes_t snd_pcm_capture_forward(struct snd_pcm_substream *subst static int snd_pcm_hwsync(struct snd_pcm_substream *substream) { - struct snd_pcm_runtime *runtime = substream->runtime; int err; snd_pcm_stream_lock_irq(substream); - switch (runtime->status->state) { - case SNDRV_PCM_STATE_DRAINING: - if (substream->stream == SNDRV_PCM_STREAM_CAPTURE) - goto __badfd; - /* Fall through */ - case SNDRV_PCM_STATE_RUNNING: - if ((err = snd_pcm_update_hw_ptr(substream)) < 0) - break; - /* Fall through */ - case SNDRV_PCM_STATE_PREPARED: - err = 0; - break; - case SNDRV_PCM_STATE_SUSPENDED: - err = -ESTRPIPE; - break; - case SNDRV_PCM_STATE_XRUN: - err = -EPIPE; - break; - default: - __badfd: - err = -EBADFD; - break; - } + err = do_pcm_hwsync(substream); snd_pcm_stream_unlock_irq(substream); return err; } @@ -2666,31 +2601,13 @@ static int snd_pcm_delay(struct snd_pcm_substream *substream, snd_pcm_sframes_t n = 0; snd_pcm_stream_lock_irq(substream); - switch (runtime->status->state) { - case SNDRV_PCM_STATE_DRAINING: - if (substream->stream == SNDRV_PCM_STREAM_CAPTURE) - goto __badfd; - /* Fall through */ - case SNDRV_PCM_STATE_RUNNING: - if ((err = snd_pcm_update_hw_ptr(substream)) < 0) - break; - /* Fall through */ - case SNDRV_PCM_STATE_PREPARED: - case SNDRV_PCM_STATE_SUSPENDED: - err = 0; + err = do_pcm_hwsync(substream); + if (!err) { if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) n = snd_pcm_playback_hw_avail(runtime); else n = snd_pcm_capture_avail(runtime); n += runtime->delay; - break; - case SNDRV_PCM_STATE_XRUN: - err = -EPIPE; - break; - default: - __badfd: - err = -EBADFD; - break; } snd_pcm_stream_unlock_irq(substream); if (!err) -- cgit From e0327a0f214154b517fa2b325acd8d42736ac95b Mon Sep 17 00:00:00 2001 From: Takashi Iwai Date: Fri, 19 May 2017 20:16:44 +0200 Subject: ALSA: pcm: Simplify forward/rewind codes Factor out the common codes in snd_pcm_*_forward() and *_rewind() functions to simplify the codes. No functional changes. Reviewd-by: Takashi Sakamoto Signed-off-by: Takashi Iwai --- sound/core/pcm_native.c | 118 ++++++++++++++++++++---------------------------- 1 file changed, 50 insertions(+), 68 deletions(-) (limited to 'sound/core/pcm_native.c') diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c index 93bd2c662c1d..ecde57afa45a 100644 --- a/sound/core/pcm_native.c +++ b/sound/core/pcm_native.c @@ -2455,34 +2455,58 @@ static int do_pcm_hwsync(struct snd_pcm_substream *substream) } } +/* increase the appl_ptr; returns the processed frames */ +static snd_pcm_sframes_t forward_appl_ptr(struct snd_pcm_substream *substream, + snd_pcm_uframes_t frames, + snd_pcm_sframes_t avail) +{ + struct snd_pcm_runtime *runtime = substream->runtime; + snd_pcm_sframes_t appl_ptr; + + if (avail <= 0) + return 0; + if (frames > (snd_pcm_uframes_t)avail) + frames = avail; + appl_ptr = runtime->control->appl_ptr + frames; + if (appl_ptr >= (snd_pcm_sframes_t)runtime->boundary) + appl_ptr -= runtime->boundary; + runtime->control->appl_ptr = appl_ptr; + return frames; +} + +/* decrease the appl_ptr; returns the processed frames */ +static snd_pcm_sframes_t rewind_appl_ptr(struct snd_pcm_substream *substream, + snd_pcm_uframes_t frames, + snd_pcm_sframes_t avail) +{ + struct snd_pcm_runtime *runtime = substream->runtime; + snd_pcm_sframes_t appl_ptr; + + if (avail <= 0) + return 0; + if (frames > (snd_pcm_uframes_t)avail) + frames = avail; + appl_ptr = runtime->control->appl_ptr - frames; + if (appl_ptr < 0) + appl_ptr += runtime->boundary; + runtime->control->appl_ptr = appl_ptr; + return frames; +} + static snd_pcm_sframes_t snd_pcm_playback_rewind(struct snd_pcm_substream *substream, snd_pcm_uframes_t frames) { struct snd_pcm_runtime *runtime = substream->runtime; - snd_pcm_sframes_t appl_ptr; snd_pcm_sframes_t ret; - snd_pcm_sframes_t hw_avail; if (frames == 0) return 0; snd_pcm_stream_lock_irq(substream); ret = do_pcm_hwsync(substream); - if (ret < 0) - goto __end; - hw_avail = snd_pcm_playback_hw_avail(runtime); - if (hw_avail <= 0) { - ret = 0; - goto __end; - } - if (frames > (snd_pcm_uframes_t)hw_avail) - frames = hw_avail; - appl_ptr = runtime->control->appl_ptr - frames; - if (appl_ptr < 0) - appl_ptr += runtime->boundary; - runtime->control->appl_ptr = appl_ptr; - ret = frames; - __end: + if (!ret) + ret = rewind_appl_ptr(substream, frames, + snd_pcm_playback_hw_avail(runtime)); snd_pcm_stream_unlock_irq(substream); return ret; } @@ -2491,30 +2515,16 @@ static snd_pcm_sframes_t snd_pcm_capture_rewind(struct snd_pcm_substream *substr snd_pcm_uframes_t frames) { struct snd_pcm_runtime *runtime = substream->runtime; - snd_pcm_sframes_t appl_ptr; snd_pcm_sframes_t ret; - snd_pcm_sframes_t hw_avail; if (frames == 0) return 0; snd_pcm_stream_lock_irq(substream); ret = do_pcm_hwsync(substream); - if (ret < 0) - goto __end; - hw_avail = snd_pcm_capture_hw_avail(runtime); - if (hw_avail <= 0) { - ret = 0; - goto __end; - } - if (frames > (snd_pcm_uframes_t)hw_avail) - frames = hw_avail; - appl_ptr = runtime->control->appl_ptr - frames; - if (appl_ptr < 0) - appl_ptr += runtime->boundary; - runtime->control->appl_ptr = appl_ptr; - ret = frames; - __end: + if (!ret) + ret = rewind_appl_ptr(substream, frames, + snd_pcm_capture_hw_avail(runtime)); snd_pcm_stream_unlock_irq(substream); return ret; } @@ -2523,30 +2533,16 @@ static snd_pcm_sframes_t snd_pcm_playback_forward(struct snd_pcm_substream *subs snd_pcm_uframes_t frames) { struct snd_pcm_runtime *runtime = substream->runtime; - snd_pcm_sframes_t appl_ptr; snd_pcm_sframes_t ret; - snd_pcm_sframes_t avail; if (frames == 0) return 0; snd_pcm_stream_lock_irq(substream); ret = do_pcm_hwsync(substream); - if (ret < 0) - goto __end; - avail = snd_pcm_playback_avail(runtime); - if (avail <= 0) { - ret = 0; - goto __end; - } - if (frames > (snd_pcm_uframes_t)avail) - frames = avail; - appl_ptr = runtime->control->appl_ptr + frames; - if (appl_ptr >= (snd_pcm_sframes_t)runtime->boundary) - appl_ptr -= runtime->boundary; - runtime->control->appl_ptr = appl_ptr; - ret = frames; - __end: + if (!ret) + ret = forward_appl_ptr(substream, frames, + snd_pcm_playback_avail(runtime)); snd_pcm_stream_unlock_irq(substream); return ret; } @@ -2555,30 +2551,16 @@ static snd_pcm_sframes_t snd_pcm_capture_forward(struct snd_pcm_substream *subst snd_pcm_uframes_t frames) { struct snd_pcm_runtime *runtime = substream->runtime; - snd_pcm_sframes_t appl_ptr; snd_pcm_sframes_t ret; - snd_pcm_sframes_t avail; if (frames == 0) return 0; snd_pcm_stream_lock_irq(substream); ret = do_pcm_hwsync(substream); - if (ret < 0) - goto __end; - avail = snd_pcm_capture_avail(runtime); - if (avail <= 0) { - ret = 0; - goto __end; - } - if (frames > (snd_pcm_uframes_t)avail) - frames = avail; - appl_ptr = runtime->control->appl_ptr + frames; - if (appl_ptr >= (snd_pcm_sframes_t)runtime->boundary) - appl_ptr -= runtime->boundary; - runtime->control->appl_ptr = appl_ptr; - ret = frames; - __end: + if (!ret) + ret = forward_appl_ptr(substream, frames, + snd_pcm_capture_avail(runtime)); snd_pcm_stream_unlock_irq(substream); return ret; } -- cgit From c2c86a97175f552fd32b339426a489c7af818123 Mon Sep 17 00:00:00 2001 From: Takashi Iwai Date: Wed, 10 May 2017 14:33:44 +0200 Subject: ALSA: pcm: Remove set_fs() in PCM core code PCM core code has a few usages of set_fs(), mostly for two codepaths: - The DELAY ioctl call from pcm_compat.c - The ioctl wrapper in kernel context for PCM OSS and other This patch removes the set_fs() usage in these places by a slight code refactoring. For the former point, snd_pcm_delay() is changed to return the value directly instead of putting the value to the given address. Each caller stores the result in an appropriate manner. For fixing the latter, snd_pcm_lib_kernel_ioctl() is changed to call the functions directly as well. For achieving it, now the function accepts only the limited set of ioctls that have been used, so far. The primary user of this function is the PCM OSS layer, and the only other user is USB UAC1 gadget driver. Both drivers don't need the full set of ioctls. Reviewed-by: Takashi Sakamoto Signed-off-by: Takashi Iwai --- sound/core/pcm_native.c | 102 ++++++++++++++++++++++++++++++------------------ 1 file changed, 63 insertions(+), 39 deletions(-) (limited to 'sound/core/pcm_native.c') diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c index ecde57afa45a..889364cbced8 100644 --- a/sound/core/pcm_native.c +++ b/sound/core/pcm_native.c @@ -181,20 +181,6 @@ void snd_pcm_stream_unlock_irqrestore(struct snd_pcm_substream *substream, } EXPORT_SYMBOL_GPL(snd_pcm_stream_unlock_irqrestore); -static inline mm_segment_t snd_enter_user(void) -{ - mm_segment_t fs = get_fs(); - set_fs(get_ds()); - return fs; -} - -static inline void snd_leave_user(mm_segment_t fs) -{ - set_fs(fs); -} - - - int snd_pcm_info(struct snd_pcm_substream *substream, struct snd_pcm_info *info) { struct snd_pcm_runtime *runtime; @@ -1081,6 +1067,7 @@ static const struct action_ops snd_pcm_action_start = { * @substream: the PCM substream instance * * Return: Zero if successful, or a negative error code. + * The stream lock must be acquired before calling this function. */ int snd_pcm_start(struct snd_pcm_substream *substream) { @@ -1088,6 +1075,13 @@ int snd_pcm_start(struct snd_pcm_substream *substream) SNDRV_PCM_STATE_RUNNING); } +/* take the stream lock and start the streams */ +static int snd_pcm_start_lock_irq(struct snd_pcm_substream *substream) +{ + return snd_pcm_action_lock_irq(&snd_pcm_action_start, substream, + SNDRV_PCM_STATE_RUNNING); +} + /* * stop callbacks */ @@ -2575,8 +2569,7 @@ static int snd_pcm_hwsync(struct snd_pcm_substream *substream) return err; } -static int snd_pcm_delay(struct snd_pcm_substream *substream, - snd_pcm_sframes_t __user *res) +static snd_pcm_sframes_t snd_pcm_delay(struct snd_pcm_substream *substream) { struct snd_pcm_runtime *runtime = substream->runtime; int err; @@ -2592,10 +2585,7 @@ static int snd_pcm_delay(struct snd_pcm_substream *substream, n += runtime->delay; } snd_pcm_stream_unlock_irq(substream); - if (!err) - if (put_user(n, res)) - err = -EFAULT; - return err; + return err < 0 ? err : n; } static int snd_pcm_sync_ptr(struct snd_pcm_substream *substream, @@ -2683,7 +2673,7 @@ static int snd_pcm_common_ioctl1(struct file *file, case SNDRV_PCM_IOCTL_RESET: return snd_pcm_reset(substream); case SNDRV_PCM_IOCTL_START: - return snd_pcm_action_lock_irq(&snd_pcm_action_start, substream, SNDRV_PCM_STATE_RUNNING); + return snd_pcm_start_lock_irq(substream); case SNDRV_PCM_IOCTL_LINK: return snd_pcm_link(substream, (int)(unsigned long) arg); case SNDRV_PCM_IOCTL_UNLINK: @@ -2695,7 +2685,16 @@ static int snd_pcm_common_ioctl1(struct file *file, case SNDRV_PCM_IOCTL_HWSYNC: return snd_pcm_hwsync(substream); case SNDRV_PCM_IOCTL_DELAY: - return snd_pcm_delay(substream, arg); + { + snd_pcm_sframes_t delay = snd_pcm_delay(substream); + snd_pcm_sframes_t __user *res = arg; + + if (delay < 0) + return delay; + if (put_user(delay, res)) + return -EFAULT; + return 0; + } case SNDRV_PCM_IOCTL_SYNC_PTR: return snd_pcm_sync_ptr(substream, arg); #ifdef CONFIG_SND_SUPPORT_OLD_API @@ -2909,30 +2908,55 @@ static long snd_pcm_capture_ioctl(struct file *file, unsigned int cmd, (void __user *)arg); } +/** + * snd_pcm_kernel_ioctl - Execute PCM ioctl in the kernel-space + * @substream: PCM substream + * @cmd: IOCTL cmd + * @arg: IOCTL argument + * + * The function is provided primarily for OSS layer and USB gadget drivers, + * and it allows only the limited set of ioctls (hw_params, sw_params, + * prepare, start, drain, drop, forward). + */ int snd_pcm_kernel_ioctl(struct snd_pcm_substream *substream, unsigned int cmd, void *arg) { - mm_segment_t fs; - int result; + snd_pcm_uframes_t *frames = arg; + snd_pcm_sframes_t result; - fs = snd_enter_user(); - switch (substream->stream) { - case SNDRV_PCM_STREAM_PLAYBACK: - result = snd_pcm_playback_ioctl1(NULL, substream, cmd, - (void __user *)arg); - break; - case SNDRV_PCM_STREAM_CAPTURE: - result = snd_pcm_capture_ioctl1(NULL, substream, cmd, - (void __user *)arg); - break; + switch (cmd) { + case SNDRV_PCM_IOCTL_FORWARD: + { + /* provided only for OSS; capture-only and no value returned */ + if (substream->stream != SNDRV_PCM_STREAM_CAPTURE) + return -EINVAL; + result = snd_pcm_capture_forward(substream, *frames); + return result < 0 ? result : 0; + } + case SNDRV_PCM_IOCTL_HW_PARAMS: + return snd_pcm_hw_params(substream, arg); + case SNDRV_PCM_IOCTL_SW_PARAMS: + return snd_pcm_sw_params(substream, arg); + case SNDRV_PCM_IOCTL_PREPARE: + return snd_pcm_prepare(substream, NULL); + case SNDRV_PCM_IOCTL_START: + return snd_pcm_start_lock_irq(substream); + case SNDRV_PCM_IOCTL_DRAIN: + return snd_pcm_drain(substream, NULL); + case SNDRV_PCM_IOCTL_DROP: + return snd_pcm_drop(substream); + case SNDRV_PCM_IOCTL_DELAY: + { + result = snd_pcm_delay(substream); + if (result < 0) + return result; + *frames = result; + return 0; + } default: - result = -EINVAL; - break; + return -EINVAL; } - snd_leave_user(fs); - return result; } - EXPORT_SYMBOL(snd_pcm_kernel_ioctl); static ssize_t snd_pcm_read(struct file *file, char __user *buf, size_t count, -- cgit From 9027c4639ef1e3254779e3033f229133222445f7 Mon Sep 17 00:00:00 2001 From: Takashi Iwai Date: Fri, 19 May 2017 20:22:33 +0200 Subject: ALSA: pcm: Call ack() whenever appl_ptr is updated Although the ack callback is supposed to be called at each appl_ptr or hw_ptr update, we missed a few opportunities: namely, forward, rewind and sync_ptr. Formerly calling ack at rewind may have leaded to unexpected results due to the forgotten negative appl_ptr update in indirect-PCM helper, which is the major user of the PCM ack callback. But now we fixed this oversights, thus we can call ack callback safely even at rewind callback -- of course with the proper handling of the error from the callback. This patch adds the calls of ack callback in the places mentioned in the above. Signed-off-by: Takashi Iwai --- sound/core/pcm_native.c | 46 +++++++++++++++++++++++++++++++++++++--------- 1 file changed, 37 insertions(+), 9 deletions(-) (limited to 'sound/core/pcm_native.c') diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c index 889364cbced8..5be549cf91e5 100644 --- a/sound/core/pcm_native.c +++ b/sound/core/pcm_native.c @@ -2449,13 +2449,35 @@ static int do_pcm_hwsync(struct snd_pcm_substream *substream) } } -/* increase the appl_ptr; returns the processed frames */ +/* 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, snd_pcm_sframes_t avail) { struct snd_pcm_runtime *runtime = substream->runtime; snd_pcm_sframes_t appl_ptr; + int ret; if (avail <= 0) return 0; @@ -2464,17 +2486,18 @@ 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; - runtime->control->appl_ptr = appl_ptr; - return frames; + ret = apply_appl_ptr(substream, appl_ptr); + return ret < 0 ? ret : frames; } -/* decrease the appl_ptr; returns the processed frames */ +/* decrease the appl_ptr; returns the processed frames or a negative error */ static snd_pcm_sframes_t rewind_appl_ptr(struct snd_pcm_substream *substream, snd_pcm_uframes_t frames, snd_pcm_sframes_t avail) { struct snd_pcm_runtime *runtime = substream->runtime; snd_pcm_sframes_t appl_ptr; + int ret; if (avail <= 0) return 0; @@ -2483,8 +2506,8 @@ 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; - runtime->control->appl_ptr = appl_ptr; - return frames; + ret = apply_appl_ptr(substream, appl_ptr); + return ret < 0 ? ret : frames; } static snd_pcm_sframes_t snd_pcm_playback_rewind(struct snd_pcm_substream *substream, @@ -2610,10 +2633,15 @@ static int snd_pcm_sync_ptr(struct snd_pcm_substream *substream, return err; } snd_pcm_stream_lock_irq(substream); - if (!(sync_ptr.flags & SNDRV_PCM_SYNC_PTR_APPL)) - control->appl_ptr = sync_ptr.c.control.appl_ptr; - else + if (!(sync_ptr.flags & SNDRV_PCM_SYNC_PTR_APPL)) { + err = apply_appl_ptr(substream, sync_ptr.c.control.appl_ptr); + if (err < 0) { + snd_pcm_stream_unlock_irq(substream); + return err; + } + } else { sync_ptr.c.control.appl_ptr = control->appl_ptr; + } if (!(sync_ptr.flags & SNDRV_PCM_SYNC_PTR_AVAIL_MIN)) control->avail_min = sync_ptr.c.control.avail_min; else -- cgit From 2c4842d3b6b3cf6db0f21e487da7e9bd3aa23090 Mon Sep 17 00:00:00 2001 From: Takashi Sakamoto Date: Fri, 26 May 2017 09:30:46 +0900 Subject: ALSA: pcm: add local header file for snd-pcm module Several files are used to construct PCM core module, a.k.a snd-pcm. Although available APIs are described in 'include/sound/pcm.h', some of them are not exported as symbols in kernel space. Such APIs are just for module local usage. This commit adds module local header file and move some function prototypes into it so that scopes of them are controlled properly and developers get no confusion from unavailable symbols. Signed-off-by: Takashi Sakamoto Signed-off-by: Takashi Iwai --- sound/core/pcm_native.c | 2 ++ 1 file changed, 2 insertions(+) (limited to 'sound/core/pcm_native.c') diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c index 5be549cf91e5..bf5d0f2acfb9 100644 --- a/sound/core/pcm_native.c +++ b/sound/core/pcm_native.c @@ -37,6 +37,8 @@ #include #include +#include "pcm_local.h" + /* * Compatibility */ -- cgit From be4e31dab0e14c1f6fa5c03b33056058b93316e2 Mon Sep 17 00:00:00 2001 From: Takashi Sakamoto Date: Wed, 7 Jun 2017 08:46:43 +0900 Subject: ALSA: pcm: tracepoints for refining PCM parameters When working for devices which support configurable modes for its data transmission or which consists of several components, developers are likely to use rules of parameters of PCM substream. However, there's no infrastructure to assist their work. In old days, ALSA PCM core got a local 'RULES_DEBUG' macro to debug refinement of parameters for PCM substream. Although this is merely a makeshift. With some modifications, we get the infrastructure. This commit is for the purpose. Refinement of mask/interval type of PCM parameters is probed as tracepoint events as 'hw_mask_param' and 'hw_interval_param' on existent 'snd_pcm' subsystem. Signed-off-by: Takashi Sakamoto Signed-off-by: Takashi Iwai --- sound/core/pcm_native.c | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) (limited to 'sound/core/pcm_native.c') diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c index bf5d0f2acfb9..b98b3ccde4f0 100644 --- a/sound/core/pcm_native.c +++ b/sound/core/pcm_native.c @@ -39,6 +39,9 @@ #include "pcm_local.h" +#define CREATE_TRACE_POINTS +#include "pcm_param_trace.h" + /* * Compatibility */ @@ -279,6 +282,9 @@ int snd_pcm_hw_refine(struct snd_pcm_substream *substream, unsigned int stamp = 2; int changed, again; + struct snd_mask __maybe_unused old_mask; + struct snd_interval __maybe_unused old_interval; + params->info = 0; params->fifo_size = 0; if (params->rmask & (1 << SNDRV_PCM_HW_PARAM_SAMPLE_BITS)) @@ -294,6 +300,9 @@ int snd_pcm_hw_refine(struct snd_pcm_substream *substream, return -EINVAL; if (!(params->rmask & (1 << k))) continue; + + if (trace_hw_mask_param_enabled()) + old_mask = *m; #ifdef RULES_DEBUG pr_debug("%s = ", snd_pcm_hw_param_names[k]); pr_cont("%04x%04x%04x%04x -> ", m->bits[3], m->bits[2], m->bits[1], m->bits[0]); @@ -302,6 +311,8 @@ int snd_pcm_hw_refine(struct snd_pcm_substream *substream, #ifdef RULES_DEBUG pr_cont("%04x%04x%04x%04x\n", m->bits[3], m->bits[2], m->bits[1], m->bits[0]); #endif + trace_hw_mask_param(substream, k, 0, &old_mask, m); + if (changed) params->cmask |= 1 << k; if (changed < 0) @@ -314,6 +325,9 @@ int snd_pcm_hw_refine(struct snd_pcm_substream *substream, return -EINVAL; if (!(params->rmask & (1 << k))) continue; + + if (trace_hw_interval_param_enabled()) + old_interval = *i; #ifdef RULES_DEBUG pr_debug("%s = ", snd_pcm_hw_param_names[k]); if (i->empty) @@ -333,6 +347,8 @@ int snd_pcm_hw_refine(struct snd_pcm_substream *substream, i->openmin ? '(' : '[', i->min, i->max, i->openmax ? ')' : ']'); #endif + trace_hw_interval_param(substream, k, 0, &old_interval, i); + if (changed) params->cmask |= 1 << k; if (changed < 0) @@ -359,6 +375,15 @@ int snd_pcm_hw_refine(struct snd_pcm_substream *substream, } if (!doit) continue; + + if (trace_hw_mask_param_enabled()) { + if (hw_is_mask(r->var)) + old_mask = *hw_param_mask(params, r->var); + } + if (trace_hw_interval_param_enabled()) { + if (hw_is_interval(r->var)) + old_interval = *hw_param_interval(params, r->var); + } #ifdef RULES_DEBUG pr_debug("Rule %d [%p]: ", k, r->func); if (r->var >= 0) { @@ -394,6 +419,14 @@ int snd_pcm_hw_refine(struct snd_pcm_substream *substream, } pr_cont("\n"); #endif + if (hw_is_mask(r->var)) { + trace_hw_mask_param(substream, r->var, k + 1, + &old_mask, hw_param_mask(params, r->var)); + } + if (hw_is_interval(r->var)) { + trace_hw_interval_param(substream, r->var, k + 1, + &old_interval, hw_param_interval(params, r->var)); + } rstamps[k] = stamp; if (changed && r->var >= 0) { params->cmask |= (1 << r->var); -- cgit From 37567c55035a3a6c6cdf060301a7d8e514627afa Mon Sep 17 00:00:00 2001 From: Takashi Sakamoto Date: Wed, 7 Jun 2017 08:46:44 +0900 Subject: ALSA: pcm: enable parameter tracepoints only when CONFIG_SND_DEBUG is enabled In a previous commit, tracepoints are added for PCM parameter processing. As long as I know, this implementation increases size of relocatable object by 35%. For vendors who are conscious of memory footprint, it brings apparent disadvantage. This commit utilizes CONFIG_SND_DEBUG configuration to enable/disable the tracepoints. Signed-off-by: Takashi Sakamoto Signed-off-by: Takashi Iwai --- sound/core/pcm_native.c | 7 +++++++ 1 file changed, 7 insertions(+) (limited to 'sound/core/pcm_native.c') diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c index b98b3ccde4f0..2ce3c98a1418 100644 --- a/sound/core/pcm_native.c +++ b/sound/core/pcm_native.c @@ -39,8 +39,15 @@ #include "pcm_local.h" +#ifdef CONFIG_SND_DEBUG #define CREATE_TRACE_POINTS #include "pcm_param_trace.h" +#else +#define trace_hw_mask_param_enabled() 0 +#define trace_hw_interval_param_enabled() 0 +#define trace_hw_mask_param(substream, type, index, prev, curr) +#define trace_hw_interval_param(substream, type, index, prev, curr) +#endif /* * Compatibility -- cgit From c6706de0ce8bc8cd1e336b8cf0acabf1adedba6c Mon Sep 17 00:00:00 2001 From: Takashi Sakamoto Date: Wed, 7 Jun 2017 08:46:45 +0900 Subject: ALSA: pcm: obsolete RULES_DEBUG local macro Added tracepoints obsoleted RULES_DEBUG local macro and relevant codes. Signed-off-by: Takashi Sakamoto Signed-off-by: Takashi Iwai --- sound/core/pcm_native.c | 89 ++++--------------------------------------------- 1 file changed, 7 insertions(+), 82 deletions(-) (limited to 'sound/core/pcm_native.c') diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c index 2ce3c98a1418..2bde07a4a87f 100644 --- a/sound/core/pcm_native.c +++ b/sound/core/pcm_native.c @@ -253,29 +253,6 @@ static bool hw_support_mmap(struct snd_pcm_substream *substream) return true; } -#undef RULES_DEBUG - -#ifdef RULES_DEBUG -#define HW_PARAM(v) [SNDRV_PCM_HW_PARAM_##v] = #v -static const char * const snd_pcm_hw_param_names[] = { - HW_PARAM(ACCESS), - HW_PARAM(FORMAT), - HW_PARAM(SUBFORMAT), - HW_PARAM(SAMPLE_BITS), - HW_PARAM(FRAME_BITS), - HW_PARAM(CHANNELS), - HW_PARAM(RATE), - HW_PARAM(PERIOD_TIME), - HW_PARAM(PERIOD_SIZE), - HW_PARAM(PERIOD_BYTES), - HW_PARAM(PERIODS), - HW_PARAM(BUFFER_TIME), - HW_PARAM(BUFFER_SIZE), - HW_PARAM(BUFFER_BYTES), - HW_PARAM(TICK_TIME), -}; -#endif - int snd_pcm_hw_refine(struct snd_pcm_substream *substream, struct snd_pcm_hw_params *params) { @@ -310,14 +287,9 @@ int snd_pcm_hw_refine(struct snd_pcm_substream *substream, if (trace_hw_mask_param_enabled()) old_mask = *m; -#ifdef RULES_DEBUG - pr_debug("%s = ", snd_pcm_hw_param_names[k]); - pr_cont("%04x%04x%04x%04x -> ", m->bits[3], m->bits[2], m->bits[1], m->bits[0]); -#endif + changed = snd_mask_refine(m, constrs_mask(constrs, k)); -#ifdef RULES_DEBUG - pr_cont("%04x%04x%04x%04x\n", m->bits[3], m->bits[2], m->bits[1], m->bits[0]); -#endif + trace_hw_mask_param(substream, k, 0, &old_mask, m); if (changed) @@ -335,25 +307,9 @@ int snd_pcm_hw_refine(struct snd_pcm_substream *substream, if (trace_hw_interval_param_enabled()) old_interval = *i; -#ifdef RULES_DEBUG - pr_debug("%s = ", snd_pcm_hw_param_names[k]); - if (i->empty) - pr_cont("empty"); - else - pr_cont("%c%u %u%c", - i->openmin ? '(' : '[', i->min, - i->max, i->openmax ? ')' : ']'); - pr_cont(" -> "); -#endif + changed = snd_interval_refine(i, constrs_interval(constrs, k)); -#ifdef RULES_DEBUG - if (i->empty) - pr_cont("empty\n"); - else - pr_cont("%c%u %u%c\n", - i->openmin ? '(' : '[', i->min, - i->max, i->openmax ? ')' : ']'); -#endif + trace_hw_interval_param(substream, k, 0, &old_interval, i); if (changed) @@ -391,41 +347,9 @@ int snd_pcm_hw_refine(struct snd_pcm_substream *substream, if (hw_is_interval(r->var)) old_interval = *hw_param_interval(params, r->var); } -#ifdef RULES_DEBUG - pr_debug("Rule %d [%p]: ", k, r->func); - if (r->var >= 0) { - pr_cont("%s = ", snd_pcm_hw_param_names[r->var]); - if (hw_is_mask(r->var)) { - m = hw_param_mask(params, r->var); - pr_cont("%x", *m->bits); - } else { - i = hw_param_interval(params, r->var); - if (i->empty) - pr_cont("empty"); - else - pr_cont("%c%u %u%c", - i->openmin ? '(' : '[', i->min, - i->max, i->openmax ? ')' : ']'); - } - } -#endif + changed = r->func(params, r); -#ifdef RULES_DEBUG - if (r->var >= 0) { - pr_cont(" -> "); - if (hw_is_mask(r->var)) - pr_cont("%x", *m->bits); - else { - if (i->empty) - pr_cont("empty"); - else - pr_cont("%c%u %u%c", - i->openmin ? '(' : '[', i->min, - i->max, i->openmax ? ')' : ']'); - } - } - pr_cont("\n"); -#endif + if (hw_is_mask(r->var)) { trace_hw_mask_param(substream, r->var, k + 1, &old_mask, hw_param_mask(params, r->var)); @@ -434,6 +358,7 @@ int snd_pcm_hw_refine(struct snd_pcm_substream *substream, trace_hw_interval_param(substream, r->var, k + 1, &old_interval, hw_param_interval(params, r->var)); } + rstamps[k] = stamp; if (changed && r->var >= 0) { params->cmask |= (1 << r->var); -- cgit From 561e1cadb4dca3783de82cfa453a142129953e4d Mon Sep 17 00:00:00 2001 From: Takashi Sakamoto Date: Fri, 9 Jun 2017 06:36:59 +0900 Subject: ALSA: pcm: add a helper function to constrain mask-type parameters Application of constraints to mask-type parameters for PCM substream is done in a call of snd_pcm_hw_refine(), while the function includes much codes and is not enough friendly to readers. This commit splits the codes to a separated function so that readers can get it easily. I leave desicion into compilers to merge the function into its callee. Signed-off-by: Takashi Sakamoto Signed-off-by: Takashi Iwai --- sound/core/pcm_native.c | 56 ++++++++++++++++++++++++++++++++----------------- 1 file changed, 37 insertions(+), 19 deletions(-) (limited to 'sound/core/pcm_native.c') diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c index 2bde07a4a87f..1dee5f960fbe 100644 --- a/sound/core/pcm_native.c +++ b/sound/core/pcm_native.c @@ -253,6 +253,39 @@ static bool hw_support_mmap(struct snd_pcm_substream *substream) return true; } +static int constrain_mask_params(struct snd_pcm_substream *substream, + struct snd_pcm_hw_params *params) +{ + struct snd_pcm_hw_constraints *constrs = + &substream->runtime->hw_constraints; + struct snd_mask *m; + unsigned int k; + struct snd_mask old_mask; + int changed; + + for (k = SNDRV_PCM_HW_PARAM_FIRST_MASK; k <= SNDRV_PCM_HW_PARAM_LAST_MASK; k++) { + m = hw_param_mask(params, k); + if (snd_mask_empty(m)) + return -EINVAL; + if (!(params->rmask & (1 << k))) + continue; + + if (trace_hw_mask_param_enabled()) + old_mask = *m; + + changed = snd_mask_refine(m, constrs_mask(constrs, k)); + + trace_hw_mask_param(substream, k, 0, &old_mask, m); + + if (changed) + params->cmask |= 1 << k; + if (changed < 0) + return changed; + } + + return 0; +} + int snd_pcm_hw_refine(struct snd_pcm_substream *substream, struct snd_pcm_hw_params *params) { @@ -265,6 +298,7 @@ int snd_pcm_hw_refine(struct snd_pcm_substream *substream, unsigned int vstamps[SNDRV_PCM_HW_PARAM_LAST_INTERVAL + 1]; unsigned int stamp = 2; int changed, again; + int err; struct snd_mask __maybe_unused old_mask; struct snd_interval __maybe_unused old_interval; @@ -278,25 +312,9 @@ int snd_pcm_hw_refine(struct snd_pcm_substream *substream, params->rate_den = 0; } - for (k = SNDRV_PCM_HW_PARAM_FIRST_MASK; k <= SNDRV_PCM_HW_PARAM_LAST_MASK; k++) { - m = hw_param_mask(params, k); - if (snd_mask_empty(m)) - return -EINVAL; - if (!(params->rmask & (1 << k))) - continue; - - if (trace_hw_mask_param_enabled()) - old_mask = *m; - - changed = snd_mask_refine(m, constrs_mask(constrs, k)); - - trace_hw_mask_param(substream, k, 0, &old_mask, m); - - if (changed) - params->cmask |= 1 << k; - if (changed < 0) - return changed; - } + err = constrain_mask_params(substream, params); + if (err < 0) + return err; for (k = SNDRV_PCM_HW_PARAM_FIRST_INTERVAL; k <= SNDRV_PCM_HW_PARAM_LAST_INTERVAL; k++) { i = hw_param_interval(params, k); -- cgit From 3432fa040211660989844209b67b414185003004 Mon Sep 17 00:00:00 2001 From: Takashi Sakamoto Date: Fri, 9 Jun 2017 06:37:00 +0900 Subject: ALSA: pcm: add a helper function to constrain interval-type parameters Application of constraints to interval-type parameters for PCM substream is done in a call of snd_pcm_hw_refine(), while the function includes much codes and is not enough friendly to readers. This commit splits the codes to a separated function so that readers can get it easily. I leave desicion into compilers to merge the function into its callee. Signed-off-by: Takashi Sakamoto Signed-off-by: Takashi Iwai --- sound/core/pcm_native.c | 55 ++++++++++++++++++++++++++++++++----------------- 1 file changed, 36 insertions(+), 19 deletions(-) (limited to 'sound/core/pcm_native.c') diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c index 1dee5f960fbe..7e811ace6bf2 100644 --- a/sound/core/pcm_native.c +++ b/sound/core/pcm_native.c @@ -286,6 +286,39 @@ static int constrain_mask_params(struct snd_pcm_substream *substream, return 0; } +static int constrain_interval_params(struct snd_pcm_substream *substream, + struct snd_pcm_hw_params *params) +{ + struct snd_pcm_hw_constraints *constrs = + &substream->runtime->hw_constraints; + struct snd_interval *i; + unsigned int k; + struct snd_interval old_interval; + int changed; + + for (k = SNDRV_PCM_HW_PARAM_FIRST_INTERVAL; k <= SNDRV_PCM_HW_PARAM_LAST_INTERVAL; k++) { + i = hw_param_interval(params, k); + if (snd_interval_empty(i)) + return -EINVAL; + if (!(params->rmask & (1 << k))) + continue; + + if (trace_hw_interval_param_enabled()) + old_interval = *i; + + changed = snd_interval_refine(i, constrs_interval(constrs, k)); + + trace_hw_interval_param(substream, k, 0, &old_interval, i); + + if (changed) + params->cmask |= 1 << k; + if (changed < 0) + return changed; + } + + return 0; +} + int snd_pcm_hw_refine(struct snd_pcm_substream *substream, struct snd_pcm_hw_params *params) { @@ -316,25 +349,9 @@ int snd_pcm_hw_refine(struct snd_pcm_substream *substream, if (err < 0) return err; - for (k = SNDRV_PCM_HW_PARAM_FIRST_INTERVAL; k <= SNDRV_PCM_HW_PARAM_LAST_INTERVAL; k++) { - i = hw_param_interval(params, k); - if (snd_interval_empty(i)) - return -EINVAL; - if (!(params->rmask & (1 << k))) - continue; - - if (trace_hw_interval_param_enabled()) - old_interval = *i; - - changed = snd_interval_refine(i, constrs_interval(constrs, k)); - - trace_hw_interval_param(substream, k, 0, &old_interval, i); - - if (changed) - params->cmask |= 1 << k; - if (changed < 0) - return changed; - } + err = constrain_interval_params(substream, params); + if (err < 0) + return err; for (k = 0; k < constrs->rules_num; k++) rstamps[k] = 0; -- cgit From 9cc07f55d42be47ad2b06dae9541d9fd964c3287 Mon Sep 17 00:00:00 2001 From: Takashi Sakamoto Date: Fri, 9 Jun 2017 06:37:01 +0900 Subject: ALSA: pcm: add a helper function to apply parameter rules Application of rules to parameters of PCM substream is done in a call of snd_pcm_hw_refine(), while the function includes much codes and is not enough friendly to readers. This commit splits the codes to a separated function so that readers can get it easily. I leave desicion into compilers to merge the function into its callee. Signed-off-by: Takashi Sakamoto Signed-off-by: Takashi Iwai --- sound/core/pcm_native.c | 77 +++++++++++++++++++++++++++++-------------------- 1 file changed, 45 insertions(+), 32 deletions(-) (limited to 'sound/core/pcm_native.c') diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c index 7e811ace6bf2..000e6e9a0c2b 100644 --- a/sound/core/pcm_native.c +++ b/sound/core/pcm_native.c @@ -319,43 +319,23 @@ static int constrain_interval_params(struct snd_pcm_substream *substream, return 0; } -int snd_pcm_hw_refine(struct snd_pcm_substream *substream, - struct snd_pcm_hw_params *params) +static int constrain_params_by_rules(struct snd_pcm_substream *substream, + struct snd_pcm_hw_params *params) { + struct snd_pcm_hw_constraints *constrs = + &substream->runtime->hw_constraints; unsigned int k; - struct snd_pcm_hardware *hw; - struct snd_interval *i = NULL; - struct snd_mask *m = NULL; - struct snd_pcm_hw_constraints *constrs = &substream->runtime->hw_constraints; unsigned int rstamps[constrs->rules_num]; unsigned int vstamps[SNDRV_PCM_HW_PARAM_LAST_INTERVAL + 1]; unsigned int stamp = 2; - int changed, again; - int err; - - struct snd_mask __maybe_unused old_mask; - struct snd_interval __maybe_unused old_interval; - - params->info = 0; - params->fifo_size = 0; - if (params->rmask & (1 << SNDRV_PCM_HW_PARAM_SAMPLE_BITS)) - params->msbits = 0; - if (params->rmask & (1 << SNDRV_PCM_HW_PARAM_RATE)) { - params->rate_num = 0; - params->rate_den = 0; - } - - err = constrain_mask_params(substream, params); - if (err < 0) - return err; - - err = constrain_interval_params(substream, params); - if (err < 0) - return err; + struct snd_mask old_mask; + struct snd_interval old_interval; + int again; + int changed; for (k = 0; k < constrs->rules_num; k++) rstamps[k] = 0; - for (k = 0; k <= SNDRV_PCM_HW_PARAM_LAST_INTERVAL; k++) + for (k = 0; k <= SNDRV_PCM_HW_PARAM_LAST_INTERVAL; k++) vstamps[k] = (params->rmask & (1 << k)) ? 1 : 0; do { again = 0; @@ -405,6 +385,39 @@ int snd_pcm_hw_refine(struct snd_pcm_substream *substream, stamp++; } } while (again); + + return 0; +} + +int snd_pcm_hw_refine(struct snd_pcm_substream *substream, + struct snd_pcm_hw_params *params) +{ + struct snd_pcm_hardware *hw; + struct snd_interval *i = NULL; + struct snd_mask *m = NULL; + int err; + + params->info = 0; + params->fifo_size = 0; + if (params->rmask & (1 << SNDRV_PCM_HW_PARAM_SAMPLE_BITS)) + params->msbits = 0; + if (params->rmask & (1 << SNDRV_PCM_HW_PARAM_RATE)) { + params->rate_num = 0; + params->rate_den = 0; + } + + err = constrain_mask_params(substream, params); + if (err < 0) + return err; + + err = constrain_interval_params(substream, params); + if (err < 0) + return err; + + err = constrain_params_by_rules(substream, params); + if (err < 0) + return err; + if (!params->msbits) { i = hw_param_interval(params, SNDRV_PCM_HW_PARAM_SAMPLE_BITS); if (snd_interval_single(i)) @@ -432,10 +445,10 @@ int snd_pcm_hw_refine(struct snd_pcm_substream *substream, i = hw_param_interval(params, SNDRV_PCM_HW_PARAM_CHANNELS); if (snd_mask_min(m) == snd_mask_max(m) && snd_interval_min(i) == snd_interval_max(i)) { - changed = substream->ops->ioctl(substream, + err = substream->ops->ioctl(substream, SNDRV_PCM_IOCTL1_FIFO_SIZE, params); - if (changed < 0) - return changed; + if (err < 0) + return err; } } params->rmask = 0; -- cgit From 0d4e399965738bb90dcee2fd5aeb15c1ccc81b42 Mon Sep 17 00:00:00 2001 From: Takashi Sakamoto Date: Fri, 9 Jun 2017 06:37:02 +0900 Subject: ALSA: pcm: use goto statement instead of while statement to reduce indentation In a process to calculate parameters of PCM substream, application of all rules is iterated several times till parameter dependencies are satisfied. In current implementation, two loops are used for the design, however this brings two-level indentation and decline readability. This commit attempts to reduce the indentation by using goto statement, instead of outer while loop. Signed-off-by: Takashi Sakamoto Signed-off-by: Takashi Iwai --- sound/core/pcm_native.c | 86 +++++++++++++++++++++++++------------------------ 1 file changed, 44 insertions(+), 42 deletions(-) (limited to 'sound/core/pcm_native.c') diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c index 000e6e9a0c2b..41aeb6facdec 100644 --- a/sound/core/pcm_native.c +++ b/sound/core/pcm_native.c @@ -337,54 +337,56 @@ static int constrain_params_by_rules(struct snd_pcm_substream *substream, rstamps[k] = 0; for (k = 0; k <= SNDRV_PCM_HW_PARAM_LAST_INTERVAL; k++) vstamps[k] = (params->rmask & (1 << k)) ? 1 : 0; - do { - again = 0; - for (k = 0; k < constrs->rules_num; k++) { - struct snd_pcm_hw_rule *r = &constrs->rules[k]; - unsigned int d; - int doit = 0; - if (r->cond && !(r->cond & params->flags)) - continue; - for (d = 0; r->deps[d] >= 0; d++) { - if (vstamps[r->deps[d]] > rstamps[k]) { - doit = 1; - break; - } +retry: + again = 0; + for (k = 0; k < constrs->rules_num; k++) { + struct snd_pcm_hw_rule *r = &constrs->rules[k]; + unsigned int d; + int doit = 0; + if (r->cond && !(r->cond & params->flags)) + continue; + for (d = 0; r->deps[d] >= 0; d++) { + if (vstamps[r->deps[d]] > rstamps[k]) { + doit = 1; + break; } - if (!doit) - continue; + } + if (!doit) + continue; - if (trace_hw_mask_param_enabled()) { - if (hw_is_mask(r->var)) - old_mask = *hw_param_mask(params, r->var); - } - if (trace_hw_interval_param_enabled()) { - if (hw_is_interval(r->var)) - old_interval = *hw_param_interval(params, r->var); - } + if (trace_hw_mask_param_enabled()) { + if (hw_is_mask(r->var)) + old_mask = *hw_param_mask(params, r->var); + } + if (trace_hw_interval_param_enabled()) { + if (hw_is_interval(r->var)) + old_interval = *hw_param_interval(params, r->var); + } - changed = r->func(params, r); + changed = r->func(params, r); - if (hw_is_mask(r->var)) { - trace_hw_mask_param(substream, r->var, k + 1, - &old_mask, hw_param_mask(params, r->var)); - } - if (hw_is_interval(r->var)) { - trace_hw_interval_param(substream, r->var, k + 1, - &old_interval, hw_param_interval(params, r->var)); - } + if (hw_is_mask(r->var)) { + trace_hw_mask_param(substream, r->var, k + 1, + &old_mask, hw_param_mask(params, r->var)); + } + if (hw_is_interval(r->var)) { + trace_hw_interval_param(substream, r->var, k + 1, + &old_interval, hw_param_interval(params, r->var)); + } - rstamps[k] = stamp; - if (changed && r->var >= 0) { - params->cmask |= (1 << r->var); - vstamps[r->var] = stamp; - again = 1; - } - if (changed < 0) - return changed; - stamp++; + rstamps[k] = stamp; + if (changed && r->var >= 0) { + params->cmask |= (1 << r->var); + vstamps[r->var] = stamp; + again = 1; } - } while (again); + if (changed < 0) + return changed; + stamp++; + } + + if (again) + goto retry; return 0; } -- cgit From d656b4a6549f0f5863b7888b25a7b20d03ecbce7 Mon Sep 17 00:00:00 2001 From: Takashi Sakamoto Date: Fri, 9 Jun 2017 06:37:03 +0900 Subject: ALSA: pcm: remove function local variable with alternative evaluation A local variable is used to judge whether a parameter should be handled due to reverse dependency of the other rules. However, this can be obsoleted by check of a sentinel in dependency array. This commit removes the local variable and check the sentinel to reduce stack usage. Signed-off-by: Takashi Sakamoto Signed-off-by: Takashi Iwai --- sound/core/pcm_native.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) (limited to 'sound/core/pcm_native.c') diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c index 41aeb6facdec..db4cdd114ed4 100644 --- a/sound/core/pcm_native.c +++ b/sound/core/pcm_native.c @@ -342,16 +342,13 @@ retry: for (k = 0; k < constrs->rules_num; k++) { struct snd_pcm_hw_rule *r = &constrs->rules[k]; unsigned int d; - int doit = 0; if (r->cond && !(r->cond & params->flags)) continue; for (d = 0; r->deps[d] >= 0; d++) { - if (vstamps[r->deps[d]] > rstamps[k]) { - doit = 1; + if (vstamps[r->deps[d]] > rstamps[k]) break; - } } - if (!doit) + if (r->deps[d] < 0) continue; if (trace_hw_mask_param_enabled()) { -- cgit From a1c06e39a9373501b4f28caf37fbccba52532f79 Mon Sep 17 00:00:00 2001 From: Takashi Sakamoto Date: Fri, 9 Jun 2017 06:37:04 +0900 Subject: ALSA: pcm: adaption of code formatting This commit modifies current for readability in below aspects: - use bool type variable instead of int type variable assigned to 0/1 - move variable definition from loop to top of the function definition Signed-off-by: Takashi Sakamoto Signed-off-by: Takashi Iwai --- sound/core/pcm_native.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) (limited to 'sound/core/pcm_native.c') diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c index db4cdd114ed4..40560e579d33 100644 --- a/sound/core/pcm_native.c +++ b/sound/core/pcm_native.c @@ -328,9 +328,11 @@ static int constrain_params_by_rules(struct snd_pcm_substream *substream, unsigned int rstamps[constrs->rules_num]; unsigned int vstamps[SNDRV_PCM_HW_PARAM_LAST_INTERVAL + 1]; unsigned int stamp = 2; + struct snd_pcm_hw_rule *r; + unsigned int d; struct snd_mask old_mask; struct snd_interval old_interval; - int again; + bool again; int changed; for (k = 0; k < constrs->rules_num; k++) @@ -338,10 +340,9 @@ static int constrain_params_by_rules(struct snd_pcm_substream *substream, for (k = 0; k <= SNDRV_PCM_HW_PARAM_LAST_INTERVAL; k++) vstamps[k] = (params->rmask & (1 << k)) ? 1 : 0; retry: - again = 0; + again = false; for (k = 0; k < constrs->rules_num; k++) { - struct snd_pcm_hw_rule *r = &constrs->rules[k]; - unsigned int d; + r = &constrs->rules[k]; if (r->cond && !(r->cond & params->flags)) continue; for (d = 0; r->deps[d] >= 0; d++) { @@ -375,7 +376,7 @@ retry: if (changed && r->var >= 0) { params->cmask |= (1 << r->var); vstamps[r->var] = stamp; - again = 1; + again = true; } if (changed < 0) return changed; @@ -453,7 +454,6 @@ int snd_pcm_hw_refine(struct snd_pcm_substream *substream, params->rmask = 0; return 0; } - EXPORT_SYMBOL(snd_pcm_hw_refine); static int snd_pcm_hw_refine_user(struct snd_pcm_substream *substream, -- cgit From b81e5ddb159490270fa2cca4f6682c4452035203 Mon Sep 17 00:00:00 2001 From: Takashi Sakamoto Date: Fri, 9 Jun 2017 06:37:05 +0900 Subject: ALSA: pcm: use helper functions to check whether parameters are determined A commit 8bea869c5e56 ("ALSA: PCM midlevel: improve fifo_size handling") allows drivers to implement calculation of fifo size in parameter structure. This calculation runs only when two of the other parameters have single value. In ALSA PCM core, there're some helper functions for the case. This commit applies the functions instead of value comparison. Signed-off-by: Takashi Sakamoto Signed-off-by: Takashi Iwai --- sound/core/pcm_native.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) (limited to 'sound/core/pcm_native.c') diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c index 40560e579d33..80275aa0bcca 100644 --- a/sound/core/pcm_native.c +++ b/sound/core/pcm_native.c @@ -443,8 +443,7 @@ int snd_pcm_hw_refine(struct snd_pcm_substream *substream, if (!params->fifo_size) { m = hw_param_mask(params, SNDRV_PCM_HW_PARAM_FORMAT); i = hw_param_interval(params, SNDRV_PCM_HW_PARAM_CHANNELS); - if (snd_mask_min(m) == snd_mask_max(m) && - snd_interval_min(i) == snd_interval_max(i)) { + if (snd_mask_single(m) && snd_interval_single(i)) { err = substream->ops->ioctl(substream, SNDRV_PCM_IOCTL1_FIFO_SIZE, params); if (err < 0) -- cgit From d81052f92c3d018ade20ecbf461004566428d9a5 Mon Sep 17 00:00:00 2001 From: Takashi Sakamoto Date: Fri, 9 Jun 2017 06:37:06 +0900 Subject: ALSA: pcm: add comment about application of rule to PCM parameters Drivers add rules of parameters to runtime of PCM substream, when applications open ALSA PCM character device. When applications call ioctl(2) with SNDRV_PCM_IOCTL_HW_REFINE or SNDRV_PCM_IOCTL_HW_PARAMS, the rules are applied to the parameters and return the result to user space. The rule can have dependency between parameters. Additionally, it can have condition flags about application of rules. Userspace applications can indicate the flags to suppress change of parameters. This commit attempts to describe the mechanism. Signed-off-by: Takashi Sakamoto Signed-off-by: Takashi Iwai --- sound/core/pcm_native.c | 53 ++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 52 insertions(+), 1 deletion(-) (limited to 'sound/core/pcm_native.c') diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c index 80275aa0bcca..422ee4629698 100644 --- a/sound/core/pcm_native.c +++ b/sound/core/pcm_native.c @@ -267,6 +267,8 @@ static int constrain_mask_params(struct snd_pcm_substream *substream, m = hw_param_mask(params, k); if (snd_mask_empty(m)) return -EINVAL; + + /* This parameter is not requested to change by a caller. */ if (!(params->rmask & (1 << k))) continue; @@ -277,6 +279,7 @@ static int constrain_mask_params(struct snd_pcm_substream *substream, trace_hw_mask_param(substream, k, 0, &old_mask, m); + /* Set corresponding flag so that the caller gets it. */ if (changed) params->cmask |= 1 << k; if (changed < 0) @@ -300,6 +303,8 @@ static int constrain_interval_params(struct snd_pcm_substream *substream, i = hw_param_interval(params, k); if (snd_interval_empty(i)) return -EINVAL; + + /* This parameter is not requested to change by a caller. */ if (!(params->rmask & (1 << k))) continue; @@ -310,6 +315,7 @@ static int constrain_interval_params(struct snd_pcm_substream *substream, trace_hw_interval_param(substream, k, 0, &old_interval, i); + /* Set corresponding flag so that the caller gets it. */ if (changed) params->cmask |= 1 << k; if (changed < 0) @@ -327,7 +333,7 @@ static int constrain_params_by_rules(struct snd_pcm_substream *substream, unsigned int k; unsigned int rstamps[constrs->rules_num]; unsigned int vstamps[SNDRV_PCM_HW_PARAM_LAST_INTERVAL + 1]; - unsigned int stamp = 2; + unsigned int stamp; struct snd_pcm_hw_rule *r; unsigned int d; struct snd_mask old_mask; @@ -335,16 +341,54 @@ static int constrain_params_by_rules(struct snd_pcm_substream *substream, bool again; int changed; + /* + * Each application of rule has own sequence number. + * + * Each member of 'rstamps' array represents the sequence number of + * recent application of corresponding rule. + */ for (k = 0; k < constrs->rules_num; k++) rstamps[k] = 0; + + /* + * Each member of 'vstamps' array represents the sequence number of + * recent application of rule in which corresponding parameters were + * changed. + * + * In initial state, elements corresponding to parameters requested by + * a caller is 1. For unrequested parameters, corresponding members + * have 0 so that the parameters are never changed anymore. + */ for (k = 0; k <= SNDRV_PCM_HW_PARAM_LAST_INTERVAL; k++) vstamps[k] = (params->rmask & (1 << k)) ? 1 : 0; + + /* Due to the above design, actual sequence number starts at 2. */ + stamp = 2; retry: + /* Apply all rules in order. */ again = false; for (k = 0; k < constrs->rules_num; k++) { r = &constrs->rules[k]; + + /* + * Check condition bits of this rule. When the rule has + * some condition bits, parameter without the bits is + * never processed. SNDRV_PCM_HW_PARAMS_NO_PERIOD_WAKEUP + * is an example of the condition bits. + */ if (r->cond && !(r->cond & params->flags)) continue; + + /* + * The 'deps' array includes maximum three dependencies + * to SNDRV_PCM_HW_PARAM_XXXs for this rule. The fourth + * member of this array is a sentinel and should be + * negative value. + * + * This rule should be processed in this time when dependent + * parameters were changed at former applications of the other + * rules. + */ for (d = 0; r->deps[d] >= 0; d++) { if (vstamps[r->deps[d]] > rstamps[k]) break; @@ -373,6 +417,12 @@ retry: } rstamps[k] = stamp; + + /* + * When the parameters is changed, notify it to the caller + * by corresponding returned bit, then preparing for next + * iteration. + */ if (changed && r->var >= 0) { params->cmask |= (1 << r->var); vstamps[r->var] = stamp; @@ -383,6 +433,7 @@ retry: stamp++; } + /* Iterate to evaluate all rules till no parameters are changed. */ if (again) goto retry; -- cgit From e02de47e3c020c7bc8ce587b1b98cfc817e7db8e Mon Sep 17 00:00:00 2001 From: Takashi Sakamoto Date: Fri, 9 Jun 2017 09:34:38 +0900 Subject: ALSA: pcm: use helper functions to refer parameters as constants To fixup some parameters, ALSA PCM core refers the other parameters as constants. There're some macros for this purpose. This commit replaces codes with them. Signed-off-by: Takashi Sakamoto Signed-off-by: Takashi Iwai --- sound/core/pcm_native.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) (limited to 'sound/core/pcm_native.c') diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c index 422ee4629698..87a507f12f2f 100644 --- a/sound/core/pcm_native.c +++ b/sound/core/pcm_native.c @@ -444,8 +444,8 @@ int snd_pcm_hw_refine(struct snd_pcm_substream *substream, struct snd_pcm_hw_params *params) { struct snd_pcm_hardware *hw; - struct snd_interval *i = NULL; - struct snd_mask *m = NULL; + const struct snd_interval *i; + const struct snd_mask *m; int err; params->info = 0; @@ -470,13 +470,13 @@ int snd_pcm_hw_refine(struct snd_pcm_substream *substream, return err; if (!params->msbits) { - i = hw_param_interval(params, SNDRV_PCM_HW_PARAM_SAMPLE_BITS); + i = hw_param_interval_c(params, SNDRV_PCM_HW_PARAM_SAMPLE_BITS); if (snd_interval_single(i)) params->msbits = snd_interval_value(i); } if (!params->rate_den) { - i = hw_param_interval(params, SNDRV_PCM_HW_PARAM_RATE); + i = hw_param_interval_c(params, SNDRV_PCM_HW_PARAM_RATE); if (snd_interval_single(i)) { params->rate_num = snd_interval_value(i); params->rate_den = 1; @@ -492,8 +492,8 @@ int snd_pcm_hw_refine(struct snd_pcm_substream *substream, SNDRV_PCM_INFO_MMAP_VALID); } if (!params->fifo_size) { - m = hw_param_mask(params, SNDRV_PCM_HW_PARAM_FORMAT); - i = hw_param_interval(params, SNDRV_PCM_HW_PARAM_CHANNELS); + m = hw_param_mask_c(params, SNDRV_PCM_HW_PARAM_FORMAT); + i = hw_param_interval_c(params, SNDRV_PCM_HW_PARAM_CHANNELS); if (snd_mask_single(m) && snd_interval_single(i)) { err = substream->ops->ioctl(substream, SNDRV_PCM_IOCTL1_FIFO_SIZE, params); -- cgit From f9a076bff053100c9c3d1d5cca33ca856688b782 Mon Sep 17 00:00:00 2001 From: Takashi Sakamoto Date: Fri, 9 Jun 2017 09:34:39 +0900 Subject: ALSA: pcm: calculate non-mask/non-interval parameters always when possible A structure for parameters of PCM runtime has parameters which are not classified as mask/interval type. They are decided only when corresponding normal parameters have unique values. * struct snd_pcm_hw_params.msbits * struct snd_pcm_hw_params.rate_num * struct snd_pcm_hw_params.rate_den * struct snd_pcm_hw_params.fifo_size Current implementation of hw_params ioctl sometimes doesn't decide these parameters even if corresponding parameters are fixed, because these parameters are evaluated before a call of snd_pcm_hw_params_choose(). This commit adds a helper function to process the parameters and call it in proper positions. Signed-off-by: Takashi Sakamoto Signed-off-by: Takashi Iwai --- sound/core/pcm_native.c | 70 +++++++++++++++++++++++++++++++------------------ 1 file changed, 44 insertions(+), 26 deletions(-) (limited to 'sound/core/pcm_native.c') diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c index 87a507f12f2f..dfe6113a6a60 100644 --- a/sound/core/pcm_native.c +++ b/sound/core/pcm_native.c @@ -440,12 +440,45 @@ retry: return 0; } +static int fixup_unreferenced_params(struct snd_pcm_substream *substream, + struct snd_pcm_hw_params *params) +{ + const struct snd_interval *i; + const struct snd_mask *m; + int err; + + if (!params->msbits) { + i = hw_param_interval_c(params, SNDRV_PCM_HW_PARAM_SAMPLE_BITS); + if (snd_interval_single(i)) + params->msbits = snd_interval_value(i); + } + + if (!params->rate_den) { + i = hw_param_interval_c(params, SNDRV_PCM_HW_PARAM_RATE); + if (snd_interval_single(i)) { + params->rate_num = snd_interval_value(i); + params->rate_den = 1; + } + } + + if (!params->fifo_size) { + m = hw_param_mask_c(params, SNDRV_PCM_HW_PARAM_FORMAT); + i = hw_param_interval_c(params, SNDRV_PCM_HW_PARAM_CHANNELS); + if (snd_mask_single(m) && snd_interval_single(i)) { + err = substream->ops->ioctl(substream, + SNDRV_PCM_IOCTL1_FIFO_SIZE, params); + if (err < 0) + return err; + } + } + + return 0; +} + int snd_pcm_hw_refine(struct snd_pcm_substream *substream, struct snd_pcm_hw_params *params) { struct snd_pcm_hardware *hw; - const struct snd_interval *i; - const struct snd_mask *m; int err; params->info = 0; @@ -469,20 +502,6 @@ int snd_pcm_hw_refine(struct snd_pcm_substream *substream, if (err < 0) return err; - if (!params->msbits) { - i = hw_param_interval_c(params, SNDRV_PCM_HW_PARAM_SAMPLE_BITS); - if (snd_interval_single(i)) - params->msbits = snd_interval_value(i); - } - - if (!params->rate_den) { - i = hw_param_interval_c(params, SNDRV_PCM_HW_PARAM_RATE); - if (snd_interval_single(i)) { - params->rate_num = snd_interval_value(i); - params->rate_den = 1; - } - } - hw = &substream->runtime->hw; if (!params->info) { params->info = hw->info & ~(SNDRV_PCM_INFO_FIFO_IN_FRAMES | @@ -491,16 +510,7 @@ int snd_pcm_hw_refine(struct snd_pcm_substream *substream, params->info &= ~(SNDRV_PCM_INFO_MMAP | SNDRV_PCM_INFO_MMAP_VALID); } - if (!params->fifo_size) { - m = hw_param_mask_c(params, SNDRV_PCM_HW_PARAM_FORMAT); - i = hw_param_interval_c(params, SNDRV_PCM_HW_PARAM_CHANNELS); - if (snd_mask_single(m) && snd_interval_single(i)) { - err = substream->ops->ioctl(substream, - SNDRV_PCM_IOCTL1_FIFO_SIZE, params); - if (err < 0) - return err; - } - } + params->rmask = 0; return 0; } @@ -517,6 +527,8 @@ static int snd_pcm_hw_refine_user(struct snd_pcm_substream *substream, return PTR_ERR(params); err = snd_pcm_hw_refine(substream, params); + if (err >= 0) + err = fixup_unreferenced_params(substream, params); if (copy_to_user(_params, params, sizeof(*params))) { if (!err) err = -EFAULT; @@ -596,6 +608,10 @@ static int snd_pcm_hw_params(struct snd_pcm_substream *substream, if (err < 0) goto _error; + err = fixup_unreferenced_params(substream, params); + if (err < 0) + goto _error; + if (substream->ops->hw_params != NULL) { err = substream->ops->hw_params(substream, params); if (err < 0) @@ -3621,6 +3637,8 @@ static int snd_pcm_hw_refine_old_user(struct snd_pcm_substream *substream, } snd_pcm_hw_convert_from_old_params(params, oparams); err = snd_pcm_hw_refine(substream, params); + if (err >= 0) + err = fixup_unreferenced_params(substream, params); snd_pcm_hw_convert_to_old_params(oparams, params); if (copy_to_user(_oparams, oparams, sizeof(*oparams))) { if (!err) -- cgit From 7802fb52564b5d6b4fdcf25a08d487897f9e4a8b Mon Sep 17 00:00:00 2001 From: Takashi Sakamoto Date: Fri, 9 Jun 2017 09:34:40 +0900 Subject: ALSA: pcm: move fixup of info flag after selecting single parameters When drivers register no flags about information of PCM hardware, ALSA PCM core fixups it roughly. Currently, this operation places in a function snd_pcm_hw_refine(). It can be moved to a function fixup_unreferenced_params() because it doesn't affects operations between these two functions. This idea is better to bundle codes with similar purposes and this commit achieves it. Signed-off-by: Takashi Sakamoto Signed-off-by: Takashi Iwai --- sound/core/pcm_native.c | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) (limited to 'sound/core/pcm_native.c') diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c index dfe6113a6a60..3293db0172db 100644 --- a/sound/core/pcm_native.c +++ b/sound/core/pcm_native.c @@ -472,13 +472,21 @@ static int fixup_unreferenced_params(struct snd_pcm_substream *substream, } } + if (!params->info) { + params->info = substream->runtime->hw.info; + params->info &= ~(SNDRV_PCM_INFO_FIFO_IN_FRAMES | + SNDRV_PCM_INFO_DRAIN_TRIGGER); + if (!hw_support_mmap(substream)) + params->info &= ~(SNDRV_PCM_INFO_MMAP | + SNDRV_PCM_INFO_MMAP_VALID); + } + return 0; } int snd_pcm_hw_refine(struct snd_pcm_substream *substream, struct snd_pcm_hw_params *params) { - struct snd_pcm_hardware *hw; int err; params->info = 0; @@ -502,16 +510,8 @@ int snd_pcm_hw_refine(struct snd_pcm_substream *substream, if (err < 0) return err; - hw = &substream->runtime->hw; - if (!params->info) { - params->info = hw->info & ~(SNDRV_PCM_INFO_FIFO_IN_FRAMES | - SNDRV_PCM_INFO_DRAIN_TRIGGER); - if (!hw_support_mmap(substream)) - params->info &= ~(SNDRV_PCM_INFO_MMAP | - SNDRV_PCM_INFO_MMAP_VALID); - } - params->rmask = 0; + return 0; } EXPORT_SYMBOL(snd_pcm_hw_refine); -- cgit From 60f96aaecb19ca294addfff0d2d0335293f3c379 Mon Sep 17 00:00:00 2001 From: Takashi Sakamoto Date: Fri, 9 Jun 2017 21:46:48 +0900 Subject: ALSA: pcm: localize snd_pcm_hw_params_choose() As of v4.12, snd_pcm_hw_params_choose() is just called in a process context of ioctl(2) with SNDRV_PCM_IOCTL_HW_PARAMS. The function locates in a different file, which has no tracepoints. This commit moves the function to a file with the tracepoints for later commit. Signed-off-by: Takashi Sakamoto Signed-off-by: Takashi Iwai --- sound/core/pcm_native.c | 40 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) (limited to 'sound/core/pcm_native.c') diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c index 3293db0172db..8d9d181b1c03 100644 --- a/sound/core/pcm_native.c +++ b/sound/core/pcm_native.c @@ -571,6 +571,46 @@ static inline void snd_pcm_timer_notify(struct snd_pcm_substream *substream, #endif } +/** + * snd_pcm_hw_param_choose - choose a configuration defined by @params + * @pcm: PCM instance + * @params: the hw_params instance + * + * Choose one configuration from configuration space defined by @params. + * The configuration chosen is that obtained fixing in this order: + * first access, first format, first subformat, min channels, + * min rate, min period time, max buffer size, min tick time + * + * Return: Zero if successful, or a negative error code on failure. + */ +static int snd_pcm_hw_params_choose(struct snd_pcm_substream *pcm, + struct snd_pcm_hw_params *params) +{ + static const int vars[] = { + SNDRV_PCM_HW_PARAM_ACCESS, + SNDRV_PCM_HW_PARAM_FORMAT, + SNDRV_PCM_HW_PARAM_SUBFORMAT, + SNDRV_PCM_HW_PARAM_CHANNELS, + SNDRV_PCM_HW_PARAM_RATE, + SNDRV_PCM_HW_PARAM_PERIOD_TIME, + SNDRV_PCM_HW_PARAM_BUFFER_SIZE, + SNDRV_PCM_HW_PARAM_TICK_TIME, + -1 + }; + const int *v; + int err; + + for (v = vars; *v != -1; v++) { + if (*v != SNDRV_PCM_HW_PARAM_BUFFER_SIZE) + err = snd_pcm_hw_param_first(pcm, params, *v, NULL); + else + err = snd_pcm_hw_param_last(pcm, params, *v, NULL); + if (snd_BUG_ON(err < 0)) + return err; + } + return 0; +} + static int snd_pcm_hw_params(struct snd_pcm_substream *substream, struct snd_pcm_hw_params *params) { -- cgit From 7b8a54aff30e96b980aa65b0b2e4ebdffcd57196 Mon Sep 17 00:00:00 2001 From: Takashi Sakamoto Date: Fri, 9 Jun 2017 21:46:49 +0900 Subject: ALSA: pcm: add tracepoints for final selection process of hardware parameters Results of ioctl(2) with SNDRV_PCM_IOCTL_HW_REFINE and SNDRV_PCM_IOCTL_HW_PARAMS are different, because the latter has single value for several parameters; e.g. channels of PCM substream. Selection of the single value is done independently of application of constraints. It's helpful for developers to trace the selection process. This commit adds tracepoints to snd_pcm_hw_params_choose() for the purpose. Signed-off-by: Takashi Sakamoto Signed-off-by: Takashi Iwai --- sound/core/pcm_native.c | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) (limited to 'sound/core/pcm_native.c') diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c index 8d9d181b1c03..076187ae8859 100644 --- a/sound/core/pcm_native.c +++ b/sound/core/pcm_native.c @@ -598,16 +598,38 @@ static int snd_pcm_hw_params_choose(struct snd_pcm_substream *pcm, -1 }; const int *v; + struct snd_mask old_mask; + struct snd_interval old_interval; int err; for (v = vars; *v != -1; v++) { + /* Keep old parameter to trace. */ + if (trace_hw_mask_param_enabled()) { + if (hw_is_mask(*v)) + old_mask = *hw_param_mask(params, *v); + } + if (trace_hw_interval_param_enabled()) { + if (hw_is_interval(*v)) + old_interval = *hw_param_interval(params, *v); + } if (*v != SNDRV_PCM_HW_PARAM_BUFFER_SIZE) err = snd_pcm_hw_param_first(pcm, params, *v, NULL); else err = snd_pcm_hw_param_last(pcm, params, *v, NULL); if (snd_BUG_ON(err < 0)) return err; + + /* Trace the parameter. */ + if (hw_is_mask(*v)) { + trace_hw_mask_param(pcm, *v, 0, &old_mask, + hw_param_mask(params, *v)); + } + if (hw_is_interval(*v)) { + trace_hw_interval_param(pcm, *v, 0, &old_interval, + hw_param_interval(params, *v)); + } } + return 0; } -- cgit From f74ae15fe3da7905b78e986ad906a333587cf160 Mon Sep 17 00:00:00 2001 From: Takashi Sakamoto Date: Sun, 11 Jun 2017 23:56:12 +0900 Subject: ALSA: pcm: return error immediately for parameters handling When refining mask/interval parameters, helper functions can return error code. This error is not handled immediately. This seems to return parameters to userspace applications in its meddle of processing. However, in general, when receiving error from system calls, the application might not handle argument buffer. It's reasonable to judge the above design as superfluity. This commit handles the error immediately. Signed-off-by: Takashi Sakamoto Signed-off-by: Takashi Iwai --- sound/core/pcm_native.c | 66 ++++++++++++++++++++++++++++--------------------- 1 file changed, 38 insertions(+), 28 deletions(-) (limited to 'sound/core/pcm_native.c') diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c index 076187ae8859..425f54827e78 100644 --- a/sound/core/pcm_native.c +++ b/sound/core/pcm_native.c @@ -276,14 +276,14 @@ static int constrain_mask_params(struct snd_pcm_substream *substream, old_mask = *m; changed = snd_mask_refine(m, constrs_mask(constrs, k)); + if (changed < 0) + return changed; trace_hw_mask_param(substream, k, 0, &old_mask, m); /* Set corresponding flag so that the caller gets it. */ if (changed) params->cmask |= 1 << k; - if (changed < 0) - return changed; } return 0; @@ -312,14 +312,14 @@ static int constrain_interval_params(struct snd_pcm_substream *substream, old_interval = *i; changed = snd_interval_refine(i, constrs_interval(constrs, k)); + if (changed < 0) + return changed; trace_hw_interval_param(substream, k, 0, &old_interval, i); /* Set corresponding flag so that the caller gets it. */ if (changed) params->cmask |= 1 << k; - if (changed < 0) - return changed; } return 0; @@ -406,6 +406,8 @@ retry: } changed = r->func(params, r); + if (changed < 0) + return changed; if (hw_is_mask(r->var)) { trace_hw_mask_param(substream, r->var, k + 1, @@ -428,8 +430,7 @@ retry: vstamps[r->var] = stamp; again = true; } - if (changed < 0) - return changed; + stamp++; } @@ -527,13 +528,16 @@ static int snd_pcm_hw_refine_user(struct snd_pcm_substream *substream, return PTR_ERR(params); err = snd_pcm_hw_refine(substream, params); - if (err >= 0) - err = fixup_unreferenced_params(substream, params); - if (copy_to_user(_params, params, sizeof(*params))) { - if (!err) - err = -EFAULT; - } + if (err < 0) + goto end; + err = fixup_unreferenced_params(substream, params); + if (err < 0) + goto end; + + if (copy_to_user(_params, params, sizeof(*params))) + err = -EFAULT; +end: kfree(params); return err; } @@ -749,11 +753,12 @@ static int snd_pcm_hw_params_user(struct snd_pcm_substream *substream, return PTR_ERR(params); err = snd_pcm_hw_params(substream, params); - if (copy_to_user(_params, params, sizeof(*params))) { - if (!err) - err = -EFAULT; - } + if (err < 0) + goto end; + if (copy_to_user(_params, params, sizeof(*params))) + err = -EFAULT; +end: kfree(params); return err; } @@ -3699,14 +3704,17 @@ static int snd_pcm_hw_refine_old_user(struct snd_pcm_substream *substream, } snd_pcm_hw_convert_from_old_params(params, oparams); err = snd_pcm_hw_refine(substream, params); - if (err >= 0) - err = fixup_unreferenced_params(substream, params); - snd_pcm_hw_convert_to_old_params(oparams, params); - if (copy_to_user(_oparams, oparams, sizeof(*oparams))) { - if (!err) - err = -EFAULT; - } + if (err < 0) + goto out_old; + + err = fixup_unreferenced_params(substream, params); + if (err < 0) + goto out_old; + snd_pcm_hw_convert_to_old_params(oparams, params); + if (copy_to_user(_oparams, oparams, sizeof(*oparams))) + err = -EFAULT; +out_old: kfree(oparams); out: kfree(params); @@ -3729,14 +3737,16 @@ static int snd_pcm_hw_params_old_user(struct snd_pcm_substream *substream, err = PTR_ERR(oparams); goto out; } + snd_pcm_hw_convert_from_old_params(params, oparams); err = snd_pcm_hw_params(substream, params); - snd_pcm_hw_convert_to_old_params(oparams, params); - if (copy_to_user(_oparams, oparams, sizeof(*oparams))) { - if (!err) - err = -EFAULT; - } + if (err < 0) + goto out_old; + snd_pcm_hw_convert_to_old_params(oparams, params); + if (copy_to_user(_oparams, oparams, sizeof(*oparams))) + err = -EFAULT; +out_old: kfree(oparams); out: kfree(params); -- cgit From 82e7d5012f73e51209305876bed2aac53b62cde3 Mon Sep 17 00:00:00 2001 From: Takashi Sakamoto Date: Sun, 11 Jun 2017 23:56:13 +0900 Subject: ALSA: pcm: probe events when parameters are changed actually At present, trace events are probed even if corresponding parameter is not actually changed. This is inconvenient. This commit improves the behaviour. Signed-off-by: Takashi Sakamoto Signed-off-by: Takashi Iwai --- sound/core/pcm_native.c | 56 +++++++++++++++++++++++++------------------------ 1 file changed, 29 insertions(+), 27 deletions(-) (limited to 'sound/core/pcm_native.c') diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c index 425f54827e78..5099078dde93 100644 --- a/sound/core/pcm_native.c +++ b/sound/core/pcm_native.c @@ -278,12 +278,12 @@ static int constrain_mask_params(struct snd_pcm_substream *substream, changed = snd_mask_refine(m, constrs_mask(constrs, k)); if (changed < 0) return changed; - - trace_hw_mask_param(substream, k, 0, &old_mask, m); + if (changed == 0) + continue; /* Set corresponding flag so that the caller gets it. */ - if (changed) - params->cmask |= 1 << k; + trace_hw_mask_param(substream, k, 0, &old_mask, m); + params->cmask |= 1 << k; } return 0; @@ -314,12 +314,12 @@ static int constrain_interval_params(struct snd_pcm_substream *substream, changed = snd_interval_refine(i, constrs_interval(constrs, k)); if (changed < 0) return changed; - - trace_hw_interval_param(substream, k, 0, &old_interval, i); + if (changed == 0) + continue; /* Set corresponding flag so that the caller gets it. */ - if (changed) - params->cmask |= 1 << k; + trace_hw_interval_param(substream, k, 0, &old_interval, i); + params->cmask |= 1 << k; } return 0; @@ -409,29 +409,29 @@ retry: if (changed < 0) return changed; - if (hw_is_mask(r->var)) { - trace_hw_mask_param(substream, r->var, k + 1, - &old_mask, hw_param_mask(params, r->var)); - } - if (hw_is_interval(r->var)) { - trace_hw_interval_param(substream, r->var, k + 1, - &old_interval, hw_param_interval(params, r->var)); - } - - rstamps[k] = stamp; - /* - * When the parameters is changed, notify it to the caller + * When the parameter is changed, notify it to the caller * by corresponding returned bit, then preparing for next * iteration. */ if (changed && r->var >= 0) { + if (hw_is_mask(r->var)) { + trace_hw_mask_param(substream, r->var, + k + 1, &old_mask, + hw_param_mask(params, r->var)); + } + if (hw_is_interval(r->var)) { + trace_hw_interval_param(substream, r->var, + k + 1, &old_interval, + hw_param_interval(params, r->var)); + } + params->cmask |= (1 << r->var); vstamps[r->var] = stamp; again = true; } - stamp++; + rstamps[k] = stamp++; } /* Iterate to evaluate all rules till no parameters are changed. */ @@ -604,7 +604,7 @@ static int snd_pcm_hw_params_choose(struct snd_pcm_substream *pcm, const int *v; struct snd_mask old_mask; struct snd_interval old_interval; - int err; + int changed; for (v = vars; *v != -1; v++) { /* Keep old parameter to trace. */ @@ -617,13 +617,15 @@ static int snd_pcm_hw_params_choose(struct snd_pcm_substream *pcm, old_interval = *hw_param_interval(params, *v); } if (*v != SNDRV_PCM_HW_PARAM_BUFFER_SIZE) - err = snd_pcm_hw_param_first(pcm, params, *v, NULL); + changed = snd_pcm_hw_param_first(pcm, params, *v, NULL); else - err = snd_pcm_hw_param_last(pcm, params, *v, NULL); - if (snd_BUG_ON(err < 0)) - return err; + changed = snd_pcm_hw_param_last(pcm, params, *v, NULL); + if (snd_BUG_ON(changed < 0)) + return changed; + if (changed == 0) + continue; - /* Trace the parameter. */ + /* Trace the changed parameter. */ if (hw_is_mask(*v)) { trace_hw_mask_param(pcm, *v, 0, &old_mask, hw_param_mask(params, *v)); -- cgit From 66e01a5cf63f2b132059d0d3d78ed737207489f2 Mon Sep 17 00:00:00 2001 From: Takashi Sakamoto Date: Mon, 12 Jun 2017 09:41:44 +0900 Subject: ALSA: pcm: unify codes to operate application-side position on PCM buffer 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 Signed-off-by: Takashi Iwai --- sound/core/pcm_native.c | 28 ++++------------------------ 1 file changed, 4 insertions(+), 24 deletions(-) (limited to 'sound/core/pcm_native.c') diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c index 5099078dde93..07995e645327 100644 --- a/sound/core/pcm_native.c +++ b/sound/core/pcm_native.c @@ -2598,27 +2598,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, @@ -2635,7 +2614,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; } @@ -2655,7 +2634,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; } @@ -2783,7 +2762,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; -- cgit From 34bcc44abb302d1586bf1eb7548be75d0f56babc Mon Sep 17 00:00:00 2001 From: Takashi Iwai Date: Tue, 24 May 2016 14:58:04 +0200 Subject: ALSA: pcm: Clean up SNDRV_PCM_IOCTL_PAUSE code Use snd_pcm_action_lock_irq() helper instead of open coding. No functional change. Reviewed-by: Takashi Sakamoto Signed-off-by: Takashi Iwai --- sound/core/pcm_native.c | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) (limited to 'sound/core/pcm_native.c') diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c index 07995e645327..798bca967c0e 100644 --- a/sound/core/pcm_native.c +++ b/sound/core/pcm_native.c @@ -2865,13 +2865,9 @@ static int snd_pcm_common_ioctl1(struct file *file, case SNDRV_PCM_IOCTL_DROP: return snd_pcm_drop(substream); case SNDRV_PCM_IOCTL_PAUSE: - { - int res; - snd_pcm_stream_lock_irq(substream); - res = snd_pcm_pause(substream, (int)(unsigned long)arg); - snd_pcm_stream_unlock_irq(substream); - return res; - } + return snd_pcm_action_lock_irq(&snd_pcm_action_pause, + substream, + (int)(unsigned long)arg); } pcm_dbg(substream->pcm, "unknown ioctl = 0x%x\n", cmd); return -ENOTTY; -- cgit From 68b4acd322494444803a3f49884ae889c8ec6689 Mon Sep 17 00:00:00 2001 From: Takashi Iwai Date: Tue, 24 May 2016 15:07:39 +0200 Subject: ALSA: pcm: Apply power lock globally to common ioctls All PCM common ioctls should run only in the powered up state, but currently only a few ioctls do the proper snd_power_lock() and snd_power_wait() invocations. Instead of adding to each place, do it commonly in the caller side, so that all these ioctls are assured to be operated at the power up state. Reviewed-by: Takashi Sakamoto Signed-off-by: Takashi Iwai --- sound/core/pcm_native.c | 56 +++++++++++++++++-------------------------------- 1 file changed, 19 insertions(+), 37 deletions(-) (limited to 'sound/core/pcm_native.c') diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c index 798bca967c0e..bd1b74aa2068 100644 --- a/sound/core/pcm_native.c +++ b/sound/core/pcm_native.c @@ -1540,14 +1540,7 @@ static const struct action_ops snd_pcm_action_resume = { static int snd_pcm_resume(struct snd_pcm_substream *substream) { - struct snd_card *card = substream->pcm->card; - int res; - - snd_power_lock(card); - if ((res = snd_power_wait(card, SNDRV_CTL_POWER_D0)) >= 0) - res = snd_pcm_action_lock_irq(&snd_pcm_action_resume, substream, 0); - snd_power_unlock(card); - return res; + return snd_pcm_action_lock_irq(&snd_pcm_action_resume, substream, 0); } #else @@ -1566,17 +1559,9 @@ static int snd_pcm_resume(struct snd_pcm_substream *substream) */ static int snd_pcm_xrun(struct snd_pcm_substream *substream) { - struct snd_card *card = substream->pcm->card; struct snd_pcm_runtime *runtime = substream->runtime; int result; - snd_power_lock(card); - if (runtime->status->state == SNDRV_PCM_STATE_SUSPENDED) { - result = snd_power_wait(card, SNDRV_CTL_POWER_D0); - if (result < 0) - goto _unlock; - } - snd_pcm_stream_lock_irq(substream); switch (runtime->status->state) { case SNDRV_PCM_STATE_XRUN: @@ -1589,8 +1574,6 @@ static int snd_pcm_xrun(struct snd_pcm_substream *substream) result = -EBADFD; } snd_pcm_stream_unlock_irq(substream); - _unlock: - snd_power_unlock(card); return result; } @@ -1694,8 +1677,6 @@ static const struct action_ops snd_pcm_action_prepare = { static int snd_pcm_prepare(struct snd_pcm_substream *substream, struct file *file) { - int res; - struct snd_card *card = substream->pcm->card; int f_flags; if (file) @@ -1703,12 +1684,8 @@ static int snd_pcm_prepare(struct snd_pcm_substream *substream, else f_flags = substream->f_flags; - snd_power_lock(card); - if ((res = snd_power_wait(card, SNDRV_CTL_POWER_D0)) >= 0) - res = snd_pcm_action_nonatomic(&snd_pcm_action_prepare, - substream, f_flags); - snd_power_unlock(card); - return res; + return snd_pcm_action_nonatomic(&snd_pcm_action_prepare, + substream, f_flags); } /* @@ -1805,15 +1782,6 @@ static int snd_pcm_drain(struct snd_pcm_substream *substream, if (runtime->status->state == SNDRV_PCM_STATE_OPEN) return -EBADFD; - snd_power_lock(card); - if (runtime->status->state == SNDRV_PCM_STATE_SUSPENDED) { - result = snd_power_wait(card, SNDRV_CTL_POWER_D0); - if (result < 0) { - snd_power_unlock(card); - return result; - } - } - if (file) { if (file->f_flags & O_NONBLOCK) nonblock = 1; @@ -1896,7 +1864,6 @@ static int snd_pcm_drain(struct snd_pcm_substream *substream, unlock: snd_pcm_stream_unlock_irq(substream); up_read(&snd_pcm_link_rwsem); - snd_power_unlock(card); return result; } @@ -2798,7 +2765,7 @@ static int snd_pcm_tstamp(struct snd_pcm_substream *substream, int __user *_arg) return 0; } -static int snd_pcm_common_ioctl1(struct file *file, +static int snd_pcm_common_ioctl(struct file *file, struct snd_pcm_substream *substream, unsigned int cmd, void __user *arg) { @@ -2873,6 +2840,21 @@ static int snd_pcm_common_ioctl1(struct file *file, return -ENOTTY; } +static int snd_pcm_common_ioctl1(struct file *file, + struct snd_pcm_substream *substream, + unsigned int cmd, void __user *arg) +{ + struct snd_card *card = substream->pcm->card; + int res; + + snd_power_lock(card); + res = snd_power_wait(card, SNDRV_CTL_POWER_D0); + if (res >= 0) + res = snd_pcm_common_ioctl(file, substream, cmd, arg); + snd_power_unlock(card); + return res; +} + static int snd_pcm_playback_ioctl1(struct file *file, struct snd_pcm_substream *substream, unsigned int cmd, void __user *arg) -- cgit From 4b95ff781e30c50298257d22a2c3743b2e5739be Mon Sep 17 00:00:00 2001 From: Takashi Iwai Date: Tue, 24 May 2016 15:08:31 +0200 Subject: ALSA: pcm: Allow dropping stream directly after resume So far, the PCM core refuses DROP ioctl when the stream in the suspended state. This was basically to avoid the invalid state change *during* the suspend. But since we protect the power change globally in the common PCM ioctl caller side, it's guaranteed that snd_pcm_drop() is called at the right power state. So we can assume that the drop of stream is safe immediately after SUSPENDED state. Reviewed-by: Takashi Sakamoto Signed-off-by: Takashi Iwai --- sound/core/pcm_native.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) (limited to 'sound/core/pcm_native.c') diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c index bd1b74aa2068..69cf9b02ac70 100644 --- a/sound/core/pcm_native.c +++ b/sound/core/pcm_native.c @@ -1883,8 +1883,7 @@ static int snd_pcm_drop(struct snd_pcm_substream *substream) runtime = substream->runtime; if (runtime->status->state == SNDRV_PCM_STATE_OPEN || - runtime->status->state == SNDRV_PCM_STATE_DISCONNECTED || - runtime->status->state == SNDRV_PCM_STATE_SUSPENDED) + runtime->status->state == SNDRV_PCM_STATE_DISCONNECTED) return -EBADFD; snd_pcm_stream_lock_irq(substream); -- cgit From 1b745cd97425f7b0b9d0c87c1b9766c31b7d0a7e Mon Sep 17 00:00:00 2001 From: Takashi Iwai Date: Tue, 24 May 2016 15:40:03 +0200 Subject: ALSA: pcm: Preprocess PAUSED or SUSPENDED stream before PREPARE Calling PREPARE ioctl to the stream in either PAUSED or SUSPENDED state may confuse some drivers that don't handle the state properly. Instead of fixing each driver, PCM core should take care of the proper state change before actually trying to (re-)prepare the stream. Namely, when the stream is in PAUSED state, it triggers PAUSE_RELEASE, and when in SUSPENDED state, it triggers STOP, before calling prepare callbacks. Reviewed-by: Takashi Sakamoto Signed-off-by: Takashi Iwai --- sound/core/pcm_native.c | 11 +++++++++++ 1 file changed, 11 insertions(+) (limited to 'sound/core/pcm_native.c') diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c index 69cf9b02ac70..0941b9c92b3f 100644 --- a/sound/core/pcm_native.c +++ b/sound/core/pcm_native.c @@ -1684,6 +1684,17 @@ static int snd_pcm_prepare(struct snd_pcm_substream *substream, else f_flags = substream->f_flags; + snd_pcm_stream_lock_irq(substream); + switch (substream->runtime->status->state) { + case SNDRV_PCM_STATE_PAUSED: + snd_pcm_pause(substream, 0); + /* fallthru */ + case SNDRV_PCM_STATE_SUSPENDED: + snd_pcm_stop(substream, SNDRV_PCM_STATE_SETUP); + break; + } + snd_pcm_stream_unlock_irq(substream); + return snd_pcm_action_nonatomic(&snd_pcm_action_prepare, substream, f_flags); } -- cgit From 4e99151435cb2e88b6d0d49939bf836c35e555a3 Mon Sep 17 00:00:00 2001 From: Takashi Iwai Date: Tue, 24 May 2016 15:53:36 +0200 Subject: ALSA: pcm: Use common PCM_RUNTIME_CHECK() for sanity checks Just a code cleanup. Reviewed-by: Takashi Sakamoto Signed-off-by: Takashi Iwai --- sound/core/pcm_native.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'sound/core/pcm_native.c') diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c index 0941b9c92b3f..05858c91c0ea 100644 --- a/sound/core/pcm_native.c +++ b/sound/core/pcm_native.c @@ -2869,7 +2869,7 @@ static int snd_pcm_playback_ioctl1(struct file *file, struct snd_pcm_substream *substream, unsigned int cmd, void __user *arg) { - if (snd_BUG_ON(!substream)) + if (PCM_RUNTIME_CHECK(substream)) return -ENXIO; if (snd_BUG_ON(substream->stream != SNDRV_PCM_STREAM_PLAYBACK)) return -EINVAL; @@ -2949,7 +2949,7 @@ static int snd_pcm_capture_ioctl1(struct file *file, struct snd_pcm_substream *substream, unsigned int cmd, void __user *arg) { - if (snd_BUG_ON(!substream)) + if (PCM_RUNTIME_CHECK(substream)) return -ENXIO; if (snd_BUG_ON(substream->stream != SNDRV_PCM_STREAM_CAPTURE)) return -EINVAL; -- cgit From e11f0f90a626f93899687b1cc909ee37dd6c5809 Mon Sep 17 00:00:00 2001 From: Takashi Sakamoto Date: Wed, 14 Jun 2017 19:30:03 +0900 Subject: ALSA: pcm: remove SNDRV_PCM_IOCTL1_INFO internal command Drivers can implement 'struct snd_pcm_ops.ioctl' to handle some requests from ALSA PCM core. These requests are internal purpose in kernel land. Usually common set of operations are used for it. SNDRV_PCM_IOCTL1_INFO is one of the requests. According to code comment, it has been obsoleted in the old days. We can see old releases in ftp.alsa-project.org. The command was firstly introduced in v0.5.0 release as SND_PCM_IOCTL1_INFO, to allow drivers to fill data of 'struct snd_pcm_channel_info' type. In v0.9.0 release, this was obsoleted by the other commands for ioctl(2) such as SNDRV_PCM_IOCTL_CHANNEL_INFO. This commit removes the long-abandoned command, bye. Signed-off-by: Takashi Sakamoto Signed-off-by: Takashi Iwai --- sound/core/pcm_native.c | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) (limited to 'sound/core/pcm_native.c') diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c index 05858c91c0ea..7e8f3656b695 100644 --- a/sound/core/pcm_native.c +++ b/sound/core/pcm_native.c @@ -212,11 +212,7 @@ int snd_pcm_info(struct snd_pcm_substream *substream, struct snd_pcm_info *info) info->subdevices_avail = pstr->substream_count - pstr->substream_opened; strlcpy(info->subname, substream->name, sizeof(info->subname)); runtime = substream->runtime; - /* AB: FIXME!!! This is definitely nonsense */ - if (runtime) { - info->sync = runtime->sync; - substream->ops->ioctl(substream, SNDRV_PCM_IOCTL1_INFO, info); - } + return 0; } -- cgit From 602d7d72c8255f80898e94562f777635efd1ddaf Mon Sep 17 00:00:00 2001 From: Takashi Iwai Date: Fri, 16 Jun 2017 16:12:30 +0200 Subject: ALSA: pcm: Follow standard EXPORT_SYMBOL() declarations Just a tidy up to follow the standard EXPORT_SYMBOL*() declarations in order to improve grep-ability. - Remove superfluous blank line before EXPORT_SYMBOL*() lines Signed-off-by: Takashi Iwai --- sound/core/pcm_native.c | 7 ------- 1 file changed, 7 deletions(-) (limited to 'sound/core/pcm_native.c') diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c index 7e8f3656b695..d35c6614fdab 100644 --- a/sound/core/pcm_native.c +++ b/sound/core/pcm_native.c @@ -1278,7 +1278,6 @@ int snd_pcm_stop(struct snd_pcm_substream *substream, snd_pcm_state_t state) { return snd_pcm_action(&snd_pcm_action_stop, substream, state); } - EXPORT_SYMBOL(snd_pcm_stop); /** @@ -1453,7 +1452,6 @@ int snd_pcm_suspend(struct snd_pcm_substream *substream) snd_pcm_stream_unlock_irqrestore(substream, flags); return err; } - EXPORT_SYMBOL(snd_pcm_suspend); /** @@ -1485,7 +1483,6 @@ int snd_pcm_suspend_all(struct snd_pcm *pcm) } return 0; } - EXPORT_SYMBOL(snd_pcm_suspend_all); /* resume */ @@ -2369,7 +2366,6 @@ void snd_pcm_release_substream(struct snd_pcm_substream *substream) } snd_pcm_detach_substream(substream); } - EXPORT_SYMBOL(snd_pcm_release_substream); int snd_pcm_open_substream(struct snd_pcm *pcm, int stream, @@ -2411,7 +2407,6 @@ int snd_pcm_open_substream(struct snd_pcm *pcm, int stream, snd_pcm_release_substream(substream); return err; } - EXPORT_SYMBOL(snd_pcm_open_substream); static int snd_pcm_open_file(struct file *file, @@ -3504,7 +3499,6 @@ int snd_pcm_lib_mmap_iomem(struct snd_pcm_substream *substream, area->vm_page_prot = pgprot_noncached(area->vm_page_prot); return vm_iomap_memory(area, runtime->dma_addr, runtime->dma_bytes); } - EXPORT_SYMBOL(snd_pcm_lib_mmap_iomem); #endif /* SNDRV_PCM_INFO_MMAP */ @@ -3553,7 +3547,6 @@ int snd_pcm_mmap_data(struct snd_pcm_substream *substream, struct file *file, atomic_inc(&substream->mmap_count); return err; } - EXPORT_SYMBOL(snd_pcm_mmap_data); static int snd_pcm_mmap(struct file *file, struct vm_area_struct *area) -- cgit From 42f945970af9df6216e3d771b4df371d02d8742c Mon Sep 17 00:00:00 2001 From: Takashi Iwai Date: Mon, 19 Jun 2017 22:39:18 +0200 Subject: ALSA: pcm: Add the explicit appl_ptr sync support Currently x86 platforms use the PCM status/control mmaps for transferring the PCM status and appl_ptr between kernel and user-spaces. The mmap is a most efficient way of communication, but it has a drawback per its nature, namely, it can't notify the change explicitly to kernel. The lack of appl_ptr update notification is a problem on a few existing drivers, but it's mostly a small issue and negligible. However, a new type of driver that uses DSP for a deep buffer management requires the exact position of appl_ptr for calculating the buffer prefetch size, and the asynchronous appl_ptr update between kernel and user-spaces becomes a significant problem for it. How can we enforce user-space to report the appl_ptr update? The way is relatively simple. Just by disabling the PCM control mmap, the user-space is supposed to fall back to the mode using SYNC_PTR ioctl, and the kernel gets control over that. This fallback mode is used in all non-x86 platforms as default, and also in the 32bit compatible model on all platforms including x86. It's been implemented already over a decade, so we can say it's fairly safe and stably working. With the help of the knowledge above, this patch introduces a new PCM info flag SNDRV_PCM_INFO_SYNC_APPLPTR for achieving the appl_ptr sync from user-space. When a driver sets this flag at open, the PCM status / control mmap is disabled, which effectively switches to SYNC_PTR mode in user-space side. In this version, both PCM status and control mmaps are disabled although only the latter, control mmap, is the target. It's because the current alsa-lib implementation supposes that both status and control mmaps are always coupled, thus it handles a fatal error when only one of them fails. Of course, the disablement of the status/control mmaps may bring a slight performance overhead. Thus, as of now, this should be used only for the dedicated devices that deserves. Note that the disablement of mmap is a sort of workaround. In the later patch, we'll introduce the way to identify the protocol version alsa-lib supports, and keep mmap working while the sync_ptr is performed together. Reviewed-by: Takashi Sakamoto Signed-off-by: Takashi Iwai --- sound/core/pcm_native.c | 23 +++++++++++++++++++++-- 1 file changed, 21 insertions(+), 2 deletions(-) (limited to 'sound/core/pcm_native.c') diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c index d35c6614fdab..9ade0c8b54a3 100644 --- a/sound/core/pcm_native.c +++ b/sound/core/pcm_native.c @@ -3376,10 +3376,29 @@ static int snd_pcm_mmap_control(struct snd_pcm_substream *substream, struct file area->vm_flags |= VM_DONTEXPAND | VM_DONTDUMP; return 0; } + +static bool pcm_status_mmap_allowed(struct snd_pcm_file *pcm_file) +{ + if (pcm_file->no_compat_mmap) + return false; + /* Disallow the status/control mmap when SYNC_APPLPTR flag is set; + * it enforces the user-space to fall back to snd_pcm_sync_ptr(), + * thus it effectively assures the manual update of appl_ptr. + * In theory, it should be enough to disallow only PCM control mmap, + * but since the current alsa-lib implementation requires both status + * and control mmaps always paired, we have to disable both of them. + */ + if (pcm_file->substream->runtime->hw.info & SNDRV_PCM_INFO_SYNC_APPLPTR) + return false; + return true; +} + #else /* ! coherent mmap */ /* * don't support mmap for status and control records. */ +#define pcm_status_mmap_allowed(pcm_file) false + static int snd_pcm_mmap_status(struct snd_pcm_substream *substream, struct file *file, struct vm_area_struct *area) { @@ -3563,11 +3582,11 @@ static int snd_pcm_mmap(struct file *file, struct vm_area_struct *area) offset = area->vm_pgoff << PAGE_SHIFT; switch (offset) { case SNDRV_PCM_MMAP_OFFSET_STATUS: - if (pcm_file->no_compat_mmap) + if (!pcm_status_mmap_allowed(pcm_file)) return -ENXIO; return snd_pcm_mmap_status(substream, file, area); case SNDRV_PCM_MMAP_OFFSET_CONTROL: - if (pcm_file->no_compat_mmap) + if (!pcm_status_mmap_allowed(pcm_file)) return -ENXIO; return snd_pcm_mmap_control(substream, file, area); default: -- cgit From 4b671f57747468d7c810caaf955f79ff1aece4d4 Mon Sep 17 00:00:00 2001 From: Takashi Iwai Date: Mon, 19 Jun 2017 23:11:54 +0200 Subject: ALSA: pcm: Add an ioctl to specify the supported protocol version We have an ioctl to inform the PCM protocol version the running kernel supports, but there is no way to know which protocol version the user-space can understand. This lack of information caused headaches in the past when we tried to extend the ABI. For example, because we couldn't guarantee the validity of the reserved bytes, we had to introduce a new ioctl SNDRV_PCM_IOCTL_STATUS_EXT for assigning a few new fields in the formerly reserved bits. If we could know that it's a new alsa-lib, we could assume the availability of the new fields, thus we could have reused the existing SNDRV_PCM_IOCTL_STATUS. In order to improve the ABI extensibility, this patch adds a new ioctl for user-space to inform its supporting protocol version to the kernel. By reporting the supported protocol from user-space, the kernel can judge which feature should be provided and which not. With the addition of the new ioctl, the PCM protocol version is bumped to 2.0.14, too. User-space checks the kernel protocol version via SNDRV_PCM_INFO_PVERSION, then it sets the supported version back via SNDRV_PCM_INFO_USER_PVERSION. Reviewed-by: Takashi Sakamoto Signed-off-by: Takashi Iwai --- sound/core/pcm_native.c | 7 +++++++ 1 file changed, 7 insertions(+) (limited to 'sound/core/pcm_native.c') diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c index 9ade0c8b54a3..1c53d93e68f2 100644 --- a/sound/core/pcm_native.c +++ b/sound/core/pcm_native.c @@ -2770,6 +2770,8 @@ static int snd_pcm_common_ioctl(struct file *file, struct snd_pcm_substream *substream, unsigned int cmd, void __user *arg) { + struct snd_pcm_file *pcm_file = file->private_data; + switch (cmd) { case SNDRV_PCM_IOCTL_PVERSION: return put_user(SNDRV_PCM_VERSION, (int __user *)arg) ? -EFAULT : 0; @@ -2779,6 +2781,11 @@ static int snd_pcm_common_ioctl(struct file *file, return 0; case SNDRV_PCM_IOCTL_TTSTAMP: return snd_pcm_tstamp(substream, arg); + case SNDRV_PCM_IOCTL_USER_PVERSION: + if (get_user(pcm_file->user_pversion, + (unsigned int __user *)arg)) + return -EFAULT; + return 0; case SNDRV_PCM_IOCTL_HW_REFINE: return snd_pcm_hw_refine_user(substream, arg); case SNDRV_PCM_IOCTL_HW_PARAMS: -- cgit From b602aa8eb1a0f52f0f9a09728b3b1c9133136656 Mon Sep 17 00:00:00 2001 From: Takashi Iwai Date: Tue, 27 Jun 2017 11:54:37 +0200 Subject: ALSA: pcm: Disable only control mmap for explicit appl_ptr sync Now that user-space (typically alsa-lib) can specify which protocol version it supports, we can optimize the kernel code depending on the reported protocol version. In this patch, we change the previous hack for enforcing the appl_ptr sync by disabling status/control mmap. Instead of forcibly disabling both mmaps, we disable only the control mmap when user-space declares the supported protocol version new enough. For older user-space, still both PCM status and control mmaps are disabled when requested by the driver due to the compatibility reason. Reviewed-by: Takashi Sakamoto Signed-off-by: Takashi Iwai --- sound/core/pcm_native.c | 22 +++++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-) (limited to 'sound/core/pcm_native.c') diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c index 1c53d93e68f2..0d1834310531 100644 --- a/sound/core/pcm_native.c +++ b/sound/core/pcm_native.c @@ -3388,12 +3388,23 @@ static bool pcm_status_mmap_allowed(struct snd_pcm_file *pcm_file) { if (pcm_file->no_compat_mmap) return false; - /* Disallow the status/control mmap when SYNC_APPLPTR flag is set; + /* See pcm_control_mmap_allowed() below. + * Since older alsa-lib requires both status and control mmaps to be + * coupled, we have to disable the status mmap for old alsa-lib, too. + */ + if (pcm_file->user_pversion < SNDRV_PROTOCOL_VERSION(2, 0, 14) && + (pcm_file->substream->runtime->hw.info & SNDRV_PCM_INFO_SYNC_APPLPTR)) + return false; + return true; +} + +static bool pcm_control_mmap_allowed(struct snd_pcm_file *pcm_file) +{ + if (pcm_file->no_compat_mmap) + return false; + /* Disallow the control mmap when SYNC_APPLPTR flag is set; * it enforces the user-space to fall back to snd_pcm_sync_ptr(), * thus it effectively assures the manual update of appl_ptr. - * In theory, it should be enough to disallow only PCM control mmap, - * but since the current alsa-lib implementation requires both status - * and control mmaps always paired, we have to disable both of them. */ if (pcm_file->substream->runtime->hw.info & SNDRV_PCM_INFO_SYNC_APPLPTR) return false; @@ -3405,6 +3416,7 @@ static bool pcm_status_mmap_allowed(struct snd_pcm_file *pcm_file) * don't support mmap for status and control records. */ #define pcm_status_mmap_allowed(pcm_file) false +#define pcm_control_mmap_allowed(pcm_file) false static int snd_pcm_mmap_status(struct snd_pcm_substream *substream, struct file *file, struct vm_area_struct *area) @@ -3593,7 +3605,7 @@ static int snd_pcm_mmap(struct file *file, struct vm_area_struct *area) return -ENXIO; return snd_pcm_mmap_status(substream, file, area); case SNDRV_PCM_MMAP_OFFSET_CONTROL: - if (!pcm_status_mmap_allowed(pcm_file)) + if (!pcm_control_mmap_allowed(pcm_file)) return -ENXIO; return snd_pcm_mmap_control(substream, file, area); default: -- cgit