Igt-dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Karthik B S <karthik.b.s@intel.com>
To: "B, Jeevan" <jeevan.b@intel.com>,
	"igt-dev@lists.freedesktop.org" <igt-dev@lists.freedesktop.org>
Cc: "Samala, Pranay" <pranay.samala@intel.com>
Subject: Re: [PATCH i-g-t] tests/chamelium/kms_chamelium_audio: Add suspend/resume audio tests
Date: Tue, 30 Jun 2026 13:16:40 +0530	[thread overview]
Message-ID: <e8711629-29a5-4074-9dea-722f3665806b@intel.com> (raw)
In-Reply-To: <DM4PR11MB63128ABA9CEB2F69C455FEDC90F72@DM4PR11MB6312.namprd11.prod.outlook.com>


On 6/30/2026 11:26 AM, B, Jeevan wrote:
>> -----Original Message-----
>> From: B S, Karthik <karthik.b.s@intel.com>
>> Sent: Tuesday, June 30, 2026 9:10 AM
>> To: B, Jeevan <jeevan.b@intel.com>; igt-dev@lists.freedesktop.org
>> Cc: Samala, Pranay <pranay.samala@intel.com>
>> Subject: Re: [PATCH i-g-t] tests/chamelium/kms_chamelium_audio: Add
>> suspend/resume audio tests
>>
>> Hi Jeevan,
>>
>> On 6/29/2026 5:31 PM, Jeevan B wrote:
>>> Add test to validate audio works after suspend/resume on HDMI/DP
>>> connector.
>>>
>>> v2: Perform a single suspend/resume for all audio combinations.
>>>       Re-enable chamelium after suspend and separate pre-suspend
>>>       and post-resume validation paths for clearer failure diagnostics.
>>> v3: Drop commit and use igt_assert_f with explicit messages to
>>>       distinguish failure in audio tests.
>>> v4: Updated test to avoid false-pass on zero executed configs.
>>>       Replace post-suspend forced re-enable with connector
>>>       re-probe/connected checks.
>>> v5: Remove unused variable and add chamelium_assert_reachable check.
>>> v6: Skip suspend/resume subtest when pre-suspend audio validation fails,
>>>       and update suspend subtest descriptions.
>>> v7: Update fixture handles cleanup regardless of skip/assert.
>>>
>>> Signed-off-by: Jeevan B <jeevan.b@intel.com>
>>> Reviewed-by: Pranay Samala <pranay.samala@intel.com>
>>> ---
>>>    tests/chamelium/kms_chamelium_audio.c  | 158
>> ++++++++++++++++++++++++-
>>>    tests/chamelium/kms_chamelium_helper.h |   3 +
>>>    2 files changed, 160 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/tests/chamelium/kms_chamelium_audio.c
>>> b/tests/chamelium/kms_chamelium_audio.c
>>> index df8d27c0c..e3c0e4b86 100644
>>> --- a/tests/chamelium/kms_chamelium_audio.c
>>> +++ b/tests/chamelium/kms_chamelium_audio.c
>>> @@ -52,6 +52,12 @@
>>>     * SUBTEST: hdmi-audio-edid
>>>     * Description: Plug a connector with an EDID suitable for audio, check
>> ALSA's
>>>     *              EDID-Like Data reports the correct audio parameters
>>> + *
>>> + * SUBTEST: dp-audio-after-suspend
>>> + * Description: Verify audio works after system suspend/resume
>>> + *
>>> + * SUBTEST: hdmi-audio-after-suspend
>>> + * Description: Verify audio works after system suspend/resume
>>>     */
>>>
>>>    /* Playback parameters control the audio signal we synthesize and
>>> send */ @@ -695,6 +701,66 @@ static bool
>> check_audio_configuration(struct alsa *alsa,
>>>    	return true;
>>>    }
>>>
>>> +static bool run_audio_test_once(chamelium_data_t *data,
>>> +				struct alsa *alsa,
>>> +				struct chamelium_port *port,
>>> +				snd_pcm_format_t format,
>>> +				int channels,
>>> +				int sampling_rate)
>>> +{
>>> +	struct audio_state state;
>>> +	bool success;
>>> +
>>> +	audio_state_init(&state, data, alsa, port, format, channels,
>>> +			 sampling_rate);
>>> +	success = test_audio_frequencies(&state);
>>> +	success &= test_audio_flatline(&state);
>>> +	audio_state_fini(&state);
>>> +
>>> +	return success;
>>> +}
>>> +
>>> +static bool run_audio_tests(chamelium_data_t *data,
>>> +			    struct alsa *alsa,
>>> +			    struct chamelium_port *port,
>>> +			    const char *audio_device)
>>> +{
>>> +	bool run = false;
>>> +	bool success = true;
>>> +	int ret, i, j;
>>> +	int channels, sampling_rate;
>>> +	snd_pcm_format_t format;
>>> +
>>> +	for (i = 0; i < test_sampling_rates_count; i++) {
>>> +		for (j = 0; j < test_formats_count; j++) {
>>> +			ret = alsa_open_output(alsa, audio_device);
>>> +			igt_assert_f(ret >= 0,
>>> +				     "Failed to open ALSA output\n");
>>> +
>>> +			format = test_formats[j];
>>> +			channels = PLAYBACK_CHANNELS;
>>> +			sampling_rate = test_sampling_rates[i];
>>> +
>>> +			if (!check_audio_configuration(alsa, format,
>>> +						       channels,
>>> +						       sampling_rate)) {
>>> +				alsa_close_output(alsa);
>>> +				continue;
>>> +			}
>>> +
>>> +			run = true;
>>> +
>>> +			success &= run_audio_test_once(data, alsa, port,
>>> +					      format, channels,
>>> +					      sampling_rate);
> Thanks for the review:-
>
> This is already being printed in audio_state_stop()
> "tests/chamelium/kms_chamelium_audio.c +1038"
> if (success)
>      log_level = IGT_LOG_DEBUG;
> else
>      log_level = IGT_LOG_CRITICAL;
>
> igt_log(IGT_LOG_DOMAIN, log_level,
>      "Audio %s test result for format %s, "
>      "sampling rate %d Hz and %d channels: %s\n",
>      state->name, snd_pcm_format_name(state->playback.format),
>      state->playback.rate, state->playback.channels,
>      success ? "ALL GREEN" : "FAILED");

Thanks for the clarification.

Regards,
Karthik.B.S
>
>> If we do need to run all the possible formats even if one of them fails, then
>> IMHO we at least need an igt_info/igt_debug calling out exactly what format
>> failed here?
>>> +
>>> +			alsa_close_output(alsa);
>>> +		}
>>> +	}
>>> +
>>> +	return run && success;
>>> +}
>>> +
>>>    static const char test_display_audio_desc[] =
>>>    	"Playback various audio signals with various audio formats/rates, "
>>>    	"capture them and check they are correct"; @@ -762,8 +828,10 @@
>>> static void test_display_audio(chamelium_data_t *data,
>>>    			sampling_rate = test_sampling_rates[i];
>>>
>>>    			if (!check_audio_configuration(alsa, format, channels,
>>> -						       sampling_rate))
>>> +						       sampling_rate)) {
>>> +				alsa_close_output(alsa);
>>>    				continue;
>>> +			}
>>>
>>>    			run = true;
>>>
>>> @@ -789,6 +857,75 @@ static void test_display_audio(chamelium_data_t
>> *data,
>>>    	free(alsa);
>>>    }
>>>
>>> +static const char test_display_audio_suspend_resume_desc[] =
>>> +	"Verify audio works after system suspend/resume"; static void
>>> +test_display_audio_suspend_resume(
>>> +				chamelium_data_t *data,
>>> +				struct chamelium_port *port,
>>> +				const char *audio_device,
>>> +				enum igt_custom_edid_type edid)
>>> +{
>>> +	struct alsa *alsa;
>>> +	igt_output_t *output;
>>> +	struct igt_fb fb;
>>> +	drmModeModeInfo *mode;
>>> +	drmModeConnector *connector;
>>> +	int fb_id;
>>> +
>>> +	igt_require(alsa_has_exclusive_access());
>>> +
>>> +	igt_require(chamelium_has_audio_support(data->chamelium,
>>> +						port));
>>> +
>>> +	alsa = alsa_init();
>>> +	igt_assert(alsa);
>>> +	data->audio_alsa = alsa;
>>> +
>>> +	igt_modeset_disable_all_outputs(&data->display);
>>> +	chamelium_reset_state(&data->display, data->chamelium,
>>> +			      port, data->ports,
>>> +			      data->port_count);
>>> +	output = chamelium_prepare_output(data, port, edid);
>>> +
>>> +	connector = chamelium_port_get_connector(data->chamelium,
>>> +						 port, false);
>>> +	data->audio_connector = connector;
>>> +	igt_assert(connector->count_modes > 0);
>>> +
>>> +	mode = &connector->modes[0];
>>> +
>>> +	fb_id = igt_create_color_pattern_fb(data->drm_fd,
>>> +					    mode->hdisplay,
>>> +					    mode->vdisplay,
>>> +					    DRM_FORMAT_XRGB8888,
>>> +					    DRM_FORMAT_MOD_LINEAR,
>>> +					    0, 0, 0, &fb);
>>> +	igt_assert(fb_id > 0);
>>> +	data->audio_fb = fb;
>>> +
>>> +	chamelium_enable_output(data, port, output,
>>> +				mode, &fb);
>>> +
>>> +	igt_skip_on_f(!run_audio_tests(data, alsa, port, audio_device),
>>> +		      "Audio test setup failed, skipping suspend/resume audio
>>> +test\n");
>>> +
>>> +	igt_system_suspend_autoresume(SUSPEND_STATE_MEM,
>>> +				      SUSPEND_TEST_NONE);
>>> +	chamelium_assert_reachable(data->chamelium, ONLINE_TIMEOUT);
>>> +
>>> +	/* Re-probe connector state after resume instead of forcing a re-
>> enable. */
>>> +	drmModeFreeConnector(connector);
>>> +	connector = chamelium_port_get_connector(data->chamelium, port,
>> true);
>>> +	data->audio_connector = connector;
>>> +	igt_assert_f(connector->connection == DRM_MODE_CONNECTED,
>>> +		     "Connector disconnected after suspend/resume\n");
>>> +	igt_assert_f(connector->count_modes > 0,
>>> +		     "No modes available after suspend/resume\n");
>>> +
>>> +	igt_assert_f(run_audio_tests(data, alsa, port, audio_device),
>>> +		     "Audio test failed after suspend\n"); }
>>> +
>>>    static const char test_display_audio_edid_desc[] =
>>>    	"Plug a connector with an EDID suitable for audio, check ALSA's "
>>>    	"EDID-Like Data reports the correct audio parameters"; @@ -854,6
>>> +991,9 @@ int igt_main()
>>>
>>>    	igt_fixture() {
>>>    		chamelium_init_test(&data);
>>> +		data.audio_alsa = NULL;
>>> +		data.audio_connector = NULL;
>>> +		data.audio_fb.fb_id = 0;
>>>    	}
>>>
>>>    	igt_describe("DisplayPort tests");
>>> @@ -866,6 +1006,11 @@ int igt_main()
>>>    	connector_subtest("dp-audio-edid", DisplayPort, &data,
>> test_display_audio_edid,
>>>    			  IGT_CUSTOM_EDID_DP_AUDIO);
>>>
>>> +	igt_describe(test_display_audio_suspend_resume_desc);
>>> +	connector_subtest("dp-audio-after-suspend", DisplayPort, &data,
>>> +			  test_display_audio_suspend_resume,
>>> +			  "HDMI", IGT_CUSTOM_EDID_DP_AUDIO);
>>> +
>>>    	igt_describe("HDMI tests");
>>>
>>>    	igt_describe(test_display_audio_desc);
>>> @@ -876,7 +1021,18 @@ int igt_main()
>>>    	connector_subtest("hdmi-audio-edid", HDMIA, &data,
>> test_display_audio_edid,
>>>    			  IGT_CUSTOM_EDID_HDMI_AUDIO);
>>>
>>> +	igt_describe(test_display_audio_suspend_resume_desc);
>>> +	connector_subtest("hdmi-audio-after-suspend", HDMIA, &data,
>>> +			  test_display_audio_suspend_resume,
>>> +			  "HDMI", IGT_CUSTOM_EDID_HDMI_AUDIO);
>>> +
>>>    	igt_fixture() {
>>> +		if (data.audio_fb.fb_id)
>> This check is redundant and already checked inside igt_remove_fb. Please
>> remove this.
>>
> Will remove this.
>
> Thanks
> Jeevan B
>> With the the above 2 updated, patch LGTM.
>>
>> Reviewed-by: Karthik B S <karthik.b.s@intel.com>
>>
>>> +			igt_remove_fb(data.drm_fd, &data.audio_fb);
>>> +		if (data.audio_connector)
>>> +			drmModeFreeConnector(data.audio_connector);
>>> +		free(data.audio_alsa);
>>> +
>>>    		igt_display_fini(&data.display);
>>>    		drm_close_driver(data.drm_fd);
>>>    	}
>>> diff --git a/tests/chamelium/kms_chamelium_helper.h
>>> b/tests/chamelium/kms_chamelium_helper.h
>>> index 606079666..65c6067db 100644
>>> --- a/tests/chamelium/kms_chamelium_helper.h
>>> +++ b/tests/chamelium/kms_chamelium_helper.h
>>> @@ -81,6 +81,9 @@ typedef struct {
>>>    	int drm_fd;
>>>
>>>    	struct chamelium_edid *edids[IGT_CUSTOM_EDID_COUNT];
>>> +	struct alsa *audio_alsa;
>>> +	struct igt_fb audio_fb;
>>> +	drmModeConnector *audio_connector;
>>>    } chamelium_data_t;
>>>
>>>    void chamelium_init_test(chamelium_data_t *data);

  reply	other threads:[~2026-06-30  7:47 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-06-29 12:01 [PATCH i-g-t] tests/chamelium/kms_chamelium_audio: Add suspend/resume audio tests Jeevan B
2026-06-29 23:32 ` ✓ i915.CI.BAT: success for tests/chamelium/kms_chamelium_audio: Add suspend/resume audio tests (rev9) Patchwork
2026-06-29 23:40 ` ✓ Xe.CI.BAT: " Patchwork
2026-06-30  3:40 ` [PATCH i-g-t] tests/chamelium/kms_chamelium_audio: Add suspend/resume audio tests Karthik B S
2026-06-30  5:56   ` B, Jeevan
2026-06-30  7:46     ` Karthik B S [this message]
2026-06-30 10:31 ` ✗ Xe.CI.FULL: failure for tests/chamelium/kms_chamelium_audio: Add suspend/resume audio tests (rev9) Patchwork
2026-06-30 10:42 ` ✗ i915.CI.Full: " Patchwork
  -- strict thread matches above, loose matches on Subject: below --
2026-06-23  8:27 [PATCH i-g-t] tests/chamelium/kms_chamelium_audio: Add suspend/resume audio tests Jeevan B
2026-06-29  6:08 ` Samala, Pranay
2026-06-19  5:38 Jeevan B
2026-06-23  8:02 ` Samala, Pranay
2026-06-11  3:52 Jeevan B
2026-05-13  6:35 Jeevan B
2026-05-11  4:18 Jeevan B
2026-05-11  6:22 ` Bilal, Mohammed
2026-05-05 18:03 Jeevan B
2026-05-07  5:13 ` Bilal, Mohammed

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=e8711629-29a5-4074-9dea-722f3665806b@intel.com \
    --to=karthik.b.s@intel.com \
    --cc=igt-dev@lists.freedesktop.org \
    --cc=jeevan.b@intel.com \
    --cc=pranay.samala@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox