[igt-dev] [PATCH i-g-t v3 00/10] tests/kms_chamelium: add pulse test

[igt-dev] [PATCH i-g-t v3 00/10] tests/kms_chamelium: add pulse test

[Simon Ser]

The series performs a refactoring of the current audio test to be able to
introduce a new one.

The new test checks both the amplitude and the channel alignment.

Changes from v2 to v3:
- Added one more patch to introduce CHAMELIUM_MAX_AUDIO_CHANNELS
- Addressed Martin's comments

Simon Ser (10):
  lib/igt_chamelium: introduce CHAMELIUM_MAX_AUDIO_CHANNELS
  tests/kms_chamelium: refactor audio test
  tests/kms_chamelium: introduce audio_state_receive
  tests/kms_chamelium: rename do_test_display_audio and
    test_audio_configuration
  tests/kms_chamelium: explain why 8-channel tests aren't performed
  lib/igt_audio: introduce audio_convert_to
  tests/kms_chamelium: add name parameter to audio_state_start
  lib/igt_audio: make audio_extract_channel_s32_le support a NULL dst
  tests/kms_chamelium: add a flatline audio test
  tests/kms_chamelium: add audio channel alignment test

 lib/igt_audio.c       |  98 ++++----
 lib/igt_audio.h       |  12 +-
 lib/igt_chamelium.c   |   8 +-
 lib/igt_chamelium.h   |   8 +-
 tests/kms_chamelium.c | 563 ++++++++++++++++++++++++++++++------------
 5 files changed, 473 insertions(+), 216 deletions(-)

--
2.21.0

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] [PATCH i-g-t v3 01/10] lib/igt_chamelium: introduce CHAMELIUM_MAX_AUDIO_CHANNELS

[Simon Ser]

This allows us not to hardcode 8 everywhere.

Signed-off-by: Simon Ser <simon.ser@intel.com>
---
 lib/igt_chamelium.c | 8 +++++---
 lib/igt_chamelium.h | 8 +++++++-
 2 files changed, 12 insertions(+), 4 deletions(-)

