[igt-dev] [PATCH i-g-t 0/3] Unify HDMI audio EDID generation

[igt-dev] [PATCH i-g-t 0/3] Unify HDMI audio EDID generation

[Simon Ser]

HDMI audio EDIDs are used in both HDMI injection tests and Chamelium audio
tests.

Simon Ser (3):
  lib/tests/igt_edid: introduce EDID sanity checks
  lib/igt_edid: add support for Speaker Allocation Data blocks
  lib/igt_kms: introduce igt_kms_get_hdmi_audio_edid

 lib/igt_edid.c              |  12 ++++
 lib/igt_edid.h              |  18 +++++
 lib/igt_kms.c               | 133 ++++++++++++++++++++----------------
 lib/igt_kms.h               |  10 ++-
 lib/tests/igt_edid.c        |  97 ++++++++++++++++++++++++++
 lib/tests/igt_hdmi_inject.c |   1 -
 lib/tests/meson.build       |   1 +
 tests/kms_chamelium.c       |  62 +----------------
 tests/kms_hdmi_inject.c     |   9 ++-
 9 files changed, 215 insertions(+), 128 deletions(-)
 create mode 100644 lib/tests/igt_edid.c

--
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/3] lib/tests/igt_edid: introduce EDID sanity checks

[Simon Ser]

The idea is to make sure we don't completely break EDIDs by performing some
basic sanity-checking in lib tests.

The test currently only checks the base and alt EDIDs. More EDIDs will be added
in the future: HDMI audio, 4K, 3D and so on.

The logic is mostly borrowed from lib/tests/igt_hdmi_inject.c. This patch is
part of the "let's unify igt_edid and igt_hdmi_inject" series.

Signed-off-by: Simon Ser <simon.ser@intel.com>
---
 lib/tests/igt_edid.c  | 96 +++++++++++++++++++++++++++++++++++++++++++
 lib/tests/meson.build |  1 +
 2 files changed, 97 insertions(+)
 create mode 100644 lib/tests/igt_edid.c

diff --git a/lib/tests/igt_edid.c b/lib/tests/igt_edid.c
new file mode 100644
index 000000000000..6cf6b5c14b53
--- /dev/null
+++ b/lib/tests/igt_edid.c
@@ -0,0 +1,96 @@
+/*
+ * Copyright © 2019 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ *
+ * Authors: Simon Ser <simon.ser@intel.com>
+ */
+
+#include "config.h"
+
+#include <stdbool.h>
+
+#include "igt_core.h"
+#include "igt_kms.h"
+#include "igt_edid.h"
+
+static const unsigned char edid_header[] = {
+	0x00, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0x00
+};
+
+/**
+ * Sanity check the header of the base EDID block.
+ */
+static bool edid_header_is_valid(const unsigned char *raw_edid)
+{
+	size_t i;
+
+	for (i = 0; i < sizeof(edid_header); i++)
+		if (raw_edid[i] != edid_header[i])
+			return false;
+
+	return true;
+}
+
+/**
+ * Sanity check the checksum of the EDID block.
+ */
+static bool edid_block_checksum(const unsigned char *raw_edid)
+{
+	size_t i;
+	unsigned char csum = 0;
+
+	for (i = 0; i < EDID_LENGTH; i++) {
+		csum += raw_edid[i];
+	}
+
+	return csum == 0;
+}
+
+typedef const unsigned char *(*get_edid_func)(void);
+
+igt_simple_main
+{
+	const struct {
+		const char *desc;
+		get_edid_func f;
+		size_t exts;
+	} funcs[] = {
+		{ "base", igt_kms_get_base_edid, 0 },
+		{ "alt", igt_kms_get_alt_edid, 0 },
+		{0},
+	}, *f;
+	const unsigned char *edid;
+	size_t i;
+
+	for (f = funcs; f->f; f++) {
+		edid = f->f();
+
+		igt_assert_f(edid_header_is_valid(edid),
+			     "invalid header on %s EDID", f->desc);
+		/* check base edid block */
+		igt_assert_f(edid_block_checksum(edid),
+			     "checksum failed on %s EDID", f->desc);
+		/* check extension blocks, if any */
+		for (i = 0; i < f->exts; i++)
+			igt_assert_f(edid_block_checksum(edid + (i + 1) * EDID_LENGTH),
+				     "CEA block checksum failed on %s EDID", f->desc);
+	}
+}
diff --git a/lib/tests/meson.build b/lib/tests/meson.build
index 9950bd59c174..e5f369f743fa 100644
--- a/lib/tests/meson.build
+++ b/lib/tests/meson.build
@@ -3,6 +3,7 @@ lib_tests = [
 	'igt_can_fail',
 	'igt_can_fail_simple',
 	'igt_conflicting_args',
+	'igt_edid',
 	'igt_exit_handler',
 	'igt_fork',
 	'igt_fork_helper',
-- 
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/3] lib/igt_edid: add support for Speaker Allocation Data blocks

[Simon Ser]

Speaker Allocation Data blocks describe which speakers are present in the
display device.

This block is required to make DisplayPort audio work.

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

diff --git a/lib/igt_edid.c b/lib/igt_edid.c
index fbdb0c06b8d7..e71136f48e14 100644
--- a/lib/igt_edid.c
+++ b/lib/igt_edid.c
@@ -348,6 +348,18 @@ size_t edid_cea_data_block_set_vsd(struct edid_cea_data_block *block,
 	return sizeof(struct edid_cea_data_block) + vsd_size;
 }
 
+size_t edid_cea_data_block_set_speaker_alloc(struct edid_cea_data_block *block,
+					     const struct cea_speaker_alloc *speakers)
+{
+	size_t size;
+
+	size = sizeof(struct cea_speaker_alloc);
+	edid_cea_data_block_init(block, EDID_CEA_DATA_SPEAKER_ALLOC, size);
+	memcpy(block->data.speakers, speakers, size);
+
+	return sizeof(struct edid_cea_data_block) + size;
+}
+
 void edid_ext_set_cea(struct edid_ext *ext, size_t data_blocks_size,
 		      uint8_t flags)
 {
diff --git a/lib/igt_edid.h b/lib/igt_edid.h
index 7edd7e38f41e..39d1842d32df 100644
--- a/lib/igt_edid.h
+++ b/lib/igt_edid.h
@@ -195,6 +195,21 @@ struct cea_vsd {
 	char data[];
 };
 
+enum cea_speaker_alloc_item {
+	CEA_SPEAKER_FRONT_LEFT_RIGHT = 1 << 0,
+	CEA_SPEAKER_LFE = 1 << 1,
+	CEA_SPEAKER_FRONT_CENTER = 1 << 2,
+	CEA_SPEAKER_REAR_LEFT_RIGHT = 1 << 3,
+	CEA_SPEAKER_REAR_CENTER = 1 << 4,
+	CEA_SPEAKER_FRONT_LEFT_RIGHT_CENTER = 1 << 5,
+	CEA_SPEAKER_REAR_LEFT_RIGHT_CENTER = 1 << 6,
+};
+
+struct cea_speaker_alloc {
+	uint8_t speakers; /* enum cea_speaker_alloc_item */
+	uint8_t reserved[2];
+} __attribute__((packed));
+
 enum edid_cea_data_type {
 	EDID_CEA_DATA_AUDIO = 1,
 	EDID_CEA_DATA_VIDEO = 2,
@@ -207,6 +222,7 @@ struct edid_cea_data_block {
 	union {
 		struct cea_sad sads[0];
 		struct cea_vsd vsds[0];
+		struct cea_speaker_alloc speakers[0];
 	} data;
 } __attribute__((packed));
 
@@ -295,6 +311,8 @@ 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);
+size_t edid_cea_data_block_set_speaker_alloc(struct edid_cea_data_block *block,
+					     const struct cea_speaker_alloc *speakers);
 void edid_ext_set_cea(struct edid_ext *ext, size_t data_blocks_size,
 		      uint8_t flags);
 
-- 
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/3] lib/igt_kms: introduce igt_kms_get_hdmi_audio_edid

[Simon Ser]

This new function uses igt_edid to generate an EDID suitable for testing HDMI
audio. It's imported from kms_chamelium with minor edits, it's used there and
in kms_hdmi_inject. A (unexported for now) generate_hdmi_audio_edid function
enables generation of EDIDs with arbitrary SAD and speaker blocks.

This obsoletes kmstest_edid_add_audio.

The sanity check for the HDMI audio EDID has been moved from
lib/tests/igt_hdmi_inject.c to lib/tests/igt_edid.c.

Signed-off-by: Simon Ser <simon.ser@intel.com>
---
 lib/igt_kms.c               | 133 ++++++++++++++++++++----------------
 lib/igt_kms.h               |  10 ++-
 lib/tests/igt_edid.c        |   1 +
 lib/tests/igt_hdmi_inject.c |   1 -
 tests/kms_chamelium.c       |  62 +----------------
 tests/kms_hdmi_inject.c     |   9 ++-
 6 files changed, 88 insertions(+), 128 deletions(-)

diff --git a/lib/igt_kms.c b/lib/igt_kms.c
index a7dc3ac3fb66..d7d711a72d27 100644
--- a/lib/igt_kms.c
+++ b/lib/igt_kms.c
@@ -26,6 +26,8 @@
  */
 
 #include "config.h"
+
+#include <assert.h>
 #include <inttypes.h>
 #include <unistd.h>
 #include <stdio.h>
@@ -180,6 +182,79 @@ const unsigned char *igt_kms_get_alt_edid(void)
 	return (unsigned char *) &edid;
 }
 
+static void
+generate_hdmi_audio_edid(unsigned char raw_edid[static HDMI_AUDIO_EDID_LENGTH],
+			 struct cea_sad *sad,
+			 struct cea_speaker_alloc *speaker_alloc)
+{
+	struct edid *edid;
+	struct edid_ext *edid_ext;
+	struct edid_cea *edid_cea;
+	char *cea_data;
+	struct edid_cea_data_block *block;
+	const struct cea_vsd *vsd;
+	size_t cea_data_size, vsd_size;
+
+	/* 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;
+
+	/* Short Audio Descriptor block */
+	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);
+
+	/* Speaker Allocation Data block */
+	block = (struct edid_cea_data_block *) &cea_data[cea_data_size];
+	cea_data_size += edid_cea_data_block_set_speaker_alloc(block,
+							       speaker_alloc);
+
+	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);
+}
+
+const unsigned char *igt_kms_get_hdmi_audio_edid(void)
+{
+	int channels;
+	uint8_t sampling_rates, sample_sizes;
+	static unsigned char raw_edid[HDMI_AUDIO_EDID_LENGTH] = {0};
+	struct cea_sad sad = {0};
+	struct cea_speaker_alloc speaker_alloc = {0};
+
+	/* Initialize the Short Audio Descriptor for PCM */
+	channels = 2;
+	sampling_rates = CEA_SAD_SAMPLING_RATE_32KHZ |
+			 CEA_SAD_SAMPLING_RATE_44KHZ |
+			 CEA_SAD_SAMPLING_RATE_48KHZ;
+	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);
+
+	/* Initialize the Speaker Allocation Data */
+	speaker_alloc.speakers = CEA_SPEAKER_FRONT_LEFT_RIGHT_CENTER;
+
+	generate_hdmi_audio_edid(raw_edid, &sad, &speaker_alloc);
+
+	return raw_edid;
+}
+
 const char * const igt_plane_prop_names[IGT_NUM_PLANE_PROPS] = {
 	[IGT_PLANE_SRC_X] = "SRC_X",
 	[IGT_PLANE_SRC_Y] = "SRC_Y",
@@ -1356,64 +1431,6 @@ void kmstest_edid_add_4k(const unsigned char *edid, size_t length,
 	update_edid_csum(new_edid.data, length);
 }
 
-/**
- * kmstest_edid_add_audio:
- * @edid: an existing valid edid block
- * @length: length of @edid
- * @new_edid_ptr: pointer to where the new edid will be placed
- * @new_length: pointer to the size of the new edid
- *
- * Makes a copy of an existing edid block and adds an extension indicating
- * basic audio support and speaker data block.
- *
- */
-void kmstest_edid_add_audio(const unsigned char *edid, size_t length,
-			    unsigned char *new_edid_ptr[], size_t *new_length)
-{
-	char vsdb_block_len = 10, audio_block_len = 4, spkr_block_len = 4;
-	struct edid_block new_edid = init_cea_block(edid, length, new_edid_ptr,
-						    new_length,
-						    vsdb_block_len +
-						    audio_block_len +
-						    spkr_block_len,
-						    DTD_SUPPORTS_AUDIO);
-	int pos = new_edid.pos;
-
-	/* audio block, short audio block descriptors  */
-	new_edid.data[pos++] = (1 << 5) | (audio_block_len - 1);
-	new_edid.data[pos++] = 0x09; /* Audio Format, PCM */
-	new_edid.data[pos++] = 0x07; /* Frequency, 32, 44.1, 48kHz  */
-	new_edid.data[pos++] = 0x07; /* Bit Rate 16, 20, 24 bit */
-
-
-	/* vsdb block ( id | length ) -- need vsdb as well
-	 * otherwise the kernel will fallback to lower clock modes */
-	new_edid.data[pos++] = 3 << 5 | (vsdb_block_len - 1);
-	/* registration id */
-	new_edid.data[pos++] = 0x3;
-	new_edid.data[pos++] = 0xc;
-	new_edid.data[pos++] = 0x0;
-	/* source physical address */
-	new_edid.data[pos++] = 0x10;
-	new_edid.data[pos++] = 0x00;
-	/* Supports_AI ... etc */
-	new_edid.data[pos++] = 0x00;
-	/* Max TMDS Clock */
-	new_edid.data[pos++] = 0x00;
-	/* Latency present, HDMI Video Present */
-	new_edid.data[pos++] = 0x20;
-	/* HDMI Video */
-	new_edid.data[pos++] = 0x00; /* 3D present */
-
-	/* speaker data block */
-	new_edid.data[pos++] = (4 << 5) | (spkr_block_len - 1);
-	new_edid.data[pos++] = (1 << 5);
-	new_edid.data[pos++] = 0x00;
-	new_edid.data[pos++] = 0x00;
-
-	update_edid_csum(new_edid.data, length);
-}
-
 /**
  * kmstest_unset_all_crtcs:
  * @drm_fd: the DRM fd
diff --git a/lib/igt_kms.h b/lib/igt_kms.h
index 38bdc08f3d50..4ac28131b6d9 100644
--- a/lib/igt_kms.h
+++ b/lib/igt_kms.h
@@ -196,7 +196,6 @@ bool kmstest_force_connector(int fd, drmModeConnector *connector,
 			     enum kmstest_force_connector_state state);
 void kmstest_edid_add_3d(const unsigned char *edid, size_t length, unsigned char *new_edid_ptr[], size_t *new_length);
 void kmstest_edid_add_4k(const unsigned char *edid, size_t length, unsigned char *new_edid_ptr[], size_t *new_length);
-void kmstest_edid_add_audio(const unsigned char *edid, size_t length, unsigned char *new_edid_ptr[], size_t *new_length);
 void kmstest_force_edid(int drm_fd, drmModeConnector *connector,
 			const unsigned char *edid, size_t length);
 
@@ -754,9 +753,14 @@ void igt_reset_connectors(void);
 
 uint32_t kmstest_get_vbl_flag(uint32_t pipe_id);
 
+struct cea_sad;
+struct cea_speaker_alloc;
+
 #define EDID_LENGTH 128
-const unsigned char* igt_kms_get_base_edid(void);
-const unsigned char* igt_kms_get_alt_edid(void);
+#define HDMI_AUDIO_EDID_LENGTH (2 * EDID_LENGTH)
+const unsigned char *igt_kms_get_base_edid(void);
+const unsigned char *igt_kms_get_alt_edid(void);
+const unsigned char *igt_kms_get_hdmi_audio_edid(void);
 
 struct udev_monitor *igt_watch_hotplug(void);
 bool igt_hotplug_detected(struct udev_monitor *mon,
diff --git a/lib/tests/igt_edid.c b/lib/tests/igt_edid.c
index 6cf6b5c14b53..a847df272525 100644
--- a/lib/tests/igt_edid.c
+++ b/lib/tests/igt_edid.c
@@ -75,6 +75,7 @@ igt_simple_main
 	} funcs[] = {
 		{ "base", igt_kms_get_base_edid, 0 },
 		{ "alt", igt_kms_get_alt_edid, 0 },
+		{ "hdmi_audio", igt_kms_get_hdmi_audio_edid, 1 },
 		{0},
 	}, *f;
 	const unsigned char *edid;
diff --git a/lib/tests/igt_hdmi_inject.c b/lib/tests/igt_hdmi_inject.c
index 9b6780a14aa6..2534b1a23acb 100644
--- a/lib/tests/igt_hdmi_inject.c
+++ b/lib/tests/igt_hdmi_inject.c
@@ -73,7 +73,6 @@ igt_simple_main
 	} funcs[] = {
 		{ "3D", kmstest_edid_add_3d },
 		{ "4k", kmstest_edid_add_4k },
-		{ "audio", kmstest_edid_add_audio },
 		{ NULL, NULL },
 	}, *f;
 
diff --git a/tests/kms_chamelium.c b/tests/kms_chamelium.c
index 8da6ec20759e..e75786d33765 100644
--- a/tests/kms_chamelium.c
+++ b/tests/kms_chamelium.c
@@ -1740,66 +1740,6 @@ test_hpd_storm_disable(data_t *data, struct chamelium_port *port, int width)
 	igt_hpd_storm_reset(data->drm_fd);
 }
 
-#define HDMI_AUDIO_EDID_SIZE (sizeof(struct edid) + sizeof(struct edid_ext))
-
-static unsigned const char *get_hdmi_audio_edid(void)
-{
-	int channels;
-	uint8_t sampling_rates, sample_sizes;
-	static unsigned char raw_edid[HDMI_AUDIO_EDID_SIZE] = {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;
-
-	/* 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);
-
-	return raw_edid;
-}
-
 static const unsigned char *get_edid(enum test_edid edid)
 {
 	switch (edid) {
@@ -1811,7 +1751,7 @@ static const unsigned char *get_edid(enum test_edid edid)
 	case TEST_EDID_ALT:
 		return igt_kms_get_alt_edid();
 	case TEST_EDID_HDMI_AUDIO:
-		return get_hdmi_audio_edid();
+		return igt_kms_get_hdmi_audio_edid();
 	}
 	assert(0); /* unreachable */
 }
diff --git a/tests/kms_hdmi_inject.c b/tests/kms_hdmi_inject.c
index a24061042c20..47f5ef2c558a 100644
--- a/tests/kms_hdmi_inject.c
+++ b/tests/kms_hdmi_inject.c
@@ -24,6 +24,7 @@
 
 #include <dirent.h>
 #include "igt.h"
+#include "igt_edid.h"
 
 #define HDISPLAY_4K	3840
 #define VDISPLAY_4K	2160
@@ -211,14 +212,14 @@ eld_is_valid(void)
 static void
 hdmi_inject_audio(int drm_fd, drmModeConnector *connector)
 {
-	unsigned char *edid;
+	const unsigned char *edid;
 	size_t length;
 	int fb_id, cid, ret, crtc_mask = -1;
 	struct igt_fb fb;
 	struct kmstest_connector_config config;
 
-	kmstest_edid_add_audio(igt_kms_get_base_edid(), EDID_LENGTH, &edid,
-			       &length);
+	edid = igt_kms_get_hdmi_audio_edid();
+	length = HDMI_AUDIO_EDID_LENGTH;
 
 	kmstest_force_edid(drm_fd, connector, edid, length);
 
@@ -261,8 +262,6 @@ hdmi_inject_audio(int drm_fd, drmModeConnector *connector)
 
 	kmstest_force_connector(drm_fd, connector, FORCE_CONNECTOR_UNSPECIFIED);
 	kmstest_force_edid(drm_fd, connector, NULL, 0);
-
-	free(edid);
 }
 
 igt_main
-- 
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: success for Unify HDMI audio EDID generation

[Patchwork]

== Series Details ==

Series: Unify HDMI audio EDID generation
URL   : https://patchwork.freedesktop.org/series/61185/
State : success

== Summary ==

CI Bug Log - changes from IGT_5017 -> IGTPW_3063
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

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

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

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

### IGT changes ###

#### Possible fixes ####

  * igt@gem_pwrite@basic:
    - fi-icl-u3:          [DMESG-WARN][1] ([fdo#107724]) -> [PASS][2]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5017/fi-icl-u3/igt@gem_pwrite@basic.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3063/fi-icl-u3/igt@gem_pwrite@basic.html

  * igt@i915_selftest@live_hangcheck:
    - fi-icl-u3:          [INCOMPLETE][3] ([fdo#107713] / [fdo#108569]) -> [PASS][4]
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5017/fi-icl-u3/igt@i915_selftest@live_hangcheck.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3063/fi-icl-u3/igt@i915_selftest@live_hangcheck.html
    - {fi-icl-dsi}:       [INCOMPLETE][5] ([fdo#107713] / [fdo#108569]) -> [PASS][6]
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5017/fi-icl-dsi/igt@i915_selftest@live_hangcheck.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3063/fi-icl-dsi/igt@i915_selftest@live_hangcheck.html

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

  * igt@kms_frontbuffer_tracking@basic:
    - fi-icl-u2:          [FAIL][9] ([fdo#103167]) -> [PASS][10]
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5017/fi-icl-u2/igt@kms_frontbuffer_tracking@basic.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3063/fi-icl-u2/igt@kms_frontbuffer_tracking@basic.html

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

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


Participating hosts (53 -> 46)
------------------------------

  Missing    (7): fi-kbl-soraka fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-byt-clapper fi-bdw-samus 


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

  * IGT: IGT_5017 -> IGTPW_3063

  CI_DRM_6147: 12a8ed2d762a29845da42f16a175579896f58dea @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_3063: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3063/
  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_3063/
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] ✓ Fi.CI.IGT: success for Unify HDMI audio EDID generation

[Patchwork]

== Series Details ==

Series: Unify HDMI audio EDID generation
URL   : https://patchwork.freedesktop.org/series/61185/
State : success

== Summary ==

CI Bug Log - changes from IGT_5017_full -> IGTPW_3063_full
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

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

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

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

### IGT changes ###

#### Issues hit ####

  * igt@gem_workarounds@suspend-resume:
    - shard-kbl:          [PASS][1] -> [INCOMPLETE][2] ([fdo#103665])
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5017/shard-kbl7/igt@gem_workarounds@suspend-resume.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3063/shard-kbl3/igt@gem_workarounds@suspend-resume.html

  * igt@i915_suspend@fence-restore-untiled:
    - shard-apl:          [PASS][3] -> [DMESG-WARN][4] ([fdo#108566]) +4 similar issues
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5017/shard-apl4/igt@i915_suspend@fence-restore-untiled.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3063/shard-apl3/igt@i915_suspend@fence-restore-untiled.html

  * igt@kms_busy@extended-pageflip-hang-newfb-render-b:
    - shard-iclb:         [PASS][5] -> [INCOMPLETE][6] ([fdo#107713])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5017/shard-iclb3/igt@kms_busy@extended-pageflip-hang-newfb-render-b.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3063/shard-iclb5/igt@kms_busy@extended-pageflip-hang-newfb-render-b.html

  * igt@kms_cursor_crc@pipe-a-cursor-suspend:
    - shard-kbl:          [PASS][7] -> [DMESG-WARN][8] ([fdo#108566]) +2 similar issues
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5017/shard-kbl2/igt@kms_cursor_crc@pipe-a-cursor-suspend.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3063/shard-kbl4/igt@kms_cursor_crc@pipe-a-cursor-suspend.html

  * igt@kms_cursor_crc@pipe-b-cursor-64x64-sliding:
    - shard-apl:          [PASS][9] -> [FAIL][10] ([fdo#103232])
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5017/shard-apl3/igt@kms_cursor_crc@pipe-b-cursor-64x64-sliding.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3063/shard-apl3/igt@kms_cursor_crc@pipe-b-cursor-64x64-sliding.html
    - shard-kbl:          [PASS][11] -> [FAIL][12] ([fdo#103232])
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5017/shard-kbl6/igt@kms_cursor_crc@pipe-b-cursor-64x64-sliding.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3063/shard-kbl7/igt@kms_cursor_crc@pipe-b-cursor-64x64-sliding.html

  * igt@kms_flip@2x-plain-flip-ts-check-interruptible:
    - shard-hsw:          [PASS][13] -> [SKIP][14] ([fdo#109271]) +12 similar issues
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5017/shard-hsw4/igt@kms_flip@2x-plain-flip-ts-check-interruptible.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3063/shard-hsw1/igt@kms_flip@2x-plain-flip-ts-check-interruptible.html

  * igt@kms_flip@modeset-vs-vblank-race:
    - shard-glk:          [PASS][15] -> [FAIL][16] ([fdo#103060])
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5017/shard-glk1/igt@kms_flip@modeset-vs-vblank-race.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3063/shard-glk2/igt@kms_flip@modeset-vs-vblank-race.html

  * igt@kms_frontbuffer_tracking@fbcpsr-1p-pri-indfb-multidraw:
    - shard-iclb:         [PASS][17] -> [FAIL][18] ([fdo#103167]) +1 similar issue
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5017/shard-iclb8/igt@kms_frontbuffer_tracking@fbcpsr-1p-pri-indfb-multidraw.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3063/shard-iclb2/igt@kms_frontbuffer_tracking@fbcpsr-1p-pri-indfb-multidraw.html

  * igt@kms_psr2_su@frontbuffer:
    - shard-iclb:         [PASS][19] -> [SKIP][20] ([fdo#109642])
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5017/shard-iclb2/igt@kms_psr2_su@frontbuffer.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3063/shard-iclb5/igt@kms_psr2_su@frontbuffer.html

  * igt@kms_psr@psr2_cursor_plane_move:
    - shard-iclb:         [PASS][21] -> [SKIP][22] ([fdo#109441])
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5017/shard-iclb2/igt@kms_psr@psr2_cursor_plane_move.html
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3063/shard-iclb5/igt@kms_psr@psr2_cursor_plane_move.html

  
#### Possible fixes ####

  * igt@gem_ctx_isolation@rcs0-s3:
    - shard-apl:          [DMESG-WARN][23] ([fdo#108566]) -> [PASS][24] +5 similar issues
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5017/shard-apl8/igt@gem_ctx_isolation@rcs0-s3.html
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3063/shard-apl1/igt@gem_ctx_isolation@rcs0-s3.html

  * igt@gem_eio@in-flight-suspend:
    - shard-kbl:          [DMESG-WARN][25] ([fdo#108566]) -> [PASS][26] +2 similar issues
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5017/shard-kbl1/igt@gem_eio@in-flight-suspend.html
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3063/shard-kbl3/igt@gem_eio@in-flight-suspend.html

  * igt@gem_eio@unwedge-stress:
    - shard-snb:          [FAIL][27] ([fdo#109661]) -> [PASS][28]
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5017/shard-snb5/igt@gem_eio@unwedge-stress.html
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3063/shard-snb7/igt@gem_eio@unwedge-stress.html

  * igt@kms_flip@2x-flip-vs-expired-vblank-interruptible:
    - shard-hsw:          [SKIP][29] ([fdo#109271]) -> [PASS][30] +21 similar issues
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5017/shard-hsw1/igt@kms_flip@2x-flip-vs-expired-vblank-interruptible.html
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3063/shard-hsw7/igt@kms_flip@2x-flip-vs-expired-vblank-interruptible.html

  * igt@kms_flip@flip-vs-expired-vblank-interruptible:
    - shard-glk:          [FAIL][31] ([fdo#105363]) -> [PASS][32]
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5017/shard-glk8/igt@kms_flip@flip-vs-expired-vblank-interruptible.html
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3063/shard-glk8/igt@kms_flip@flip-vs-expired-vblank-interruptible.html

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-draw-pwrite:
    - shard-apl:          [FAIL][33] ([fdo#103167]) -> [PASS][34]
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5017/shard-apl8/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-draw-pwrite.html
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3063/shard-apl6/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-draw-pwrite.html
    - shard-kbl:          [FAIL][35] ([fdo#103167]) -> [PASS][36]
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5017/shard-kbl3/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-draw-pwrite.html
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3063/shard-kbl6/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-draw-pwrite.html

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-pri-indfb-draw-render:
    - shard-iclb:         [FAIL][37] ([fdo#103167]) -> [PASS][38] +6 similar issues
   [37]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5017/shard-iclb5/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-pri-indfb-draw-render.html
   [38]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3063/shard-iclb7/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-pri-indfb-draw-render.html

  * igt@kms_frontbuffer_tracking@fbc-2p-primscrn-cur-indfb-draw-render:
    - shard-glk:          [FAIL][39] ([fdo#103167]) -> [PASS][40]
   [39]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5017/shard-glk1/igt@kms_frontbuffer_tracking@fbc-2p-primscrn-cur-indfb-draw-render.html
   [40]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3063/shard-glk5/igt@kms_frontbuffer_tracking@fbc-2p-primscrn-cur-indfb-draw-render.html

  * igt@kms_plane_lowres@pipe-a-tiling-y:
    - shard-iclb:         [FAIL][41] ([fdo#103166]) -> [PASS][42]
   [41]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5017/shard-iclb6/igt@kms_plane_lowres@pipe-a-tiling-y.html
   [42]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3063/shard-iclb2/igt@kms_plane_lowres@pipe-a-tiling-y.html

  * igt@kms_psr@psr2_sprite_plane_move:
    - shard-iclb:         [SKIP][43] ([fdo#109441]) -> [PASS][44] +1 similar issue
   [43]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5017/shard-iclb6/igt@kms_psr@psr2_sprite_plane_move.html
   [44]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3063/shard-iclb2/igt@kms_psr@psr2_sprite_plane_move.html

  * igt@kms_setmode@basic:
    - shard-kbl:          [FAIL][45] ([fdo#99912]) -> [PASS][46]
   [45]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5017/shard-kbl1/igt@kms_setmode@basic.html
   [46]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3063/shard-kbl3/igt@kms_setmode@basic.html

  * igt@kms_vblank@pipe-a-ts-continuation-dpms-suspend:
    - shard-hsw:          [INCOMPLETE][47] ([fdo#103540]) -> [PASS][48]
   [47]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5017/shard-hsw6/igt@kms_vblank@pipe-a-ts-continuation-dpms-suspend.html
   [48]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3063/shard-hsw5/igt@kms_vblank@pipe-a-ts-continuation-dpms-suspend.html

  
#### Warnings ####

  * igt@gem_mmap_gtt@forked-big-copy-odd:
    - shard-iclb:         [TIMEOUT][49] ([fdo#109673]) -> [INCOMPLETE][50] ([fdo#107713] / [fdo#109100]) +1 similar issue
   [49]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5017/shard-iclb8/igt@gem_mmap_gtt@forked-big-copy-odd.html
   [50]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3063/shard-iclb2/igt@gem_mmap_gtt@forked-big-copy-odd.html

  * igt@gem_tiled_swapping@non-threaded:
    - shard-hsw:          [FAIL][51] ([fdo#108686]) -> [INCOMPLETE][52] ([fdo#103540])
   [51]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5017/shard-hsw6/igt@gem_tiled_swapping@non-threaded.html
   [52]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3063/shard-hsw6/igt@gem_tiled_swapping@non-threaded.html

  * igt@prime_vgem@wait-bsd1:
    - shard-snb:          [INCOMPLETE][53] ([fdo#105411]) -> [FAIL][54] ([fdo#110764])
   [53]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5017/shard-snb2/igt@prime_vgem@wait-bsd1.html
   [54]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3063/shard-snb6/igt@prime_vgem@wait-bsd1.html

  
  [fdo#103060]: https://bugs.freedesktop.org/show_bug.cgi?id=103060
  [fdo#103166]: https://bugs.freedesktop.org/show_bug.cgi?id=103166
  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#103232]: https://bugs.freedesktop.org/show_bug.cgi?id=103232
  [fdo#103540]: https://bugs.freedesktop.org/show_bug.cgi?id=103540
  [fdo#103665]: https://bugs.freedesktop.org/show_bug.cgi?id=103665
  [fdo#105363]: https://bugs.freedesktop.org/show_bug.cgi?id=105363
  [fdo#105411]: https://bugs.freedesktop.org/show_bug.cgi?id=105411
  [fdo#107713]: https://bugs.freedesktop.org/show_bug.cgi?id=107713
  [fdo#108566]: https://bugs.freedesktop.org/show_bug.cgi?id=108566
  [fdo#108686]: https://bugs.freedesktop.org/show_bug.cgi?id=108686
  [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#109661]: https://bugs.freedesktop.org/show_bug.cgi?id=109661
  [fdo#109673]: https://bugs.freedesktop.org/show_bug.cgi?id=109673
  [fdo#110764]: https://bugs.freedesktop.org/show_bug.cgi?id=110764
  [fdo#99912]: https://bugs.freedesktop.org/show_bug.cgi?id=99912


Participating hosts (7 -> 6)
------------------------------

  Missing    (1): shard-skl 


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

  * IGT: IGT_5017 -> IGTPW_3063

  CI_DRM_6147: 12a8ed2d762a29845da42f16a175579896f58dea @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_3063: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3063/
  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_3063/
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t 1/3] lib/tests/igt_edid: introduce EDID sanity checks

[Martin Peres]

On 27/05/2019 15:03, Simon Ser wrote:
> The idea is to make sure we don't completely break EDIDs by performing some
> basic sanity-checking in lib tests.
> 
> The test currently only checks the base and alt EDIDs. More EDIDs will be added
> in the future: HDMI audio, 4K, 3D and so on.
> 
> The logic is mostly borrowed from lib/tests/igt_hdmi_inject.c. This patch is
> part of the "let's unify igt_edid and igt_hdmi_inject" series.

Reviewed-by: Martin Peres <martin.peres@linux.intel.com>
> 
> Signed-off-by: Simon Ser <simon.ser@intel.com>
> ---
>  lib/tests/igt_edid.c  | 96 +++++++++++++++++++++++++++++++++++++++++++
>  lib/tests/meson.build |  1 +
>  2 files changed, 97 insertions(+)
>  create mode 100644 lib/tests/igt_edid.c
> 
> diff --git a/lib/tests/igt_edid.c b/lib/tests/igt_edid.c
> new file mode 100644
> index 000000000000..6cf6b5c14b53
> --- /dev/null
> +++ b/lib/tests/igt_edid.c
> @@ -0,0 +1,96 @@
> +/*
> + * Copyright © 2019 Intel Corporation
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the next
> + * paragraph) shall be included in all copies or substantial portions of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
> + * IN THE SOFTWARE.
> + *
> + * Authors: Simon Ser <simon.ser@intel.com>
> + */
> +
> +#include "config.h"
> +
> +#include <stdbool.h>
> +
> +#include "igt_core.h"
> +#include "igt_kms.h"
> +#include "igt_edid.h"
> +
> +static const unsigned char edid_header[] = {
> +	0x00, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0x00
> +};
> +
> +/**
> + * Sanity check the header of the base EDID block.
> + */
> +static bool edid_header_is_valid(const unsigned char *raw_edid)
> +{
> +	size_t i;
> +
> +	for (i = 0; i < sizeof(edid_header); i++)
> +		if (raw_edid[i] != edid_header[i])
> +			return false;
> +
> +	return true;
> +}
> +
> +/**
> + * Sanity check the checksum of the EDID block.
> + */
> +static bool edid_block_checksum(const unsigned char *raw_edid)
> +{
> +	size_t i;
> +	unsigned char csum = 0;
> +
> +	for (i = 0; i < EDID_LENGTH; i++) {
> +		csum += raw_edid[i];
> +	}
> +
> +	return csum == 0;
> +}
> +
> +typedef const unsigned char *(*get_edid_func)(void);
> +
> +igt_simple_main
> +{
> +	const struct {
> +		const char *desc;
> +		get_edid_func f;
> +		size_t exts;
> +	} funcs[] = {
> +		{ "base", igt_kms_get_base_edid, 0 },
> +		{ "alt", igt_kms_get_alt_edid, 0 },
> +		{0},
> +	}, *f;
> +	const unsigned char *edid;
> +	size_t i;
> +
> +	for (f = funcs; f->f; f++) {
> +		edid = f->f();
> +
> +		igt_assert_f(edid_header_is_valid(edid),
> +			     "invalid header on %s EDID", f->desc);
> +		/* check base edid block */
> +		igt_assert_f(edid_block_checksum(edid),
> +			     "checksum failed on %s EDID", f->desc);
> +		/* check extension blocks, if any */
> +		for (i = 0; i < f->exts; i++)
> +			igt_assert_f(edid_block_checksum(edid + (i + 1) * EDID_LENGTH),
> +				     "CEA block checksum failed on %s EDID", f->desc);
> +	}
> +}
> diff --git a/lib/tests/meson.build b/lib/tests/meson.build
> index 9950bd59c174..e5f369f743fa 100644
> --- a/lib/tests/meson.build
> +++ b/lib/tests/meson.build
> @@ -3,6 +3,7 @@ lib_tests = [
>  	'igt_can_fail',
>  	'igt_can_fail_simple',
>  	'igt_conflicting_args',
> +	'igt_edid',
>  	'igt_exit_handler',
>  	'igt_fork',
>  	'igt_fork_helper',
> 
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t 2/3] lib/igt_edid: add support for Speaker Allocation Data blocks

[Martin Peres]

On 27/05/2019 15:03, Simon Ser wrote:
> Speaker Allocation Data blocks describe which speakers are present in the
> display device.
> 
> This block is required to make DisplayPort audio work.
> 
> Signed-off-by: Simon Ser <simon.ser@intel.com>
> ---
>  lib/igt_edid.c | 12 ++++++++++++
>  lib/igt_edid.h | 18 ++++++++++++++++++
>  2 files changed, 30 insertions(+)
> 
> diff --git a/lib/igt_edid.c b/lib/igt_edid.c
> index fbdb0c06b8d7..e71136f48e14 100644
> --- a/lib/igt_edid.c
> +++ b/lib/igt_edid.c
> @@ -348,6 +348,18 @@ size_t edid_cea_data_block_set_vsd(struct edid_cea_data_block *block,
>  	return sizeof(struct edid_cea_data_block) + vsd_size;
>  }
>  
> +size_t edid_cea_data_block_set_speaker_alloc(struct edid_cea_data_block *block,
> +					     const struct cea_speaker_alloc *speakers)
> +{
> +	size_t size;
> +
> +	size = sizeof(struct cea_speaker_alloc);
> +	edid_cea_data_block_init(block, EDID_CEA_DATA_SPEAKER_ALLOC, size);
> +	memcpy(block->data.speakers, speakers, size);
> +
> +	return sizeof(struct edid_cea_data_block) + size;
> +}
> +
>  void edid_ext_set_cea(struct edid_ext *ext, size_t data_blocks_size,
>  		      uint8_t flags)
>  {
> diff --git a/lib/igt_edid.h b/lib/igt_edid.h
> index 7edd7e38f41e..39d1842d32df 100644
> --- a/lib/igt_edid.h
> +++ b/lib/igt_edid.h
> @@ -195,6 +195,21 @@ struct cea_vsd {
>  	char data[];
>  };
>  
> +enum cea_speaker_alloc_item {

Is that the official name? Having alloc in the name of an enum is a
little odd...

> +	CEA_SPEAKER_FRONT_LEFT_RIGHT = 1 << 0,
> +	CEA_SPEAKER_LFE = 1 << 1,
> +	CEA_SPEAKER_FRONT_CENTER = 1 << 2,
> +	CEA_SPEAKER_REAR_LEFT_RIGHT = 1 << 3,
> +	CEA_SPEAKER_REAR_CENTER = 1 << 4,
> +	CEA_SPEAKER_FRONT_LEFT_RIGHT_CENTER = 1 << 5,
> +	CEA_SPEAKER_REAR_LEFT_RIGHT_CENTER = 1 << 6,
> +};
> +
> +struct cea_speaker_alloc {
> +	uint8_t speakers; /* enum cea_speaker_alloc_item */
> +	uint8_t reserved[2];
> +} __attribute__((packed));
> +
>  enum edid_cea_data_type {
>  	EDID_CEA_DATA_AUDIO = 1,
>  	EDID_CEA_DATA_VIDEO = 2,
> @@ -207,6 +222,7 @@ struct edid_cea_data_block {
>  	union {
>  		struct cea_sad sads[0];
>  		struct cea_vsd vsds[0];
> +		struct cea_speaker_alloc speakers[0];

Why [0]? Shouldn't this all be [1]?

>  	} data;
>  } __attribute__((packed));
>  
> @@ -295,6 +311,8 @@ 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);
> +size_t edid_cea_data_block_set_speaker_alloc(struct edid_cea_data_block *block,
> +					     const struct cea_speaker_alloc *speakers);
>  void edid_ext_set_cea(struct edid_ext *ext, size_t data_blocks_size,
>  		      uint8_t flags);
>  
> 
_______________________________________________
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/3] lib/igt_kms: introduce igt_kms_get_hdmi_audio_edid

[Martin Peres]

On 27/05/2019 15:03, Simon Ser wrote:
> This new function uses igt_edid to generate an EDID suitable for testing HDMI
> audio. It's imported from kms_chamelium with minor edits, it's used there and
> in kms_hdmi_inject. A (unexported for now) generate_hdmi_audio_edid function
> enables generation of EDIDs with arbitrary SAD and speaker blocks.
> 
> This obsoletes kmstest_edid_add_audio.
> 
> The sanity check for the HDMI audio EDID has been moved from
> lib/tests/igt_hdmi_inject.c to lib/tests/igt_edid.c.
> 
> Signed-off-by: Simon Ser <simon.ser@intel.com>
> ---
>  lib/igt_kms.c               | 133 ++++++++++++++++++++----------------
>  lib/igt_kms.h               |  10 ++-
>  lib/tests/igt_edid.c        |   1 +
>  lib/tests/igt_hdmi_inject.c |   1 -
>  tests/kms_chamelium.c       |  62 +----------------
>  tests/kms_hdmi_inject.c     |   9 ++-
>  6 files changed, 88 insertions(+), 128 deletions(-)
> 
> diff --git a/lib/igt_kms.c b/lib/igt_kms.c
> index a7dc3ac3fb66..d7d711a72d27 100644
> --- a/lib/igt_kms.c
> +++ b/lib/igt_kms.c
> @@ -26,6 +26,8 @@
>   */
>  
>  #include "config.h"
> +
> +#include <assert.h>
>  #include <inttypes.h>
>  #include <unistd.h>
>  #include <stdio.h>
> @@ -180,6 +182,79 @@ const unsigned char *igt_kms_get_alt_edid(void)
>  	return (unsigned char *) &edid;
>  }
>  
> +static void
> +generate_hdmi_audio_edid(unsigned char raw_edid[static HDMI_AUDIO_EDID_LENGTH],
> +			 struct cea_sad *sad,
> +			 struct cea_speaker_alloc *speaker_alloc)
> +{
> +	struct edid *edid;
> +	struct edid_ext *edid_ext;
> +	struct edid_cea *edid_cea;
> +	char *cea_data;
> +	struct edid_cea_data_block *block;
> +	const struct cea_vsd *vsd;
> +	size_t cea_data_size, vsd_size;
> +
> +	/* 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;
> +
> +	/* Short Audio Descriptor block */
> +	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);
> +
> +	/* Speaker Allocation Data block */
> +	block = (struct edid_cea_data_block *) &cea_data[cea_data_size];
> +	cea_data_size += edid_cea_data_block_set_speaker_alloc(block,
> +							       speaker_alloc);
> +
> +	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);
> +}

Looks pretty neat!

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

> +
> +const unsigned char *igt_kms_get_hdmi_audio_edid(void)
> +{
> +	int channels;
> +	uint8_t sampling_rates, sample_sizes;
> +	static unsigned char raw_edid[HDMI_AUDIO_EDID_LENGTH] = {0};
> +	struct cea_sad sad = {0};
> +	struct cea_speaker_alloc speaker_alloc = {0};
> +
> +	/* Initialize the Short Audio Descriptor for PCM */
> +	channels = 2;
> +	sampling_rates = CEA_SAD_SAMPLING_RATE_32KHZ |
> +			 CEA_SAD_SAMPLING_RATE_44KHZ |
> +			 CEA_SAD_SAMPLING_RATE_48KHZ;
> +	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);
> +
> +	/* Initialize the Speaker Allocation Data */
> +	speaker_alloc.speakers = CEA_SPEAKER_FRONT_LEFT_RIGHT_CENTER;
> +
> +	generate_hdmi_audio_edid(raw_edid, &sad, &speaker_alloc);
> +
> +	return raw_edid;
> +}
> +
>  const char * const igt_plane_prop_names[IGT_NUM_PLANE_PROPS] = {
>  	[IGT_PLANE_SRC_X] = "SRC_X",
>  	[IGT_PLANE_SRC_Y] = "SRC_Y",
> @@ -1356,64 +1431,6 @@ void kmstest_edid_add_4k(const unsigned char *edid, size_t length,
>  	update_edid_csum(new_edid.data, length);
>  }
>  
> -/**
> - * kmstest_edid_add_audio:
> - * @edid: an existing valid edid block
> - * @length: length of @edid
> - * @new_edid_ptr: pointer to where the new edid will be placed
> - * @new_length: pointer to the size of the new edid
> - *
> - * Makes a copy of an existing edid block and adds an extension indicating
> - * basic audio support and speaker data block.
> - *
> - */
> -void kmstest_edid_add_audio(const unsigned char *edid, size_t length,
> -			    unsigned char *new_edid_ptr[], size_t *new_length)
> -{
> -	char vsdb_block_len = 10, audio_block_len = 4, spkr_block_len = 4;
> -	struct edid_block new_edid = init_cea_block(edid, length, new_edid_ptr,
> -						    new_length,
> -						    vsdb_block_len +
> -						    audio_block_len +
> -						    spkr_block_len,
> -						    DTD_SUPPORTS_AUDIO);
> -	int pos = new_edid.pos;
> -
> -	/* audio block, short audio block descriptors  */
> -	new_edid.data[pos++] = (1 << 5) | (audio_block_len - 1);
> -	new_edid.data[pos++] = 0x09; /* Audio Format, PCM */
> -	new_edid.data[pos++] = 0x07; /* Frequency, 32, 44.1, 48kHz  */
> -	new_edid.data[pos++] = 0x07; /* Bit Rate 16, 20, 24 bit */
> -
> -
> -	/* vsdb block ( id | length ) -- need vsdb as well
> -	 * otherwise the kernel will fallback to lower clock modes */
> -	new_edid.data[pos++] = 3 << 5 | (vsdb_block_len - 1);
> -	/* registration id */
> -	new_edid.data[pos++] = 0x3;
> -	new_edid.data[pos++] = 0xc;
> -	new_edid.data[pos++] = 0x0;
> -	/* source physical address */
> -	new_edid.data[pos++] = 0x10;
> -	new_edid.data[pos++] = 0x00;
> -	/* Supports_AI ... etc */
> -	new_edid.data[pos++] = 0x00;
> -	/* Max TMDS Clock */
> -	new_edid.data[pos++] = 0x00;
> -	/* Latency present, HDMI Video Present */
> -	new_edid.data[pos++] = 0x20;
> -	/* HDMI Video */
> -	new_edid.data[pos++] = 0x00; /* 3D present */
> -
> -	/* speaker data block */
> -	new_edid.data[pos++] = (4 << 5) | (spkr_block_len - 1);
> -	new_edid.data[pos++] = (1 << 5);
> -	new_edid.data[pos++] = 0x00;
> -	new_edid.data[pos++] = 0x00;
> -
> -	update_edid_csum(new_edid.data, length);
> -}
> -
>  /**
>   * kmstest_unset_all_crtcs:
>   * @drm_fd: the DRM fd
> diff --git a/lib/igt_kms.h b/lib/igt_kms.h
> index 38bdc08f3d50..4ac28131b6d9 100644
> --- a/lib/igt_kms.h
> +++ b/lib/igt_kms.h
> @@ -196,7 +196,6 @@ bool kmstest_force_connector(int fd, drmModeConnector *connector,
>  			     enum kmstest_force_connector_state state);
>  void kmstest_edid_add_3d(const unsigned char *edid, size_t length, unsigned char *new_edid_ptr[], size_t *new_length);
>  void kmstest_edid_add_4k(const unsigned char *edid, size_t length, unsigned char *new_edid_ptr[], size_t *new_length);
> -void kmstest_edid_add_audio(const unsigned char *edid, size_t length, unsigned char *new_edid_ptr[], size_t *new_length);
>  void kmstest_force_edid(int drm_fd, drmModeConnector *connector,
>  			const unsigned char *edid, size_t length);
>  
> @@ -754,9 +753,14 @@ void igt_reset_connectors(void);
>  
>  uint32_t kmstest_get_vbl_flag(uint32_t pipe_id);
>  
> +struct cea_sad;
> +struct cea_speaker_alloc;
> +
>  #define EDID_LENGTH 128
> -const unsigned char* igt_kms_get_base_edid(void);
> -const unsigned char* igt_kms_get_alt_edid(void);
> +#define HDMI_AUDIO_EDID_LENGTH (2 * EDID_LENGTH)
> +const unsigned char *igt_kms_get_base_edid(void);
> +const unsigned char *igt_kms_get_alt_edid(void);
> +const unsigned char *igt_kms_get_hdmi_audio_edid(void);
>  
>  struct udev_monitor *igt_watch_hotplug(void);
>  bool igt_hotplug_detected(struct udev_monitor *mon,
> diff --git a/lib/tests/igt_edid.c b/lib/tests/igt_edid.c
> index 6cf6b5c14b53..a847df272525 100644
> --- a/lib/tests/igt_edid.c
> +++ b/lib/tests/igt_edid.c
> @@ -75,6 +75,7 @@ igt_simple_main
>  	} funcs[] = {
>  		{ "base", igt_kms_get_base_edid, 0 },
>  		{ "alt", igt_kms_get_alt_edid, 0 },
> +		{ "hdmi_audio", igt_kms_get_hdmi_audio_edid, 1 },
>  		{0},
>  	}, *f;
>  	const unsigned char *edid;
> diff --git a/lib/tests/igt_hdmi_inject.c b/lib/tests/igt_hdmi_inject.c
> index 9b6780a14aa6..2534b1a23acb 100644
> --- a/lib/tests/igt_hdmi_inject.c
> +++ b/lib/tests/igt_hdmi_inject.c
> @@ -73,7 +73,6 @@ igt_simple_main
>  	} funcs[] = {
>  		{ "3D", kmstest_edid_add_3d },
>  		{ "4k", kmstest_edid_add_4k },
> -		{ "audio", kmstest_edid_add_audio },
>  		{ NULL, NULL },
>  	}, *f;
>  
> diff --git a/tests/kms_chamelium.c b/tests/kms_chamelium.c
> index 8da6ec20759e..e75786d33765 100644
> --- a/tests/kms_chamelium.c
> +++ b/tests/kms_chamelium.c
> @@ -1740,66 +1740,6 @@ test_hpd_storm_disable(data_t *data, struct chamelium_port *port, int width)
>  	igt_hpd_storm_reset(data->drm_fd);
>  }
>  
> -#define HDMI_AUDIO_EDID_SIZE (sizeof(struct edid) + sizeof(struct edid_ext))
> -
> -static unsigned const char *get_hdmi_audio_edid(void)
> -{
> -	int channels;
> -	uint8_t sampling_rates, sample_sizes;
> -	static unsigned char raw_edid[HDMI_AUDIO_EDID_SIZE] = {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;
> -
> -	/* 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);
> -
> -	return raw_edid;
> -}
> -
>  static const unsigned char *get_edid(enum test_edid edid)
>  {
>  	switch (edid) {
> @@ -1811,7 +1751,7 @@ static const unsigned char *get_edid(enum test_edid edid)
>  	case TEST_EDID_ALT:
>  		return igt_kms_get_alt_edid();
>  	case TEST_EDID_HDMI_AUDIO:
> -		return get_hdmi_audio_edid();
> +		return igt_kms_get_hdmi_audio_edid();
>  	}
>  	assert(0); /* unreachable */
>  }
> diff --git a/tests/kms_hdmi_inject.c b/tests/kms_hdmi_inject.c
> index a24061042c20..47f5ef2c558a 100644
> --- a/tests/kms_hdmi_inject.c
> +++ b/tests/kms_hdmi_inject.c
> @@ -24,6 +24,7 @@
>  
>  #include <dirent.h>
>  #include "igt.h"
> +#include "igt_edid.h"
>  
>  #define HDISPLAY_4K	3840
>  #define VDISPLAY_4K	2160
> @@ -211,14 +212,14 @@ eld_is_valid(void)
>  static void
>  hdmi_inject_audio(int drm_fd, drmModeConnector *connector)
>  {
> -	unsigned char *edid;
> +	const unsigned char *edid;
>  	size_t length;
>  	int fb_id, cid, ret, crtc_mask = -1;
>  	struct igt_fb fb;
>  	struct kmstest_connector_config config;
>  
> -	kmstest_edid_add_audio(igt_kms_get_base_edid(), EDID_LENGTH, &edid,
> -			       &length);
> +	edid = igt_kms_get_hdmi_audio_edid();
> +	length = HDMI_AUDIO_EDID_LENGTH;
>  
>  	kmstest_force_edid(drm_fd, connector, edid, length);
>  
> @@ -261,8 +262,6 @@ hdmi_inject_audio(int drm_fd, drmModeConnector *connector)
>  
>  	kmstest_force_connector(drm_fd, connector, FORCE_CONNECTOR_UNSPECIFIED);
>  	kmstest_force_edid(drm_fd, connector, NULL, 0);
> -
> -	free(edid);
>  }
>  
>  igt_main
> 
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t 2/3] lib/igt_edid: add support for Speaker Allocation Data blocks

[Ser, Simon]

On Mon, 2019-06-03 at 16:13 +0300, Martin Peres wrote:
> On 27/05/2019 15:03, Simon Ser wrote:
> > Speaker Allocation Data blocks describe which speakers are present in the
> > display device.
> > 
> > This block is required to make DisplayPort audio work.
> > 
> > Signed-off-by: Simon Ser <simon.ser@intel.com>
> > ---
> >  lib/igt_edid.c | 12 ++++++++++++
> >  lib/igt_edid.h | 18 ++++++++++++++++++
> >  2 files changed, 30 insertions(+)
> > 
> > diff --git a/lib/igt_edid.c b/lib/igt_edid.c
> > index fbdb0c06b8d7..e71136f48e14 100644
> > --- a/lib/igt_edid.c
> > +++ b/lib/igt_edid.c
> > @@ -348,6 +348,18 @@ size_t edid_cea_data_block_set_vsd(struct edid_cea_data_block *block,
> >  	return sizeof(struct edid_cea_data_block) + vsd_size;
> >  }
> >  
> > +size_t edid_cea_data_block_set_speaker_alloc(struct edid_cea_data_block *block,
> > +					     const struct cea_speaker_alloc *speakers)
> > +{
> > +	size_t size;
> > +
> > +	size = sizeof(struct cea_speaker_alloc);
> > +	edid_cea_data_block_init(block, EDID_CEA_DATA_SPEAKER_ALLOC, size);
> > +	memcpy(block->data.speakers, speakers, size);
> > +
> > +	return sizeof(struct edid_cea_data_block) + size;
> > +}
> > +
> >  void edid_ext_set_cea(struct edid_ext *ext, size_t data_blocks_size,
> >  		      uint8_t flags)
> >  {
> > diff --git a/lib/igt_edid.h b/lib/igt_edid.h
> > index 7edd7e38f41e..39d1842d32df 100644
> > --- a/lib/igt_edid.h
> > +++ b/lib/igt_edid.h
> > @@ -195,6 +195,21 @@ struct cea_vsd {
> >  	char data[];
> >  };
> >  
> > +enum cea_speaker_alloc_item {
> 
> Is that the official name? Having alloc in the name of an enum is a
> little odd...

Yes, "Speaker Allocation Data" is the official name.

> > +	CEA_SPEAKER_FRONT_LEFT_RIGHT = 1 << 0,
> > +	CEA_SPEAKER_LFE = 1 << 1,
> > +	CEA_SPEAKER_FRONT_CENTER = 1 << 2,
> > +	CEA_SPEAKER_REAR_LEFT_RIGHT = 1 << 3,
> > +	CEA_SPEAKER_REAR_CENTER = 1 << 4,
> > +	CEA_SPEAKER_FRONT_LEFT_RIGHT_CENTER = 1 << 5,
> > +	CEA_SPEAKER_REAR_LEFT_RIGHT_CENTER = 1 << 6,
> > +};
> > +
> > +struct cea_speaker_alloc {
> > +	uint8_t speakers; /* enum cea_speaker_alloc_item */
> > +	uint8_t reserved[2];
> > +} __attribute__((packed));
> > +
> >  enum edid_cea_data_type {
> >  	EDID_CEA_DATA_AUDIO = 1,
> >  	EDID_CEA_DATA_VIDEO = 2,
> > @@ -207,6 +222,7 @@ struct edid_cea_data_block {
> >  	union {
> >  		struct cea_sad sads[0];
> >  		struct cea_vsd vsds[0];
> > +		struct cea_speaker_alloc speakers[0];
> 
> Why [0]? Shouldn't this all be [1]?

EDID allows a variable number of these, so this struct has variable
length. The array must have a zero size otherwise sizeof-based
computations will be off (each of the members of the union have a
different size). The idea is to allow indexing any of these from the
end of the header block.

Alternatively, in all places where we access these fields we could cast
the edid_cea_data_block to a char *, then add sizeof(struct
edid_cea_data_block), then cast to a struct cea_speaker_alloc *. This
sounds a little annoying to write and to read, though.

> >  	} data;
> >  } __attribute__((packed));
> >  
> > @@ -295,6 +311,8 @@ 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);
> > +size_t edid_cea_data_block_set_speaker_alloc(struct edid_cea_data_block *block,
> > +					     const struct cea_speaker_alloc *speakers);
> >  void edid_ext_set_cea(struct edid_ext *ext, size_t data_blocks_size,
> >  		      uint8_t flags);
> >  
> > 
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t 2/3] lib/igt_edid: add support for Speaker Allocation Data blocks

[Martin Peres]

On 03/06/2019 16:28, Ser, Simon wrote:
> On Mon, 2019-06-03 at 16:13 +0300, Martin Peres wrote:
>> On 27/05/2019 15:03, Simon Ser wrote:
>>> Speaker Allocation Data blocks describe which speakers are present in the
>>> display device.
>>>
>>> This block is required to make DisplayPort audio work.
>>>
>>> Signed-off-by: Simon Ser <simon.ser@intel.com>
>>> ---
>>>  lib/igt_edid.c | 12 ++++++++++++
>>>  lib/igt_edid.h | 18 ++++++++++++++++++
>>>  2 files changed, 30 insertions(+)
>>>
>>> diff --git a/lib/igt_edid.c b/lib/igt_edid.c
>>> index fbdb0c06b8d7..e71136f48e14 100644
>>> --- a/lib/igt_edid.c
>>> +++ b/lib/igt_edid.c
>>> @@ -348,6 +348,18 @@ size_t edid_cea_data_block_set_vsd(struct edid_cea_data_block *block,
>>>  	return sizeof(struct edid_cea_data_block) + vsd_size;
>>>  }
>>>  
>>> +size_t edid_cea_data_block_set_speaker_alloc(struct edid_cea_data_block *block,
>>> +					     const struct cea_speaker_alloc *speakers)
>>> +{
>>> +	size_t size;
>>> +
>>> +	size = sizeof(struct cea_speaker_alloc);
>>> +	edid_cea_data_block_init(block, EDID_CEA_DATA_SPEAKER_ALLOC, size);
>>> +	memcpy(block->data.speakers, speakers, size);
>>> +
>>> +	return sizeof(struct edid_cea_data_block) + size;
>>> +}
>>> +
>>>  void edid_ext_set_cea(struct edid_ext *ext, size_t data_blocks_size,
>>>  		      uint8_t flags)
>>>  {
>>> diff --git a/lib/igt_edid.h b/lib/igt_edid.h
>>> index 7edd7e38f41e..39d1842d32df 100644
>>> --- a/lib/igt_edid.h
>>> +++ b/lib/igt_edid.h
>>> @@ -195,6 +195,21 @@ struct cea_vsd {
>>>  	char data[];
>>>  };
>>>  
>>> +enum cea_speaker_alloc_item {
>>
>> Is that the official name? Having alloc in the name of an enum is a
>> little odd...
> 
> Yes, "Speaker Allocation Data" is the official name.

ACK

> 
>>> +	CEA_SPEAKER_FRONT_LEFT_RIGHT = 1 << 0,
>>> +	CEA_SPEAKER_LFE = 1 << 1,
>>> +	CEA_SPEAKER_FRONT_CENTER = 1 << 2,
>>> +	CEA_SPEAKER_REAR_LEFT_RIGHT = 1 << 3,
>>> +	CEA_SPEAKER_REAR_CENTER = 1 << 4,
>>> +	CEA_SPEAKER_FRONT_LEFT_RIGHT_CENTER = 1 << 5,
>>> +	CEA_SPEAKER_REAR_LEFT_RIGHT_CENTER = 1 << 6,
>>> +};
>>> +
>>> +struct cea_speaker_alloc {
>>> +	uint8_t speakers; /* enum cea_speaker_alloc_item */
>>> +	uint8_t reserved[2];
>>> +} __attribute__((packed));
>>> +
>>>  enum edid_cea_data_type {
>>>  	EDID_CEA_DATA_AUDIO = 1,
>>>  	EDID_CEA_DATA_VIDEO = 2,
>>> @@ -207,6 +222,7 @@ struct edid_cea_data_block {
>>>  	union {
>>>  		struct cea_sad sads[0];
>>>  		struct cea_vsd vsds[0];
>>> +		struct cea_speaker_alloc speakers[0];
>>
>> Why [0]? Shouldn't this all be [1]?
> 
> EDID allows a variable number of these, so this struct has variable
> length. The array must have a zero size otherwise sizeof-based
> computations will be off (each of the members of the union have a
> different size). The idea is to allow indexing any of these from the
> end of the header block.
> 
> Alternatively, in all places where we access these fields we could cast
> the edid_cea_data_block to a char *, then add sizeof(struct
> edid_cea_data_block), then cast to a struct cea_speaker_alloc *. This
> sounds a little annoying to write and to read, though.

I see... Well, sounds like an acceptable reasoning. Is it documented on
top of the union already?

Regardless:

Reviewed-by: Martin Peres <martin.peres@linux.intel.com>
> 
>>>  	} data;
>>>  } __attribute__((packed));
>>>  
>>> @@ -295,6 +311,8 @@ 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);
>>> +size_t edid_cea_data_block_set_speaker_alloc(struct edid_cea_data_block *block,
>>> +					     const struct cea_speaker_alloc *speakers);
>>>  void edid_ext_set_cea(struct edid_ext *ext, size_t data_blocks_size,
>>>  		      uint8_t flags);
>>>  
>>>
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev