[igt-dev] [PATCH i-g-t 0/7] tests/kms_chamelium: add hdmi-audio test

[igt-dev] [PATCH i-g-t 0/7] tests/kms_chamelium: add hdmi-audio test

[Simon Ser]

This patch series adds a new hdmi-audio test to the Chamelium suite.

Generating our own EDID is necessary to get HDMI audio to work. Patches 1 and 2
add the necessary features to lib/igt_edid. Patch 3 uses this library to
generate the appropriate EDID. Other patches are minor fixes and improvements.

Simon Ser (7):
  lib/igt_edid: add support for Short Audio Descriptors
  lib/igt_edid: add support for Vendor Specific Data blocks
  tests/kms_chamelium: generate an EDID with audio support
  test/kms_chamelium: disable >48KHz audio tests
  tests/kms_chamelium: enable audio test on HDMI ports
  tests/kms_chamelium: don't abort audio test on first fail
  HAX: add dp-audio test to fast-feedback

 lib/igt_chamelium.c                   |   9 +-
 lib/igt_edid.c                        | 113 +++++++++++++++--
 lib/igt_edid.h                        | 103 +++++++++++++++-
 tests/intel-ci/fast-feedback.testlist |   1 +
 tests/kms_chamelium.c                 | 171 ++++++++++++++++++--------
 5 files changed, 332 insertions(+), 65 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 1/7] lib/igt_edid: add support for Short Audio Descriptors

[Simon Ser]

Short Audio Descriptors (SADs) can be wrapped in an EDID extension to advertise
audio support.

The EDID structure is as follows:
- The 128-byte EDID block contains a field with the number of 128-byte
  extension blocks that follow.
- Each extension block has a tag which specifies its type. The tag we're
  interested in is CEA-861.
- The CEA block has a few flags, including one that indicates whether basic
  audio is supported. The CEA block contains several sub-blocks of variable
  size. There are two types of CEA sub-blocks:
  - Detailed Timing Descriptors (DTDs): additional video timings
  - Data Block Collection: these can detail video, audio, speaker placement and
    other pieces of information about the display.
  We're interested in audio blocks.
- Audio blocks contain one or more Short Audio Descriptors (SADs). A SAD is a
  3-byte record describing a supported format.
- SADs can describe support for the PCM format, including sampling rate,
  sample size and channels.

The igt_edid library intentionally exposes all of this complexity because it
would be nice to generate all kind of valid EDIDs and test the kernel handles
them correctly (e.g. multiple SADs in different CEA blocks, or any EDID we find
in the wild really).

Signed-off-by: Simon Ser <simon.ser@intel.com>
---
 lib/igt_edid.c | 71 +++++++++++++++++++++++++++++++++++++-----
 lib/igt_edid.h | 84 ++++++++++++++++++++++++++++++++++++++++++++++++--
 2 files changed, 145 insertions(+), 10 deletions(-)

diff --git a/lib/igt_edid.c b/lib/igt_edid.c
index 9d604b13..d01defb0 100644
--- a/lib/igt_edid.c
+++ b/lib/igt_edid.c
@@ -252,19 +252,74 @@ void edid_init_with_mode(struct edid *edid, drmModeModeInfo *mode)
 				   EDID_DETAIL_MONITOR_NAME, "IGT");
 }
 
+static uint8_t compute_checksum(const uint8_t *buf, size_t size)
+{
+	size_t i;
+	uint8_t sum = 0;
+
+	assert(size > 0);
+	for (i = 0; i < size - 1; i++) {
+		sum += buf[i];
+	}
+
+	return 256 - sum;
+}
+
 /**
  * edid_update_checksum: compute and update the EDID checksum
  */
 void edid_update_checksum(struct edid *edid)
 {
-	size_t i;
-	const uint8_t *buf = (const uint8_t *) edid;
-	uint8_t sum = 0;
+	edid->checksum = compute_checksum((uint8_t *) edid,
+					  sizeof(struct edid));
+}
 
-	/* calculate checksum */
-	for (i = 0; i < sizeof(struct edid) - 1; i++) {
-		sum = sum + buf[i];
-	}
+/**
+ * cea_sad_init_pcm:
+ * @channels: the number of supported channels (max. 8)
+ * @sampling_rates: bitfield of enum cea_sad_sampling_rate
+ * @sample_sizes: bitfield of enum cea_sad_pcm_sample_size
+ *
+ * Initialize a Short Audio Descriptor to advertise PCM support.
+ */
+void cea_sad_init_pcm(struct cea_sad *sad, int channels,
+		      uint8_t sampling_rates, uint8_t sample_sizes)
+{
+	assert(channels <= 8);
+	sad->format_channels = CEA_SAD_FORMAT_PCM << 3 | (channels - 1);
+	sad->sampling_rates = sampling_rates;
+	sad->bitrate = sample_sizes;
+}
+
+/**
+ * edid_ext_set_cea_sad: set an extension block to be CEA SAD
+ */
+void edid_ext_set_cea_sad(struct edid_ext *ext, const struct cea_sad *sads,
+			  size_t sads_len)
+{
+	struct edid_cea *cea = &ext->data.cea;
+	struct edid_cea_data_block *data_block;
+	size_t sads_size, data_block_size;
 
-	edid->checksum = 256 - sum;
+	memset(ext, 0, sizeof(struct edid_ext));
+
+	sads_size = sizeof(struct cea_sad) * sads_len;
+	data_block_size = sizeof(struct edid_cea_data_block) + sads_size;
+
+	ext->tag = EDID_EXT_CEA;
+
+	cea->revision = 3;
+	cea->dtd_start = 4 + data_block_size;
+	cea->misc = 1 << 6; /* basic audio, no DTD */
+
+	assert(sads_size <= 0xFF);
+	data_block = (struct edid_cea_data_block *) cea->data;
+	data_block->type_len = EDID_CEA_DATA_AUDIO << 5 | sads_size;
+	memcpy(data_block->data.sads, sads, sads_size);
+}
+
+void edid_ext_update_cea_checksum(struct edid_ext *ext)
+{
+	ext->data.cea.checksum = compute_checksum((uint8_t *) ext,
+						  sizeof(struct edid_ext));
 }
diff --git a/lib/igt_edid.h b/lib/igt_edid.h
index d0963033..860ed929 100644
--- a/lib/igt_edid.h
+++ b/lib/igt_edid.h
@@ -148,6 +148,80 @@ struct detailed_timing {
 	} data;
 } __attribute__((packed));
 
+enum cea_sad_format {
+	CEA_SAD_FORMAT_PCM = 1,
+	CEA_SAD_FORMAT_AC3 = 2,
+	CEA_SAD_FORMAT_MPEG1 = 3, /* Layers 1 & 2 */
+	CEA_SAD_FORMAT_MP3 = 4,
+	CEA_SAD_FORMAT_MPEG2 = 5,
+	CEA_SAD_FORMAT_AAC = 6,
+	CEA_SAD_FORMAT_DTS = 7,
+	CEA_SAD_FORMAT_ATRAC = 8,
+	CEA_SAD_FORMAT_SACD = 9, /* One-bit audio */
+	CEA_SAD_FORMAT_DD_PLUS = 10,
+	CEA_SAD_FORMAT_DTS_HD = 11,
+	CEA_SAD_FORMAT_DOLBY = 12, /* MLP/Dolby TrueHD */
+	CEA_SAD_FORMAT_DST = 13,
+	CEA_SAD_FORMAT_WMA = 14, /* Microsoft WMA Pro */
+};
+
+enum cea_sad_sampling_rate {
+	CEA_SAD_SAMPLING_RATE_32KHZ = 1 << 0,
+	CEA_SAD_SAMPLING_RATE_44KHZ = 1 << 1,
+	CEA_SAD_SAMPLING_RATE_48KHZ = 1 << 2,
+	CEA_SAD_SAMPLING_RATE_88KHZ = 1 << 3,
+	CEA_SAD_SAMPLING_RATE_96KHZ = 1 << 4,
+	CEA_SAD_SAMPLING_RATE_176KHZ = 1 << 5,
+	CEA_SAD_SAMPLING_RATE_192KHZ = 1 << 6,
+};
+
+/* for PCM only */
+enum cea_sad_pcm_sample_size {
+	CEA_SAD_SAMPLE_SIZE_16 = 1 << 0,
+	CEA_SAD_SAMPLE_SIZE_20 = 1 << 1,
+	CEA_SAD_SAMPLE_SIZE_24 = 1 << 2,
+};
+
+/* Short Audio Descriptor */
+struct cea_sad {
+	uint8_t format_channels;
+	uint8_t sampling_rates;
+	uint8_t bitrate;
+} __attribute__((packed));
+
+enum edid_cea_data_type {
+	EDID_CEA_DATA_AUDIO = 1,
+	EDID_CEA_DATA_VIDEO = 2,
+	EDID_CEA_DATA_VENDOR_SPECIFIC = 3,
+	EDID_CEA_DATA_SPEAKER_ALLOC = 4,
+};
+
+struct edid_cea_data_block {
+	uint8_t type_len; /* type is from enum edid_cea_data_type */
+	union {
+		struct cea_sad sads[0];
+	} data;
+} __attribute__((packed));
+
+struct edid_cea {
+	uint8_t revision;
+	uint8_t dtd_start;
+	uint8_t misc;
+	char data[123]; /* DBC & DTD collection, padded with zeros */
+	uint8_t checksum;
+} __attribute__((packed));
+
+enum edid_ext_tag {
+	EDID_EXT_CEA = 0x02,
+};
+
+struct edid_ext {
+	uint8_t tag; /* enum edid_ext_tag */
+	union {
+		struct edid_cea cea;
+	} data;
+} __attribute__((packed));
+
 struct edid {
 	char header[8];
 	/* Vendor & product info */
@@ -183,9 +257,9 @@ struct edid {
 	/* Detailing timings 1-4 */
 	struct detailed_timing detailed_timings[DETAILED_TIMINGS_LEN];
 	/* Number of 128 byte ext. blocks */
-	uint8_t extensions;
-	/* Checksum */
+	uint8_t extensions_len;
 	uint8_t checksum;
+	struct edid_ext extensions[];
 } __attribute__((packed));
 
 void edid_init(struct edid *edid);
@@ -199,4 +273,10 @@ void detailed_timing_set_string(struct detailed_timing *dt,
 				enum detailed_non_pixel_type type,
 				const char *str);
 
+void cea_sad_init_pcm(struct cea_sad *sad, int channels,
+		      uint8_t sampling_rates, uint8_t sample_sizes);
+void edid_ext_set_cea_sad(struct edid_ext *ext, const struct cea_sad *sads,
+			  size_t sads_len);
+void edid_ext_update_cea_checksum(struct edid_ext *ext);
+
 #endif
-- 
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 2/7] lib/igt_edid: add support for Vendor Specific Data blocks

[Simon Ser]

For some reason HDMI audio won't work unless you cast a magic IEEE Registration
Identifier alongside with its appropriate Components of Source Physical
Address. The easiest way to do this is to capture a wild HDMI EDID, study it,
and blindly copy bytes because you don't understand anything about their
possible meaning (if any).

This commit also changes the SAD API exposed by igt_edid, to allow for both a
SAD block and a VSD block to be included in the same CEA block.

Signed-off-by: Simon Ser <simon.ser@intel.com>
---
 lib/igt_edid.c | 72 +++++++++++++++++++++++++++++++++++++++-----------
 lib/igt_edid.h | 23 ++++++++++++++--
 2 files changed, 78 insertions(+), 17 deletions(-)

diff --git a/lib/igt_edid.c b/lib/igt_edid.c
index d01defb0..fbdb0c06 100644
--- a/lib/igt_edid.c
+++ b/lib/igt_edid.c
@@ -292,30 +292,72 @@ void cea_sad_init_pcm(struct cea_sad *sad, int channels,
 }
 
 /**
- * edid_ext_set_cea_sad: set an extension block to be CEA SAD
+ * cea_vsd_get_hdmi_default:
+ *
+ * Returns the default Vendor Specific Data block for HDMI.
  */
-void edid_ext_set_cea_sad(struct edid_ext *ext, const struct cea_sad *sads,
-			  size_t sads_len)
+const struct cea_vsd *cea_vsd_get_hdmi_default(size_t *size)
 {
-	struct edid_cea *cea = &ext->data.cea;
-	struct edid_cea_data_block *data_block;
-	size_t sads_size, data_block_size;
+	static char raw[sizeof(struct cea_vsd) + 4] = {0};
+	struct cea_vsd *vsd;
+
+	*size = sizeof(raw);
+
+	/* Magic incantation. Works better if you orient your screen in the
+	 * direction of the VESA headquarters. */
+	vsd = (struct cea_vsd *) raw;
+	vsd->ieee_oui[0] = 0x03;
+	vsd->ieee_oui[1] = 0x0C;
+	vsd->ieee_oui[2] = 0x00;
+	vsd->data[0] = 0x10;
+	vsd->data[1] = 0x00;
+	vsd->data[2] = 0x38;
+	vsd->data[3] = 0x2D;
+
+	return vsd;
+}
+
+static void edid_cea_data_block_init(struct edid_cea_data_block *block,
+				     enum edid_cea_data_type type, size_t size)
+{
+	assert(size <= 0xFF);
+	block->type_len = type << 5 | size;
+}
 
-	memset(ext, 0, sizeof(struct edid_ext));
+size_t edid_cea_data_block_set_sad(struct edid_cea_data_block *block,
+				   const struct cea_sad *sads, size_t sads_len)
+{
+	size_t sads_size;
 
 	sads_size = sizeof(struct cea_sad) * sads_len;
-	data_block_size = sizeof(struct edid_cea_data_block) + sads_size;
+	edid_cea_data_block_init(block, EDID_CEA_DATA_AUDIO, sads_size);
+
+	memcpy(block->data.sads, sads, sads_size);
+
+	return sizeof(struct edid_cea_data_block) + sads_size;
+}
+
+size_t edid_cea_data_block_set_vsd(struct edid_cea_data_block *block,
+				   const struct cea_vsd *vsd, size_t vsd_size)
+{
+	edid_cea_data_block_init(block, EDID_CEA_DATA_VENDOR_SPECIFIC,
+				 vsd_size);
+
+	memcpy(block->data.vsds, vsd, vsd_size);
+
+	return sizeof(struct edid_cea_data_block) + vsd_size;
+}
+
+void edid_ext_set_cea(struct edid_ext *ext, size_t data_blocks_size,
+		      uint8_t flags)
+{
+	struct edid_cea *cea = &ext->data.cea;
 
 	ext->tag = EDID_EXT_CEA;
 
 	cea->revision = 3;
-	cea->dtd_start = 4 + data_block_size;
-	cea->misc = 1 << 6; /* basic audio, no DTD */
-
-	assert(sads_size <= 0xFF);
-	data_block = (struct edid_cea_data_block *) cea->data;
-	data_block->type_len = EDID_CEA_DATA_AUDIO << 5 | sads_size;
-	memcpy(data_block->data.sads, sads, sads_size);
+	cea->dtd_start = 4 + data_blocks_size;
+	cea->misc = flags; /* just flags, no DTD */
 }
 
 void edid_ext_update_cea_checksum(struct edid_ext *ext)
diff --git a/lib/igt_edid.h b/lib/igt_edid.h
index 860ed929..790d1984 100644
--- a/lib/igt_edid.h
+++ b/lib/igt_edid.h
@@ -189,6 +189,12 @@ struct cea_sad {
 	uint8_t bitrate;
 } __attribute__((packed));
 
