Re: [igt-dev] [PATCH i-g-t] audio: Avoid test_integrity failing if there's no HW support

Ser, Simon
2019-04-23T11:45:09Z

On Tue, 2019-04-23 at 09:09 +0100, Chris Wilson wrote:
> Before asserting that the HDMI audio loopback works after resume, first
> require it to be present before suspend -- as not all machines are setup
> with the right HDMI audio capture.
>
> More strictly, one would expect that an audio capture device setup
> before suspend will remain functional after resume. An exercise left for
> the reader.

What if audio works without suspend, but breaks with suspend? That new
test makes it less easy to understand whether audio works at all or if
it breaks because of suspend.

Side note, I wouldn't bother too much with patches since all the audio
tests will get replaced by Chamelium audio tests (see my
"tests/kms_chamelium: add dp-audio test" series).

> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=110499
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Simon Ser <simon.ser@intel.com>
> Cc: Paul Kocialkowski <paul.kocialkowski@linux.intel.com>
> ---
> tests/audio.c | 18 +++++++-----------
> 1 file changed, 7 insertions(+), 11 deletions(-)
>
> diff --git a/tests/audio.c b/tests/audio.c
> index 560876a33..28d18fb29 100644
> --- a/tests/audio.c
> +++ b/tests/audio.c
> @@ -94,7 +94,7 @@ static int input_callback(void *data, short *buffer, int frames)
> return 0;
> }
>
> -static void test_integrity(const char *device_name)
> +static bool test_integrity(const char *device_name)
> {
> struct test_data data;
> int sampling_rate;
> @@ -150,39 +150,35 @@ static void test_integrity(const char *device_name)
> data.streak = 0;
>
> ret = alsa_run(data.alsa, RUN_TIMEOUT);
> - igt_assert(ret > 0);
>
> audio_signal_clean(data.signal);
> free(data.signal);
>
> alsa_close_output(data.alsa);
>
> - run = true;
> + run = ret > 0;
> }
>
> - /* Make sure we tested at least one frequency */
> - igt_assert(run);
> -
> alsa_close_input(data.alsa);
> free(data.alsa);
> +
> + return run;
> }
>
> static void test_suspend_resume_integrity(const char *device_name,
> enum igt_suspend_state state,
> enum igt_suspend_test test)
> {
> - test_integrity(device_name);
> + /* Make sure we tested at least one frequency */
> + igt_require(test_integrity(device_name));
>
> igt_system_suspend_autoresume(state, test);
>
> - test_integrity(device_name);
> + igt_assert(test_integrity(device_name));
> }
>
> igt_main
> {
> - igt_subtest("hdmi-integrity")
> - test_integrity("HDMI");
> -
> igt_subtest("hdmi-integrity-after-suspend")
> test_suspend_resume_integrity("HDMI", SUSPEND_STATE_MEM,
> SUSPEND_TEST_NONE);