diff --git a/lib/igt_chamelium.c b/lib/igt_chamelium.c
index cfdf7617a65a..75f03d8469aa 100644
--- a/lib/igt_chamelium.c
+++ b/lib/igt_chamelium.c
@@ -1020,7 +1020,7 @@ bool chamelium_has_audio_support(struct chamelium *chamelium,
  */
 void chamelium_get_audio_channel_mapping(struct chamelium *chamelium,
 					 struct chamelium_port *port,
-					 int mapping[static 8])
+					 int mapping[static CHAMELIUM_MAX_AUDIO_CHANNELS])
 {
 	xmlrpc_value *res, *res_channel;
 	int res_len, i;
@@ -1028,7 +1028,7 @@ void chamelium_get_audio_channel_mapping(struct chamelium *chamelium,
 	res = chamelium_rpc(chamelium, port, "GetAudioChannelMapping", "(i)",
 			    port->id);
 	res_len = xmlrpc_array_size(&chamelium->env, res);
-	igt_assert(res_len == 8);
+	igt_assert(res_len == CHAMELIUM_MAX_AUDIO_CHANNELS);
 	for (i = 0; i < res_len; i++) {
 		xmlrpc_array_read_item(&chamelium->env, res, i, &res_channel);
 		xmlrpc_read_int(&chamelium->env, res_channel, &mapping[i]);
@@ -1058,8 +1058,10 @@ static void audio_format_from_xml(struct chamelium *chamelium,
 
 	if (rate)
 		xmlrpc_read_int(&chamelium->env, res_rate, rate);
-	if (channels)
+	if (channels) {
 		xmlrpc_read_int(&chamelium->env, res_channel, channels);
+		igt_assert(*channels <= CHAMELIUM_MAX_AUDIO_CHANNELS);
+	}
 
 	xmlrpc_DECREF(res_channel);
 	xmlrpc_DECREF(res_sample_format);
diff --git a/lib/igt_chamelium.h b/lib/igt_chamelium.h
index 33362b266ce4..28f726c91e83 100644
--- a/lib/igt_chamelium.h
+++ b/lib/igt_chamelium.h
@@ -65,6 +65,12 @@ struct chamelium_audio_file {
  */
 #define CHAMELIUM_DEFAULT_EDID 0
 
+/**
+ * CHAMELIUM_MAX_AUDIO_CHANNELS: the maximum number of audio capture channels
+ * supported by Chamelium.
+ */
+#define CHAMELIUM_MAX_AUDIO_CHANNELS 8
+
 struct chamelium *chamelium_init(int drm_fd);
 void chamelium_deinit(struct chamelium *chamelium);
 void chamelium_reset(struct chamelium *chamelium);
@@ -116,7 +122,7 @@ bool chamelium_has_audio_support(struct chamelium *chamelium,
 				 struct chamelium_port *port);
 void chamelium_get_audio_channel_mapping(struct chamelium *chamelium,
 					 struct chamelium_port *port,
-					 int mapping[static 8]);
+					 int mapping[static CHAMELIUM_MAX_AUDIO_CHANNELS]);
 void chamelium_get_audio_format(struct chamelium *chamelium,
 				struct chamelium_port *port,
 				int *rate, int *channels);
-- 
2.21.0

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] [PATCH i-g-t v3 02/10] tests/kms_chamelium: refactor audio test

[Simon Ser]

Instead of shoving everything into do_test_display_audio, extract the logic to
initialize, start, stop, finish an audio test in helper functions. The struct
audio_state now carries all audio-related state.

This will allow to easily add more audio tests that don't use sine waves (via
audio_signal). This is necessary for future delay and amplitude tests.

Signed-off-by: Simon Ser <simon.ser@intel.com>
Reviewed-by: Martin Peres <martin.peres@linux.intel.com>
---
 tests/kms_chamelium.c | 336 ++++++++++++++++++++++++------------------
 1 file changed, 195 insertions(+), 141 deletions(-)

diff --git a/tests/kms_chamelium.c b/tests/kms_chamelium.c
index 8da6ec20759e..27693681728e 100644
--- a/tests/kms_chamelium.c
+++ b/tests/kms_chamelium.c
@@ -812,17 +812,179 @@ static const snd_pcm_format_t test_formats[] = {
 static const size_t test_formats_count = sizeof(test_formats) / sizeof(test_formats[0]);
 
 struct audio_state {
+	struct alsa *alsa;
+	struct chamelium *chamelium;
+	struct chamelium_port *port;
+	struct chamelium_stream *stream;
+
+	/* The capture format is only available after capture has started. */
+	struct {
+		snd_pcm_format_t format;
+		int channels;
+		int rate;
+	} playback, capture;
+
 	struct audio_signal *signal;
-	snd_pcm_format_t format;
+	int channel_mapping[CHAMELIUM_MAX_AUDIO_CHANNELS];
+
+	int dump_fd;
+	char *dump_path;
+
+	pthread_t thread;
 	atomic_bool run;
 };
 
+static void audio_state_init(struct audio_state *state, data_t *data,
+			     struct alsa *alsa, struct chamelium_port *port,
+			     snd_pcm_format_t format, int channels, int rate)
+{
+	memset(state, 0, sizeof(*state));
+	state->dump_fd = -1;
+
+	state->alsa = alsa;
+	state->chamelium = data->chamelium;
+	state->port = port;
+
+	state->playback.format = format;
+	state->playback.channels = channels;
+	state->playback.rate = rate;
+
+	alsa_configure_output(alsa, format, channels, rate);
+
+	state->stream = chamelium_stream_init();
+	igt_assert(state->stream);
+}
+
+static void audio_state_fini(struct audio_state *state)
+{
+	chamelium_stream_deinit(state->stream);
+}
+
+static void *run_audio_thread(void *data)
+{
+	struct alsa *alsa = data;
+
+	alsa_run(alsa, -1);
+	return NULL;
+}
+
+static void audio_state_start(struct audio_state *state)
+{
+	int ret;
+	bool ok;
+	size_t i, j;
+	enum chamelium_stream_realtime_mode stream_mode;
+	char dump_suffix[64];
+
+	igt_debug("Starting test with playback format %s, sampling rate %d Hz "
+		  "and %d channels\n",
+		  snd_pcm_format_name(state->playback.format),
+		  state->playback.rate, state->playback.channels);
+
+	chamelium_start_capturing_audio(state->chamelium, state->port, false);
+
+	stream_mode = CHAMELIUM_STREAM_REALTIME_STOP_WHEN_OVERFLOW;
+	ok = chamelium_stream_dump_realtime_audio(state->stream, stream_mode);
+	igt_assert(ok);
+
+	/* Start playing audio */
+	state->run = true;
+	ret = pthread_create(&state->thread, NULL,
+			     run_audio_thread, state->alsa);
+	igt_assert(ret == 0);
+
+	/* The Chamelium device only supports this PCM format. */
+	state->capture.format = SND_PCM_FORMAT_S32_LE;
+
+	/* Only after we've started playing audio, we can retrieve the capture
+	 * format used by the Chamelium device. */
+	chamelium_get_audio_format(state->chamelium, state->port,
+				   &state->capture.rate,
+				   &state->capture.channels);
+	if (state->capture.rate == 0) {
+		igt_debug("Audio receiver doesn't indicate the capture "
+			 "sampling rate, assuming it's %d Hz\n",
+			 state->playback.rate);
+		state->capture.rate = state->playback.rate;
+	}
+
+	chamelium_get_audio_channel_mapping(state->chamelium, state->port,
+					    state->channel_mapping);
+	/* Make sure we can capture all channels we send. */
+	for (i = 0; i < state->playback.channels; i++) {
+		ok = false;
+		for (j = 0; j < state->capture.channels; j++) {
+			if (state->channel_mapping[j] == i) {
+				ok = true;
+				break;
+			}
+		}
+		igt_assert(ok);
+	}
+
+	if (igt_frame_dump_is_enabled()) {
+		snprintf(dump_suffix, sizeof(dump_suffix),
+			 "capture-%s-%dch-%dHz",
+			 snd_pcm_format_name(state->playback.format),
+			 state->playback.channels, state->playback.rate);
+
+		state->dump_fd = audio_create_wav_file_s32_le(dump_suffix,
+							      state->capture.rate,
+							      state->capture.channels,
+							      &state->dump_path);
+		igt_assert(state->dump_fd >= 0);
+	}
+}
+
+static void audio_state_stop(struct audio_state *state, bool success)
+{
+	bool ok;
+	int ret;
+	struct chamelium_audio_file *audio_file;
+
+	igt_debug("Stopping audio playback\n");
+	state->run = false;
+	ret = pthread_join(state->thread, NULL);
+	igt_assert(ret == 0);
+
+	ok = chamelium_stream_stop_realtime_audio(state->stream);
+	igt_assert(ok);
+
+	audio_file = chamelium_stop_capturing_audio(state->chamelium,
+						    state->port);
+	if (audio_file) {
+		igt_debug("Audio file saved on the Chamelium in %s\n",
+			  audio_file->path);
+		chamelium_destroy_audio_file(audio_file);
+	}
+
+	if (state->dump_fd >= 0) {
+		close(state->dump_fd);
+		state->dump_fd = -1;
+
+		if (success) {
+			/* Test succeeded, no need to keep the captured data */
+			unlink(state->dump_path);
+		} else
+			igt_debug("Saved captured audio data to %s\n",
+				  state->dump_path);
+		free(state->dump_path);
+		state->dump_path = NULL;
+	}
+
+	igt_debug("Audio test result for format %s, sampling rate %d Hz and "
+		  "%d channels: %s\n",
+		  snd_pcm_format_name(state->playback.format),
+		  state->playback.rate, state->playback.channels,
+		  success ? "ALL GREEN" : "FAILED");
+}
+
 static int
 audio_output_callback(void *data, void *buffer, int samples)
 {
 	struct audio_state *state = data;
 
-	switch (state->format) {
+	switch (state->playback.format) {
 	case SND_PCM_FORMAT_S16_LE:
 		audio_signal_fill_s16_le(state->signal, buffer, samples);
 		break;
@@ -839,55 +1001,19 @@ audio_output_callback(void *data, void *buffer, int samples)
 	return state->run ? 0 : -1;
 }
 
-static void *
-run_audio_thread(void *data)
+static bool do_test_display_audio(struct audio_state *state)
 {
-	struct alsa *alsa = data;
-
-	alsa_run(alsa, -1);
-	return NULL;
-}
-
-static bool
-do_test_display_audio(data_t *data, struct chamelium_port *port,
-		      struct alsa *alsa, snd_pcm_format_t playback_format,
-		      int playback_channels, int playback_rate)
-{
-	int ret, capture_rate, capture_channels, msec, freq, step;
-	struct chamelium_audio_file *audio_file;
-	struct chamelium_stream *stream;
-	enum chamelium_stream_realtime_mode stream_mode;
-	struct audio_signal *signal;
+	int msec, freq, step;
 	int32_t *recv, *buf;
 	double *channel;
 	size_t i, j, streak, page_count;
 	size_t recv_len, buf_len, buf_cap, buf_size, channel_len;
 	bool ok, success;
-	char dump_suffix[64];
-	char *dump_path = NULL;
-	int dump_fd = -1;
-	pthread_t thread;
-	struct audio_state state = {};
-	int channel_mapping[8], capture_chan;
-
-	igt_debug("Testing with playback format %s, sampling rate %d Hz and "
-		  "%d channels\n",
-		  snd_pcm_format_name(playback_format),
-		  playback_rate, playback_channels);
-	alsa_configure_output(alsa, playback_format,
-			      playback_channels, playback_rate);
+	int capture_chan;
 
-	chamelium_start_capturing_audio(data->chamelium, port, false);
-
-	stream = chamelium_stream_init();
-	igt_assert(stream);
-
-	stream_mode = CHAMELIUM_STREAM_REALTIME_STOP_WHEN_OVERFLOW;
-	ok = chamelium_stream_dump_realtime_audio(stream, stream_mode);
-	igt_assert(ok);
-
-	signal = audio_signal_init(playback_channels, playback_rate);
-	igt_assert(signal);
+	state->signal = audio_signal_init(state->playback.channels,
+					  state->playback.rate);
+	igt_assert(state->signal);
 
 	/* We'll choose different frequencies per channel to make sure they are
 	 * independent from each other. To do so, we'll add a different offset
@@ -900,62 +1026,21 @@ do_test_display_audio(data_t *data, struct chamelium_port *port,
 	 * later on. We cannot retrieve the capture rate before starting
 	 * playing audio, so we don't really have the choice.
 	 */
-	step = 2 * playback_rate / CAPTURE_SAMPLES;
+	step = 2 * state->playback.rate / CAPTURE_SAMPLES;
 	for (i = 0; i < test_frequencies_count; i++) {
-		for (j = 0; j < playback_channels; j++) {
+		for (j = 0; j < state->playback.channels; j++) {
 			freq = test_frequencies[i] + j * step;
-			audio_signal_add_frequency(signal, freq, j);
+			audio_signal_add_frequency(state->signal, freq, j);
 		}
 	}
-	audio_signal_synthesize(signal);
+	audio_signal_synthesize(state->signal);
 
-	state.signal = signal;
-	state.format = playback_format;
-	state.run = true;
-	alsa_register_output_callback(alsa, audio_output_callback, &state,
+	alsa_register_output_callback(state->alsa, audio_output_callback, state,
 				      PLAYBACK_SAMPLES);
 
-	/* Start playing audio */
-	ret = pthread_create(&thread, NULL, run_audio_thread, alsa);
-	igt_assert(ret == 0);
+	audio_state_start(state);
 
-	/* Only after we've started playing audio, we can retrieve the capture
-	 * format used by the Chamelium device. */
-	chamelium_get_audio_format(data->chamelium, port,
-				   &capture_rate, &capture_channels);
-	if (capture_rate == 0) {
-		igt_debug("Audio receiver doesn't indicate the capture "
-			 "sampling rate, assuming it's %d Hz\n", playback_rate);
-		capture_rate = playback_rate;
-	} else
-		igt_assert(capture_rate == playback_rate);
-
-	chamelium_get_audio_channel_mapping(data->chamelium, port,
-					    channel_mapping);
-	/* Make sure we can capture all channels we send. */
-	for (i = 0; i < playback_channels; i++) {
-		ok = false;
-		for (j = 0; j < capture_channels; j++) {
-			if (channel_mapping[j] == i) {
-				ok = true;
-				break;
-			}
-		}
-		igt_assert(ok);
-	}
-
-	if (igt_frame_dump_is_enabled()) {
-		snprintf(dump_suffix, sizeof(dump_suffix),
-			 "capture-%s-%dch-%dHz",
-			 snd_pcm_format_name(playback_format),
-			 playback_channels, playback_rate);
-
-		dump_fd = audio_create_wav_file_s32_le(dump_suffix,
-						       capture_rate,
-						       capture_channels,
-						       &dump_path);
-		igt_assert(dump_fd >= 0);
-	}
+	igt_assert(state->capture.rate == state->playback.rate);
 
 	/* Needs to be a multiple of 128, because that's the number of samples
 	 * we get per channel each time we receive an audio page from the
@@ -970,7 +1055,7 @@ do_test_display_audio(data_t *data, struct chamelium_port *port,
 	channel_len = CAPTURE_SAMPLES;
 	channel = malloc(sizeof(double) * channel_len);
 
-	buf_cap = capture_channels * channel_len;
+	buf_cap = state->capture.channels * channel_len;
 	buf = malloc(sizeof(int32_t) * buf_cap);
 	buf_len = 0;
 
@@ -982,7 +1067,7 @@ do_test_display_audio(data_t *data, struct chamelium_port *port,
 	msec = 0;
 	i = 0;
 	while (!success && msec < AUDIO_TIMEOUT) {
-		ok = chamelium_stream_receive_realtime_audio(stream,
+		ok = chamelium_stream_receive_realtime_audio(state->stream,
 							     &page_count,
 							     &recv, &recv_len);
 		igt_assert(ok);
@@ -994,26 +1079,27 @@ do_test_display_audio(data_t *data, struct chamelium_port *port,
 			continue;
 		igt_assert(buf_len == buf_cap);
 
-		if (dump_fd >= 0) {
+		if (state->dump_fd >= 0) {
 			buf_size = buf_len * sizeof(int32_t);
-			igt_assert(write(dump_fd, buf, buf_size) == buf_size);
+			igt_assert(write(state->dump_fd, buf, buf_size) == buf_size);
 		}
 
-		msec = i * channel_len / (double) capture_rate * 1000;
+		msec = i * channel_len / (double) state->capture.rate * 1000;
 		igt_debug("Detecting audio signal, t=%d msec\n", msec);
 
-		for (j = 0; j < playback_channels; j++) {
-			capture_chan = channel_mapping[j];
+		for (j = 0; j < state->playback.channels; j++) {
+			capture_chan = state->channel_mapping[j];
 			igt_assert(capture_chan >= 0);
 			igt_debug("Processing channel %zu (captured as "
 				  "channel %d)\n", j, capture_chan);
 
 			audio_extract_channel_s32_le(channel, channel_len,
 						     buf, buf_len,
-						     capture_channels,
+						     state->capture.channels,
 						     capture_chan);
 
-			if (audio_signal_detect(signal, capture_rate, j,
+			if (audio_signal_detect(state->signal,
+						state->capture.rate, j,
 						channel, channel_len))
 				streak++;
 			else
@@ -1023,49 +1109,15 @@ do_test_display_audio(data_t *data, struct chamelium_port *port,
 		buf_len = 0;
 		i++;
 
-		success = streak == MIN_STREAK * playback_channels;
+		success = streak == MIN_STREAK * state->playback.channels;
 	}
 
-	igt_debug("Stopping audio playback\n");
-	state.run = false;
-	ret = pthread_join(thread, NULL);
-	igt_assert(ret == 0);
-
-	alsa_close_output(alsa);
-
-	igt_debug("Audio test result for format %s, sampling rate %d Hz and "
-		  "%d channels: %s\n",
-		  snd_pcm_format_name(playback_format),
-		  playback_rate, playback_channels,
-		  success ? "ALL GREEN" : "FAILED");
-
-	if (dump_fd >= 0) {
-		close(dump_fd);
-		if (success) {
-			/* Test succeeded, no need to keep the captured data */
-			unlink(dump_path);
-		} else
-			igt_debug("Saved captured audio data to %s\n", dump_path);
-		free(dump_path);
-	}
+	audio_state_stop(state, success);
 
 	free(recv);
 	free(buf);
 	free(channel);
-
-	ok = chamelium_stream_stop_realtime_audio(stream);
-	igt_assert(ok);
-
-	audio_file = chamelium_stop_capturing_audio(data->chamelium,
-						    port);
-	if (audio_file) {
-		igt_debug("Audio file saved on the Chamelium in %s\n",
-			  audio_file->path);
-		chamelium_destroy_audio_file(audio_file);
-	}
-
-	audio_signal_fini(signal);
-	chamelium_stream_deinit(stream);
+	audio_signal_fini(state->signal);
 
 	return success;
 }
@@ -1112,6 +1164,7 @@ test_display_audio(data_t *data, struct chamelium_port *port,
 	int fb_id, i, j;
 	int channels, sampling_rate;
 	snd_pcm_format_t format;
+	struct audio_state state;
 
 	igt_require(alsa_has_exclusive_access());
 
@@ -1161,9 +1214,10 @@ test_display_audio(data_t *data, struct chamelium_port *port,
 
 			run = true;
 
-			success &= do_test_display_audio(data, port, alsa,
-							 format, channels,
-							 sampling_rate);
+			audio_state_init(&state, data, alsa, port,
+					 format, channels, sampling_rate);
+			success &= do_test_display_audio(&state);
+			audio_state_fini(&state);
 
 			alsa_close_output(alsa);
 		}
-- 
2.21.0

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] [PATCH i-g-t v3 03/10] tests/kms_chamelium: introduce audio_state_receive

[Simon Ser]

This extracts the logic receiving audio pages in do_test_display_audio into a
new audio_state_receive function. The function takes care of updating the
current msec counter and dumping the received data in a file.

Signed-off-by: Simon Ser <simon.ser@intel.com>
Reviewed-by: Martin Peres <martin.peres@linux.intel.com>
---
 tests/kms_chamelium.c | 55 ++++++++++++++++++++++++++++---------------
 1 file changed, 36 insertions(+), 19 deletions(-)

diff --git a/tests/kms_chamelium.c b/tests/kms_chamelium.c
index 27693681728e..71489e5df9d7 100644
--- a/tests/kms_chamelium.c
+++ b/tests/kms_chamelium.c
@@ -827,6 +827,9 @@ struct audio_state {
 	struct audio_signal *signal;
 	int channel_mapping[CHAMELIUM_MAX_AUDIO_CHANNELS];
 
+	size_t recv_pages;
+	int msec;
+
 	int dump_fd;
 	char *dump_path;
 
@@ -876,6 +879,9 @@ static void audio_state_start(struct audio_state *state)
 	enum chamelium_stream_realtime_mode stream_mode;
 	char dump_suffix[64];
 
+	state->recv_pages = 0;
+	state->msec = 0;
+
 	igt_debug("Starting test with playback format %s, sampling rate %d Hz "
 		  "and %d channels\n",
 		  snd_pcm_format_name(state->playback.format),
@@ -936,6 +942,29 @@ static void audio_state_start(struct audio_state *state)
 	}
 }
 
+static void audio_state_receive(struct audio_state *state,
+				int32_t **recv, size_t *recv_len)
+{
+	bool ok;
+	size_t page_count;
+	size_t recv_size;
+
+	ok = chamelium_stream_receive_realtime_audio(state->stream,
+						     &page_count,
+						     recv, recv_len);
+	igt_assert(ok);
+
+	state->msec = state->recv_pages * *recv_len
+		      / (double) state->capture.channels
+		      / (double) state->capture.rate * 1000;
+	state->recv_pages++;
+
+	if (state->dump_fd >= 0) {
+		recv_size = *recv_len * sizeof(int32_t);
+		igt_assert(write(state->dump_fd, *recv, recv_size) == recv_size);
+	}
+}
+
 static void audio_state_stop(struct audio_state *state, bool success)
 {
 	bool ok;
@@ -1003,12 +1032,12 @@ audio_output_callback(void *data, void *buffer, int samples)
 
 static bool do_test_display_audio(struct audio_state *state)
 {
-	int msec, freq, step;
+	int freq, step;
 	int32_t *recv, *buf;
 	double *channel;
-	size_t i, j, streak, page_count;
-	size_t recv_len, buf_len, buf_cap, buf_size, channel_len;
-	bool ok, success;
+	size_t i, j, streak;
+	size_t recv_len, buf_len, buf_cap, channel_len;
+	bool success;
 	int capture_chan;
 
 	state->signal = audio_signal_init(state->playback.channels,
@@ -1064,13 +1093,8 @@ static bool do_test_display_audio(struct audio_state *state)
 
 	success = false;
 	streak = 0;
-	msec = 0;
-	i = 0;
-	while (!success && msec < AUDIO_TIMEOUT) {
-		ok = chamelium_stream_receive_realtime_audio(state->stream,
-							     &page_count,
-							     &recv, &recv_len);
-		igt_assert(ok);
+	while (!success && state->msec < AUDIO_TIMEOUT) {
+		audio_state_receive(state, &recv, &recv_len);
 
 		memcpy(&buf[buf_len], recv, recv_len * sizeof(int32_t));
 		buf_len += recv_len;
@@ -1079,13 +1103,7 @@ static bool do_test_display_audio(struct audio_state *state)
 			continue;
 		igt_assert(buf_len == buf_cap);
 
-		if (state->dump_fd >= 0) {
-			buf_size = buf_len * sizeof(int32_t);
-			igt_assert(write(state->dump_fd, buf, buf_size) == buf_size);
-		}
-
-		msec = i * channel_len / (double) state->capture.rate * 1000;
-		igt_debug("Detecting audio signal, t=%d msec\n", msec);
+		igt_debug("Detecting audio signal, t=%d msec\n", state->msec);
 
 		for (j = 0; j < state->playback.channels; j++) {
 			capture_chan = state->channel_mapping[j];
@@ -1107,7 +1125,6 @@ static bool do_test_display_audio(struct audio_state *state)
 		}
 
 		buf_len = 0;
-		i++;
 
 		success = streak == MIN_STREAK * state->playback.channels;
 	}
-- 
2.21.0

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] [PATCH i-g-t v3 04/10] tests/kms_chamelium: rename do_test_display_audio and test_audio_configuration

[Simon Ser]

- Rename do_test_display_audio to test_audio_frequencies to prepare for a
  future amplitude/delay test
- Rename test_audio_configuration to check_audio_configuration because this
  function doesn't execute any test, it just checks whether we can perform
  audio tests using a particular configuration

Signed-off-by: Simon Ser <simon.ser@intel.com>
Reviewed-by: Martin Peres <martin.peres@linux.intel.com>
---
 tests/kms_chamelium.c | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/tests/kms_chamelium.c b/tests/kms_chamelium.c
index 71489e5df9d7..f934a5bee74a 100644
--- a/tests/kms_chamelium.c
+++ b/tests/kms_chamelium.c
@@ -1009,7 +1009,7 @@ static void audio_state_stop(struct audio_state *state, bool success)
 }
 
 static int
-audio_output_callback(void *data, void *buffer, int samples)
+audio_output_frequencies_callback(void *data, void *buffer, int samples)
 {
 	struct audio_state *state = data;
 
@@ -1030,7 +1030,7 @@ audio_output_callback(void *data, void *buffer, int samples)
 	return state->run ? 0 : -1;
 }
 
-static bool do_test_display_audio(struct audio_state *state)
+static bool test_audio_frequencies(struct audio_state *state)
 {
 	int freq, step;
 	int32_t *recv, *buf;
@@ -1064,7 +1064,8 @@ static bool do_test_display_audio(struct audio_state *state)
 	}
 	audio_signal_synthesize(state->signal);
 
-	alsa_register_output_callback(state->alsa, audio_output_callback, state,
+	alsa_register_output_callback(state->alsa,
+				      audio_output_frequencies_callback, state,
 				      PLAYBACK_SAMPLES);
 
 	audio_state_start(state);
@@ -1139,8 +1140,8 @@ static bool do_test_display_audio(struct audio_state *state)
 	return success;
 }
 
-static bool test_audio_configuration(struct alsa *alsa, snd_pcm_format_t format,
-				     int channels, int sampling_rate)
+static bool check_audio_configuration(struct alsa *alsa, snd_pcm_format_t format,
+				      int channels, int sampling_rate)
 {
 	if (!alsa_test_output_configuration(alsa, format, channels,
 					    sampling_rate)) {
@@ -1225,15 +1226,15 @@ test_display_audio(data_t *data, struct chamelium_port *port,
 			channels = PLAYBACK_CHANNELS;
 			sampling_rate = test_sampling_rates[i];
 
-			if (!test_audio_configuration(alsa, format, channels,
-						      sampling_rate))
+			if (!check_audio_configuration(alsa, format, channels,
+						       sampling_rate))
 				continue;
 
 			run = true;
 
 			audio_state_init(&state, data, alsa, port,
 					 format, channels, sampling_rate);
-			success &= do_test_display_audio(&state);
+			success &= test_audio_frequencies(&state);
 			audio_state_fini(&state);
 
 			alsa_close_output(alsa);
-- 
2.21.0

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] [PATCH i-g-t v3 05/10] tests/kms_chamelium: explain why 8-channel tests aren't performed

[Simon Ser]

Also add a reference to the relevant Chromium bug.

Signed-off-by: Simon Ser <simon.ser@intel.com>
Reviewed-by: Martin Peres <martin.peres@linux.intel.com>
---
 tests/kms_chamelium.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/tests/kms_chamelium.c b/tests/kms_chamelium.c
index f934a5bee74a..8c63cee5e2e4 100644
--- a/tests/kms_chamelium.c
+++ b/tests/kms_chamelium.c
@@ -1221,7 +1221,9 @@ test_display_audio(data_t *data, struct chamelium_port *port,
 			ret = alsa_open_output(alsa, audio_device);
 			igt_assert(ret >= 0);
 
-			/* TODO: playback on all 8 available channels */
+			/* TODO: playback on all 8 available channels (this
+			 * isn't supported by Chamelium devices yet, see
+			 * https://crbug.com/950917) */
 			format = test_formats[j];
 			channels = PLAYBACK_CHANNELS;
 			sampling_rate = test_sampling_rates[i];
-- 
2.21.0

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] [PATCH i-g-t v3 06/10] lib/igt_audio: introduce audio_convert_to

[Simon Ser]

This function converts normalized doubles into an ALSA PCM format.

Instead of having per-format audio_signal_fill_* functions, we can only have
audio_signal_fill that outputs normalized doubles. Then in ALSA's playback
callback, we can simply use the new audio_convert_to function to fill the
buffer.

This makes the test code simpler and prevents code duplication when another
ALSA playback callback is implemented.

This adds a dependency of igt_audio over ALSA for the PCM format enum, but I
don't think this is a concern, since I don't see the point of using igt_audio
without igt_alsa. If this is an issue, it would always be possible to replace
ALSA's enum with our own in the future.

Signed-off-by: Simon Ser <simon.ser@intel.com>
Reviewed-by: Martin Peres <martin.peres@linux.intel.com>
---
 lib/igt_audio.c       | 87 +++++++++++++++++++++----------------------
 lib/igt_audio.h       | 12 +++---
 tests/kms_chamelium.c | 22 ++++-------
 3 files changed, 55 insertions(+), 66 deletions(-)

diff --git a/lib/igt_audio.c b/lib/igt_audio.c
index ea2d83a5cad0..be665b03116f 100644
--- a/lib/igt_audio.c
+++ b/lib/igt_audio.c
@@ -304,51 +304,6 @@ void audio_signal_fill(struct audio_signal *signal, double *buffer,
 	audio_sanity_check(buffer, signal->channels * samples);
 }
 
-void audio_signal_fill_s16_le(struct audio_signal *signal, int16_t *buffer,
-			      size_t samples)
-{
-	double *tmp;
-	size_t i;
-
-	tmp = malloc(sizeof(double) * signal->channels * samples);
-	audio_signal_fill(signal, tmp, samples);
-
-	for (i = 0; i < signal->channels * samples; ++i)
-		buffer[i] = INT16_MAX * tmp[i];
-
-	free(tmp);
-}
-
-void audio_signal_fill_s24_le(struct audio_signal *signal, int32_t *buffer,
-			      size_t samples)
-{
-	double *tmp;
-	size_t i;
-
-	tmp = malloc(sizeof(double) * signal->channels * samples);
-	audio_signal_fill(signal, tmp, samples);
-
-	for (i = 0; i < signal->channels * samples; ++i)
-		buffer[i] = 0x7FFFFF * tmp[i];
-
-	free(tmp);
-}
-
-void audio_signal_fill_s32_le(struct audio_signal *signal, int32_t *buffer,
-			      size_t samples)
-{
-	double *tmp;
-	size_t i;
-
-	tmp = malloc(sizeof(double) * signal->channels * samples);
-	audio_signal_fill(signal, tmp, samples);
-
-	for (i = 0; i < signal->channels * samples; ++i)
-		buffer[i] = INT32_MAX * tmp[i];
-
-	free(tmp);
-}
-
 /**
  * Checks that frequencies specified in signal, and only those, are included
  * in the input data.
@@ -508,6 +463,48 @@ size_t audio_extract_channel_s32_le(double *dst, size_t dst_cap,
 	return dst_len;
 }
 
+static void audio_convert_to_s16_le(int16_t *dst, double *src, size_t len)
+{
+	size_t i;
+
+	for (i = 0; i < len; ++i)
+		dst[i] = INT16_MAX * src[i];
+}
+
+static void audio_convert_to_s24_le(int32_t *dst, double *src, size_t len)
+{
+	size_t i;
+
+	for (i = 0; i < len; ++i)
+		dst[i] = 0x7FFFFF * src[i];
+}
+
+static void audio_convert_to_s32_le(int32_t *dst, double *src, size_t len)
+{
+	size_t i;
+
+	for (i = 0; i < len; ++i)
+		dst[i] = INT32_MAX * src[i];
+}
+
+void audio_convert_to(void *dst, double *src, size_t len,
+		      snd_pcm_format_t format)
+{
+	switch (format) {
+	case SND_PCM_FORMAT_S16_LE:
+		audio_convert_to_s16_le(dst, src, len);
+		break;
+	case SND_PCM_FORMAT_S24_LE:
+		audio_convert_to_s24_le(dst, src, len);
+		break;
+	case SND_PCM_FORMAT_S32_LE:
+		audio_convert_to_s32_le(dst, src, len);
+		break;
+	default:
+		assert(false); /* unreachable */
+	}
+}
+
 #define RIFF_TAG "RIFF"
 #define WAVE_TAG "WAVE"
 #define FMT_TAG "fmt "
diff --git a/lib/igt_audio.h b/lib/igt_audio.h
index c8de70871faa..5c910c27304d 100644
--- a/lib/igt_audio.h
+++ b/lib/igt_audio.h
@@ -32,6 +32,8 @@
 #include <stdbool.h>
 #include <stdint.h>
 
+#include <alsa/asoundlib.h>
+
 struct audio_signal;
 
 struct audio_signal *audio_signal_init(int channels, int sampling_rate);
@@ -41,18 +43,14 @@ int audio_signal_add_frequency(struct audio_signal *signal, int frequency,
 void audio_signal_synthesize(struct audio_signal *signal);
 void audio_signal_reset(struct audio_signal *signal);
 void audio_signal_fill(struct audio_signal *signal, double *buffer,
-		       size_t buffer_len);
-void audio_signal_fill_s16_le(struct audio_signal *signal, int16_t *buffer,
-			      size_t buffer_len);
-void audio_signal_fill_s24_le(struct audio_signal *signal, int32_t *buffer,
-			      size_t buffer_len);
-void audio_signal_fill_s32_le(struct audio_signal *signal, int32_t *buffer,
-			      size_t buffer_len);
+		       size_t samples);
 bool audio_signal_detect(struct audio_signal *signal, int sampling_rate,
 			 int channel, const double *samples, size_t samples_len);
 size_t audio_extract_channel_s32_le(double *dst, size_t dst_cap,
 				    int32_t *src, size_t src_len,
 				    int n_channels, int channel);
+void audio_convert_to(void *dst, double *src, size_t len,
+		      snd_pcm_format_t format);
 int audio_create_wav_file_s32_le(const char *qualifier, uint32_t sample_rate,
 				 uint16_t channels, char **path);
 
diff --git a/tests/kms_chamelium.c b/tests/kms_chamelium.c
index 8c63cee5e2e4..065cb1676a3b 100644
--- a/tests/kms_chamelium.c
+++ b/tests/kms_chamelium.c
@@ -1012,20 +1012,14 @@ static int
 audio_output_frequencies_callback(void *data, void *buffer, int samples)
 {
 	struct audio_state *state = data;
-
-	switch (state->playback.format) {
-	case SND_PCM_FORMAT_S16_LE:
-		audio_signal_fill_s16_le(state->signal, buffer, samples);
-		break;
-	case SND_PCM_FORMAT_S24_LE:
-		audio_signal_fill_s24_le(state->signal, buffer, samples);
-		break;
-	case SND_PCM_FORMAT_S32_LE:
-		audio_signal_fill_s32_le(state->signal, buffer, samples);
-		break;
-	default:
-		assert(false); /* unreachable */
-	}
+	double *tmp;
+	size_t len;
+
+	len = samples * state->playback.channels;
+	tmp = malloc(len * sizeof(double));
+	audio_signal_fill(state->signal, tmp, samples);
+	audio_convert_to(buffer, tmp, len, state->playback.format);
+	free(tmp);
 
 	return state->run ? 0 : -1;
 }
-- 
2.21.0

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] [PATCH i-g-t v3 07/10] tests/kms_chamelium: add name parameter to audio_state_start

[Simon Ser]

This identifies the audio test name. This is required for adding multiple
audio tests.

Signed-off-by: Simon Ser <simon.ser@intel.com>
Reviewed-by: Martin Peres <martin.peres@linux.intel.com>
---
 tests/kms_chamelium.c | 24 ++++++++++++++----------
 1 file changed, 14 insertions(+), 10 deletions(-)

diff --git a/tests/kms_chamelium.c b/tests/kms_chamelium.c
index 065cb1676a3b..40ca93687c20 100644
--- a/tests/kms_chamelium.c
+++ b/tests/kms_chamelium.c
@@ -824,6 +824,7 @@ struct audio_state {
 		int rate;
 	} playback, capture;
 
+	char *name;
 	struct audio_signal *signal;
 	int channel_mapping[CHAMELIUM_MAX_AUDIO_CHANNELS];
 
@@ -861,6 +862,7 @@ static void audio_state_init(struct audio_state *state, data_t *data,
 static void audio_state_fini(struct audio_state *state)
 {
 	chamelium_stream_deinit(state->stream);
+	free(state->name);
 }
 
 static void *run_audio_thread(void *data)
@@ -871,7 +873,7 @@ static void *run_audio_thread(void *data)
 	return NULL;
 }
 
-static void audio_state_start(struct audio_state *state)
+static void audio_state_start(struct audio_state *state, const char *name)
 {
 	int ret;
 	bool ok;
@@ -879,12 +881,14 @@ static void audio_state_start(struct audio_state *state)
 	enum chamelium_stream_realtime_mode stream_mode;
 	char dump_suffix[64];
 
+	free(state->name);
+	state->name = strdup(name);
 	state->recv_pages = 0;
 	state->msec = 0;
 
-	igt_debug("Starting test with playback format %s, sampling rate %d Hz "
-		  "and %d channels\n",
-		  snd_pcm_format_name(state->playback.format),
+	igt_debug("Starting %s test with playback format %s, "
+		  "sampling rate %d Hz and %d channels\n",
+		  name, snd_pcm_format_name(state->playback.format),
 		  state->playback.rate, state->playback.channels);
 
 	chamelium_start_capturing_audio(state->chamelium, state->port, false);
@@ -930,8 +934,8 @@ static void audio_state_start(struct audio_state *state)
 
 	if (igt_frame_dump_is_enabled()) {
 		snprintf(dump_suffix, sizeof(dump_suffix),
-			 "capture-%s-%dch-%dHz",
-			 snd_pcm_format_name(state->playback.format),
+			 "capture-%s-%s-%dch-%dHz",
+			 name, snd_pcm_format_name(state->playback.format),
 			 state->playback.channels, state->playback.rate);
 
 		state->dump_fd = audio_create_wav_file_s32_le(dump_suffix,
@@ -1001,9 +1005,9 @@ static void audio_state_stop(struct audio_state *state, bool success)
 		state->dump_path = NULL;
 	}
 
-	igt_debug("Audio test result for format %s, sampling rate %d Hz and "
-		  "%d channels: %s\n",
-		  snd_pcm_format_name(state->playback.format),
+	igt_debug("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");
 }
@@ -1062,7 +1066,7 @@ static bool test_audio_frequencies(struct audio_state *state)
 				      audio_output_frequencies_callback, state,
 				      PLAYBACK_SAMPLES);
 
-	audio_state_start(state);
+	audio_state_start(state, "frequencies");
 
 	igt_assert(state->capture.rate == state->playback.rate);
 
-- 
2.21.0

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] [PATCH i-g-t v3 08/10] lib/igt_audio: make audio_extract_channel_s32_le support a NULL dst

[Simon Ser]

This adds a snprintf-like behaviour to audio_extract_channel_s32_le.

Signed-off-by: Simon Ser <simon.ser@intel.com>
Reviewed-by: Martin Peres <martin.peres@linux.intel.com>
---
 lib/igt_audio.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/lib/igt_audio.c b/lib/igt_audio.c
index be665b03116f..08c0fb6af0db 100644
--- a/lib/igt_audio.c
+++ b/lib/igt_audio.c
@@ -445,7 +445,13 @@ bool audio_signal_detect(struct audio_signal *signal, int sampling_rate,
 }
 
 /**
- * Extracts a single channel from a multi-channel S32_LE input buffer.
+ * audio_extract_channel_s32_le: extracts a single channel from a multi-channel
+ * S32_LE input buffer.
+ *
+ * If dst_cap is zero, no copy is performed. This can be used to compute the
+ * minimum required capacity.
+ *
+ * Returns: the number of samples extracted.
  */
 size_t audio_extract_channel_s32_le(double *dst, size_t dst_cap,
 				    int32_t *src, size_t src_len,
@@ -456,6 +462,9 @@ size_t audio_extract_channel_s32_le(double *dst, size_t dst_cap,
 	igt_assert(channel < n_channels);
 	igt_assert(src_len % n_channels == 0);
 	dst_len = src_len / n_channels;
+	if (dst_cap == 0)
+		return dst_len;
+
 	igt_assert(dst_len <= dst_cap);
 	for (i = 0; i < dst_len; i++)
 		dst[i] = (double) src[i * n_channels + channel] / INT32_MAX;
-- 
2.21.0

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] [PATCH i-g-t v3 09/10] tests/kms_chamelium: add a flatline audio test

[Simon Ser]

This commit adds a flatline test alongside the existing frequencies test.

The test sends a constant value and checks that the amplitude is correct. A
window is used to check that each sample is within acceptable bounds. The test
is stopped as soon as 3 audio pages pass the test.

Signed-off-by: Simon Ser <simon.ser@intel.com>
Reviewed-by: Martin Peres <martin.peres@linux.intel.com>
---
 tests/kms_chamelium.c | 101 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 101 insertions(+)

diff --git a/tests/kms_chamelium.c b/tests/kms_chamelium.c
index 40ca93687c20..451a616f1a2e 100644
--- a/tests/kms_chamelium.c
+++ b/tests/kms_chamelium.c
@@ -772,6 +772,9 @@ test_display_frame_dump(data_t *data, struct chamelium_port *port)
 /* A streak of 3 gives confidence that the signal is good. */
 #define MIN_STREAK 3
 
+#define FLATLINE_AMPLITUDE 0.9 /* normalized, ie. in [0, 1] */
+#define FLATLINE_ACCURACY 0.001 /* ± 0.1% of the full amplitude */
+
 /* TODO: enable >48KHz rates, these are not reliable */
 static int test_sampling_rates[] = {
 	32000,
@@ -1138,6 +1141,103 @@ static bool test_audio_frequencies(struct audio_state *state)
 	return success;
 }
 
+static int audio_output_flatline_callback(void *data, void *buffer,
+					     int samples)
+{
+	struct audio_state *state = data;
+	double *tmp;
+	size_t len, i;
+
+	len = samples * state->playback.channels;
+	tmp = malloc(len * sizeof(double));
+	for (i = 0; i < len; i++)
+		tmp[i] = FLATLINE_AMPLITUDE;
+	audio_convert_to(buffer, tmp, len, state->playback.format);
+	free(tmp);
+
+	return state->run ? 0 : -1;
+}
+
+static bool detect_flatline_amplitude(double *buf, size_t buf_len)
+{
+	double min, max;
+	size_t i;
+	bool ok;
+
+	min = max = NAN;
+	for (i = 0; i < buf_len; i++) {
+		if (isnan(min) || buf[i] < min)
+			min = buf[i];
+		if (isnan(max) || buf[i] > max)
+			max = buf[i];
+	}
+
+	ok = (min >= FLATLINE_AMPLITUDE - FLATLINE_ACCURACY &&
+	      max <= FLATLINE_AMPLITUDE + FLATLINE_ACCURACY);
+	if (ok)
+		igt_debug("Flatline detected\n");
+	else
+		igt_debug("Flatline not detected (min=%f, max=%f)\n",
+			  min, max);
+	return ok;
+}
+
+static bool test_audio_flatline(struct audio_state *state)
+{
+	bool success;
+	int32_t *recv;
+	size_t recv_len, i, channel_len;
+	int streak, capture_chan;
+	double *channel;
+
+	alsa_register_output_callback(state->alsa,
+				      audio_output_flatline_callback, state,
+				      PLAYBACK_SAMPLES);
+
+	audio_state_start(state, "flatline");
+
+	recv = NULL;
+	recv_len = 0;
+	success = false;
+	while (!success && state->msec < AUDIO_TIMEOUT) {
+		audio_state_receive(state, &recv, &recv_len);
+
+		igt_debug("Detecting audio signal, t=%d msec\n", state->msec);
+
+		for (i = 0; i < state->playback.channels; i++) {
+			capture_chan = state->channel_mapping[i];
+			igt_assert(capture_chan >= 0);
+			igt_debug("Processing channel %zu (captured as "
+				  "channel %d)\n", i, capture_chan);
+
+			channel_len = audio_extract_channel_s32_le(NULL, 0,
+								   recv, recv_len,
+								   state->capture.channels,
+								   capture_chan);
+			channel = malloc(channel_len * sizeof(double));
+			audio_extract_channel_s32_le(channel, channel_len,
+						     recv, recv_len,
+						     state->capture.channels,
+						     capture_chan);
+
+			if (detect_flatline_amplitude(channel, channel_len))
+				streak++;
+			else
+				streak = 0;
+
+			free(channel);
+		}
+
+		success = streak == MIN_STREAK * state->playback.channels;
+	}
+
+	audio_state_stop(state, success);
+
+	free(recv);
+
+	return success;
+}
+
 static bool check_audio_configuration(struct alsa *alsa, snd_pcm_format_t format,
 				      int channels, int sampling_rate)
 {
@@ -1235,6 +1335,7 @@ test_display_audio(data_t *data, struct chamelium_port *port,
 			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);
 
 			alsa_close_output(alsa);
-- 
2.21.0

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] [PATCH i-g-t v3 10/10] tests/kms_chamelium: add audio channel alignment test

[Simon Ser]

The previous pulse test only checked the signal amplitude.

This commit adds a new check to the flatline test: channel alignment. This check
makes sure there is no time shift between each channel.

This is achieved by first sending a positive signal and then a falling edge.
Edges in each channel should be aligned.

The index of each channel's falling edge is stored in number of samples. I
chose not to implement a per-page test because the edge could be right between
two pages.

Signed-off-by: Simon Ser <simon.ser@intel.com>
Reviewed-by: Martin Peres <martin.peres@linux.intel.com>
---
 tests/kms_chamelium.c | 100 ++++++++++++++++++++++++++++++++++++------
 1 file changed, 86 insertions(+), 14 deletions(-)

diff --git a/tests/kms_chamelium.c b/tests/kms_chamelium.c
index 451a616f1a2e..49c3de0b7beb 100644
--- a/tests/kms_chamelium.c
+++ b/tests/kms_chamelium.c
@@ -773,7 +773,8 @@ test_display_frame_dump(data_t *data, struct chamelium_port *port)
 #define MIN_STREAK 3
 
 #define FLATLINE_AMPLITUDE 0.9 /* normalized, ie. in [0, 1] */
-#define FLATLINE_ACCURACY 0.001 /* ± 0.1% of the full amplitude */
+#define FLATLINE_AMPLITUDE_ACCURACY 0.001 /* ± 0.1 % of the full amplitude */
+#define FLATLINE_ALIGN_ACCURACY 0 /* number of samples */
 
 /* TODO: enable >48KHz rates, these are not reliable */
 static int test_sampling_rates[] = {
@@ -828,7 +829,7 @@ struct audio_state {
 	} playback, capture;
 
 	char *name;
-	struct audio_signal *signal;
+	struct audio_signal *signal; /* for frequencies test only */
 	int channel_mapping[CHAMELIUM_MAX_AUDIO_CHANNELS];
 
 	size_t recv_pages;
@@ -839,6 +840,7 @@ struct audio_state {
 
 	pthread_t thread;
 	atomic_bool run;
+	atomic_bool positive; /* for pulse test only */
 };
 
 static void audio_state_init(struct audio_state *state, data_t *data,
@@ -1151,16 +1153,16 @@ static int audio_output_flatline_callback(void *data, void *buffer,
 	len = samples * state->playback.channels;
 	tmp = malloc(len * sizeof(double));
 	for (i = 0; i < len; i++)
-		tmp[i] = FLATLINE_AMPLITUDE;
+		tmp[i] = (state->positive ? 1 : -1) * FLATLINE_AMPLITUDE;
 	audio_convert_to(buffer, tmp, len, state->playback.format);
 	free(tmp);
 
 	return state->run ? 0 : -1;
 }
 
-static bool detect_flatline_amplitude(double *buf, size_t buf_len)
+static bool detect_flatline_amplitude(double *buf, size_t buf_len, bool pos)
 {
-	double min, max;
+	double expected, min, max;
 	size_t i;
 	bool ok;
 
@@ -1172,34 +1174,63 @@ static bool detect_flatline_amplitude(double *buf, size_t buf_len)
 			max = buf[i];
 	}
 
-	ok = (min >= FLATLINE_AMPLITUDE - FLATLINE_ACCURACY &&
-	      max <= FLATLINE_AMPLITUDE + FLATLINE_ACCURACY);
+	expected = (pos ? 1 : -1) * FLATLINE_AMPLITUDE;
+	ok = (min >= expected - FLATLINE_AMPLITUDE_ACCURACY &&
+	      max <= expected + FLATLINE_AMPLITUDE_ACCURACY);
 	if (ok)
-		igt_debug("Flatline detected\n");
+		igt_debug("Flatline wave amplitude detected\n");
 	else
-		igt_debug("Flatline not detected (min=%f, max=%f)\n",
+		igt_debug("Flatline amplitude not detected (min=%f, max=%f)\n",
 			  min, max);
 	return ok;
 }
 
+static ssize_t detect_falling_edge(double *buf, size_t buf_len)
+{
+	size_t i;
+
+	for (i = 0; i < buf_len; i++) {
+		if (buf[i] < 0)
+			return i;
+	}
+
+	return -1;
+}
+
+/** test_audio_flatline:
+ *
+ * Send a constant value (one positive, then a negative one) and check that:
+ *
+ * - The amplitude of the flatline is correct
+ * - All channels switch from a positive signal to a negative one at the same
+ *   time (ie. all channels are aligned)
+ */
 static bool test_audio_flatline(struct audio_state *state)
 {
-	bool success;
+	bool success, amp_success, align_success;
 	int32_t *recv;
 	size_t recv_len, i, channel_len;
+	ssize_t j;
 	int streak, capture_chan;
 	double *channel;
+	int falling_edges[CHAMELIUM_MAX_AUDIO_CHANNELS];
 
 	alsa_register_output_callback(state->alsa,
 				      audio_output_flatline_callback, state,
 				      PLAYBACK_SAMPLES);
 
+	/* Start by sending a positive signal */
+	state->positive = true;
+
 	audio_state_start(state, "flatline");
 
+	for (i = 0; i < state->playback.channels; i++)
+		falling_edges[i] = -1;
+
 	recv = NULL;
 	recv_len = 0;
-	success = false;
-	while (!success && state->msec < AUDIO_TIMEOUT) {
+	amp_success = false;
+	while (!amp_success && state->msec < AUDIO_TIMEOUT) {
 		audio_state_receive(state, &recv, &recv_len);
 
 		igt_debug("Detecting audio signal, t=%d msec\n", state->msec);
@@ -1220,17 +1251,58 @@ static bool test_audio_flatline(struct audio_state *state)
 						     state->capture.channels,
 						     capture_chan);
 
-			if (detect_flatline_amplitude(channel, channel_len))
+			/* Check whether the amplitude is fine */
+			if (detect_flatline_amplitude(channel, channel_len,
+						      state->positive))
 				streak++;
 			else
 				streak = 0;
 
+			/* If we're now sending a negative signal, detect the
+			 * falling edge */
+			j = detect_falling_edge(channel, channel_len);
+			if (!state->positive && j >= 0) {
+				falling_edges[i] = recv_len * state->recv_pages
+						   + j;
+			}
+
 			free(channel);
 		}
 
-		success = streak == MIN_STREAK * state->playback.channels;
+		amp_success = streak == MIN_STREAK * state->playback.channels;
+
+		if (amp_success && state->positive) {
+			/* Switch to a negative signal after we've detected the
+			 * positive one. */
+			state->positive = false;
+			amp_success = false;
+			streak = 0;
+			igt_debug("Switching to negative square wave\n");
+		}
+	}
+
+	/* Check alignment between all channels by comparing the index of the
+	 * falling edge. */
+	align_success = true;
+	for (i = 0; i < state->playback.channels; i++) {
+		if (falling_edges[i] < 0) {
+			igt_debug("Falling edge not detected for channel %zu\n",
+				  i);
+			align_success = false;
+			continue;
+		}
+
+		if (abs(falling_edges[0] - falling_edges[i]) >
+		    FLATLINE_ALIGN_ACCURACY) {
+			igt_debug("Channel alignment mismatch: "
+				  "channel 0 has a falling edge at index %d "
+				  "while channel %zu has index %d\n",
+				  falling_edges[0], i, falling_edges[i]);
+			align_success = false;
+		}
 	}
 
+	success = amp_success && align_success;
 	audio_state_stop(state, success);
 
 	free(recv);
-- 
2.21.0

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t v3 01/10] lib/igt_chamelium: introduce CHAMELIUM_MAX_AUDIO_CHANNELS

[Peres, Martin]

On 27/05/2019 17:35, Ser, Simon wrote:
> This allows us not to hardcode 8 everywhere.
> 
> Signed-off-by: Simon Ser <simon.ser@intel.com>

The series is:
Reviewed-by: Martin Peres <martin.peres@linux.intel.com>

Please land the patches as soon as you get the CI results!

> ---
>  lib/igt_chamelium.c | 8 +++++---
>  lib/igt_chamelium.h | 8 +++++++-
>  2 files changed, 12 insertions(+), 4 deletions(-)
> 
> diff --git a/lib/igt_chamelium.c b/lib/igt_chamelium.c
> index cfdf7617a65a..75f03d8469aa 100644
> --- a/lib/igt_chamelium.c
> +++ b/lib/igt_chamelium.c
> @@ -1020,7 +1020,7 @@ bool chamelium_has_audio_support(struct chamelium *chamelium,
>   */
>  void chamelium_get_audio_channel_mapping(struct chamelium *chamelium,
>  					 struct chamelium_port *port,
> -					 int mapping[static 8])
> +					 int mapping[static CHAMELIUM_MAX_AUDIO_CHANNELS])
>  {
>  	xmlrpc_value *res, *res_channel;
>  	int res_len, i;
> @@ -1028,7 +1028,7 @@ void chamelium_get_audio_channel_mapping(struct chamelium *chamelium,
>  	res = chamelium_rpc(chamelium, port, "GetAudioChannelMapping", "(i)",
>  			    port->id);
>  	res_len = xmlrpc_array_size(&chamelium->env, res);
> -	igt_assert(res_len == 8);
> +	igt_assert(res_len == CHAMELIUM_MAX_AUDIO_CHANNELS);
>  	for (i = 0; i < res_len; i++) {
>  		xmlrpc_array_read_item(&chamelium->env, res, i, &res_channel);
>  		xmlrpc_read_int(&chamelium->env, res_channel, &mapping[i]);
> @@ -1058,8 +1058,10 @@ static void audio_format_from_xml(struct chamelium *chamelium,
>  
>  	if (rate)
>  		xmlrpc_read_int(&chamelium->env, res_rate, rate);
> -	if (channels)
> +	if (channels) {
>  		xmlrpc_read_int(&chamelium->env, res_channel, channels);
> +		igt_assert(*channels <= CHAMELIUM_MAX_AUDIO_CHANNELS);
> +	}
>  
>  	xmlrpc_DECREF(res_channel);
>  	xmlrpc_DECREF(res_sample_format);
> diff --git a/lib/igt_chamelium.h b/lib/igt_chamelium.h
> index 33362b266ce4..28f726c91e83 100644
> --- a/lib/igt_chamelium.h
> +++ b/lib/igt_chamelium.h
> @@ -65,6 +65,12 @@ struct chamelium_audio_file {
>   */
>  #define CHAMELIUM_DEFAULT_EDID 0
>  
> +/**
> + * CHAMELIUM_MAX_AUDIO_CHANNELS: the maximum number of audio capture channels
> + * supported by Chamelium.
> + */
> +#define CHAMELIUM_MAX_AUDIO_CHANNELS 8
> +
>  struct chamelium *chamelium_init(int drm_fd);
>  void chamelium_deinit(struct chamelium *chamelium);
>  void chamelium_reset(struct chamelium *chamelium);
> @@ -116,7 +122,7 @@ bool chamelium_has_audio_support(struct chamelium *chamelium,
>  				 struct chamelium_port *port);
>  void chamelium_get_audio_channel_mapping(struct chamelium *chamelium,
>  					 struct chamelium_port *port,
> -					 int mapping[static 8]);
> +					 int mapping[static CHAMELIUM_MAX_AUDIO_CHANNELS]);
>  void chamelium_get_audio_format(struct chamelium *chamelium,
>  				struct chamelium_port *port,
>  				int *rate, int *channels);
> 

---------------------------------------------------------------------
Intel Finland Oy
Registered Address: PL 281, 00181 Helsinki 
Business Identity Code: 0357606 - 4 
Domiciled in Helsinki 

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] ✓ Fi.CI.BAT: success for tests/kms_chamelium: add pulse test (rev3)

[Patchwork]

== Series Details ==

Series: tests/kms_chamelium: add pulse test (rev3)
URL   : https://patchwork.freedesktop.org/series/61111/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_6149 -> IGTPW_3064
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/61111/revisions/3/mbox/

Known issues
------------

  Here are the changes found in IGTPW_3064 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@i915_selftest@live_hangcheck:
    - fi-icl-y:           [PASS][1] -> [INCOMPLETE][2] ([fdo#107713] / [fdo#108569])
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6149/fi-icl-y/igt@i915_selftest@live_hangcheck.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3064/fi-icl-y/igt@i915_selftest@live_hangcheck.html

  * igt@kms_cursor_legacy@basic-busy-flip-before-cursor-atomic:
    - fi-icl-u3:          [PASS][3] -> [DMESG-WARN][4] ([fdo#107724]) +2 similar issues
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6149/fi-icl-u3/igt@kms_cursor_legacy@basic-busy-flip-before-cursor-atomic.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3064/fi-icl-u3/igt@kms_cursor_legacy@basic-busy-flip-before-cursor-atomic.html

  
#### Possible fixes ####

  * igt@gem_basic@create-fd-close:
    - {fi-icl-dsi}:       [DMESG-WARN][5] ([fdo#106107]) -> [PASS][6]
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6149/fi-icl-dsi/igt@gem_basic@create-fd-close.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3064/fi-icl-dsi/igt@gem_basic@create-fd-close.html
    - fi-cml-u2:          [INCOMPLETE][7] ([fdo#110566]) -> [PASS][8]
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6149/fi-cml-u2/igt@gem_basic@create-fd-close.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3064/fi-cml-u2/igt@gem_basic@create-fd-close.html

  * igt@gem_exec_suspend@basic-s3:
    - fi-blb-e6850:       [INCOMPLETE][9] ([fdo#107718]) -> [PASS][10]
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6149/fi-blb-e6850/igt@gem_exec_suspend@basic-s3.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3064/fi-blb-e6850/igt@gem_exec_suspend@basic-s3.html

  * igt@i915_pm_rpm@module-reload:
    - fi-skl-6770hq:      [FAIL][11] ([fdo#108511]) -> [PASS][12]
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6149/fi-skl-6770hq/igt@i915_pm_rpm@module-reload.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3064/fi-skl-6770hq/igt@i915_pm_rpm@module-reload.html

  * igt@i915_selftest@live_contexts:
    - fi-bdw-gvtdvm:      [DMESG-FAIL][13] ([fdo#110235]) -> [PASS][14]
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6149/fi-bdw-gvtdvm/igt@i915_selftest@live_contexts.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3064/fi-bdw-gvtdvm/igt@i915_selftest@live_contexts.html

  * {igt@i915_selftest@live_reset}:
    - fi-skl-iommu:       [INCOMPLETE][15] ([fdo#108602]) -> [PASS][16]
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6149/fi-skl-iommu/igt@i915_selftest@live_reset.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3064/fi-skl-iommu/igt@i915_selftest@live_reset.html

  * igt@kms_cursor_legacy@basic-flip-before-cursor-varying-size:
    - fi-icl-u3:          [DMESG-WARN][17] ([fdo#107724]) -> [PASS][18]
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6149/fi-icl-u3/igt@kms_cursor_legacy@basic-flip-before-cursor-varying-size.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3064/fi-icl-u3/igt@kms_cursor_legacy@basic-flip-before-cursor-varying-size.html

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#106107]: https://bugs.freedesktop.org/show_bug.cgi?id=106107
  [fdo#107713]: https://bugs.freedesktop.org/show_bug.cgi?id=107713
  [fdo#107718]: https://bugs.freedesktop.org/show_bug.cgi?id=107718
  [fdo#107724]: https://bugs.freedesktop.org/show_bug.cgi?id=107724
  [fdo#108511]: https://bugs.freedesktop.org/show_bug.cgi?id=108511
  [fdo#108569]: https://bugs.freedesktop.org/show_bug.cgi?id=108569
  [fdo#108602]: https://bugs.freedesktop.org/show_bug.cgi?id=108602
  [fdo#110235]: https://bugs.freedesktop.org/show_bug.cgi?id=110235
  [fdo#110566]: https://bugs.freedesktop.org/show_bug.cgi?id=110566


Participating hosts (52 -> 45)
------------------------------

  Additional (1): fi-bdw-5557u 
  Missing    (8): fi-kbl-soraka fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-ctg-p8600 fi-byt-clapper fi-bdw-samus 


Build changes
-------------

  * IGT: IGT_5017 -> IGTPW_3064

  CI_DRM_6149: e8f06c34fa3c76fa94010485e07b89935fa8b9b5 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_3064: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3064/
  IGT_5017: 2892adce93fb8eea3d764dc0f766a202d9dcef37 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3064/
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] ✓ Fi.CI.IGT: success for tests/kms_chamelium: add pulse test (rev3)

[Patchwork]

== Series Details ==

Series: tests/kms_chamelium: add pulse test (rev3)
URL   : https://patchwork.freedesktop.org/series/61111/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_6149_full -> IGTPW_3064_full
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/61111/revisions/3/mbox/

Known issues
------------

  Here are the changes found in IGTPW_3064_full that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@gem_eio@in-flight-suspend:
    - shard-glk:          [PASS][1] -> [FAIL][2] ([fdo#110667])
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6149/shard-glk7/igt@gem_eio@in-flight-suspend.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3064/shard-glk6/igt@gem_eio@in-flight-suspend.html

  * igt@gem_pwrite@big-gtt-forwards:
    - shard-glk:          [PASS][3] -> [INCOMPLETE][4] ([fdo#103359] / [k.org#198133])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6149/shard-glk6/igt@gem_pwrite@big-gtt-forwards.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3064/shard-glk3/igt@gem_pwrite@big-gtt-forwards.html

  * igt@gem_workarounds@suspend-resume:
    - shard-apl:          [PASS][5] -> [DMESG-WARN][6] ([fdo#108566]) +5 similar issues
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6149/shard-apl3/igt@gem_workarounds@suspend-resume.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3064/shard-apl8/igt@gem_workarounds@suspend-resume.html

  * igt@kms_flip@2x-plain-flip-fb-recreate-interruptible:
    - shard-hsw:          [PASS][7] -> [SKIP][8] ([fdo#109271]) +18 similar issues
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6149/shard-hsw6/igt@kms_flip@2x-plain-flip-fb-recreate-interruptible.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3064/shard-hsw1/igt@kms_flip@2x-plain-flip-fb-recreate-interruptible.html

  * igt@kms_flip@plain-flip-fb-recreate-interruptible:
    - shard-glk:          [PASS][9] -> [FAIL][10] ([fdo#100368])
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6149/shard-glk8/igt@kms_flip@plain-flip-fb-recreate-interruptible.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3064/shard-glk3/igt@kms_flip@plain-flip-fb-recreate-interruptible.html

  * igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-pri-indfb-draw-blt:
    - shard-iclb:         [PASS][11] -> [FAIL][12] ([fdo#103167]) +6 similar issues
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6149/shard-iclb1/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-pri-indfb-draw-blt.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3064/shard-iclb7/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-pri-indfb-draw-blt.html

  * igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a:
    - shard-kbl:          [PASS][13] -> [DMESG-WARN][14] ([fdo#108566]) +1 similar issue
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6149/shard-kbl7/igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3064/shard-kbl1/igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a.html

  * igt@kms_psr@psr2_cursor_plane_onoff:
    - shard-iclb:         [PASS][15] -> [SKIP][16] ([fdo#109441]) +1 similar issue
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6149/shard-iclb2/igt@kms_psr@psr2_cursor_plane_onoff.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3064/shard-iclb3/igt@kms_psr@psr2_cursor_plane_onoff.html

  * igt@kms_vblank@pipe-b-ts-continuation-dpms-suspend:
    - shard-iclb:         [PASS][17] -> [INCOMPLETE][18] ([fdo#107713])
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6149/shard-iclb3/igt@kms_vblank@pipe-b-ts-continuation-dpms-suspend.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3064/shard-iclb3/igt@kms_vblank@pipe-b-ts-continuation-dpms-suspend.html

  
#### Possible fixes ####

  * igt@gem_tiled_swapping@non-threaded:
    - shard-glk:          [DMESG-WARN][19] ([fdo#108686]) -> [PASS][20]
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6149/shard-glk8/igt@gem_tiled_swapping@non-threaded.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3064/shard-glk3/igt@gem_tiled_swapping@non-threaded.html

  * igt@i915_pm_rpm@legacy-planes:
    - shard-iclb:         [INCOMPLETE][21] ([fdo#107713] / [fdo#108840] / [fdo#109960]) -> [PASS][22]
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6149/shard-iclb2/igt@i915_pm_rpm@legacy-planes.html
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3064/shard-iclb1/igt@i915_pm_rpm@legacy-planes.html

  * igt@i915_suspend@fence-restore-tiled2untiled:
    - shard-apl:          [DMESG-WARN][23] ([fdo#108566]) -> [PASS][24] +6 similar issues
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6149/shard-apl3/igt@i915_suspend@fence-restore-tiled2untiled.html
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3064/shard-apl8/igt@i915_suspend@fence-restore-tiled2untiled.html

  * igt@kms_flip@2x-flip-vs-expired-vblank-interruptible:
    - shard-hsw:          [SKIP][25] ([fdo#109271]) -> [PASS][26] +24 similar issues
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6149/shard-hsw1/igt@kms_flip@2x-flip-vs-expired-vblank-interruptible.html
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3064/shard-hsw8/igt@kms_flip@2x-flip-vs-expired-vblank-interruptible.html

  * igt@kms_flip@flip-vs-expired-vblank-interruptible:
    - shard-glk:          [FAIL][27] ([fdo#105363]) -> [PASS][28]
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6149/shard-glk7/igt@kms_flip@flip-vs-expired-vblank-interruptible.html
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3064/shard-glk2/igt@kms_flip@flip-vs-expired-vblank-interruptible.html

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-draw-pwrite:
    - shard-apl:          [FAIL][29] ([fdo#103167]) -> [PASS][30]
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6149/shard-apl2/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-draw-pwrite.html
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3064/shard-apl5/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-draw-pwrite.html
    - shard-kbl:          [FAIL][31] ([fdo#103167]) -> [PASS][32]
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6149/shard-kbl6/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-draw-pwrite.html
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3064/shard-kbl7/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-draw-pwrite.html

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-render:
    - shard-iclb:         [FAIL][33] ([fdo#103167]) -> [PASS][34]
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6149/shard-iclb1/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-render.html
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3064/shard-iclb1/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-render.html

  * igt@kms_frontbuffer_tracking@fbc-2p-primscrn-cur-indfb-draw-render:
    - shard-glk:          [FAIL][35] ([fdo#103167]) -> [PASS][36]
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6149/shard-glk6/igt@kms_frontbuffer_tracking@fbc-2p-primscrn-cur-indfb-draw-render.html
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3064/shard-glk5/igt@kms_frontbuffer_tracking@fbc-2p-primscrn-cur-indfb-draw-render.html

  * igt@kms_psr2_su@frontbuffer:
    - shard-iclb:         [SKIP][37] ([fdo#109642]) -> [PASS][38]
   [37]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6149/shard-iclb3/igt@kms_psr2_su@frontbuffer.html
   [38]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3064/shard-iclb2/igt@kms_psr2_su@frontbuffer.html

  * igt@kms_psr@no_drrs:
    - shard-iclb:         [FAIL][39] ([fdo#108341]) -> [PASS][40]
   [39]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6149/shard-iclb1/igt@kms_psr@no_drrs.html
   [40]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3064/shard-iclb2/igt@kms_psr@no_drrs.html

  
#### Warnings ####

  * igt@gem_mmap_gtt@forked-big-copy:
    - shard-iclb:         [TIMEOUT][41] ([fdo#109673]) -> [INCOMPLETE][42] ([fdo#107713] / [fdo#109100])
   [41]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6149/shard-iclb8/igt@gem_mmap_gtt@forked-big-copy.html
   [42]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3064/shard-iclb2/igt@gem_mmap_gtt@forked-big-copy.html

  * igt@gem_mmap_gtt@forked-big-copy-xy:
    - shard-iclb:         [INCOMPLETE][43] ([fdo#107713] / [fdo#109100]) -> [TIMEOUT][44] ([fdo#109673])
   [43]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6149/shard-iclb3/igt@gem_mmap_gtt@forked-big-copy-xy.html
   [44]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3064/shard-iclb1/igt@gem_mmap_gtt@forked-big-copy-xy.html

  * igt@gem_tiled_swapping@non-threaded:
    - shard-hsw:          [FAIL][45] ([fdo#108686]) -> [INCOMPLETE][46] ([fdo#103540])
   [45]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6149/shard-hsw6/igt@gem_tiled_swapping@non-threaded.html
   [46]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3064/shard-hsw2/igt@gem_tiled_swapping@non-threaded.html

  * igt@gem_userptr_blits@process-exit-gtt-busy:
    - shard-apl:          [INCOMPLETE][47] ([fdo#103927]) -> [SKIP][48] ([fdo#109271])
   [47]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6149/shard-apl3/igt@gem_userptr_blits@process-exit-gtt-busy.html
   [48]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3064/shard-apl3/igt@gem_userptr_blits@process-exit-gtt-busy.html

  
  [fdo#100368]: https://bugs.freedesktop.org/show_bug.cgi?id=100368
  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#103359]: https://bugs.freedesktop.org/show_bug.cgi?id=103359
  [fdo#103540]: https://bugs.freedesktop.org/show_bug.cgi?id=103540
  [fdo#103927]: https://bugs.freedesktop.org/show_bug.cgi?id=103927
  [fdo#105363]: https://bugs.freedesktop.org/show_bug.cgi?id=105363
  [fdo#107713]: https://bugs.freedesktop.org/show_bug.cgi?id=107713
  [fdo#108341]: https://bugs.freedesktop.org/show_bug.cgi?id=108341
  [fdo#108566]: https://bugs.freedesktop.org/show_bug.cgi?id=108566
  [fdo#108686]: https://bugs.freedesktop.org/show_bug.cgi?id=108686
  [fdo#108840]: https://bugs.freedesktop.org/show_bug.cgi?id=108840
  [fdo#109100]: https://bugs.freedesktop.org/show_bug.cgi?id=109100
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109441]: https://bugs.freedesktop.org/show_bug.cgi?id=109441
  [fdo#109642]: https://bugs.freedesktop.org/show_bug.cgi?id=109642
  [fdo#109673]: https://bugs.freedesktop.org/show_bug.cgi?id=109673
  [fdo#109960]: https://bugs.freedesktop.org/show_bug.cgi?id=109960
  [fdo#110667]: https://bugs.freedesktop.org/show_bug.cgi?id=110667
  [k.org#198133]: https://bugzilla.kernel.org/show_bug.cgi?id=198133


Participating hosts (10 -> 6)
------------------------------

  Missing    (4): pig-skl-6260u shard-skl pig-hsw-4770r pig-glk-j5005 


Build changes
-------------

  * IGT: IGT_5017 -> IGTPW_3064
  * Piglit: piglit_4509 -> None

  CI_DRM_6149: e8f06c34fa3c76fa94010485e07b89935fa8b9b5 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_3064: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3064/
  IGT_5017: 2892adce93fb8eea3d764dc0f766a202d9dcef37 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3064/
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t v3 09/10] tests/kms_chamelium: add a flatline audio test

[Tvrtko Ursulin]

On 27/05/2019 15:34, Simon Ser wrote:
> This commit adds a flatline test alongside the existing frequencies test.
> 
> The test sends a constant value and checks that the amplitude is correct. A
> window is used to check that each sample is within acceptable bounds. The test
> is stopped as soon as 3 audio pages pass the test.
> 
> Signed-off-by: Simon Ser <simon.ser@intel.com>
> Reviewed-by: Martin Peres <martin.peres@linux.intel.com>
> ---
>   tests/kms_chamelium.c | 101 ++++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 101 insertions(+)
> 
> diff --git a/tests/kms_chamelium.c b/tests/kms_chamelium.c
> index 40ca93687c20..451a616f1a2e 100644
> --- a/tests/kms_chamelium.c
> +++ b/tests/kms_chamelium.c
> @@ -772,6 +772,9 @@ test_display_frame_dump(data_t *data, struct chamelium_port *port)
>   /* A streak of 3 gives confidence that the signal is good. */
>   #define MIN_STREAK 3
>   
> +#define FLATLINE_AMPLITUDE 0.9 /* normalized, ie. in [0, 1] */

I assume the test is making triple sure it only ever outputs this signal 
to connectors connected to Chamelium, in all possible scenarios? (I am 
thinking it could be dangerous to some amps/speakers if by some kind of 
accident.)

Regards,

Tvrtko

> +#define FLATLINE_ACCURACY 0.001 /* ± 0.1% of the full amplitude */
> +
>   /* TODO: enable >48KHz rates, these are not reliable */
>   static int test_sampling_rates[] = {
>   	32000,
> @@ -1138,6 +1141,103 @@ static bool test_audio_frequencies(struct audio_state *state)
>   	return success;
>   }
>   
> +static int audio_output_flatline_callback(void *data, void *buffer,
> +					     int samples)
> +{
> +	struct audio_state *state = data;
> +	double *tmp;
> +	size_t len, i;
> +
> +	len = samples * state->playback.channels;
> +	tmp = malloc(len * sizeof(double));
> +	for (i = 0; i < len; i++)
> +		tmp[i] = FLATLINE_AMPLITUDE;
> +	audio_convert_to(buffer, tmp, len, state->playback.format);
> +	free(tmp);
> +
> +	return state->run ? 0 : -1;
> +}
> +
> +static bool detect_flatline_amplitude(double *buf, size_t buf_len)
> +{
> +	double min, max;
> +	size_t i;
> +	bool ok;
> +
> +	min = max = NAN;
> +	for (i = 0; i < buf_len; i++) {
> +		if (isnan(min) || buf[i] < min)
> +			min = buf[i];
> +		if (isnan(max) || buf[i] > max)
> +			max = buf[i];
> +	}
> +
> +	ok = (min >= FLATLINE_AMPLITUDE - FLATLINE_ACCURACY &&
> +	      max <= FLATLINE_AMPLITUDE + FLATLINE_ACCURACY);
> +	if (ok)
> +		igt_debug("Flatline detected\n");
> +	else
> +		igt_debug("Flatline not detected (min=%f, max=%f)\n",
> +			  min, max);
> +	return ok;
> +}
> +
> +static bool test_audio_flatline(struct audio_state *state)
> +{
> +	bool success;
> +	int32_t *recv;
> +	size_t recv_len, i, channel_len;
> +	int streak, capture_chan;
> +	double *channel;
> +
> +	alsa_register_output_callback(state->alsa,
> +				      audio_output_flatline_callback, state,
> +				      PLAYBACK_SAMPLES);
> +
> +	audio_state_start(state, "flatline");
> +
> +	recv = NULL;
> +	recv_len = 0;
> +	success = false;
> +	while (!success && state->msec < AUDIO_TIMEOUT) {
> +		audio_state_receive(state, &recv, &recv_len);
> +
> +		igt_debug("Detecting audio signal, t=%d msec\n", state->msec);
> +
> +		for (i = 0; i < state->playback.channels; i++) {
> +			capture_chan = state->channel_mapping[i];
> +			igt_assert(capture_chan >= 0);
> +			igt_debug("Processing channel %zu (captured as "
> +				  "channel %d)\n", i, capture_chan);
> +
> +			channel_len = audio_extract_channel_s32_le(NULL, 0,
> +								   recv, recv_len,
> +								   state->capture.channels,
> +								   capture_chan);
> +			channel = malloc(channel_len * sizeof(double));
> +			audio_extract_channel_s32_le(channel, channel_len,
> +						     recv, recv_len,
> +						     state->capture.channels,
> +						     capture_chan);
> +
> +			if (detect_flatline_amplitude(channel, channel_len))
> +				streak++;
> +			else
> +				streak = 0;
> +
> +			free(channel);
> +		}
> +
> +		success = streak == MIN_STREAK * state->playback.channels;
> +	}
> +
> +	audio_state_stop(state, success);
> +
> +	free(recv);
> +
> +	return success;
> +}
> +
>   static bool check_audio_configuration(struct alsa *alsa, snd_pcm_format_t format,
>   				      int channels, int sampling_rate)
>   {
> @@ -1235,6 +1335,7 @@ test_display_audio(data_t *data, struct chamelium_port *port,
>   			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);
>   
>   			alsa_close_output(alsa);
> 
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t v3 09/10] tests/kms_chamelium: add a flatline audio test

[Ser, Simon]

On Tue, 2019-06-04 at 09:38 +0100, Tvrtko Ursulin wrote:
> On 27/05/2019 15:34, Simon Ser wrote:
> > This commit adds a flatline test alongside the existing frequencies test.
> > 
> > The test sends a constant value and checks that the amplitude is correct. A
> > window is used to check that each sample is within acceptable bounds. The test
> > is stopped as soon as 3 audio pages pass the test.
> > 
> > Signed-off-by: Simon Ser <simon.ser@intel.com>
> > Reviewed-by: Martin Peres <martin.peres@linux.intel.com>
> > ---
> >   tests/kms_chamelium.c | 101 ++++++++++++++++++++++++++++++++++++++++++
> >   1 file changed, 101 insertions(+)
> > 
> > diff --git a/tests/kms_chamelium.c b/tests/kms_chamelium.c
> > index 40ca93687c20..451a616f1a2e 100644
> > --- a/tests/kms_chamelium.c
> > +++ b/tests/kms_chamelium.c
> > @@ -772,6 +772,9 @@ test_display_frame_dump(data_t *data, struct chamelium_port *port)
> >   /* A streak of 3 gives confidence that the signal is good. */
> >   #define MIN_STREAK 3
> >   
> > +#define FLATLINE_AMPLITUDE 0.9 /* normalized, ie. in [0, 1] */
> 
> I assume the test is making triple sure it only ever outputs this signal 
> to connectors connected to Chamelium, in all possible scenarios? (I am 
> thinking it could be dangerous to some amps/speakers if by some kind of 
> accident.)

Not at all. The signal is sent to all HDMI/DP ports.

I have to check whether it's easy to match ALSA outputs to monitor
names.

Martin, is this a concern?

> Regards,
> 
> Tvrtko
> 
> > +#define FLATLINE_ACCURACY 0.001 /* ± 0.1% of the full amplitude */
> > +
> >   /* TODO: enable >48KHz rates, these are not reliable */
> >   static int test_sampling_rates[] = {
> >   	32000,
> > @@ -1138,6 +1141,103 @@ static bool test_audio_frequencies(struct audio_state *state)
> >   	return success;
> >   }
> >   
> > +static int audio_output_flatline_callback(void *data, void *buffer,
> > +					     int samples)
> > +{
> > +	struct audio_state *state = data;
> > +	double *tmp;
> > +	size_t len, i;
> > +
> > +	len = samples * state->playback.channels;
> > +	tmp = malloc(len * sizeof(double));
> > +	for (i = 0; i < len; i++)
> > +		tmp[i] = FLATLINE_AMPLITUDE;
> > +	audio_convert_to(buffer, tmp, len, state->playback.format);
> > +	free(tmp);
> > +
> > +	return state->run ? 0 : -1;
> > +}
> > +
> > +static bool detect_flatline_amplitude(double *buf, size_t buf_len)
> > +{
> > +	double min, max;
> > +	size_t i;
> > +	bool ok;
> > +
> > +	min = max = NAN;
> > +	for (i = 0; i < buf_len; i++) {
> > +		if (isnan(min) || buf[i] < min)
> > +			min = buf[i];
> > +		if (isnan(max) || buf[i] > max)
> > +			max = buf[i];
> > +	}
> > +
> > +	ok = (min >= FLATLINE_AMPLITUDE - FLATLINE_ACCURACY &&
> > +	      max <= FLATLINE_AMPLITUDE + FLATLINE_ACCURACY);
> > +	if (ok)
> > +		igt_debug("Flatline detected\n");
> > +	else
> > +		igt_debug("Flatline not detected (min=%f, max=%f)\n",
> > +			  min, max);
> > +	return ok;
> > +}
> > +
> > +static bool test_audio_flatline(struct audio_state *state)
> > +{
> > +	bool success;
> > +	int32_t *recv;
> > +	size_t recv_len, i, channel_len;
> > +	int streak, capture_chan;
> > +	double *channel;
> > +
> > +	alsa_register_output_callback(state->alsa,
> > +				      audio_output_flatline_callback, state,
> > +				      PLAYBACK_SAMPLES);
> > +
> > +	audio_state_start(state, "flatline");
> > +
> > +	recv = NULL;
> > +	recv_len = 0;
> > +	success = false;
> > +	while (!success && state->msec < AUDIO_TIMEOUT) {
> > +		audio_state_receive(state, &recv, &recv_len);
> > +
> > +		igt_debug("Detecting audio signal, t=%d msec\n", state->msec);
> > +
> > +		for (i = 0; i < state->playback.channels; i++) {
> > +			capture_chan = state->channel_mapping[i];
> > +			igt_assert(capture_chan >= 0);
> > +			igt_debug("Processing channel %zu (captured as "
> > +				  "channel %d)\n", i, capture_chan);
> > +
> > +			channel_len = audio_extract_channel_s32_le(NULL, 0,
> > +								   recv, recv_len,
> > +								   state->capture.channels,
> > +								   capture_chan);
> > +			channel = malloc(channel_len * sizeof(double));
> > +			audio_extract_channel_s32_le(channel, channel_len,
> > +						     recv, recv_len,
> > +						     state->capture.channels,
> > +						     capture_chan);
> > +
> > +			if (detect_flatline_amplitude(channel, channel_len))
> > +				streak++;
> > +			else
> > +				streak = 0;
> > +
> > +			free(channel);
> > +		}
> > +
> > +		success = streak == MIN_STREAK * state->playback.channels;
> > +	}
> > +
> > +	audio_state_stop(state, success);
> > +
> > +	free(recv);
> > +
> > +	return success;
> > +}
> > +
> >   static bool check_audio_configuration(struct alsa *alsa, snd_pcm_format_t format,
> >   				      int channels, int sampling_rate)
> >   {
> > @@ -1235,6 +1335,7 @@ test_display_audio(data_t *data, struct chamelium_port *port,
> >   			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);
> >   
> >   			alsa_close_output(alsa);
> > 
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t v3 09/10] tests/kms_chamelium: add a flatline audio test

[Peres, Martin]

On 04/06/2019 14:22, Ser, Simon wrote:
> On Tue, 2019-06-04 at 09:38 +0100, Tvrtko Ursulin wrote:
>> On 27/05/2019 15:34, Simon Ser wrote:
>>> This commit adds a flatline test alongside the existing frequencies test.
>>>
>>> The test sends a constant value and checks that the amplitude is correct. A
>>> window is used to check that each sample is within acceptable bounds. The test
>>> is stopped as soon as 3 audio pages pass the test.
>>>
>>> Signed-off-by: Simon Ser <simon.ser@intel.com>
>>> Reviewed-by: Martin Peres <martin.peres@linux.intel.com>
>>> ---
>>>   tests/kms_chamelium.c | 101 ++++++++++++++++++++++++++++++++++++++++++
>>>   1 file changed, 101 insertions(+)
>>>
>>> diff --git a/tests/kms_chamelium.c b/tests/kms_chamelium.c
>>> index 40ca93687c20..451a616f1a2e 100644
>>> --- a/tests/kms_chamelium.c
>>> +++ b/tests/kms_chamelium.c
>>> @@ -772,6 +772,9 @@ test_display_frame_dump(data_t *data, struct chamelium_port *port)
>>>   /* A streak of 3 gives confidence that the signal is good. */
>>>   #define MIN_STREAK 3
>>>   
>>> +#define FLATLINE_AMPLITUDE 0.9 /* normalized, ie. in [0, 1] */
>>
>> I assume the test is making triple sure it only ever outputs this signal 
>> to connectors connected to Chamelium, in all possible scenarios? (I am 
>> thinking it could be dangerous to some amps/speakers if by some kind of 
>> accident.)
> 
> Not at all. The signal is sent to all HDMI/DP ports.
> 
> I have to check whether it's easy to match ALSA outputs to monitor
> names.
> 
> Martin, is this a concern?

This is true that a non-zero constant voltage could be damaging for
speakers as it can make them overheat without us hearing anything
(constant position == no sound heard, but Ohm's law still applies). It
would take longer than 1s though... On top of this, all speakers (except
subwoofers) have high-pass filters that should remove the DC-offset so
all we should be left with is a nice pop which might or might not be
loud depending on how powerful the speakers are and how loud their
settings are. Multi-kW systems definitely don't like them, but how
likely is it that people would run IGT on it? :D

That being said, if we can associate the alsa output to a certain
connector (the one we are reading the sound from), then it would
actually be a good thing to test the sound on this connector only, since
it would allow us to verify that the mapping is indeed correct!

Martin
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t v3 09/10] tests/kms_chamelium: add a flatline audio test

[Tvrtko Ursulin]

On 04/06/2019 13:26, Peres, Martin wrote:
> On 04/06/2019 14:22, Ser, Simon wrote:
>> On Tue, 2019-06-04 at 09:38 +0100, Tvrtko Ursulin wrote:
>>> On 27/05/2019 15:34, Simon Ser wrote:
>>>> This commit adds a flatline test alongside the existing frequencies test.
>>>>
>>>> The test sends a constant value and checks that the amplitude is correct. A
>>>> window is used to check that each sample is within acceptable bounds. The test
>>>> is stopped as soon as 3 audio pages pass the test.
>>>>
>>>> Signed-off-by: Simon Ser <simon.ser@intel.com>
>>>> Reviewed-by: Martin Peres <martin.peres@linux.intel.com>
>>>> ---
>>>>    tests/kms_chamelium.c | 101 ++++++++++++++++++++++++++++++++++++++++++
>>>>    1 file changed, 101 insertions(+)
>>>>
>>>> diff --git a/tests/kms_chamelium.c b/tests/kms_chamelium.c
>>>> index 40ca93687c20..451a616f1a2e 100644
>>>> --- a/tests/kms_chamelium.c
>>>> +++ b/tests/kms_chamelium.c
>>>> @@ -772,6 +772,9 @@ test_display_frame_dump(data_t *data, struct chamelium_port *port)
>>>>    /* A streak of 3 gives confidence that the signal is good. */
>>>>    #define MIN_STREAK 3
>>>>    
>>>> +#define FLATLINE_AMPLITUDE 0.9 /* normalized, ie. in [0, 1] */
>>>
>>> I assume the test is making triple sure it only ever outputs this signal
>>> to connectors connected to Chamelium, in all possible scenarios? (I am
>>> thinking it could be dangerous to some amps/speakers if by some kind of
>>> accident.)
>>
>> Not at all. The signal is sent to all HDMI/DP ports.
>>
>> I have to check whether it's easy to match ALSA outputs to monitor
>> names.
>>
>> Martin, is this a concern?
> 
> This is true that a non-zero constant voltage could be damaging for
> speakers as it can make them overheat without us hearing anything
> (constant position == no sound heard, but Ohm's law still applies). It
> would take longer than 1s though... On top of this, all speakers (except
> subwoofers) have high-pass filters that should remove the DC-offset so
> all we should be left with is a nice pop which might or might not be
> loud depending on how powerful the speakers are and how loud their
> settings are. Multi-kW systems definitely don't like them, but how
> likely is it that people would run IGT on it? :D

Why would all speakers have high-pass filters? I would be surprised if 
full range ones do, and N-way definitely do not in their totality. 
Considering the range of laptop speakers, monitor speakers, TVs, etc, 
all of varying quality, I'd be quite conservative and cautious before 
outputting even one second of +90% DC signal. I know it is just IGT and 
so extremely unlikely that if anything fries it would be anyone apart 
from us frying our own stuff, but, still, do we have to use DC as test 
signal?

> That being said, if we can associate the alsa output to a certain
> connector (the one we are reading the sound from), then it would
> actually be a good thing to test the sound on this connector only, since
> it would allow us to verify that the mapping is indeed correct!

Yeah.. and why do we even test this in such detail? I mean outputting 
different signals and stuff. Why not just send a sine wave beep or 
something and check it was captured? I did not figure out from the 
commit message why DC.

Regards,

Tvrtko
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t v3 09/10] tests/kms_chamelium: add a flatline audio test

[Ser, Simon]

On Tue, 2019-06-04 at 13:59 +0100, Tvrtko Ursulin wrote:
> On 04/06/2019 13:26, Peres, Martin wrote:
> > On 04/06/2019 14:22, Ser, Simon wrote:
> > > On Tue, 2019-06-04 at 09:38 +0100, Tvrtko Ursulin wrote:
> > > > On 27/05/2019 15:34, Simon Ser wrote:
> > > > > This commit adds a flatline test alongside the existing frequencies test.
> > > > > 
> > > > > The test sends a constant value and checks that the amplitude is correct. A
> > > > > window is used to check that each sample is within acceptable bounds. The test
> > > > > is stopped as soon as 3 audio pages pass the test.
> > > > > 
> > > > > Signed-off-by: Simon Ser <simon.ser@intel.com>
> > > > > Reviewed-by: Martin Peres <martin.peres@linux.intel.com>
> > > > > ---
> > > > >    tests/kms_chamelium.c | 101 ++++++++++++++++++++++++++++++++++++++++++
> > > > >    1 file changed, 101 insertions(+)
> > > > > 
> > > > > diff --git a/tests/kms_chamelium.c b/tests/kms_chamelium.c
> > > > > index 40ca93687c20..451a616f1a2e 100644
> > > > > --- a/tests/kms_chamelium.c
> > > > > +++ b/tests/kms_chamelium.c
> > > > > @@ -772,6 +772,9 @@ test_display_frame_dump(data_t *data, struct chamelium_port *port)
> > > > >    /* A streak of 3 gives confidence that the signal is good. */
> > > > >    #define MIN_STREAK 3
> > > > >    
> > > > > +#define FLATLINE_AMPLITUDE 0.9 /* normalized, ie. in [0, 1] */
> > > > 
> > > > I assume the test is making triple sure it only ever outputs this signal
> > > > to connectors connected to Chamelium, in all possible scenarios? (I am
> > > > thinking it could be dangerous to some amps/speakers if by some kind of
> > > > accident.)
> > > 
> > > Not at all. The signal is sent to all HDMI/DP ports.
> > > 
> > > I have to check whether it's easy to match ALSA outputs to monitor
> > > names.
> > > 
> > > Martin, is this a concern?
> > 
> > This is true that a non-zero constant voltage could be damaging for
> > speakers as it can make them overheat without us hearing anything
> > (constant position == no sound heard, but Ohm's law still applies). It
> > would take longer than 1s though... On top of this, all speakers (except
> > subwoofers) have high-pass filters that should remove the DC-offset so
> > all we should be left with is a nice pop which might or might not be
> > loud depending on how powerful the speakers are and how loud their
> > settings are. Multi-kW systems definitely don't like them, but how
> > likely is it that people would run IGT on it? :D
> 
> Why would all speakers have high-pass filters? I would be surprised if 
> full range ones do, and N-way definitely do not in their totality. 
> Considering the range of laptop speakers, monitor speakers, TVs, etc, 
> all of varying quality, I'd be quite conservative and cautious before 
> outputting even one second of +90% DC signal. I know it is just IGT and 
> so extremely unlikely that if anything fries it would be anyone apart 
> from us frying our own stuff, but, still, do we have to use DC as test 
> signal?

DC is just so simple. It also allows us to check that all samples are
in a very precise range.

Would it be fine to use a 25% DC?

> > That being said, if we can associate the alsa output to a certain
> > connector (the one we are reading the sound from), then it would
> > actually be a good thing to test the sound on this connector only, since
> > it would allow us to verify that the mapping is indeed correct!
> 
> Yeah.. and why do we even test this in such detail? I mean outputting 
> different signals and stuff. 

Because audio could be broken in a lot of different ways.

> Why not just send a sine wave beep or 
> something and check it was captured? I did not figure out from the 
> commit message why DC.

We already have sine wave tests, but sine waves make it complicated to
check that amplitude is correct for all samples.
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t v3 09/10] tests/kms_chamelium: add a flatline audio test

[Ser, Simon]

On Tue, 2019-06-04 at 13:26 +0100, Peres, Martin wrote:
> On 04/06/2019 14:22, Ser, Simon wrote:
> > On Tue, 2019-06-04 at 09:38 +0100, Tvrtko Ursulin wrote:
> > > On 27/05/2019 15:34, Simon Ser wrote:
> > > > This commit adds a flatline test alongside the existing frequencies test.
> > > > 
> > > > The test sends a constant value and checks that the amplitude is correct. A
> > > > window is used to check that each sample is within acceptable bounds. The test
> > > > is stopped as soon as 3 audio pages pass the test.
> > > > 
> > > > Signed-off-by: Simon Ser <simon.ser@intel.com>
> > > > Reviewed-by: Martin Peres <martin.peres@linux.intel.com>
> > > > ---
> > > >   tests/kms_chamelium.c | 101 ++++++++++++++++++++++++++++++++++++++++++
> > > >   1 file changed, 101 insertions(+)
> > > > 
> > > > diff --git a/tests/kms_chamelium.c b/tests/kms_chamelium.c
> > > > index 40ca93687c20..451a616f1a2e 100644
> > > > --- a/tests/kms_chamelium.c
> > > > +++ b/tests/kms_chamelium.c
> > > > @@ -772,6 +772,9 @@ test_display_frame_dump(data_t *data, struct chamelium_port *port)
> > > >   /* A streak of 3 gives confidence that the signal is good. */
> > > >   #define MIN_STREAK 3
> > > >   
> > > > +#define FLATLINE_AMPLITUDE 0.9 /* normalized, ie. in [0, 1] */
> > > 
> > > I assume the test is making triple sure it only ever outputs this signal 
> > > to connectors connected to Chamelium, in all possible scenarios? (I am 
> > > thinking it could be dangerous to some amps/speakers if by some kind of 
> > > accident.)
> > 
> > Not at all. The signal is sent to all HDMI/DP ports.
> > 
> > I have to check whether it's easy to match ALSA outputs to monitor
> > names.
> > 
> > Martin, is this a concern?
> 
> This is true that a non-zero constant voltage could be damaging for
> speakers as it can make them overheat without us hearing anything
> (constant position == no sound heard, but Ohm's law still applies). It
> would take longer than 1s though... On top of this, all speakers (except
> subwoofers) have high-pass filters that should remove the DC-offset so
> all we should be left with is a nice pop which might or might not be
> loud depending on how powerful the speakers are and how loud their
> settings are. Multi-kW systems definitely don't like them, but how
> likely is it that people would run IGT on it? :D
> 
> That being said, if we can associate the alsa output to a certain
> connector (the one we are reading the sound from), then it would
> actually be a good thing to test the sound on this connector only, since
> it would allow us to verify that the mapping is indeed correct!

Yes, I agree. I'll look into this, but I'm not sure it's possible with
the current kernel API.

TBH that's something that could be useful to usespace too…
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t v3 09/10] tests/kms_chamelium: add a flatline audio test

[Tvrtko Ursulin]

On 04/06/2019 15:06, Ser, Simon wrote:
> On Tue, 2019-06-04 at 13:59 +0100, Tvrtko Ursulin wrote:
>> On 04/06/2019 13:26, Peres, Martin wrote:
>>> On 04/06/2019 14:22, Ser, Simon wrote:
>>>> On Tue, 2019-06-04 at 09:38 +0100, Tvrtko Ursulin wrote:
>>>>> On 27/05/2019 15:34, Simon Ser wrote:
>>>>>> This commit adds a flatline test alongside the existing frequencies test.
>>>>>>
>>>>>> The test sends a constant value and checks that the amplitude is correct. A
>>>>>> window is used to check that each sample is within acceptable bounds. The test
>>>>>> is stopped as soon as 3 audio pages pass the test.
>>>>>>
>>>>>> Signed-off-by: Simon Ser <simon.ser@intel.com>
>>>>>> Reviewed-by: Martin Peres <martin.peres@linux.intel.com>
>>>>>> ---
>>>>>>     tests/kms_chamelium.c | 101 ++++++++++++++++++++++++++++++++++++++++++
>>>>>>     1 file changed, 101 insertions(+)
>>>>>>
>>>>>> diff --git a/tests/kms_chamelium.c b/tests/kms_chamelium.c
>>>>>> index 40ca93687c20..451a616f1a2e 100644
>>>>>> --- a/tests/kms_chamelium.c
>>>>>> +++ b/tests/kms_chamelium.c
>>>>>> @@ -772,6 +772,9 @@ test_display_frame_dump(data_t *data, struct chamelium_port *port)
>>>>>>     /* A streak of 3 gives confidence that the signal is good. */
>>>>>>     #define MIN_STREAK 3
>>>>>>     
>>>>>> +#define FLATLINE_AMPLITUDE 0.9 /* normalized, ie. in [0, 1] */
>>>>>
>>>>> I assume the test is making triple sure it only ever outputs this signal
>>>>> to connectors connected to Chamelium, in all possible scenarios? (I am
>>>>> thinking it could be dangerous to some amps/speakers if by some kind of
>>>>> accident.)
>>>>
>>>> Not at all. The signal is sent to all HDMI/DP ports.
>>>>
>>>> I have to check whether it's easy to match ALSA outputs to monitor
>>>> names.
>>>>
>>>> Martin, is this a concern?
>>>
>>> This is true that a non-zero constant voltage could be damaging for
>>> speakers as it can make them overheat without us hearing anything
>>> (constant position == no sound heard, but Ohm's law still applies). It
>>> would take longer than 1s though... On top of this, all speakers (except
>>> subwoofers) have high-pass filters that should remove the DC-offset so
>>> all we should be left with is a nice pop which might or might not be
>>> loud depending on how powerful the speakers are and how loud their
>>> settings are. Multi-kW systems definitely don't like them, but how
>>> likely is it that people would run IGT on it? :D
>>
>> Why would all speakers have high-pass filters? I would be surprised if
>> full range ones do, and N-way definitely do not in their totality.
>> Considering the range of laptop speakers, monitor speakers, TVs, etc,
>> all of varying quality, I'd be quite conservative and cautious before
>> outputting even one second of +90% DC signal. I know it is just IGT and
>> so extremely unlikely that if anything fries it would be anyone apart
>> from us frying our own stuff, but, still, do we have to use DC as test
>> signal?
> 
> DC is just so simple. It also allows us to check that all samples are
> in a very precise range.

Why is this test important? Is there some path in i915 which can mess up 
with individual sample levels? Like, it will lower 127 to 100, but won't 
affect 126? (In signed 8-bit for simplicity.)

> Would it be fine to use a 25% DC?

Must be better by definition but it would be best to ask ALSA folks.

>>> That being said, if we can associate the alsa output to a certain
>>> connector (the one we are reading the sound from), then it would
>>> actually be a good thing to test the sound on this connector only, since
>>> it would allow us to verify that the mapping is indeed correct!
>>
>> Yeah.. and why do we even test this in such detail? I mean outputting
>> different signals and stuff.
> 
> Because audio could be broken in a lot of different ways.
I don't understand if this "broken in a lot of different ways" is 
something i915 can affect or it more belongs in ALSA testing?

>> Why not just send a sine wave beep or
>> something and check it was captured? I did not figure out from the
>> commit message why DC.
> 
> We already have sine wave tests, but sine waves make it complicated to
> check that amplitude is correct for all samples.

Yeah, I don't get it as said above.

Regards,

Tvrtko
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t v3 09/10] tests/kms_chamelium: add a flatline audio test

[Ser, Simon]

On Wed, 2019-06-05 at 11:51 +0100, Tvrtko Ursulin wrote:
> On 04/06/2019 15:06, Ser, Simon wrote:
> > On Tue, 2019-06-04 at 13:59 +0100, Tvrtko Ursulin wrote:
> > > On 04/06/2019 13:26, Peres, Martin wrote:
> > > > On 04/06/2019 14:22, Ser, Simon wrote:
> > > > > On Tue, 2019-06-04 at 09:38 +0100, Tvrtko Ursulin wrote:
> > > > > > On 27/05/2019 15:34, Simon Ser wrote:
> > > > > > > This commit adds a flatline test alongside the existing frequencies test.
> > > > > > > 
> > > > > > > The test sends a constant value and checks that the amplitude is correct. A
> > > > > > > window is used to check that each sample is within acceptable bounds. The test
> > > > > > > is stopped as soon as 3 audio pages pass the test.
> > > > > > > 
> > > > > > > Signed-off-by: Simon Ser <simon.ser@intel.com>
> > > > > > > Reviewed-by: Martin Peres <martin.peres@linux.intel.com>
> > > > > > > ---
> > > > > > >     tests/kms_chamelium.c | 101 ++++++++++++++++++++++++++++++++++++++++++
> > > > > > >     1 file changed, 101 insertions(+)
> > > > > > > 
> > > > > > > diff --git a/tests/kms_chamelium.c b/tests/kms_chamelium.c
> > > > > > > index 40ca93687c20..451a616f1a2e 100644
> > > > > > > --- a/tests/kms_chamelium.c
> > > > > > > +++ b/tests/kms_chamelium.c
> > > > > > > @@ -772,6 +772,9 @@ test_display_frame_dump(data_t *data, struct chamelium_port *port)
> > > > > > >     /* A streak of 3 gives confidence that the signal is good. */
> > > > > > >     #define MIN_STREAK 3
> > > > > > >     
> > > > > > > +#define FLATLINE_AMPLITUDE 0.9 /* normalized, ie. in [0, 1] */
> > > > > > 
> > > > > > I assume the test is making triple sure it only ever outputs this signal
> > > > > > to connectors connected to Chamelium, in all possible scenarios? (I am
> > > > > > thinking it could be dangerous to some amps/speakers if by some kind of
> > > > > > accident.)
> > > > > 
> > > > > Not at all. The signal is sent to all HDMI/DP ports.
> > > > > 
> > > > > I have to check whether it's easy to match ALSA outputs to monitor
> > > > > names.
> > > > > 
> > > > > Martin, is this a concern?
> > > > 
> > > > This is true that a non-zero constant voltage could be damaging for
> > > > speakers as it can make them overheat without us hearing anything
> > > > (constant position == no sound heard, but Ohm's law still applies). It
> > > > would take longer than 1s though... On top of this, all speakers (except
> > > > subwoofers) have high-pass filters that should remove the DC-offset so
> > > > all we should be left with is a nice pop which might or might not be
> > > > loud depending on how powerful the speakers are and how loud their
> > > > settings are. Multi-kW systems definitely don't like them, but how
> > > > likely is it that people would run IGT on it? :D
> > > 
> > > Why would all speakers have high-pass filters? I would be surprised if
> > > full range ones do, and N-way definitely do not in their totality.
> > > Considering the range of laptop speakers, monitor speakers, TVs, etc,
> > > all of varying quality, I'd be quite conservative and cautious before
> > > outputting even one second of +90% DC signal. I know it is just IGT and
> > > so extremely unlikely that if anything fries it would be anyone apart
> > > from us frying our own stuff, but, still, do we have to use DC as test
> > > signal?
> > 
> > DC is just so simple. It also allows us to check that all samples are
> > in a very precise range.
> 
> Why is this test important? Is there some path in i915 which can mess up 
> with individual sample levels? Like, it will lower 127 to 100, but won't 
> affect 126? (In signed 8-bit for simplicity.)

Quoting Martin Peres:

   Cross-component validation is every component's responsibility: each
   component needs to check its integration with the latest released
   version, and the last LTS of each dependency/user (at least at a
   weekly cadence). It is also their job to report bugs (I certainly
   do)! i915 CI already runs Piglit (GL testsuite) both in post- and
   pre-merge, and we'll run Media and Compute tests (we need to test
   more versions of mesa though).

Yes, we want to make sure the audio is perfect. The approach used with
the Chamelium is black-box: I don't know how i915 works, how ALSA
works, I don't know whether i915 touches the samples or not. I just
want to make sure actual users get proper sound.

If the test fails because of an ALSA bug, I'll be more than happy to
report the bug to ALSA and to make sure it gets fixed so that our users
don't run into it.

If nobody wants to have end-to-end tests, then nothing will get
properly tested. I can imagine someone working with ALSA saying that
the DRM path won't be tested because it involves i915. In the end this
means bad coverage.

What do you think?

> > Would it be fine to use a 25% DC?
> 
> Must be better by definition but it would be best to ask ALSA folks.
> 
> > > > That being said, if we can associate the alsa output to a certain
> > > > connector (the one we are reading the sound from), then it would
> > > > actually be a good thing to test the sound on this connector only, since
> > > > it would allow us to verify that the mapping is indeed correct!
> > > 
> > > Yeah.. and why do we even test this in such detail? I mean outputting
> > > different signals and stuff.
> > 
> > Because audio could be broken in a lot of different ways.
> I don't understand if this "broken in a lot of different ways" is 
> something i915 can affect or it more belongs in ALSA testing?

(See above)

> > > Why not just send a sine wave beep or
> > > something and check it was captured? I did not figure out from the
> > > commit message why DC.
> > 
> > We already have sine wave tests, but sine waves make it complicated to
> > check that amplitude is correct for all samples.
> 
> Yeah, I don't get it as said above.
> 
> Regards,
> 
> Tvrtko
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t v3 09/10] tests/kms_chamelium: add a flatline audio test

[Tvrtko Ursulin]

On 05/06/2019 14:04, Ser, Simon wrote:
> On Wed, 2019-06-05 at 11:51 +0100, Tvrtko Ursulin wrote:
>> On 04/06/2019 15:06, Ser, Simon wrote:
>>> On Tue, 2019-06-04 at 13:59 +0100, Tvrtko Ursulin wrote:
>>>> On 04/06/2019 13:26, Peres, Martin wrote:
>>>>> On 04/06/2019 14:22, Ser, Simon wrote:
>>>>>> On Tue, 2019-06-04 at 09:38 +0100, Tvrtko Ursulin wrote:
>>>>>>> On 27/05/2019 15:34, Simon Ser wrote:
>>>>>>>> This commit adds a flatline test alongside the existing frequencies test.
>>>>>>>>
>>>>>>>> The test sends a constant value and checks that the amplitude is correct. A
>>>>>>>> window is used to check that each sample is within acceptable bounds. The test
>>>>>>>> is stopped as soon as 3 audio pages pass the test.
>>>>>>>>
>>>>>>>> Signed-off-by: Simon Ser <simon.ser@intel.com>
>>>>>>>> Reviewed-by: Martin Peres <martin.peres@linux.intel.com>
>>>>>>>> ---
>>>>>>>>      tests/kms_chamelium.c | 101 ++++++++++++++++++++++++++++++++++++++++++
>>>>>>>>      1 file changed, 101 insertions(+)
>>>>>>>>
>>>>>>>> diff --git a/tests/kms_chamelium.c b/tests/kms_chamelium.c
>>>>>>>> index 40ca93687c20..451a616f1a2e 100644
>>>>>>>> --- a/tests/kms_chamelium.c
>>>>>>>> +++ b/tests/kms_chamelium.c
>>>>>>>> @@ -772,6 +772,9 @@ test_display_frame_dump(data_t *data, struct chamelium_port *port)
>>>>>>>>      /* A streak of 3 gives confidence that the signal is good. */
>>>>>>>>      #define MIN_STREAK 3
>>>>>>>>      
>>>>>>>> +#define FLATLINE_AMPLITUDE 0.9 /* normalized, ie. in [0, 1] */
>>>>>>>
>>>>>>> I assume the test is making triple sure it only ever outputs this signal
>>>>>>> to connectors connected to Chamelium, in all possible scenarios? (I am
>>>>>>> thinking it could be dangerous to some amps/speakers if by some kind of
>>>>>>> accident.)
>>>>>>
>>>>>> Not at all. The signal is sent to all HDMI/DP ports.
>>>>>>
>>>>>> I have to check whether it's easy to match ALSA outputs to monitor
>>>>>> names.
>>>>>>
>>>>>> Martin, is this a concern?
>>>>>
>>>>> This is true that a non-zero constant voltage could be damaging for
>>>>> speakers as it can make them overheat without us hearing anything
>>>>> (constant position == no sound heard, but Ohm's law still applies). It
>>>>> would take longer than 1s though... On top of this, all speakers (except
>>>>> subwoofers) have high-pass filters that should remove the DC-offset so
>>>>> all we should be left with is a nice pop which might or might not be
>>>>> loud depending on how powerful the speakers are and how loud their
>>>>> settings are. Multi-kW systems definitely don't like them, but how
>>>>> likely is it that people would run IGT on it? :D
>>>>
>>>> Why would all speakers have high-pass filters? I would be surprised if
>>>> full range ones do, and N-way definitely do not in their totality.
>>>> Considering the range of laptop speakers, monitor speakers, TVs, etc,
>>>> all of varying quality, I'd be quite conservative and cautious before
>>>> outputting even one second of +90% DC signal. I know it is just IGT and
>>>> so extremely unlikely that if anything fries it would be anyone apart
>>>> from us frying our own stuff, but, still, do we have to use DC as test
>>>> signal?
>>>
>>> DC is just so simple. It also allows us to check that all samples are
>>> in a very precise range.
>>
>> Why is this test important? Is there some path in i915 which can mess up
>> with individual sample levels? Like, it will lower 127 to 100, but won't
>> affect 126? (In signed 8-bit for simplicity.)
> 
> Quoting Martin Peres:
> 
>     Cross-component validation is every component's responsibility: each
>     component needs to check its integration with the latest released
>     version, and the last LTS of each dependency/user (at least at a
>     weekly cadence). It is also their job to report bugs (I certainly
>     do)! i915 CI already runs Piglit (GL testsuite) both in post- and
>     pre-merge, and we'll run Media and Compute tests (we need to test
>     more versions of mesa though).
> 
> Yes, we want to make sure the audio is perfect. The approach used with
> the Chamelium is black-box: I don't know how i915 works, how ALSA
> works, I don't know whether i915 touches the samples or not. I just
> want to make sure actual users get proper sound.
> 
> If the test fails because of an ALSA bug, I'll be more than happy to
> report the bug to ALSA and to make sure it gets fixed so that our users
> don't run into it.
> 
> If nobody wants to have end-to-end tests, then nothing will get
> properly tested. I can imagine someone working with ALSA saying that
> the DRM path won't be tested because it involves i915. In the end this
> means bad coverage.
> 
> What do you think?

Note that Piglit tests that we run, and media and compute tests that we 
plan to (I hope so at least for the latter), will not be written by i915 
developers but respective component owners. From this follows that it 
may very well make sense to run some ALSA test suite (or cross-cooperate 
to write or adapt one, depending on what is available), but less so to 
write our own.

When you get to the point where simplest way of figuring out whether 
sound works is not enough is where I would draw a line. And as you 
already seem to be past this point my recommendation would be to 
reconsider, especially the DC output test. Or at least reach out to ALSA 
folks to get their opinion on how safe or unsafe that might be against a 
wide palette of random consumer class junk hardware.

Regards,

Tvrtko

> 
>>> Would it be fine to use a 25% DC?
>>
>> Must be better by definition but it would be best to ask ALSA folks.
>>
>>>>> That being said, if we can associate the alsa output to a certain
>>>>> connector (the one we are reading the sound from), then it would
>>>>> actually be a good thing to test the sound on this connector only, since
>>>>> it would allow us to verify that the mapping is indeed correct!
>>>>
>>>> Yeah.. and why do we even test this in such detail? I mean outputting
>>>> different signals and stuff.
>>>
>>> Because audio could be broken in a lot of different ways.
>> I don't understand if this "broken in a lot of different ways" is
>> something i915 can affect or it more belongs in ALSA testing?
> 
> (See above)
> 
>>>> Why not just send a sine wave beep or
>>>> something and check it was captured? I did not figure out from the
>>>> commit message why DC.
>>>
>>> We already have sine wave tests, but sine waves make it complicated to
>>> check that amplitude is correct for all samples.
>>
>> Yeah, I don't get it as said above.
>>
>> Regards,
>>
>> Tvrtko
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev