Add audio EDID tests to Chamelium

Prior to this commit, we were using the Chamelium's default EDID for DP audio. This relies on the fact that this EDID supports audio and has correct audio paremeters for our testing.

Generating our own EDID is less error-prone and will allow us to test different audio parameters.

v2:
- Replace {HDMI,DP}_AUDIO_EDID_LENGTH with AUDIO_EDID_LENGTH (Arek)
- Don't hardcode EDIDs array length (Arek)

Signed-off-by Simon Ser <[email protected]>

---
 lib/igt_kms.c           | 46 +++++++++++++++++++++++++++++++++--------
 lib/igt_kms.h           |  3 ++-
 tests/kms_chamelium.c   | 10 ++++++---
 tests/kms_hdmi_inject.c |  2 +-
 4 files changed, 47 insertions(+), 14 deletions(-)

lib/igt_kms.c +0 -0

index d7d711a72d27..011b064579a8 100644

lib/igt_kms.c +35 -6

+		    struct cea_speaker_alloc *speaker_alloc)
 {
 	struct edid *edid;
 	struct edid_ext *edid_ext;

@@ -210,10 +210,12 @@ generate_hdmi_audio_edid(unsigned char raw_edid[static HDMI_AUDIO_EDID_LENGTH],
 	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);
+	if (with_vsd) {
+		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];

@@ -233,7 +235,33 @@ 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};
+	static unsigned char raw_edid[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_audio_edid(raw_edid, true, &sad, &speaker_alloc);
+
+	return raw_edid;
+}
+
+const unsigned char *igt_kms_get_dp_audio_edid(void)
+{
+	int channels;
+	uint8_t sampling_rates, sample_sizes;
+	static unsigned char raw_edid[AUDIO_EDID_LENGTH] = {0};
 	struct cea_sad sad = {0};
 	struct cea_speaker_alloc speaker_alloc = {0};
 

@@ -250,7 +278,7 @@ const unsigned char *igt_kms_get_hdmi_audio_edid(void)
 	/* Initialize the Speaker Allocation Data */
 	speaker_alloc.speakers = CEA_SPEAKER_FRONT_LEFT_RIGHT_CENTER;
 
-	generate_hdmi_audio_edid(raw_edid, &sad, &speaker_alloc);
+	generate_audio_edid(raw_edid, false, &sad, &speaker_alloc);
 
 	return raw_edid;
 }

lib/igt_kms.h +2 -1

index 4ac28131b6d9..5df9f18c0bd1 100644
@@ -757,10 +757,11 @@ struct cea_sad;
 struct cea_speaker_alloc;
 
 #define EDID_LENGTH 128
-#define HDMI_AUDIO_EDID_LENGTH (2 * EDID_LENGTH)
+#define 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);
+const unsigned char *igt_kms_get_dp_audio_edid(void);
 
 struct udev_monitor *igt_watch_hotplug(void);
 bool igt_hotplug_detected(struct udev_monitor *mon,

tests/kms_chamelium.c +0 -0

index 52e3f0953621..3beea0c623bd 100644

tests/kms_chamelium.c +6 -3

 };
+#define TEST_EDID_COUNT 5
 
 typedef struct {
 	struct chamelium *chamelium;

@@ -49,7 +51,7 @@ typedef struct {
 
 	int drm_fd;
 
-	int edids[4];
+	int edids[TEST_EDID_COUNT];
 } data_t;
 
 #define HOTPLUG_TIMEOUT 20 /* seconds */

@@ -1997,6 +1999,8 @@ static const unsigned char *get_edid(enum test_edid edid)
 		return igt_kms_get_alt_edid();
 	case TEST_EDID_HDMI_AUDIO:
 		return igt_kms_get_hdmi_audio_edid();
+	case TEST_EDID_DP_AUDIO:
+		return igt_kms_get_dp_audio_edid();
 	}
 	assert(0); /* unreachable */
 }

@@ -2031,7 +2035,7 @@ igt_main
 						 &data.port_count);
 
 		data.edids[TEST_EDID_CHAMELIUM_DEFAULT] = CHAMELIUM_DEFAULT_EDID;
-		for (i = 1; i < sizeof(data.edids) / sizeof(data.edids[0]); ++i) {
+		for (i = 1; i < TEST_EDID_COUNT; ++i) {
 			data.edids[i] = chamelium_new_edid(data.chamelium,
 							   get_edid(i));
 		}

@@ -2113,7 +2117,7 @@ igt_main
 		 * Use the Chamelium's default EDID for DP audio. */
 		connector_subtest("dp-audio", DisplayPort)
 			test_display_audio(&data, port, "HDMI",
-					   TEST_EDID_CHAMELIUM_DEFAULT);
+					   TEST_EDID_DP_AUDIO);
 	}
 
 	igt_subtest_group {

tests/kms_hdmi_inject.c +0 -0

index 754a7407b441..8c0d1333db19 100644

Quoted 26 lines · Simon Ser

Prior to this commit, we were using the Chamelium's default EDID for DP audio. This relies on the fact that this EDID supports audio and has correct audio paremeters for our testing.

Generating our own EDID is less error-prone and will allow us to test different audio parameters.

v2:
- Replace {HDMI,DP}_AUDIO_EDID_LENGTH with AUDIO_EDID_LENGTH (Arek)
- Don't hardcode EDIDs array length (Arek)

Signed-off-by Simon Ser <[email protected]>

---
 lib/igt_kms.c           | 46 +++++++++++++++++++++++++++++++++--------
 lib/igt_kms.h           |  3 ++-
 tests/kms_chamelium.c   | 10 ++++++---
 tests/kms_hdmi_inject.c |  2 +-
 4 files changed, 47 insertions(+), 14 deletions(-)

lib/igt_kms.c +0 -0

index d7d711a72d27..011b064579a8 100644
@@ -183,9 +183,9 @@ const unsigned char *igt_kms_get_alt_edid(void)
 }
 

lib/igt_kms.c const unsigned char *igt_kms_get_alt_edid(void) · line 187

Quoted 6 lines

lib/igt_kms.c +2 -3

 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)
+generate_audio_edid(unsigned char raw_edid[static AUDIO_EDID_LENGTH],
+		    bool with_vsd, struct cea_sad *sad,

with_vsd is not exactly super clear as to what it does, right? How about with_hdmi_audio_vsd?

Quoted 80 lines

lib/igt_kms.c +35 -6

+		    struct cea_speaker_alloc *speaker_alloc)
 {
 	struct edid *edid;
 	struct edid_ext *edid_ext;

@@ -210,10 +210,12 @@ generate_hdmi_audio_edid(unsigned char raw_edid[static HDMI_AUDIO_EDID_LENGTH],
 	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);
+	if (with_vsd) {
+		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];

@@ -233,7 +235,33 @@ 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};
+	static unsigned char raw_edid[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_audio_edid(raw_edid, true, &sad, &speaker_alloc);
+
+	return raw_edid;
+}
+
+const unsigned char *igt_kms_get_dp_audio_edid(void)
+{
+	int channels;
+	uint8_t sampling_rates, sample_sizes;
+	static unsigned char raw_edid[AUDIO_EDID_LENGTH] = {0};
 	struct cea_sad sad = {0};
 	struct cea_speaker_alloc speaker_alloc = {0};
 

@@ -250,7 +278,7 @@ const unsigned char *igt_kms_get_hdmi_audio_edid(void)
 	/* Initialize the Speaker Allocation Data */
 	speaker_alloc.speakers = CEA_SPEAKER_FRONT_LEFT_RIGHT_CENTER;
 
-	generate_hdmi_audio_edid(raw_edid, &sad, &speaker_alloc);
+	generate_audio_edid(raw_edid, false, &sad, &speaker_alloc);
 
 	return raw_edid;
 }

lib/igt_kms.h +2 -1

index 4ac28131b6d9..5df9f18c0bd1 100644
@@ -757,10 +757,11 @@ struct cea_sad;
 struct cea_speaker_alloc;
 
 #define EDID_LENGTH 128
-#define HDMI_AUDIO_EDID_LENGTH (2 * EDID_LENGTH)
+#define 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);
+const unsigned char *igt_kms_get_dp_audio_edid(void);
 

tests/kms_chamelium.c enum test_edid { · line 42

Quoted 11 lines

lib/igt_kms.h +0 -0

 struct udev_monitor *igt_watch_hotplug(void);
 bool igt_hotplug_detected(struct udev_monitor *mon,

tests/kms_chamelium.c +1 -0

index 52e3f0953621..3beea0c623bd 100644
@@ -39,7 +39,9 @@ enum test_edid {
 	TEST_EDID_BASE,
 	TEST_EDID_ALT,
 	TEST_EDID_HDMI_AUDIO,
+	TEST_EDID_DP_AUDIO,

Move TEST_EDID_COUNT here, so as it will automatically get the right value when you change the amount of tests.

With this particular change done, this is :

Reviewed-by Martin Peres <[email protected]>

The rename is up to you.

Quoted 48 lines

tests/kms_chamelium.c +6 -3

 };
+#define TEST_EDID_COUNT 5
 
 typedef struct {
 	struct chamelium *chamelium;

@@ -49,7 +51,7 @@ typedef struct {
 
 	int drm_fd;
 
-	int edids[4];
+	int edids[TEST_EDID_COUNT];
 } data_t;
 
 #define HOTPLUG_TIMEOUT 20 /* seconds */

@@ -1997,6 +1999,8 @@ static const unsigned char *get_edid(enum test_edid edid)
 		return igt_kms_get_alt_edid();
 	case TEST_EDID_HDMI_AUDIO:
 		return igt_kms_get_hdmi_audio_edid();
+	case TEST_EDID_DP_AUDIO:
+		return igt_kms_get_dp_audio_edid();
 	}
 	assert(0); /* unreachable */
 }

@@ -2031,7 +2035,7 @@ igt_main
 						 &data.port_count);
 
 		data.edids[TEST_EDID_CHAMELIUM_DEFAULT] = CHAMELIUM_DEFAULT_EDID;
-		for (i = 1; i < sizeof(data.edids) / sizeof(data.edids[0]); ++i) {
+		for (i = 1; i < TEST_EDID_COUNT; ++i) {
 			data.edids[i] = chamelium_new_edid(data.chamelium,
 							   get_edid(i));
 		}

@@ -2113,7 +2117,7 @@ igt_main
 		 * Use the Chamelium's default EDID for DP audio. */
 		connector_subtest("dp-audio", DisplayPort)
 			test_display_audio(&data, port, "HDMI",
-					   TEST_EDID_CHAMELIUM_DEFAULT);
+					   TEST_EDID_DP_AUDIO);
 	}
 
 	igt_subtest_group {

tests/kms_hdmi_inject.c +0 -0

index 754a7407b441..8c0d1333db19 100644
@@ -149,7 +149,7 @@ hdmi_inject_audio(int drm_fd, drmModeConnector *connector)
 	struct kmstest_connector_config config;
 

tests/kms_hdmi_inject.c hdmi_inject_audio(int drm_fd, drmModeConnector *connector) · line 155

Quoted 7 lines

tests/kms_hdmi_inject.c +1 -1

 	edid = igt_kms_get_hdmi_audio_edid();
-	length = HDMI_AUDIO_EDID_LENGTH;
+	length = AUDIO_EDID_LENGTH;
 
 	kmstest_force_edid(drm_fd, connector, edid, length);
 

The existing eld_has_igt function doesn't allow the caller to retrieve the parsed ELD and check audio parameters.

Signed-off-by Simon Ser <[email protected]>

---
 lib/igt_eld.c | 20 +++++++++++++-------
 lib/igt_eld.h |  1 +
 2 files changed, 14 insertions(+), 7 deletions(-)

lib/igt_eld.c +13 -7

index 1bb294e3fb2a..7217962ee5ca 100644
@@ -200,16 +200,14 @@ static bool eld_parse_entry(const char *path, struct eld_entry *eld)
 	return monitor_present;
 }
 
-/** eld_has_igt: check whether ALSA has detected the audio-capable IGT EDID by
- * parsing ELD entries */
-bool eld_has_igt(void)
+/** eld_get_igt: retrieve the ALSA ELD entry matching the IGT EDID */
+bool eld_get_igt(struct eld_entry *eld)
 {
 	DIR *dir;
 	struct dirent *dirent;
 	int i;
 	char card[64];
 	char path[PATH_MAX];
-	struct eld_entry eld;
 
 	for (i = 0; i < 8; i++) {
 		snprintf(card, sizeof(card), "/proc/asound/card%d", i);

@@ -224,16 +222,16 @@ bool eld_has_igt(void)
 
 			snprintf(path, sizeof(path), "%s/%s", card,
 				 dirent->d_name);
-			if (!eld_parse_entry(path, &eld)) {
+			if (!eld_parse_entry(path, eld)) {
 				continue;
 			}
 
-			if (!eld.valid) {
+			if (!eld->valid) {
 				igt_debug("Skipping invalid ELD: %s\n", path);
 				continue;
 			}
 
-			if (strcmp(eld.monitor_name, "IGT") == 0) {
+			if (strcmp(eld->monitor_name, "IGT") == 0) {
 				closedir(dir);
 				return true;
 			}

@@ -243,3 +241,11 @@ bool eld_has_igt(void)
 
 	return false;
 }
+
+/** eld_has_igt: check whether ALSA has detected the audio-capable IGT EDID by
+ * parsing ELD entries */
+bool eld_has_igt(void)
+{
+	struct eld_entry eld;
+	return eld_get_igt(&eld);
+}

lib/igt_eld.h +0 -0

index e16187884d4b..7c4489f054f1 100644

This test enables a Chamelium port with an audio-friendly EDID, and then checks that the EDID-Like Data exposed by ALSA matches our expectations.

Signed-off-by Simon Ser <[email protected]>

---
 tests/kms_chamelium.c | 58 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 58 insertions(+)

tests/kms_chamelium.c +35 -0

index 3beea0c623bd..1a5271bb770f 100644
@@ -28,6 +28,7 @@
 #include "igt.h"
 #include "igt_vc4.h"
 #include "igt_edid.h"
+#include "igt_eld.h"
 
 #include <fcntl.h>
 #include <pthread.h>

@@ -1428,6 +1429,55 @@ test_display_audio(data_t *data, struct chamelium_port *port,
 	free(alsa);
 }
 
+static void
+test_display_audio_edid(data_t *data, struct chamelium_port *port,
+			enum test_edid edid)
+{
+	igt_output_t *output;
+	igt_plane_t *primary;
+	struct igt_fb fb;
+	drmModeModeInfo *mode;
+	drmModeConnector *connector;
+	int fb_id;
+	struct eld_entry eld;
+	struct eld_sad *sad;
+
+	reset_state(data, port);
+
+	output = prepare_output(data, port, edid);
+	connector = chamelium_port_get_connector(data->chamelium, port, false);
+	primary = igt_output_get_plane_type(output, DRM_PLANE_TYPE_PRIMARY);
+	igt_assert(primary);
+
+	/* Enable the output because audio cannot be played on inactive
+	 * connectors. */
+	igt_assert(connector->count_modes > 0);
+	mode = &connector->modes[0];
+
+	fb_id = igt_create_color_pattern_fb(data->drm_fd,
+					    mode->hdisplay, mode->vdisplay,
+					    DRM_FORMAT_XRGB8888,
+					    LOCAL_DRM_FORMAT_MOD_NONE,
+					    0, 0, 0, &fb);
+	igt_assert(fb_id > 0);
+
+	enable_output(data, port, output, mode, &fb);
+

tests/kms_chamelium.c +9 -0

+
+	igt_remove_fb(data->drm_fd, &fb);
+
+	drmModeFreeConnector(connector);
+}
 
 static void randomize_plane_stride(data_t *data,
 				   uint32_t width, uint32_t height,

@@ -2118,6 +2168,10 @@ igt_main
 		connector_subtest("dp-audio", DisplayPort)
 			test_display_audio(&data, port, "HDMI",
 					   TEST_EDID_DP_AUDIO);
+
+		connector_subtest("dp-audio-edid", DisplayPort)
+			test_display_audio_edid(&data, port,
+						TEST_EDID_DP_AUDIO);
 	}
 
 	igt_subtest_group {

Quoted 63 lines · Simon Ser

This test enables a Chamelium port with an audio-friendly EDID, and then checks that the EDID-Like Data exposed by ALSA matches our expectations.

Signed-off-by Simon Ser <[email protected]>

---
 tests/kms_chamelium.c | 58 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 58 insertions(+)

tests/kms_chamelium.c +41 -0

index 3beea0c623bd..1a5271bb770f 100644
@@ -28,6 +28,7 @@
 #include "igt.h"
 #include "igt_vc4.h"
 #include "igt_edid.h"
+#include "igt_eld.h"
 
 #include <fcntl.h>
 #include <pthread.h>

@@ -1428,6 +1429,55 @@ test_display_audio(data_t *data, struct chamelium_port *port,
 	free(alsa);
 }
 
+static void
+test_display_audio_edid(data_t *data, struct chamelium_port *port,
+			enum test_edid edid)
+{
+	igt_output_t *output;
+	igt_plane_t *primary;
+	struct igt_fb fb;
+	drmModeModeInfo *mode;
+	drmModeConnector *connector;
+	int fb_id;
+	struct eld_entry eld;
+	struct eld_sad *sad;
+
+	reset_state(data, port);
+
+	output = prepare_output(data, port, edid);
+	connector = chamelium_port_get_connector(data->chamelium, port, false);
+	primary = igt_output_get_plane_type(output, DRM_PLANE_TYPE_PRIMARY);
+	igt_assert(primary);
+
+	/* Enable the output because audio cannot be played on inactive
+	 * connectors. */
+	igt_assert(connector->count_modes > 0);
+	mode = &connector->modes[0];
+
+	fb_id = igt_create_color_pattern_fb(data->drm_fd,
+					    mode->hdisplay, mode->vdisplay,
+					    DRM_FORMAT_XRGB8888,
+					    LOCAL_DRM_FORMAT_MOD_NONE,
+					    0, 0, 0, &fb);
+	igt_assert(fb_id > 0);
+
+	enable_output(data, port, output, mode, &fb);
+
+	igt_assert(eld_get_igt(&eld));
+	igt_assert(eld.sads_len == 1);
+
+	sad = &eld.sads[0];
+	igt_assert(sad->coding_type == CEA_SAD_FORMAT_PCM);
+	igt_assert(sad->channels == 2);

tests/kms_chamelium.c test_display_audio(data_t *data, struct chamelium_port *port, · line 1475

Quoted 4 lines

tests/kms_chamelium.c +4 -0

+	igt_assert(sad->rates == (CEA_SAD_SAMPLING_RATE_32KHZ |
+		   CEA_SAD_SAMPLING_RATE_44KHZ | CEA_SAD_SAMPLING_RATE_48KHZ));
+	igt_assert(sad->bits == (CEA_SAD_SAMPLE_SIZE_16 |
+		   CEA_SAD_SAMPLE_SIZE_20 | CEA_SAD_SAMPLE_SIZE_24));

If we have only one tests for one EDID, I would suggest trying more coding_types and channels to make it less typical (we want to detect hardcoded values).

With channels = 8 and a couple of other coding_types, the patch is:

Reviewed-by Martin Peres <[email protected]>

Any plans on generating broken ELDs and seeing how Alsa parses that?

Martin

Quoted 29 lines

tests/kms_chamelium.c +13 -0

+
+	igt_remove_fb(data->drm_fd, &fb);
+
+	drmModeFreeConnector(connector);
+}
 
 static void randomize_plane_stride(data_t *data,
 				   uint32_t width, uint32_t height,

@@ -2118,6 +2168,10 @@ igt_main
 		connector_subtest("dp-audio", DisplayPort)
 			test_display_audio(&data, port, "HDMI",
 					   TEST_EDID_DP_AUDIO);
+
+		connector_subtest("dp-audio-edid", DisplayPort)
+			test_display_audio_edid(&data, port,
+						TEST_EDID_DP_AUDIO);
 	}
 
 	igt_subtest_group {

@@ -2269,6 +2323,10 @@ igt_main
 		connector_subtest("hdmi-audio", HDMIA)
 			test_display_audio(&data, port, "HDMI",
 					   TEST_EDID_HDMI_AUDIO);
+
+		connector_subtest("hdmi-audio-edid", HDMIA)
+			test_display_audio_edid(&data, port,
+						TEST_EDID_HDMI_AUDIO);
 	}
 

tests/kms_chamelium.c igt_main · line 2332

Quoted 2 lines

tests/kms_chamelium.c +0 -0

 	igt_subtest_group {

Quoted 64 lines · Martin Peres

Quoted 63 lines · Simon Ser

This test enables a Chamelium port with an audio-friendly EDID, and then checks that the EDID-Like Data exposed by ALSA matches our expectations.

Signed-off-by Simon Ser <[email protected]>

---
 tests/kms_chamelium.c | 58 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 58 insertions(+)

tests/kms_chamelium.c +41 -0

index 3beea0c623bd..1a5271bb770f 100644
@@ -28,6 +28,7 @@
 #include "igt.h"
 #include "igt_vc4.h"
 #include "igt_edid.h"
+#include "igt_eld.h"
 
 #include <fcntl.h>
 #include <pthread.h>

@@ -1428,6 +1429,55 @@ test_display_audio(data_t *data, struct chamelium_port *port,
 	free(alsa);
 }
 
+static void
+test_display_audio_edid(data_t *data, struct chamelium_port *port,
+			enum test_edid edid)
+{
+	igt_output_t *output;
+	igt_plane_t *primary;
+	struct igt_fb fb;
+	drmModeModeInfo *mode;
+	drmModeConnector *connector;
+	int fb_id;
+	struct eld_entry eld;
+	struct eld_sad *sad;
+
+	reset_state(data, port);
+
+	output = prepare_output(data, port, edid);
+	connector = chamelium_port_get_connector(data->chamelium, port, false);
+	primary = igt_output_get_plane_type(output, DRM_PLANE_TYPE_PRIMARY);
+	igt_assert(primary);
+
+	/* Enable the output because audio cannot be played on inactive
+	 * connectors. */
+	igt_assert(connector->count_modes > 0);
+	mode = &connector->modes[0];
+
+	fb_id = igt_create_color_pattern_fb(data->drm_fd,
+					    mode->hdisplay, mode->vdisplay,
+					    DRM_FORMAT_XRGB8888,
+					    LOCAL_DRM_FORMAT_MOD_NONE,
+					    0, 0, 0, &fb);
+	igt_assert(fb_id > 0);
+
+	enable_output(data, port, output, mode, &fb);
+
+	igt_assert(eld_get_igt(&eld));
+	igt_assert(eld.sads_len == 1);
+
+	sad = &eld.sads[0];
+	igt_assert(sad->coding_type == CEA_SAD_FORMAT_PCM);
+	igt_assert(sad->channels == 2);

Quoted 8 lines

tests/kms_chamelium.c test_display_audio(data_t *data, struct chamelium_port *port, · line 1475

Quoted 4 lines

tests/kms_chamelium.c +4 -0

+	igt_assert(sad->rates == (CEA_SAD_SAMPLING_RATE_32KHZ |
+		   CEA_SAD_SAMPLING_RATE_44KHZ | CEA_SAD_SAMPLING_RATE_48KHZ));
+	igt_assert(sad->bits == (CEA_SAD_SAMPLE_SIZE_16 |
+		   CEA_SAD_SAMPLE_SIZE_20 | CEA_SAD_SAMPLE_SIZE_24));

If we have only one tests for one EDID, I would suggest trying more coding_types and channels to make it less typical (we want to detect hardcoded values).

So, the issue is that right now the EDID matches the capabilities of the Chamelium receiver. I'm not too comfortable with advertising an EDID with more than what the Chamelium supports for audio tests.

I agree that we need more EDIDs with different params. I have plans to send a patch for this.

Quoted 5 lines

With channels = 8 and a couple of other coding_types, the patch is:

Reviewed-by Martin Peres <[email protected]>

Any plans on generating broken ELDs and seeing how Alsa parses that?

Yeah, broken EDIDs/ELDs would be interesting, at least to make sure it doesn't freak out completely.

Quoted 31 lines

Martin

Quoted 29 lines

tests/kms_chamelium.c +13 -0

+
+	igt_remove_fb(data->drm_fd, &fb);
+
+	drmModeFreeConnector(connector);
+}
 
 static void randomize_plane_stride(data_t *data,
 				   uint32_t width, uint32_t height,

@@ -2118,6 +2168,10 @@ igt_main
 		connector_subtest("dp-audio", DisplayPort)
 			test_display_audio(&data, port, "HDMI",
 					   TEST_EDID_DP_AUDIO);
+
+		connector_subtest("dp-audio-edid", DisplayPort)
+			test_display_audio_edid(&data, port,
+						TEST_EDID_DP_AUDIO);
 	}
 
 	igt_subtest_group {

@@ -2269,6 +2323,10 @@ igt_main
 		connector_subtest("hdmi-audio", HDMIA)
 			test_display_audio(&data, port, "HDMI",
 					   TEST_EDID_HDMI_AUDIO);
+
+		connector_subtest("hdmi-audio-edid", HDMIA)
+			test_display_audio_edid(&data, port,
+						TEST_EDID_HDMI_AUDIO);
 	}
 

Quoted 2 lines

tests/kms_chamelium.c igt_main · line 2332

Quoted 2 lines

tests/kms_chamelium.c +0 -0

 	igt_subtest_group {

Quoted 74 lines · Simon

Quoted 72 lines · Martin Peres

Quoted 57 lines · Simon Ser

This test enables a Chamelium port with an audio-friendly EDID, and then checks that the EDID-Like Data exposed by ALSA matches our expectations.

Signed-off-by Simon Ser <[email protected]>

---
 tests/kms_chamelium.c | 58 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 58 insertions(+)

tests/kms_chamelium.c +35 -0

index 3beea0c623bd..1a5271bb770f 100644
@@ -28,6 +28,7 @@
 #include "igt.h"
 #include "igt_vc4.h"
 #include "igt_edid.h"
+#include "igt_eld.h"
 
 #include <fcntl.h>
 #include <pthread.h>

@@ -1428,6 +1429,55 @@ test_display_audio(data_t *data, struct chamelium_port *port,
 	free(alsa);
 }
 
+static void
+test_display_audio_edid(data_t *data, struct chamelium_port *port,
+			enum test_edid edid)
+{
+	igt_output_t *output;
+	igt_plane_t *primary;
+	struct igt_fb fb;
+	drmModeModeInfo *mode;
+	drmModeConnector *connector;
+	int fb_id;
+	struct eld_entry eld;
+	struct eld_sad *sad;
+
+	reset_state(data, port);
+
+	output = prepare_output(data, port, edid);
+	connector = chamelium_port_get_connector(data->chamelium, port, false);
+	primary = igt_output_get_plane_type(output, DRM_PLANE_TYPE_PRIMARY);
+	igt_assert(primary);
+
+	/* Enable the output because audio cannot be played on inactive
+	 * connectors. */
+	igt_assert(connector->count_modes > 0);
+	mode = &connector->modes[0];
+
+	fb_id = igt_create_color_pattern_fb(data->drm_fd,
+					    mode->hdisplay, mode->vdisplay,
+					    DRM_FORMAT_XRGB8888,
+					    LOCAL_DRM_FORMAT_MOD_NONE,
+					    0, 0, 0, &fb);
+	igt_assert(fb_id > 0);
+
+	enable_output(data, port, output, mode, &fb);
+

tests/kms_chamelium.c test_display_audio(data_t *data, struct chamelium_port *port, · line 1475

Quoted 10 lines

tests/kms_chamelium.c +10 -0

+	igt_assert(eld_get_igt(&eld));
+	igt_assert(eld.sads_len == 1);
+
+	sad = &eld.sads[0];
+	igt_assert(sad->coding_type == CEA_SAD_FORMAT_PCM);
+	igt_assert(sad->channels == 2);
+	igt_assert(sad->rates == (CEA_SAD_SAMPLING_RATE_32KHZ |
+		   CEA_SAD_SAMPLING_RATE_44KHZ | CEA_SAD_SAMPLING_RATE_48KHZ));
+	igt_assert(sad->bits == (CEA_SAD_SAMPLE_SIZE_16 |
+		   CEA_SAD_SAMPLE_SIZE_20 | CEA_SAD_SAMPLE_SIZE_24));

If we have only one tests for one EDID, I would suggest trying more coding_types and channels to make it less typical (we want to detect hardcoded values).

Quoted 3 lines

So, the issue is that right now the EDID matches the capabilities of the Chamelium receiver. I'm not too comfortable with advertising an EDID with more than what the Chamelium supports for audio tests.

Well, we are not going to send anything to the chamelium, so not sure why you are afraid of anything :s

Quoted 2 lines

I agree that we need more EDIDs with different params. I have plans to send a patch for this.

OK!

Quoted 7 lines

Quoted 4 lines

With channels = 8 and a couple of other coding_types, the patch is:

Reviewed-by Martin Peres <[email protected]>

Any plans on generating broken ELDs and seeing how Alsa parses that?

Yeah, broken EDIDs/ELDs would be interesting, at least to make sure it doesn't freak out completely.

Exactly! :)

Quoted 30 lines

Quoted 30 lines

Martin

Quoted 29 lines

tests/kms_chamelium.c +13 -0

+
+	igt_remove_fb(data->drm_fd, &fb);
+
+	drmModeFreeConnector(connector);
+}
 
 static void randomize_plane_stride(data_t *data,
 				   uint32_t width, uint32_t height,

@@ -2118,6 +2168,10 @@ igt_main
 		connector_subtest("dp-audio", DisplayPort)
 			test_display_audio(&data, port, "HDMI",
 					   TEST_EDID_DP_AUDIO);
+
+		connector_subtest("dp-audio-edid", DisplayPort)
+			test_display_audio_edid(&data, port,
+						TEST_EDID_DP_AUDIO);
 	}
 
 	igt_subtest_group {

@@ -2269,6 +2323,10 @@ igt_main
 		connector_subtest("hdmi-audio", HDMIA)
 			test_display_audio(&data, port, "HDMI",
 					   TEST_EDID_HDMI_AUDIO);
+
+		connector_subtest("hdmi-audio-edid", HDMIA)
+			test_display_audio_edid(&data, port,
+						TEST_EDID_HDMI_AUDIO);
 	}
 

Quoted 2 lines

Quoted 2 lines

tests/kms_chamelium.c igt_main · line 2332

Quoted 2 lines

tests/kms_chamelium.c +0 -0

 	igt_subtest_group {

Quoted 75 lines · Martin Peres

Quoted 74 lines · Simon

Quoted 72 lines · Martin Peres

Quoted 57 lines · Simon Ser

This test enables a Chamelium port with an audio-friendly EDID, and then checks that the EDID-Like Data exposed by ALSA matches our expectations.

Signed-off-by Simon Ser <[email protected]>

---
 tests/kms_chamelium.c | 58 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 58 insertions(+)

tests/kms_chamelium.c +35 -0

index 3beea0c623bd..1a5271bb770f 100644
@@ -28,6 +28,7 @@
 #include "igt.h"
 #include "igt_vc4.h"
 #include "igt_edid.h"
+#include "igt_eld.h"
 
 #include <fcntl.h>
 #include <pthread.h>

@@ -1428,6 +1429,55 @@ test_display_audio(data_t *data, struct chamelium_port *port,
 	free(alsa);
 }
 
+static void
+test_display_audio_edid(data_t *data, struct chamelium_port *port,
+			enum test_edid edid)
+{
+	igt_output_t *output;
+	igt_plane_t *primary;
+	struct igt_fb fb;
+	drmModeModeInfo *mode;
+	drmModeConnector *connector;
+	int fb_id;
+	struct eld_entry eld;
+	struct eld_sad *sad;
+
+	reset_state(data, port);
+
+	output = prepare_output(data, port, edid);
+	connector = chamelium_port_get_connector(data->chamelium, port, false);
+	primary = igt_output_get_plane_type(output, DRM_PLANE_TYPE_PRIMARY);
+	igt_assert(primary);
+
+	/* Enable the output because audio cannot be played on inactive
+	 * connectors. */
+	igt_assert(connector->count_modes > 0);
+	mode = &connector->modes[0];
+
+	fb_id = igt_create_color_pattern_fb(data->drm_fd,
+					    mode->hdisplay, mode->vdisplay,
+					    DRM_FORMAT_XRGB8888,
+					    LOCAL_DRM_FORMAT_MOD_NONE,
+					    0, 0, 0, &fb);
+	igt_assert(fb_id > 0);
+
+	enable_output(data, port, output, mode, &fb);
+

tests/kms_chamelium.c test_display_audio(data_t *data, struct chamelium_port *port, · line 1475

Quoted 10 lines

tests/kms_chamelium.c +10 -0

+	igt_assert(eld_get_igt(&eld));
+	igt_assert(eld.sads_len == 1);
+
+	sad = &eld.sads[0];
+	igt_assert(sad->coding_type == CEA_SAD_FORMAT_PCM);
+	igt_assert(sad->channels == 2);
+	igt_assert(sad->rates == (CEA_SAD_SAMPLING_RATE_32KHZ |
+		   CEA_SAD_SAMPLING_RATE_44KHZ | CEA_SAD_SAMPLING_RATE_48KHZ));
+	igt_assert(sad->bits == (CEA_SAD_SAMPLE_SIZE_16 |
+		   CEA_SAD_SAMPLE_SIZE_20 | CEA_SAD_SAMPLE_SIZE_24));

If we have only one tests for one EDID, I would suggest trying more coding_types and channels to make it less typical (we want to detect hardcoded values).

Quoted 6 lines

Quoted 3 lines

So, the issue is that right now the EDID matches the capabilities of the Chamelium receiver. I'm not too comfortable with advertising an EDID with more than what the Chamelium supports for audio tests.

Well, we are not going to send anything to the chamelium, so not sure why you are afraid of anything :s

The same EDID is used for audio tests.

Quoted 46 lines

Quoted 3 lines

I agree that we need more EDIDs with different params. I have plans to send a patch for this.

OK!

Quoted 7 lines

Quoted 4 lines

With channels = 8 and a couple of other coding_types, the patch is:

Reviewed-by Martin Peres <[email protected]>

Any plans on generating broken ELDs and seeing how Alsa parses that?

Yeah, broken EDIDs/ELDs would be interesting, at least to make sure it doesn't freak out completely.

Exactly! :)

Quoted 30 lines

Quoted 30 lines

Martin

Quoted 29 lines

tests/kms_chamelium.c +13 -0

+
+	igt_remove_fb(data->drm_fd, &fb);
+
+	drmModeFreeConnector(connector);
+}
 
 static void randomize_plane_stride(data_t *data,
 				   uint32_t width, uint32_t height,

@@ -2118,6 +2168,10 @@ igt_main
 		connector_subtest("dp-audio", DisplayPort)
 			test_display_audio(&data, port, "HDMI",
 					   TEST_EDID_DP_AUDIO);
+
+		connector_subtest("dp-audio-edid", DisplayPort)
+			test_display_audio_edid(&data, port,
+						TEST_EDID_DP_AUDIO);
 	}
 
 	igt_subtest_group {

@@ -2269,6 +2323,10 @@ igt_main
 		connector_subtest("hdmi-audio", HDMIA)
 			test_display_audio(&data, port, "HDMI",
 					   TEST_EDID_HDMI_AUDIO);
+
+		connector_subtest("hdmi-audio-edid", HDMIA)
+			test_display_audio_edid(&data, port,
+						TEST_EDID_HDMI_AUDIO);
 	}
 

Quoted 2 lines

Quoted 2 lines

Quoted 2 lines

tests/kms_chamelium.c igt_main · line 2332

Quoted 2 lines

tests/kms_chamelium.c +0 -0

 	igt_subtest_group {