All of 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.