[alsa-devel] [PATCH 0/2] ALSA: pcm: trace selection process of hardware parameters

Hi,
This patchset is a revised version of my former RFC.
[alsa-devel] [PATCH RFC 00/21] ALSA: pcm: add tracepoints for PCM params operation http://mailman.alsa-project.org/pipermail/alsa-devel/2017-May/120548.html
Unlike SNDRV_PCM_IOCTL_HW_REFINE, returned data from invocation of ioctl(2) with SNDRV_PCM_IOCTL_HW_PARAMS includes single value for below parameters:
* 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
This is an additional selection process of hardware parameters in service routine of the command.
This patchset adds tracepoint events to probe the selection process so that developers can realize final set of parameters by tracing data.
Below is a sample. The first three entries are final applications of constraint rules. The rest is newly added by this patchset.
$ trace-cmd report ... hw_interval_param: pcmC0D0p 021/023 PERIOD_SIZE 0 1 [1000 1000] 0 1 [1000 1000] hw_interval_param: pcmC0D0p 022/023 BUFFER_BYTES 0 1 [8000 8000] 0 1 [8000 8000] hw_interval_param: pcmC0D0p 023/023 RATE 0 1 [8000 8000] 0 1 [8000 8000] hw_mask_param: pcmC0D0p 000/023 ACCESS 00000000000000000000000000000001 00000000000000000000000000000001 hw_mask_param: pcmC0D0p 000/023 FORMAT 00000000000000000000000000000004 00000000000000000000000000000004 hw_mask_param: pcmC0D0p 000/023 SUBFORMAT 00000000000000000000000000000001 00000000000000000000000000000001 hw_interval_param: pcmC0D0p 000/023 CHANNELS 0 1 [1 1] 0 1 [1 1] hw_interval_param: pcmC0D0p 000/023 RATE 0 1 [8000 8000] 0 1 [8000 8000] hw_interval_param: pcmC0D0p 000/023 PERIOD_TIME 0 1 [125000 125000] 0 1 [125000 125000] hw_interval_param: pcmC0D0p 000/023 BUFFER_SIZE 0 1 [4000 4000] 0 1 [4000 4000] hw_interval_param: pcmC0D0p 000/023 TICK_TIME 0 1 [0 0] 0 1 [0 0]
Takashi Sakamoto (3): ALSA: pcm: localize snd_pcm_hw_params_choose() ALSA: pcm: add tracepoints for final selection process of hardware parameters ALSA: pcm: use friendly name for id of PCM substream in trace print
sound/core/pcm_lib.c | 40 ---------------------------- sound/core/pcm_local.h | 3 --- sound/core/pcm_native.c | 62 ++++++++++++++++++++++++++++++++++++++++++++ sound/core/pcm_param_trace.h | 8 +++--- 4 files changed, 66 insertions(+), 47 deletions(-)

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 o-takashi@sakamocchi.jp --- sound/core/pcm_lib.c | 40 ---------------------------------------- sound/core/pcm_local.h | 3 --- sound/core/pcm_native.c | 40 ++++++++++++++++++++++++++++++++++++++++ 3 files changed, 40 insertions(+), 43 deletions(-)
diff --git a/sound/core/pcm_lib.c b/sound/core/pcm_lib.c index 95b8ef15029f..9dc7bbfe8853 100644 --- a/sound/core/pcm_lib.c +++ b/sound/core/pcm_lib.c @@ -1700,46 +1700,6 @@ int snd_pcm_hw_param_last(struct snd_pcm_substream *pcm,
EXPORT_SYMBOL(snd_pcm_hw_param_last);
-/** - * 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. - */ -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_lib_ioctl_reset(struct snd_pcm_substream *substream, void *arg) { diff --git a/sound/core/pcm_local.h b/sound/core/pcm_local.h index 34c66decaaf2..e4bf2af62b02 100644 --- a/sound/core/pcm_local.h +++ b/sound/core/pcm_local.h @@ -24,9 +24,6 @@ void snd_interval_mulkdiv(const struct snd_interval *a, unsigned int k, int snd_pcm_hw_constraints_init(struct snd_pcm_substream *substream); int snd_pcm_hw_constraints_complete(struct snd_pcm_substream *substream);
-int snd_pcm_hw_params_choose(struct snd_pcm_substream *substream, - struct snd_pcm_hw_params *params); - int snd_pcm_hw_constraint_mask(struct snd_pcm_runtime *runtime, snd_pcm_hw_param_t var, u_int32_t mask);
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) {

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 o-takashi@sakamocchi.jp --- sound/core/pcm_native.c | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+)
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; }

Use the same print format of snd_pcm_debug_name() for userspace tracing program.
Suggested-by: Takashi Iwai tiwai@suse.de Signed-off-by: Takashi Sakamoto o-takashi@sakamocchi.jp --- sound/core/pcm_param_trace.h | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/sound/core/pcm_param_trace.h b/sound/core/pcm_param_trace.h index 872922326b38..86c8d658a25c 100644 --- a/sound/core/pcm_param_trace.h +++ b/sound/core/pcm_param_trace.h @@ -49,11 +49,11 @@ TRACE_EVENT(hw_mask_param, memcpy(__entry->prev_bits, prev->bits, sizeof(__u32) * 8); memcpy(__entry->curr_bits, curr->bits, sizeof(__u32) * 8); ), - TP_printk("%d,%d,%d,%d %03d/%03d %s %08x%08x%08x%08x %08x%08x%08x%08x", + TP_printk("pcmC%dD%d%s:%d %03d/%03d %s %08x%08x%08x%08x %08x%08x%08x%08x", __entry->card, __entry->device, + __entry->direction ? "c" : "p", __entry->subdevice, - __entry->direction, __entry->index, __entry->total, __print_symbolic(__entry->type, hw_param_labels), @@ -109,11 +109,11 @@ TRACE_EVENT(hw_interval_param, __entry->curr_integer = curr->integer; __entry->curr_empty = curr->empty; ), - TP_printk("%d,%d,%d,%d %03d/%03d %s %d %d %s%u %u%s %d %d %s%u %u%s", + TP_printk("pcmC%dD%d%s:%d %03d/%03d %s %d %d %s%u %u%s %d %d %s%u %u%s", __entry->card, __entry->device, + __entry->direction ? "c" : "p", __entry->subdevice, - __entry->direction, __entry->index, __entry->total, __print_symbolic(__entry->type, hw_param_labels),

On Fri, 09 Jun 2017 14:46:47 +0200, Takashi Sakamoto wrote:
Hi,
This patchset is a revised version of my former RFC.
[alsa-devel] [PATCH RFC 00/21] ALSA: pcm: add tracepoints for PCM params operation http://mailman.alsa-project.org/pipermail/alsa-devel/2017-May/120548.html
Unlike SNDRV_PCM_IOCTL_HW_REFINE, returned data from invocation of ioctl(2) with SNDRV_PCM_IOCTL_HW_PARAMS includes single value for below parameters:
- 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
This is an additional selection process of hardware parameters in service routine of the command.
This patchset adds tracepoint events to probe the selection process so that developers can realize final set of parameters by tracing data.
Below is a sample. The first three entries are final applications of constraint rules. The rest is newly added by this patchset.
$ trace-cmd report ... hw_interval_param: pcmC0D0p 021/023 PERIOD_SIZE 0 1 [1000 1000] 0 1 [1000 1000] hw_interval_param: pcmC0D0p 022/023 BUFFER_BYTES 0 1 [8000 8000] 0 1 [8000 8000] hw_interval_param: pcmC0D0p 023/023 RATE 0 1 [8000 8000] 0 1 [8000 8000] hw_mask_param: pcmC0D0p 000/023 ACCESS 00000000000000000000000000000001 00000000000000000000000000000001 hw_mask_param: pcmC0D0p 000/023 FORMAT 00000000000000000000000000000004 00000000000000000000000000000004 hw_mask_param: pcmC0D0p 000/023 SUBFORMAT 00000000000000000000000000000001 00000000000000000000000000000001 hw_interval_param: pcmC0D0p 000/023 CHANNELS 0 1 [1 1] 0 1 [1 1] hw_interval_param: pcmC0D0p 000/023 RATE 0 1 [8000 8000] 0 1 [8000 8000] hw_interval_param: pcmC0D0p 000/023 PERIOD_TIME 0 1 [125000 125000] 0 1 [125000 125000] hw_interval_param: pcmC0D0p 000/023 BUFFER_SIZE 0 1 [4000 4000] 0 1 [4000 4000] hw_interval_param: pcmC0D0p 000/023 TICK_TIME 0 1 [0 0] 0 1 [0 0]
Takashi Sakamoto (3): ALSA: pcm: localize snd_pcm_hw_params_choose() ALSA: pcm: add tracepoints for final selection process of hardware parameters ALSA: pcm: use friendly name for id of PCM substream in trace print
Applied all three patches (although the cover letter says two :)
thanks,
Takashi
participants (2)
-
Takashi Iwai
-
Takashi Sakamoto