+/* Vendor Specific Data */
+struct cea_vsd {
+	uint8_t ieee_oui[3];
+	char data[];
+};
+
 enum edid_cea_data_type {
 	EDID_CEA_DATA_AUDIO = 1,
 	EDID_CEA_DATA_VIDEO = 2,
@@ -200,9 +206,17 @@ struct edid_cea_data_block {
 	uint8_t type_len; /* type is from enum edid_cea_data_type */
 	union {
 		struct cea_sad sads[0];
+		struct cea_vsd vsds[0];
 	} data;
 } __attribute__((packed));
 
+enum edid_cea_flag {
+	EDID_CEA_YCBCR422 = 1 << 4,
+	EDID_CEA_YCBCR444 = 1 << 5,
+	EDID_CEA_BASIC_AUDIO = 1 << 6,
+	EDID_CEA_UNDERSCAN = 1 << 7,
+};
+
 struct edid_cea {
 	uint8_t revision;
 	uint8_t dtd_start;
@@ -275,8 +289,13 @@ void detailed_timing_set_string(struct detailed_timing *dt,
 
 void cea_sad_init_pcm(struct cea_sad *sad, int channels,
 		      uint8_t sampling_rates, uint8_t sample_sizes);
-void edid_ext_set_cea_sad(struct edid_ext *ext, const struct cea_sad *sads,
-			  size_t sads_len);
 void edid_ext_update_cea_checksum(struct edid_ext *ext);
+const struct cea_vsd *cea_vsd_get_hdmi_default(size_t *size);
+size_t edid_cea_data_block_set_sad(struct edid_cea_data_block *block,
+				   const struct cea_sad *sads, size_t sads_len);
+size_t edid_cea_data_block_set_vsd(struct edid_cea_data_block *block,
+				   const struct cea_vsd *vsd, size_t vsd_size);
+void edid_ext_set_cea(struct edid_ext *ext, size_t data_blocks_size,
+		      uint8_t flags);
 
 #endif
-- 
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 3/7] tests/kms_chamelium: generate an EDID with audio support

[Simon Ser]

The default Chamelium EDID works great with DisplayPort but has issues with
HDMI. The EDID advertises itself as a DisplayPort-only device, which causes
issues with HDMI audio. Additionally, the EDID doesn't contain a magic
incantation within a vendor-specific data block that is needed for audio to
work.

This patch makes it so an EDID is generated and set by IGT. The EDID is the
base IGT EDID patched to append a Short Audio Descriptor and Vendor Specific
Data block extension.

This generated is suitable for HDMI audio. For DisplayPort audio, we keep using
the Chamelium's default EDID.

The global variable holding the test frequencies has been renamed to prevent
shadowing local variables.

Signed-off-by: Simon Ser <simon.ser@intel.com>
---
 lib/igt_chamelium.c   |   9 +++-
 tests/kms_chamelium.c | 123 ++++++++++++++++++++++++++++++------------
 2 files changed, 95 insertions(+), 37 deletions(-)

diff --git a/lib/igt_chamelium.c b/lib/igt_chamelium.c
index ffc68f35..44445444 100644
--- a/lib/igt_chamelium.c
+++ b/lib/igt_chamelium.c
@@ -38,6 +38,7 @@
 #include "igt_chamelium.h"
 #include "igt_core.h"
 #include "igt_aux.h"
+#include "igt_edid.h"
 #include "igt_frame.h"
 #include "igt_list.h"
 #include "igt_kms.h"
@@ -499,14 +500,18 @@ void chamelium_schedule_hpd_toggle(struct chamelium *chamelium,
  *
  * Returns: The ID of the EDID uploaded to the chamelium.
  */
-int chamelium_new_edid(struct chamelium *chamelium, const unsigned char *edid)
+int chamelium_new_edid(struct chamelium *chamelium,
+		       const unsigned char *raw_edid)
 {
 	xmlrpc_value *res;
 	struct chamelium_edid *allocated_edid;
 	int edid_id;
+	struct edid *edid = (struct edid *) raw_edid;
+	size_t edid_size = sizeof(struct edid) +
+			   edid->extensions_len * sizeof(struct edid_ext);
 
 	res = chamelium_rpc(chamelium, NULL, "CreateEdid", "(6)",
-			    edid, EDID_LENGTH);
+			    raw_edid, edid_size);
 
 	xmlrpc_read_int(&chamelium->env, res, &edid_id);
 	xmlrpc_DECREF(res);
diff --git a/tests/kms_chamelium.c b/tests/kms_chamelium.c
index 714e5e06..126af226 100644
--- a/tests/kms_chamelium.c
+++ b/tests/kms_chamelium.c
@@ -27,6 +27,7 @@
 #include "config.h"
 #include "igt.h"
 #include "igt_vc4.h"
+#include "igt_edid.h"
 
 #include <fcntl.h>
 #include <pthread.h>
@@ -414,8 +415,7 @@ test_suspend_resume_edid_change(data_t *data, struct chamelium_port *port,
 }
 
 static igt_output_t *
-prepare_output(data_t *data,
-	       struct chamelium_port *port, bool set_edid)
+prepare_output(data_t *data, struct chamelium_port *port, int edid_id)
 {
 	igt_display_t *display = &data->display;
 	igt_output_t *output;
@@ -428,10 +428,10 @@ prepare_output(data_t *data,
 	igt_require(res = drmModeGetResources(data->drm_fd));
 
 	/* The chamelium's default EDID has a lot of resolutions, way more then
-	 * we need to test
+	 * we need to test. Additionally the default EDID doesn't support HDMI
+	 * audio.
 	 */
-	if (set_edid)
-		chamelium_port_set_edid(data->chamelium, port, data->edid_id);
+	chamelium_port_set_edid(data->chamelium, port, edid_id);
 
 	chamelium_plug(data->chamelium, port);
 	wait_for_connector(data, port, DRM_MODE_CONNECTED);
@@ -616,7 +616,7 @@ static void test_display_one_mode(data_t *data, struct chamelium_port *port,
 
 	reset_state(data, port);
 
-	output = prepare_output(data, port, true);
+	output = prepare_output(data, port, data->edid_id);
 	connector = chamelium_port_get_connector(data->chamelium, port, false);
 	primary = igt_output_get_plane_type(output, DRM_PLANE_TYPE_PRIMARY);
 	igt_assert(primary);
@@ -647,7 +647,7 @@ static void test_display_all_modes(data_t *data, struct chamelium_port *port,
 
 	reset_state(data, port);
 
-	output = prepare_output(data, port, true);
+	output = prepare_output(data, port, data->edid_id);
 	connector = chamelium_port_get_connector(data->chamelium, port, false);
 	primary = igt_output_get_plane_type(output, DRM_PLANE_TYPE_PRIMARY);
 	igt_assert(primary);
@@ -682,7 +682,7 @@ test_display_frame_dump(data_t *data, struct chamelium_port *port)
 
 	reset_state(data, port);
 
-	output = prepare_output(data, port, true);
+	output = prepare_output(data, port, data->edid_id);
 	connector = chamelium_port_get_connector(data->chamelium, port, false);
 	primary = igt_output_get_plane_type(output, DRM_PLANE_TYPE_PRIMARY);
 	igt_assert(primary);
@@ -725,7 +725,7 @@ 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
 
-static int sampling_rates[] = {
+static int test_sampling_rates[] = {
 	32000,
 	44100,
 	48000,
@@ -735,7 +735,7 @@ static int sampling_rates[] = {
 	192000,
 };
 
-static int sampling_rates_count = sizeof(sampling_rates) / sizeof(int);
+static int test_sampling_rates_count = sizeof(test_sampling_rates) / sizeof(int);
 
 static int test_frequencies[] = {
 	300,
@@ -986,7 +986,7 @@ do_test_display_audio(data_t *data, struct chamelium_port *port,
 
 static void
 test_display_audio(data_t *data, struct chamelium_port *port,
-		   const char *audio_device)
+		   const char *audio_device, int edid_id)
 {
 	bool run = false;
 	struct alsa *alsa;
@@ -1005,10 +1005,7 @@ test_display_audio(data_t *data, struct chamelium_port *port,
 
 	reset_state(data, port);
 
-	/* Use the default Chamelium EDID for this test, as the base IGT EDID
-	 * doesn't advertise audio support (see drm_detect_monitor_audio in
-	 * the kernel tree). */
-	output = prepare_output(data, port, false);
+	output = prepare_output(data, port, edid_id);
 	connector = chamelium_port_get_connector(data->chamelium, port, false);
 	primary = igt_output_get_plane_type(output, DRM_PLANE_TYPE_PRIMARY);
 	igt_assert(primary);
@@ -1027,14 +1024,14 @@ test_display_audio(data_t *data, struct chamelium_port *port,
 
 	enable_output(data, port, output, mode, &fb);
 
-	for (i = 0; i < sampling_rates_count; i++) {
+	for (i = 0; i < test_sampling_rates_count; i++) {
 		ret = alsa_open_output(alsa, audio_device);
 		igt_assert(ret >= 0);
 
 		/* TODO: playback on all 8 available channels */
 		run |= do_test_display_audio(data, port, alsa,
 					     PLAYBACK_CHANNELS,
-					     sampling_rates[i]);
+					     test_sampling_rates[i]);
 
 		alsa_close_output(alsa);
 	}
@@ -1377,7 +1374,7 @@ static void test_display_planes_random(data_t *data,
 	reset_state(data, port);
 
 	/* Find the connector and pipe. */
-	output = prepare_output(data, port, true);
+	output = prepare_output(data, port, data->edid_id);
 
 	mode = igt_output_get_mode(output);
 
@@ -1557,7 +1554,17 @@ static data_t data;
 igt_main
 {
 	struct chamelium_port *port;
-	int edid_id, alt_edid_id, p;
+	int p, channels;
+	uint8_t sampling_rates, sample_sizes;
+	char raw_edid[sizeof(struct edid) + sizeof(struct edid_ext)] = {0};
+	struct edid *edid;
+	struct edid_ext *edid_ext;
+	struct edid_cea *edid_cea;
+	char *cea_data;
+	struct edid_cea_data_block *block;
+	struct cea_sad sad = {0};
+	const struct cea_vsd *vsd;
+	size_t cea_data_size, vsd_size;
 
 	igt_fixture {
 		igt_skip_on_simulation();
@@ -1569,12 +1576,52 @@ igt_main
 		data.ports = chamelium_get_ports(data.chamelium,
 						 &data.port_count);
 
-		edid_id = chamelium_new_edid(data.chamelium,
-					     igt_kms_get_base_edid());
-		alt_edid_id = chamelium_new_edid(data.chamelium,
+		/* Initialize the Short Audio Descriptor for PCM */
+		channels = 2; /* TODO: speaker alloc blocks for > 2 channels */
+		sampling_rates = CEA_SAD_SAMPLING_RATE_32KHZ |
+				 CEA_SAD_SAMPLING_RATE_44KHZ |
+				 CEA_SAD_SAMPLING_RATE_48KHZ |
+				 CEA_SAD_SAMPLING_RATE_88KHZ |
+				 CEA_SAD_SAMPLING_RATE_96KHZ |
+				 CEA_SAD_SAMPLING_RATE_176KHZ |
+				 CEA_SAD_SAMPLING_RATE_192KHZ;
+		sample_sizes = CEA_SAD_SAMPLE_SIZE_16 |
+			       CEA_SAD_SAMPLE_SIZE_20 |
+			       CEA_SAD_SAMPLE_SIZE_24;
+		cea_sad_init_pcm(&sad, channels, sampling_rates, sample_sizes);
+
+		/* Create a new EDID from the base IGT EDID, and add an
+		 * extension that advertises audio support. */
+		edid = (struct edid *) raw_edid;
+		memcpy(edid, igt_kms_get_base_edid(), sizeof(struct edid));
+		edid->extensions_len = 1;
+		edid_ext = &edid->extensions[0];
+		edid_cea = &edid_ext->data.cea;
+		cea_data = edid_cea->data;
+		cea_data_size = 0;
+
+		block = (struct edid_cea_data_block *) &cea_data[cea_data_size];
+		cea_data_size += edid_cea_data_block_set_sad(block, &sad, 1);
+
+		/* A Vendor Specific Data block is needed for HDMI audio */
+		block = (struct edid_cea_data_block *) &cea_data[cea_data_size];
+		vsd = cea_vsd_get_hdmi_default(&vsd_size);
+		cea_data_size += edid_cea_data_block_set_vsd(block, vsd,
+							     vsd_size);
+
+		igt_assert(cea_data_size <= sizeof(edid_cea->data));
+
+		edid_ext_set_cea(edid_ext, cea_data_size,
+				 EDID_CEA_BASIC_AUDIO);
+
+		edid_update_checksum(edid);
+		edid_ext_update_cea_checksum(edid_ext);
+
+		data.edid_id = chamelium_new_edid(data.chamelium,
+						  (unsigned char *) raw_edid);
+
+		data.alt_edid_id = chamelium_new_edid(data.chamelium,
 						 igt_kms_get_alt_edid());
-		data.edid_id = edid_id;
-		data.alt_edid_id = alt_edid_id;
 
 		/* So fbcon doesn't try to reprobe things itself */
 		kmstest_set_vt_graphics_mode();
@@ -1598,9 +1645,9 @@ igt_main
 					   HPD_TOGGLE_COUNT_FAST);
 
 		connector_subtest("dp-edid-read", DisplayPort) {
-			test_edid_read(&data, port, edid_id,
+			test_edid_read(&data, port, data.edid_id,
 				       igt_kms_get_base_edid());
-			test_edid_read(&data, port, alt_edid_id,
+			test_edid_read(&data, port, data.alt_edid_id,
 				       igt_kms_get_alt_edid());
 		}
 
@@ -1626,13 +1673,15 @@ igt_main
 			test_suspend_resume_edid_change(&data, port,
 							SUSPEND_STATE_MEM,
 							SUSPEND_TEST_NONE,
-							edid_id, alt_edid_id);
+							data.edid_id,
+							data.alt_edid_id);
 
 		connector_subtest("dp-edid-change-during-hibernate", DisplayPort)
 			test_suspend_resume_edid_change(&data, port,
 							SUSPEND_STATE_DISK,
 							SUSPEND_TEST_DEVICES,
-							edid_id, alt_edid_id);
+							data.edid_id,
+							data.alt_edid_id);
 
 		connector_subtest("dp-crc-single", DisplayPort)
 			test_display_all_modes(&data, port, DRM_FORMAT_XRGB8888,
@@ -1649,8 +1698,10 @@ igt_main
 		connector_subtest("dp-frame-dump", DisplayPort)
 			test_display_frame_dump(&data, port);
 
+		/* The EDID we generate advertises HDMI audio, not DP audio.
+		 * Use the Chamelium's default EDID for DP audio. */
 		connector_subtest("dp-audio", DisplayPort)
-			test_display_audio(&data, port, "HDMI");
+			test_display_audio(&data, port, "HDMI", 0);
 	}
 
 	igt_subtest_group {
@@ -1668,9 +1719,9 @@ igt_main
 					   HPD_TOGGLE_COUNT_FAST);
 
 		connector_subtest("hdmi-edid-read", HDMIA) {
-			test_edid_read(&data, port, edid_id,
+			test_edid_read(&data, port, data.edid_id,
 				       igt_kms_get_base_edid());
-			test_edid_read(&data, port, alt_edid_id,
+			test_edid_read(&data, port, data.alt_edid_id,
 				       igt_kms_get_alt_edid());
 		}
 
@@ -1696,13 +1747,15 @@ igt_main
 			test_suspend_resume_edid_change(&data, port,
 							SUSPEND_STATE_MEM,
 							SUSPEND_TEST_NONE,
-							edid_id, alt_edid_id);
+							data.edid_id,
+							data.alt_edid_id);
 
 		connector_subtest("hdmi-edid-change-during-hibernate", HDMIA)
 			test_suspend_resume_edid_change(&data, port,
 							SUSPEND_STATE_DISK,
 							SUSPEND_TEST_DEVICES,
-							edid_id, alt_edid_id);
+							data.edid_id,
+							data.alt_edid_id);
 
 		connector_subtest("hdmi-crc-single", HDMIA)
 			test_display_all_modes(&data, port, DRM_FORMAT_XRGB8888,
@@ -1813,9 +1866,9 @@ igt_main
 			test_basic_hotplug(&data, port, HPD_TOGGLE_COUNT_FAST);
 
 		connector_subtest("vga-edid-read", VGA) {
-			test_edid_read(&data, port, edid_id,
+			test_edid_read(&data, port, data.edid_id,
 				       igt_kms_get_base_edid());
-			test_edid_read(&data, port, alt_edid_id,
+			test_edid_read(&data, port, data.alt_edid_id,
 				       igt_kms_get_alt_edid());
 		}
 
-- 
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 4/7] test/kms_chamelium: disable >48KHz audio tests

[Simon Ser]

These weren't tested on DisplayPort (because the Chamelium's default EDID
doesn't advertise them) so this doesn't remove some test cases.

On HDMI they don't work reliably: sometimes no audio data is captured by the
Chamelium. This needs to be investigated, but for now let's just disable them.

Signed-off-by: Simon Ser <simon.ser@intel.com>
---
 tests/kms_chamelium.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/tests/kms_chamelium.c b/tests/kms_chamelium.c
index 126af226..718e4aad 100644
--- a/tests/kms_chamelium.c
+++ b/tests/kms_chamelium.c
@@ -725,14 +725,15 @@ 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
 
+/* TODO: enable >48KHz rates, these are not reliable */
 static int test_sampling_rates[] = {
 	32000,
 	44100,
 	48000,
-	88200,
-	96000,
-	176400,
-	192000,
+	/* 88200, */
+	/* 96000, */
+	/* 176400, */
+	/* 192000, */
 };
 
 static int test_sampling_rates_count = sizeof(test_sampling_rates) / sizeof(int);
-- 
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 5/7] tests/kms_chamelium: enable audio test on HDMI ports

[Simon Ser]

Now that we have all the required EDID bits in place, we can add a new
hdmi-audio test.

Signed-off-by: Simon Ser <simon.ser@intel.com>
---
 tests/kms_chamelium.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/tests/kms_chamelium.c b/tests/kms_chamelium.c
index 718e4aad..7e84cd1c 100644
--- a/tests/kms_chamelium.c
+++ b/tests/kms_chamelium.c
@@ -1852,6 +1852,9 @@ igt_main
 
 		connector_subtest("hdmi-frame-dump", HDMIA)
 			test_display_frame_dump(&data, port);
+
+		connector_subtest("hdmi-audio", HDMIA)
+			test_display_audio(&data, port, "HDMI", data.edid_id);
 	}
 
 	igt_subtest_group {
-- 
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 6/7] tests/kms_chamelium: don't abort audio test on first fail

[Simon Ser]

The audio test is repeated with a few different frequencies. When the test
fails for one frequency, continue testing other frequencies so that we get a
better understanding of the situation: is only one frequency failing, or are
all of them failing?

Signed-off-by: Simon Ser <simon.ser@intel.com>
---
 tests/kms_chamelium.c | 38 +++++++++++++++++++++++---------------
 1 file changed, 23 insertions(+), 15 deletions(-)

diff --git a/tests/kms_chamelium.c b/tests/kms_chamelium.c
index 7e84cd1c..5efbee92 100644
--- a/tests/kms_chamelium.c
+++ b/tests/kms_chamelium.c
@@ -794,15 +794,6 @@ do_test_display_audio(data_t *data, struct chamelium_port *port,
 	struct audio_state state = {};
 	int channel_mapping[8], capture_chan;
 
-	if (!alsa_test_output_configuration(alsa, playback_channels,
-					    playback_rate)) {
-		igt_debug("Skipping test with sample rate %d Hz and %d channels "
-			  "because at least one of the selected output devices "
-			  "doesn't support this configuration\n",
-			  playback_rate, playback_channels);
-		return false;
-	}
-
 	igt_debug("Testing with playback sampling rate %d Hz and %d channels\n",
 		  playback_rate, playback_channels);
 	alsa_configure_output(alsa, playback_channels, playback_rate);
@@ -981,15 +972,14 @@ do_test_display_audio(data_t *data, struct chamelium_port *port,
 	audio_signal_fini(signal);
 	chamelium_stream_deinit(stream);
 
-	igt_assert(success);
-	return true;
+	return success;
 }
 
 static void
 test_display_audio(data_t *data, struct chamelium_port *port,
 		   const char *audio_device, int edid_id)
 {
-	bool run = false;
+	bool run, success;
 	struct alsa *alsa;
 	int ret;
 	igt_output_t *output;
@@ -998,6 +988,7 @@ test_display_audio(data_t *data, struct chamelium_port *port,
 	drmModeModeInfo *mode;
 	drmModeConnector *connector;
 	int fb_id, i;
+	int channels, sampling_rate;
 
 	igt_require(alsa_has_exclusive_access());
 
@@ -1025,20 +1016,37 @@ test_display_audio(data_t *data, struct chamelium_port *port,
 
 	enable_output(data, port, output, mode, &fb);
 
+	run = false;
+	success = true;
 	for (i = 0; i < test_sampling_rates_count; i++) {
 		ret = alsa_open_output(alsa, audio_device);
 		igt_assert(ret >= 0);
 
 		/* TODO: playback on all 8 available channels */
-		run |= do_test_display_audio(data, port, alsa,
-					     PLAYBACK_CHANNELS,
-					     test_sampling_rates[i]);
+		channels = PLAYBACK_CHANNELS;
+		sampling_rate = test_sampling_rates[i];
+
+		if (!alsa_test_output_configuration(alsa, channels,
+						    sampling_rate)) {
+			igt_debug("Skipping test with sample rate %d Hz and %d channels "
+				  "because at least one of the selected output devices "
+				  "doesn't support this configuration\n",
+				  channels, sampling_rate);
+			continue;
+		}
+
+		run = true;
+
+		success &= do_test_display_audio(data, port, alsa, channels,
+						 sampling_rate);
 
 		alsa_close_output(alsa);
 	}
 
 	/* Make sure we tested at least one frequency. */
 	igt_assert(run);
+	/* Make sure all runs were successful. */
+	igt_assert(success);
 
 	igt_remove_fb(data->drm_fd, &fb);
 
-- 
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 7/7] HAX: add dp-audio test to fast-feedback

[Simon Ser]

Waiting for the whole series to be complete to decide whether or not we want
this test in fast-feedback.

Signed-off-by: Simon Ser <simon.ser@intel.com>
---
 tests/intel-ci/fast-feedback.testlist | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tests/intel-ci/fast-feedback.testlist b/tests/intel-ci/fast-feedback.testlist
index 40475b1a..28f5fa55 100644
--- a/tests/intel-ci/fast-feedback.testlist
+++ b/tests/intel-ci/fast-feedback.testlist
@@ -175,6 +175,7 @@ igt@kms_addfb_basic@unused-pitches
 igt@kms_busy@basic-flip-a
 igt@kms_busy@basic-flip-b
 igt@kms_busy@basic-flip-c
+igt@kms_chamelium@dp-audio
 igt@kms_chamelium@dp-hpd-fast
 igt@kms_chamelium@dp-edid-read
 igt@kms_chamelium@dp-crc-fast
-- 
2.21.0

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

[igt-dev] ✗ Fi.CI.BAT: failure for tests/kms_chamelium: add hdmi-audio test

[Patchwork]

== Series Details ==

Series: tests/kms_chamelium: add hdmi-audio test
URL   : https://patchwork.freedesktop.org/series/60003/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_6006 -> IGTPW_2929
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with IGTPW_2929 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in IGTPW_2929, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/60003/revisions/1/mbox/

Possible new issues
-------------------

  Here are the unknown changes that may have been introduced in IGTPW_2929:

### IGT changes ###

#### Possible regressions ####

  * igt@kms_chamelium@hdmi-edid-read:
    - fi-kbl-7567u:       [PASS][1] -> [FAIL][2]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6006/fi-kbl-7567u/igt@kms_chamelium@hdmi-edid-read.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2929/fi-kbl-7567u/igt@kms_chamelium@hdmi-edid-read.html

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

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

### IGT changes ###

#### Issues hit ####

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

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

  * igt@kms_frontbuffer_tracking@basic:
    - fi-byt-clapper:     [PASS][7] -> [FAIL][8] ([fdo#103167])
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6006/fi-byt-clapper/igt@kms_frontbuffer_tracking@basic.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2929/fi-byt-clapper/igt@kms_frontbuffer_tracking@basic.html

  
  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#107713]: https://bugs.freedesktop.org/show_bug.cgi?id=107713
  [fdo#108569]: https://bugs.freedesktop.org/show_bug.cgi?id=108569
  [fdo#110235 ]: https://bugs.freedesktop.org/show_bug.cgi?id=110235 


Participating hosts (25 -> 24)
------------------------------

  Additional (9): fi-hsw-peppy fi-skl-6770hq fi-skl-6260u fi-ilk-650 fi-hsw-4770 fi-bxt-j4205 fi-ivb-3770 fi-icl-dsi fi-skl-6600u 
  Missing    (10): fi-kbl-soraka fi-ilk-m540 fi-skl-guc fi-byt-squawks fi-apl-guc fi-kbl-guc fi-kbl-8809g fi-bsw-kefka fi-byt-n2820 fi-skl-6700k2 


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

  * IGT: IGT_4968 -> IGTPW_2929

  CI_DRM_6006: f2b80d9616c3ced3d5e04bba276c4e57265faabb @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_2929: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2929/
  IGT_4968: caed251990f35bfe45368f803980071a73e36315 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools



== Testlist changes ==

+igt@kms_chamelium@hdmi-audio

== Logs ==

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

Re: [igt-dev] [PATCH i-g-t 3/7] tests/kms_chamelium: generate an EDID with audio support

[Arkadiusz Hiler]

On Fri, Apr 26, 2019 at 07:21:06PM +0300, Simon Ser wrote:
> The default Chamelium EDID works great with DisplayPort but has issues with
> HDMI. The EDID advertises itself as a DisplayPort-only device, which causes
> issues with HDMI audio. Additionally, the EDID doesn't contain a magic
> incantation within a vendor-specific data block that is needed for audio to
> work.
> 
> This patch makes it so an EDID is generated and set by IGT. The EDID is the
> base IGT EDID patched to append a Short Audio Descriptor and Vendor Specific
> Data block extension.
> 
> This generated is suitable for HDMI audio. For DisplayPort audio, we keep using
                ^ EDID/one
> the Chamelium's default EDID.
> 
> The global variable holding the test frequencies has been renamed to prevent
> shadowing local variables.
           ^ by

> Signed-off-by: Simon Ser <simon.ser@intel.com>
> ---
>  lib/igt_chamelium.c   |   9 +++-
>  tests/kms_chamelium.c | 123 ++++++++++++++++++++++++++++++------------
>  2 files changed, 95 insertions(+), 37 deletions(-)
> 
> diff --git a/tests/kms_chamelium.c b/tests/kms_chamelium.c
> index 714e5e06..126af226 100644
> --- a/tests/kms_chamelium.c
> +++ b/tests/kms_chamelium.c
> @@ -1626,13 +1673,15 @@ igt_main
>  			test_suspend_resume_edid_change(&data, port,
>  							SUSPEND_STATE_MEM,
>  							SUSPEND_TEST_NONE,
> -							edid_id, alt_edid_id);
> +							data.edid_id,
> +							data.alt_edid_id);
>  
>  		connector_subtest("dp-edid-change-during-hibernate", DisplayPort)
>  			test_suspend_resume_edid_change(&data, port,
>  							SUSPEND_STATE_DISK,
>  							SUSPEND_TEST_DEVICES,
> -							edid_id, alt_edid_id);
> +							data.edid_id,
> +							data.alt_edid_id);
>  
>  		connector_subtest("dp-crc-single", DisplayPort)
>  			test_display_all_modes(&data, port, DRM_FORMAT_XRGB8888,
> @@ -1649,8 +1698,10 @@ igt_main
>  		connector_subtest("dp-frame-dump", DisplayPort)
>  			test_display_frame_dump(&data, port);
>  
> +		/* The EDID we generate advertises HDMI audio, not DP audio.
> +		 * Use the Chamelium's default EDID for DP audio. */
>  		connector_subtest("dp-audio", DisplayPort)
> -			test_display_audio(&data, port, "HDMI");
> +			test_display_audio(&data, port, "HDMI", 0);

#define CHAMELIUM_DEFAULT_EDID 0

or so

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

Re: [igt-dev] [PATCH i-g-t 5/7] tests/kms_chamelium: enable audio test on HDMI ports

[Arkadiusz Hiler]

On Fri, Apr 26, 2019 at 07:21:08PM +0300, Simon Ser wrote:
> Now that we have all the required EDID bits in place, we can add a new
> hdmi-audio test.
> 
> Signed-off-by: Simon Ser <simon.ser@intel.com>
> ---
>  tests/kms_chamelium.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/tests/kms_chamelium.c b/tests/kms_chamelium.c
> index 718e4aad..7e84cd1c 100644
> --- a/tests/kms_chamelium.c
> +++ b/tests/kms_chamelium.c
> @@ -1852,6 +1852,9 @@ igt_main
>  
>  		connector_subtest("hdmi-frame-dump", HDMIA)
>  			test_display_frame_dump(&data, port);
> +
> +		connector_subtest("hdmi-audio", HDMIA)
> +			test_display_audio(&data, port, "HDMI", data.edid_id);

edid_hdmi_with_audio_id?

we will quite likely start having more of those anyway

>  	}
>  
>  	igt_subtest_group {
> -- 
> 2.21.0
> 
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] ✗ Fi.CI.BAT: failure for tests/kms_chamelium: add hdmi-audio test

[Arkadiusz Hiler]

On Fri, Apr 26, 2019 at 05:13:21PM +0000, Patchwork wrote:
> == Series Details ==
> 
> Series: tests/kms_chamelium: add hdmi-audio test
> URL   : https://patchwork.freedesktop.org/series/60003/
> State : failure
> 
> == Summary ==
> 
> CI Bug Log - changes from CI_DRM_6006 -> IGTPW_2929
> ====================================================
> 
> Summary
> -------
> 
>   **FAILURE**
> 
>   Serious unknown changes coming with IGTPW_2929 absolutely need to be
>   verified manually.
>   
>   If you think the reported changes have nothing to do with the changes
>   introduced in IGTPW_2929, please notify your bug team to allow them
>   to document this new failure mode, which will reduce false positives in CI.
> 
>   External URL: https://patchwork.freedesktop.org/api/1.0/series/60003/revisions/1/mbox/
> 
> Possible new issues
> -------------------
> 
>   Here are the unknown changes that may have been introduced in IGTPW_2929:
> 
> ### IGT changes ###
> 
> #### Possible regressions ####
> 
>   * igt@kms_chamelium@hdmi-edid-read:
>     - fi-kbl-7567u:       [PASS][1] -> [FAIL][2]
>    [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6006/fi-kbl-7567u/igt@kms_chamelium@hdmi-edid-read.html
>    [2]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2929/fi-kbl-7567u/igt@kms_chamelium@hdmi-edid-read.html

This is a real regression:

tests/kms_chamelium.c:1763
----------------------------------------------
connector_subtest("hdmi-edid-read", HDMIA) {
	test_edid_read(&data, port, data.edid_id,
		       igt_kms_get_base_edid());
	test_edid_read(&data, port, data.alt_edid_id,
		       igt_kms_get_alt_edid());
}
----------------------------------------------

data.edid_id is this new edid for HDMI, yet the blob we provide for
comparison is the "base igt edid".

Having to pass both representations is rather error-prone.

I think we should just use IDs and when we are registering new EDID,
we should store it in a map id -> EDID. Then such tests would take
edid_id only, and retrieve the blob from the map.

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