This series provides a bunch of quick updates which should make the
coverage from pcm-test a bit more useful, it adds some support for
skipping tests when the hardware/driver is unable to support the
requested configuration, support for providing user visible descriptions
and then expands the set of cases we cover to include more sample rates
and channel counts. This should exercise switching between 8kHz and
44.1kHz based rates and ensure that clocking doesn't get confused by
non-stereo channel counts, both of which are I expect common real world
errors, at least for embedded cards.
v4:
- Rebase onto v6.2-rc1.
v3:
- "Rebase" onto Takashi's current tree (with a revert).
- Include Jaroslav's changes to specify all tests in the configuration
file parsing.
- Add a new "description" field to the configuration instead of trying
to name the tests.
- Always run both default and per-system tests, logging our success at
setting the per-system configurations as a separate test since they
shouldn't fail.
v2:
- Rebase onto Takashi's current tree.
- Tweak the buffer sizes for the newly added cases, don't be quite
so ambitious in how big a buffer we request for 96kHz and don't
go quite so small for 8kHz since some devices start hitting lower
limits on period size and struggle to deliver accurate timing.
To: Takashi Iwai <[email protected]>
To: Jaroslav Kysela <[email protected]>
To: Shuah Khan <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Signed-off-by: Mark Brown <[email protected]>
---
Jaroslav Kysela (1):
kselftest/alsa: pcm - move more configuration to configuration files
Mark Brown (6):
kselftest/alsa: pcm - Drop recent coverage improvement changes
kselftest/alsa: pcm - Always run the default set of tests
kselftest/alsa: pcm - skip tests when we fail to set params
kselftest/alsa: pcm - Support optional description for tests
kselftest/alsa: pcm - Provide descriptions for the default tests
kselftest/alsa: pcm - Add more coverage by default
tools/testing/selftests/alsa/Makefile | 2 +-
tools/testing/selftests/alsa/alsa-local.h | 3 +
tools/testing/selftests/alsa/conf.c | 26 ++-
.../alsa/conf.d/Lenovo_ThinkPad_P1_Gen2.conf | 43 +++--
tools/testing/selftests/alsa/pcm-test.c | 205 ++++++++++++++-------
tools/testing/selftests/alsa/pcm-test.conf | 63 +++++++
6 files changed, 250 insertions(+), 92 deletions(-)
---
base-commit: 1b929c02afd37871d5afb9d498426f83432e71c2
change-id: 20221208-alsa-pcm-test-hacks-f6c1aa76bd2c
Best regards,
--
Mark Brown <[email protected]>
In preparation to adopting a better, more comprehensive approach to
adding the coverage that was just added using some changes from Jaroslav
which were sent at the same time the recently added improvements were
being applied drop what was applied. This reverts:
7d721baea138 "kselftest/alsa: Add more coverage of sample rates and channel counts"
ee12040dd53a "kselftest/alsa: Provide more meaningful names for tests"
ae95efd9754c "kselftest/alsa: Don't any configuration in the sample config"
8370d9b00c92 Revert "kselftest/alsa: Report failures to set the requested channels as skips"
f944f8b539ea "kselftest/alsa: Report failures to set the requested sample rate as skips"
22eeb8f531c1 "kselftest/alsa: Refactor pcm-test to list the tests to run in a struct"
Reviewed-by: Jaroslav Kysela <[email protected]>
Signed-off-by: Mark Brown <[email protected]>
---
.../alsa/conf.d/Lenovo_ThinkPad_P1_Gen2.conf | 35 ++++-----
tools/testing/selftests/alsa/pcm-test.c | 88 +++++++------------
---
.../alsa/conf.d/Lenovo_ThinkPad_P1_Gen2.conf | 35 ++++-----
tools/testing/selftests/alsa/pcm-test.c | 88 +++++++---------------
2 files changed, 42 insertions(+), 81 deletions(-)
diff --git a/tools/testing/selftests/alsa/conf.d/Lenovo_ThinkPad_P1_Gen2.conf b/tools/testing/selftests/alsa/conf.d/Lenovo_ThinkPad_P1_Gen2.conf
index 9eca985e0c08..0a83f35d43eb 100644
--- a/tools/testing/selftests/alsa/conf.d/Lenovo_ThinkPad_P1_Gen2.conf
+++ b/tools/testing/selftests/alsa/conf.d/Lenovo_ThinkPad_P1_Gen2.conf
@@ -39,25 +39,22 @@ card.hda {
#
pcm.0.0 {
PLAYBACK {
- #
- # Uncomment to override values for specific tests
- #
- #test_name1 {
- # access RW_INTERLEAVED
- # format S16_LE
- # rate 48000
- # channels 2
- # period_size 512
- # buffer_size 4096
- #}
- #test_name2 {
- # access RW_INTERLEAVED
- # format S16_LE
- # rate 48000
- # channels 2
- # period_size 24000
- # buffer_size 192000
- #}
+ test.time1 {
+ access RW_INTERLEAVED # can be omitted - default
+ format S16_LE # can be omitted - default
+ rate 48000 # can be omitted - default
+ channels 2 # can be omitted - default
+ period_size 512
+ buffer_size 4096
+ }
+ test.time2 {
+ access RW_INTERLEAVED
+ format S16_LE
+ rate 48000
+ channels 2
+ period_size 24000
+ buffer_size 192000
+ }
}
CAPTURE {
# use default tests, check for the presence
diff --git a/tools/testing/selftests/alsa/pcm-test.c b/tools/testing/selftests/alsa/pcm-test.c
index f293c7d81009..6e7dfc395b98 100644
--- a/tools/testing/selftests/alsa/pcm-test.c
+++ b/tools/testing/selftests/alsa/pcm-test.c
@@ -37,15 +37,6 @@ struct pcm_data *pcm_list = NULL;
int num_missing = 0;
struct pcm_data *pcm_missing = NULL;
-struct time_test_def {
- const char *cfg_prefix;
- const char *format;
- long rate;
- long channels;
- long period_size;
- long buffer_size;
-};
-
void timestamp_now(timestamp_t *tstamp)
{
if (clock_gettime(CLOCK_MONOTONIC_RAW, tstamp))
@@ -229,7 +220,9 @@ static void find_pcms(void)
}
static void test_pcm_time1(struct pcm_data *data,
- const struct time_test_def *test)
+ const char *cfg_prefix, const char *sformat,
+ long srate, long schannels,
+ long speriod_size, long sbuffer_size)
{
char name[64], key[128], msg[256];
const char *cs;
@@ -241,32 +234,30 @@ static void test_pcm_time1(struct pcm_data *data,
snd_pcm_sframes_t frames;
long long ms;
long rate, channels, period_size, buffer_size;
- unsigned int rchannels;
unsigned int rrate;
snd_pcm_uframes_t rperiod_size, rbuffer_size, start_threshold;
timestamp_t tstamp;
bool pass = false, automatic = true;
snd_pcm_hw_params_t *hw_params;
snd_pcm_sw_params_t *sw_params;
- bool skip = false;
snd_pcm_hw_params_alloca(&hw_params);
snd_pcm_sw_params_alloca(&sw_params);
- cs = conf_get_string(data->pcm_config, test->cfg_prefix, "format", test->format);
+ cs = conf_get_string(data->pcm_config, cfg_prefix, "format", sformat);
format = snd_pcm_format_value(cs);
if (format == SND_PCM_FORMAT_UNKNOWN)
ksft_exit_fail_msg("Wrong format '%s'\n", cs);
- rate = conf_get_long(data->pcm_config, test->cfg_prefix, "rate", test->rate);
- channels = conf_get_long(data->pcm_config, test->cfg_prefix, "channels", test->channels);
- period_size = conf_get_long(data->pcm_config, test->cfg_prefix, "period_size", test->period_size);
- buffer_size = conf_get_long(data->pcm_config, test->cfg_prefix, "buffer_size", test->buffer_size);
+ rate = conf_get_long(data->pcm_config, cfg_prefix, "rate", srate);
+ channels = conf_get_long(data->pcm_config, cfg_prefix, "channels", schannels);
+ period_size = conf_get_long(data->pcm_config, cfg_prefix, "period_size", speriod_size);
+ buffer_size = conf_get_long(data->pcm_config, cfg_prefix, "buffer_size", sbuffer_size);
- automatic = strcmp(test->format, snd_pcm_format_name(format)) == 0 &&
- test->rate == rate &&
- test->channels == channels &&
- test->period_size == period_size &&
- test->buffer_size == buffer_size;
+ automatic = strcmp(sformat, snd_pcm_format_name(format)) == 0 &&
+ srate == rate &&
+ schannels == channels &&
+ speriod_size == period_size &&
+ sbuffer_size == buffer_size;
samples = malloc((rate * channels * snd_pcm_format_physical_width(format)) / 8);
if (!samples)
@@ -302,7 +293,7 @@ static void test_pcm_time1(struct pcm_data *data,
if (automatic && format == SND_PCM_FORMAT_S16_LE) {
format = SND_PCM_FORMAT_S32_LE;
ksft_print_msg("%s.%d.%d.%d.%s.%s format S16_LE -> S32_LE\n",
- test->cfg_prefix,
+ cfg_prefix,
data->card, data->device, data->subdevice,
snd_pcm_stream_name(data->stream),
snd_pcm_access_name(access));
@@ -311,17 +302,11 @@ static void test_pcm_time1(struct pcm_data *data,
snd_pcm_format_name(format), snd_strerror(err));
goto __close;
}
- rchannels = channels;
- err = snd_pcm_hw_params_set_channels_near(handle, hw_params, &rchannels);
+ err = snd_pcm_hw_params_set_channels(handle, hw_params, channels);
if (err < 0) {
snprintf(msg, sizeof(msg), "snd_pcm_hw_params_set_channels %ld: %s", channels, snd_strerror(err));
goto __close;
}
- if (rchannels != channels) {
- snprintf(msg, sizeof(msg), "channels unsupported %ld != %ld", channels, rchannels);
- skip = true;
- goto __close;
- }
rrate = rate;
err = snd_pcm_hw_params_set_rate_near(handle, hw_params, &rrate, 0);
if (err < 0) {
@@ -329,8 +314,7 @@ static void test_pcm_time1(struct pcm_data *data,
goto __close;
}
if (rrate != rate) {
- snprintf(msg, sizeof(msg), "rate unsupported %ld != %ld", rate, rrate);
- skip = true;
+ snprintf(msg, sizeof(msg), "rate mismatch %ld != %ld", rate, rrate);
goto __close;
}
rperiod_size = period_size;
@@ -378,7 +362,7 @@ static void test_pcm_time1(struct pcm_data *data,
}
ksft_print_msg("%s.%d.%d.%d.%s hw_params.%s.%s.%ld.%ld.%ld.%ld sw_params.%ld\n",
- test->cfg_prefix,
+ cfg_prefix,
data->card, data->device, data->subdevice,
snd_pcm_stream_name(data->stream),
snd_pcm_access_name(access),
@@ -426,40 +410,21 @@ static void test_pcm_time1(struct pcm_data *data,
msg[0] = '\0';
pass = true;
__close:
- if (!skip) {
- ksft_test_result(pass, "%s.%d.%d.%d.%s%s%s\n",
- test->cfg_prefix,
- data->card, data->device, data->subdevice,
- snd_pcm_stream_name(data->stream),
- msg[0] ? " " : "", msg);
- } else {
- ksft_test_result_skip("%s.%d.%d.%d.%s%s%s\n",
- test->cfg_prefix,
- data->card, data->device,
- data->subdevice,
- snd_pcm_stream_name(data->stream),
- msg[0] ? " " : "", msg);
- }
+ ksft_test_result(pass, "%s.%d.%d.%d.%s%s%s\n",
+ cfg_prefix,
+ data->card, data->device, data->subdevice,
+ snd_pcm_stream_name(data->stream),
+ msg[0] ? " " : "", msg);
free(samples);
if (handle)
snd_pcm_close(handle);
}
-static const struct time_test_def time_tests[] = {
- /* name format rate chan period buffer */
- { "8k.1.big", "S16_LE", 8000, 2, 8000, 32000 },
- { "8k.2.big", "S16_LE", 8000, 2, 8000, 32000 },
- { "44k1.2.big", "S16_LE", 44100, 2, 22050, 192000 },
- { "48k.2.small", "S16_LE", 48000, 2, 512, 4096 },
- { "48k.2.big", "S16_LE", 48000, 2, 24000, 192000 },
- { "48k.6.big", "S16_LE", 48000, 6, 48000, 576000 },
- { "96k.2.big", "S16_LE", 96000, 2, 48000, 192000 },
-};
+#define TESTS_PER_PCM 2
int main(void)
{
struct pcm_data *pcm;
- int i;
ksft_print_header();
@@ -467,7 +432,7 @@ int main(void)
find_pcms();
- ksft_set_plan(num_missing + num_pcms * ARRAY_SIZE(time_tests));
+ ksft_set_plan(num_missing + num_pcms * TESTS_PER_PCM);
for (pcm = pcm_missing; pcm != NULL; pcm = pcm->next) {
ksft_test_result(false, "test.missing.%d.%d.%d.%s\n",
@@ -476,9 +441,8 @@ int main(void)
}
for (pcm = pcm_list; pcm != NULL; pcm = pcm->next) {
- for (i = 0; i < ARRAY_SIZE(time_tests); i++) {
- test_pcm_time1(pcm, &time_tests[i]);
- }
+ test_pcm_time1(pcm, "test.time1", "S16_LE", 48000, 2, 512, 4096);
+ test_pcm_time1(pcm, "test.time2", "S16_LE", 48000, 2, 24000, 192000);
}
conf_free();
--
2.30.2
On Tue, 27 Dec 2022 18:06:46 +0100,
Mark Brown wrote:
>
> This series provides a bunch of quick updates which should make the
> coverage from pcm-test a bit more useful, it adds some support for
> skipping tests when the hardware/driver is unable to support the
> requested configuration, support for providing user visible descriptions
> and then expands the set of cases we cover to include more sample rates
> and channel counts. This should exercise switching between 8kHz and
> 44.1kHz based rates and ensure that clocking doesn't get confused by
> non-stereo channel counts, both of which are I expect common real world
> errors, at least for embedded cards.
>
> v4:
> - Rebase onto v6.2-rc1.
> v3:
> - "Rebase" onto Takashi's current tree (with a revert).
> - Include Jaroslav's changes to specify all tests in the configuration
> file parsing.
> - Add a new "description" field to the configuration instead of trying
> to name the tests.
> - Always run both default and per-system tests, logging our success at
> setting the per-system configurations as a separate test since they
> shouldn't fail.
> v2:
> - Rebase onto Takashi's current tree.
> - Tweak the buffer sizes for the newly added cases, don't be quite
> so ambitious in how big a buffer we request for 96kHz and don't
> go quite so small for 8kHz since some devices start hitting lower
> limits on period size and struggle to deliver accurate timing.
>
> To: Takashi Iwai <[email protected]>
> To: Jaroslav Kysela <[email protected]>
> To: Shuah Khan <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> Cc: [email protected]
> Signed-off-by: Mark Brown <[email protected]>
Applied all patches now to for-next branch.
thanks,
Takashi