Add audio EDID tests to Chamelium

There are two reasons why I want to introduce this library:

- I want to use it from the Chamelium tests for DisplayPort
- I want to expand it to also check that audio parameters parsed by ALSA are correct (formats, sampling rates, sample sizes and so on)

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

---
 lib/igt_eld.c           | 111 ++++++++++++++++++++++++++++++++++++++++
 lib/igt_eld.h           |  35 +++++++++++++
 lib/meson.build         |   1 +
 tests/kms_hdmi_inject.c |  80 ++---------------------------
 4 files changed, 152 insertions(+), 75 deletions(-)
 create mode 100644 lib/igt_eld.c
 create mode 100644 lib/igt_eld.h

lib/igt_eld.c new file +51 -0

new file mode 100644
index 000000000000..8e0dcc306e85
@@ -0,0 +1,111 @@
+/*
+ * 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 <[email protected]>
+ */
+
+#include "config.h"
+
+#include <dirent.h>
+#include <stdint.h>
+#include <stdio.h>
+#include <string.h>
+
+#include "igt_eld.h"
+
+/**
+ * EDID-Like Data (ELD) is metadata parsed and exposed by ALSA for HDMI and
+ * DisplayPort connectors supporting audio. This includes the monitor name and
+ * the supported audio parameters (formats, sampling rates, sample sizes and so
+ * on).
+ */
+
+/** eld_entry_is_igt: checks whether an ELD entry is mapped to the IGT EDID */
+static bool eld_entry_is_igt(const char *path)
+{
+	FILE *in;
+	char buf[1024];
+	uint8_t eld_valid = 0;
+	uint8_t mon_valid = 0;
+
+	in = fopen(path, "r");
+	if (!in)

lib/igt_eld.c new file +50 -0

+		}
+
+		if (!strncasecmp(line, "monitor_name", 12) &&
+				strstr(line, "IGT")) {
+			mon_valid++;
+		}
+	}
+
+	fclose(in);
+	if (mon_valid && eld_valid)
+		return true;
+
+	return false;
+}
+
+/** eld_has_igt: check whether ALSA has detected the audio-capable IGT EDID by
+ * parsing ELD entries */
+bool eld_has_igt(void)
+{
+	DIR *dir;
+	struct dirent *snd_hda;
+	int i;
+
+	for (i = 0; i < 8; i++) {
+		char cards[128];
+
+		snprintf(cards, sizeof(cards), "/proc/asound/card%d", i);
+		dir = opendir(cards);
+		if (!dir)
+			continue;
+
+		while ((snd_hda = readdir(dir))) {
+			char fpath[PATH_MAX];
+
+			if (*snd_hda->d_name == '.' ||
+			    strstr(snd_hda->d_name, "eld") == 0)
+				continue;
+
+			snprintf(fpath, sizeof(fpath), "%s/%s", cards,
+				 snd_hda->d_name);
+			if (eld_entry_is_igt(fpath)) {
+				closedir(dir);
+				return true;
+			}
+		}
+		closedir(dir);
+	}
+
+	return false;
+}

lib/igt_eld.h new file +35 -0

new file mode 100644
index 000000000000..27f876d9f631
@@ -0,0 +1,35 @@
+/*
+ * 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 <[email protected]>
+ */
+
+#ifndef IGT_ELD_H
+#define IGT_ELD_H
+
+#include "config.h"
+
+#include <stdbool.h>
+
+bool eld_has_igt(void);
+
+#endif

lib/meson.build +1 -0

index cdb450e1e762..844e0abcd919 100644
@@ -59,6 +59,7 @@ lib_sources = [
 	'igt_psr.c',
 	'igt_amd.c',
 	'igt_edid.c',
+	'igt_eld.c',
 ]
 
 lib_deps = [

tests/kms_hdmi_inject.c +4 -74

index a24061042c20..eba25046cead 100644
@@ -22,8 +22,12 @@
  *
  */
 
+#include "config.h"
+
 #include <dirent.h>
+
 #include "igt.h"
+#include "igt_eld.h"
 
 #define HDISPLAY_4K	3840
 #define VDISPLAY_4K	2160

@@ -134,80 +138,6 @@ hdmi_inject_4k(int drm_fd, drmModeConnector *connector)
 	free(edid);
 }
 
-/** eld_entry_is_igt: checks whether an ELD entry is mapped to the IGT EDID */
-static bool
-eld_entry_is_igt(const char* path)
-{
-	FILE *in;
-	char buf[1024];
-	uint8_t eld_valid = 0;
-	uint8_t mon_valid = 0;
-
-	in = fopen(path, "r");
-	if (!in)
-		return false;
-
-	memset(buf, 0, 1024);
-
-	while ((fgets(buf, 1024, in)) != NULL) {
-
-		char *line = buf;
-
-		if (!strncasecmp(line, "eld_valid", 9) &&
-				strstr(line, "1")) {
-			eld_valid++;
-		}
-
-		if (!strncasecmp(line, "monitor_name", 12) &&
-				strstr(line, "IGT")) {
-			mon_valid++;
-		}
-	}
-
-	fclose(in);
-	if (mon_valid && eld_valid)
-		return true;
-
-	return false;
-}
-
-/** eld_is_valid: check whether ALSA has detected the audio-capable IGT EDID by
- * parsing ELD entries */
-static bool
-eld_is_valid(void)
-{
-	DIR *dir;
-	struct dirent *snd_hda;
-	int i;
-
-	for (i = 0; i < 8; i++) {
-		char cards[128];
-
-		snprintf(cards, sizeof(cards), "/proc/asound/card%d", i);
-		dir = opendir(cards);
-		if (!dir)
-			continue;
-
-		while ((snd_hda = readdir(dir))) {
-			char fpath[PATH_MAX];
-
-			if (*snd_hda->d_name == '.' ||
-			    strstr(snd_hda->d_name, "eld") == 0)
-				continue;
-
-			snprintf(fpath, sizeof(fpath), "%s/%s", cards,
-				 snd_hda->d_name);
-			if (eld_entry_is_igt(fpath)) {
-				closedir(dir);
-				return true;
-			}
-		}
-		closedir(dir);
-	}
-
-	return false;
-}
-
 static void
 hdmi_inject_audio(int drm_fd, drmModeConnector *connector)
 {

Make the ELD enumeration more robust, and implement proper parsing for ELD fields. This will become useful when other ELD fields (formats, sample rates, sample sizes) will be parsed and checked.

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

---
 lib/igt_eld.c | 94 +++++++++++++++++++++++++++++++--------------------
 lib/igt_eld.h |  5 +++
 2 files changed, 63 insertions(+), 36 deletions(-)

lib/igt_eld.c +4 -0

index 8e0dcc306e85..a198001a56d7 100644
@@ -30,8 +30,12 @@
 #include <stdio.h>
 #include <string.h>
 
+#include "igt_core.h"
 #include "igt_eld.h"
 
+#define ELD_PREFIX "eld#"
+#define ELD_DELIM " \t"
+
 /**
  * EDID-Like Data (ELD) is metadata parsed and exposed by ALSA for HDMI and
  * DisplayPort connectors supporting audio. This includes the monitor name and

lib/igt_eld.c +6 -11

-static bool eld_entry_is_igt(const char *path)
+static bool eld_parse_entry(const char *path, struct eld_entry *eld)
 {
-	FILE *in;
+	FILE *f;
 	char buf[1024];
-	uint8_t eld_valid = 0;
-	uint8_t mon_valid = 0;
-
-	in = fopen(path, "r");
-	if (!in)
-		return false;
+	char *key, *value;
+	size_t len;
+	bool monitor_present;
 
-	memset(buf, 0, 1024);
+	memset(eld, 0, sizeof(*eld));
 
-	while ((fgets(buf, 1024, in)) != NULL) {
-		char *line = buf;

lib/igt_eld.c +25 -15

+
+		if (strcmp(key, "monitor_present") == 0)
+			monitor_present = strcmp(value, "1") == 0;
+		else if (strcmp(key, "eld_valid") == 0)
+			eld->valid = strcmp(value, "1") == 0;
+		else if (strcmp(key, "monitor_name") == 0)
+			snprintf(eld->monitor_name, sizeof(eld->monitor_name),
+				 "%s", value);
+	}
 
-		if (!strncasecmp(line, "monitor_name", 12) &&
-				strstr(line, "IGT")) {
-			mon_valid++;
-		}
+	if (ferror(f) != 0) {
+		igt_debug("Failed to read ELD file: %d\n", ferror(f));
+		return false;
 	}
 
-	fclose(in);
-	if (mon_valid && eld_valid)
-		return true;
+	fclose(f);
 
-	return false;
+	return monitor_present;
 }
 
 /** eld_has_igt: check whether ALSA has detected the audio-capable IGT EDID by

@@ -79,27 +93,35 @@ static bool eld_entry_is_igt(const char *path)
 bool eld_has_igt(void)
 {
 	DIR *dir;
-	struct dirent *snd_hda;
+	struct dirent *dirent;
 	int i;
+	char card[64];
+	char path[PATH_MAX];
+	struct eld_entry eld;
 
 	for (i = 0; i < 8; i++) {
-		char cards[128];
-
-		snprintf(cards, sizeof(cards), "/proc/asound/card%d", i);
-		dir = opendir(cards);
+		snprintf(card, sizeof(card), "/proc/asound/card%d", i);
+		dir = opendir(card);
 		if (!dir)
 			continue;
 
-		while ((snd_hda = readdir(dir))) {
-			char fpath[PATH_MAX];
+		while ((dirent = readdir(dir))) {
+			if (strncmp(dirent->d_name, ELD_PREFIX,
+				    strlen(ELD_PREFIX)) != 0)
+				continue;
+

lib/igt_eld.c +2 -3

 				continue;
+			}
 
-			snprintf(fpath, sizeof(fpath), "%s/%s", cards,
-				 snd_hda->d_name);
-			if (eld_entry_is_igt(fpath)) {
+			if (strcmp(eld.monitor_name, "IGT") == 0) {
 				closedir(dir);
 				return true;
 			}

lib/igt_eld.h +0 -0

index 27f876d9f631..21fe3537acf9 100644
@@ -30,6 +30,11 @@
 

Quoted 28 lines · Simon Ser

Make the ELD enumeration more robust, and implement proper parsing for ELD fields. This will become useful when other ELD fields (formats, sample rates, sample sizes) will be parsed and checked.

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

---
 lib/igt_eld.c | 94 +++++++++++++++++++++++++++++++--------------------
 lib/igt_eld.h |  5 +++
 2 files changed, 63 insertions(+), 36 deletions(-)

lib/igt_eld.c +4 -0

index 8e0dcc306e85..a198001a56d7 100644
@@ -30,8 +30,12 @@
 #include <stdio.h>
 #include <string.h>
 
+#include "igt_core.h"
 #include "igt_eld.h"
 
+#define ELD_PREFIX "eld#"
+#define ELD_DELIM " \t"
+
 /**
  * EDID-Like Data (ELD) is metadata parsed and exposed by ALSA for HDMI and
  * DisplayPort connectors supporting audio. This includes the monitor name and

lib/igt_eld.c line 42

Quoted 4 lines

lib/igt_eld.c +0 -1

@@ -39,39 +43,49 @@
  * on).
  */
 
-/** eld_entry_is_igt: checks whether an ELD entry is mapped to the IGT EDID */

Posting an example of eld here would be helpful:

$ cat /proc/asound/card0/eld#0.2
monitor_present         1
eld_valid               1
monitor_name            U2879G6

connection_type         DisplayPort
eld_version             [0x2] CEA-861D or below
edid_version            [0x3] CEA-861-B, C or D
manufacture_id          0xe305
product_id              0x2879
port_id                 0x800
support_hdcp            0
support_ai              0
audio_sync_delay        0
speakers                [0x1] FL/FR
sad_count               1
sad0_coding_type        [0x1] LPCM
sad0_channels           2
sad0_rates              [0xe0] 32000 44100 48000
sad0_bits               [0xe0000] 16 20 24

Quoted 21 lines

lib/igt_eld.c +6 -11

-static bool eld_entry_is_igt(const char *path)
+static bool eld_parse_entry(const char *path, struct eld_entry *eld)
 {
-	FILE *in;
+	FILE *f;
 	char buf[1024];
-	uint8_t eld_valid = 0;
-	uint8_t mon_valid = 0;
-
-	in = fopen(path, "r");
-	if (!in)
-		return false;
+	char *key, *value;
+	size_t len;
+	bool monitor_present;
 
-	memset(buf, 0, 1024);
+	memset(eld, 0, sizeof(*eld));
 
-	while ((fgets(buf, 1024, in)) != NULL) {
-		char *line = buf;

lib/igt_eld.c line 62

Quoted 10 lines

lib/igt_eld.c +5 -4

+	f = fopen(path, "r");
+	if (!f) {
+		igt_debug("Failed to open ELD file: %s\n", path);
+		return false;
+	}
 
-		if (!strncasecmp(line, "eld_valid", 9) &&
-				strstr(line, "1")) {
-			eld_valid++;
-		}

Good riddance! Ignore my comment from patch 1!

lib/igt_eld.c line 68

Quoted 7 lines

lib/igt_eld.c +7 -0

+	while ((fgets(buf, sizeof(buf), f)) != NULL) {
+		len = strlen(buf);
+		if (buf[len - 1] == '\n')
+			buf[len - 1] = '\0';
+
+		key = strtok(buf, ELD_DELIM);
+		value = strtok(NULL, "");

Hmm, I had forgotten that strok was stateful when str == NULL!

lib/igt_eld.c line 70

Quoted 2 lines

lib/igt_eld.c +2 -0

+		/* Skip whitespace at the beginning */
+		value += strspn(value, ELD_DELIM);

Thanks for teaching me a new function :) This is quite practical!

After doing more python work, going back to parsing with C is a little rough :D Not as bad as ASM though!

Quoted 63 lines

lib/igt_eld.c +30 -15

+
+		if (strcmp(key, "monitor_present") == 0)
+			monitor_present = strcmp(value, "1") == 0;
+		else if (strcmp(key, "eld_valid") == 0)
+			eld->valid = strcmp(value, "1") == 0;
+		else if (strcmp(key, "monitor_name") == 0)
+			snprintf(eld->monitor_name, sizeof(eld->monitor_name),
+				 "%s", value);
+	}
 
-		if (!strncasecmp(line, "monitor_name", 12) &&
-				strstr(line, "IGT")) {
-			mon_valid++;
-		}
+	if (ferror(f) != 0) {
+		igt_debug("Failed to read ELD file: %d\n", ferror(f));
+		return false;
 	}
 
-	fclose(in);
-	if (mon_valid && eld_valid)
-		return true;
+	fclose(f);
 
-	return false;
+	return monitor_present;
 }
 
 /** eld_has_igt: check whether ALSA has detected the audio-capable IGT EDID by

@@ -79,27 +93,35 @@ static bool eld_entry_is_igt(const char *path)
 bool eld_has_igt(void)
 {
 	DIR *dir;
-	struct dirent *snd_hda;
+	struct dirent *dirent;
 	int i;
+	char card[64];
+	char path[PATH_MAX];
+	struct eld_entry eld;
 
 	for (i = 0; i < 8; i++) {
-		char cards[128];
-
-		snprintf(cards, sizeof(cards), "/proc/asound/card%d", i);
-		dir = opendir(cards);
+		snprintf(card, sizeof(card), "/proc/asound/card%d", i);
+		dir = opendir(card);
 		if (!dir)
 			continue;
 
-		while ((snd_hda = readdir(dir))) {
-			char fpath[PATH_MAX];
+		while ((dirent = readdir(dir))) {
+			if (strncmp(dirent->d_name, ELD_PREFIX,
+				    strlen(ELD_PREFIX)) != 0)
+				continue;
+
+			snprintf(path, sizeof(path), "%s/%s", card,
+				 dirent->d_name);
+			if (!eld_parse_entry(path, &eld)) {
+				continue;
+			}
 

lib/igt_eld.c static bool eld_entry_is_igt(const char *path) · line 120

Quoted 4 lines

lib/igt_eld.c +2 -2

-			if (*snd_hda->d_name == '.' ||
-			    strstr(snd_hda->d_name, "eld") == 0)
+			if (!eld.valid) {
+				igt_debug("Skipping invalid ELD: %s\n", path);

The message is a little confusing. How about "Skipping non-ELD file:
%s\n" instead?

Anyway, I like where this is going!

Reviewed-by Martin Peres <[email protected]>

Quoted 25 lines

lib/igt_eld.c +2 -3

 				continue;
+			}
 
-			snprintf(fpath, sizeof(fpath), "%s/%s", cards,
-				 snd_hda->d_name);
-			if (eld_entry_is_igt(fpath)) {
+			if (strcmp(eld.monitor_name, "IGT") == 0) {
 				closedir(dir);
 				return true;
 			}

lib/igt_eld.h +5 -0

index 27f876d9f631..21fe3537acf9 100644
@@ -30,6 +30,11 @@
 
 #include <stdbool.h>
 
+struct eld_entry {
+	bool valid;
+	char monitor_name[16];
+};
+
 bool eld_has_igt(void);
 

lib/igt_eld.h line 40

Quoted 2 lines

lib/igt_eld.h +0 -0

 #endif

Quoted 101 lines · Martin Peres

Quoted 28 lines · Simon Ser

Make the ELD enumeration more robust, and implement proper parsing for ELD fields. This will become useful when other ELD fields (formats, sample rates, sample sizes) will be parsed and checked.

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

---
 lib/igt_eld.c | 94 +++++++++++++++++++++++++++++++--------------------
 lib/igt_eld.h |  5 +++
 2 files changed, 63 insertions(+), 36 deletions(-)

lib/igt_eld.c +4 -0

index 8e0dcc306e85..a198001a56d7 100644
@@ -30,8 +30,12 @@
 #include <stdio.h>
 #include <string.h>
 
+#include "igt_core.h"
 #include "igt_eld.h"
 
+#define ELD_PREFIX "eld#"
+#define ELD_DELIM " \t"
+
 /**
  * EDID-Like Data (ELD) is metadata parsed and exposed by ALSA for HDMI and
  * DisplayPort connectors supporting audio. This includes the monitor name and

lib/igt_eld.c line 42

Quoted 4 lines

lib/igt_eld.c +0 -1

@@ -39,39 +43,49 @@
  * on).
  */
 
-/** eld_entry_is_igt: checks whether an ELD entry is mapped to the IGT EDID */

Posting an example of eld here would be helpful:

$ cat /proc/asound/card0/eld#0.2
monitor_present         1
eld_valid               1
monitor_name            U2879G6

connection_type         DisplayPort
eld_version             [0x2] CEA-861D or below
edid_version            [0x3] CEA-861-B, C or D
manufacture_id          0xe305
product_id              0x2879
port_id                 0x800
support_hdcp            0
support_ai              0
audio_sync_delay        0
speakers                [0x1] FL/FR
sad_count               1
sad0_coding_type        [0x1] LPCM
sad0_channels           2
sad0_rates              [0xe0] 32000 44100 48000
sad0_bits               [0xe0000] 16 20 24

Quoted 21 lines

lib/igt_eld.c +6 -11

-static bool eld_entry_is_igt(const char *path)
+static bool eld_parse_entry(const char *path, struct eld_entry *eld)
 {
-	FILE *in;
+	FILE *f;
 	char buf[1024];
-	uint8_t eld_valid = 0;
-	uint8_t mon_valid = 0;
-
-	in = fopen(path, "r");
-	if (!in)
-		return false;
+	char *key, *value;
+	size_t len;
+	bool monitor_present;
 
-	memset(buf, 0, 1024);
+	memset(eld, 0, sizeof(*eld));
 
-	while ((fgets(buf, 1024, in)) != NULL) {
-		char *line = buf;

lib/igt_eld.c line 62

Quoted 10 lines

lib/igt_eld.c +5 -4

+	f = fopen(path, "r");
+	if (!f) {
+		igt_debug("Failed to open ELD file: %s\n", path);
+		return false;
+	}
 
-		if (!strncasecmp(line, "eld_valid", 9) &&
-				strstr(line, "1")) {
-			eld_valid++;
-		}

Good riddance! Ignore my comment from patch 1!

lib/igt_eld.c line 68

Quoted 7 lines

lib/igt_eld.c +7 -0

+	while ((fgets(buf, sizeof(buf), f)) != NULL) {
+		len = strlen(buf);
+		if (buf[len - 1] == '\n')
+			buf[len - 1] = '\0';
+
+		key = strtok(buf, ELD_DELIM);
+		value = strtok(NULL, "");

Hmm, I had forgotten that strok was stateful when str == NULL!

Quoted 7 lines

lib/igt_eld.c line 70

Quoted 2 lines

lib/igt_eld.c +2 -0

+		/* Skip whitespace at the beginning */
+		value += strspn(value, ELD_DELIM);

Thanks for teaching me a new function :) This is quite practical!

After doing more python work, going back to parsing with C is a little rough :D Not as bad as ASM though!

Eheh, I can imagine that :P

Quoted 63 lines

Quoted 63 lines

lib/igt_eld.c +30 -15

+
+		if (strcmp(key, "monitor_present") == 0)
+			monitor_present = strcmp(value, "1") == 0;
+		else if (strcmp(key, "eld_valid") == 0)
+			eld->valid = strcmp(value, "1") == 0;
+		else if (strcmp(key, "monitor_name") == 0)
+			snprintf(eld->monitor_name, sizeof(eld->monitor_name),
+				 "%s", value);
+	}
 
-		if (!strncasecmp(line, "monitor_name", 12) &&
-				strstr(line, "IGT")) {
-			mon_valid++;
-		}
+	if (ferror(f) != 0) {
+		igt_debug("Failed to read ELD file: %d\n", ferror(f));
+		return false;
 	}
 
-	fclose(in);
-	if (mon_valid && eld_valid)
-		return true;
+	fclose(f);
 
-	return false;
+	return monitor_present;
 }
 
 /** eld_has_igt: check whether ALSA has detected the audio-capable IGT EDID by

@@ -79,27 +93,35 @@ static bool eld_entry_is_igt(const char *path)
 bool eld_has_igt(void)
 {
 	DIR *dir;
-	struct dirent *snd_hda;
+	struct dirent *dirent;
 	int i;
+	char card[64];
+	char path[PATH_MAX];
+	struct eld_entry eld;
 
 	for (i = 0; i < 8; i++) {
-		char cards[128];
-
-		snprintf(cards, sizeof(cards), "/proc/asound/card%d", i);
-		dir = opendir(cards);
+		snprintf(card, sizeof(card), "/proc/asound/card%d", i);
+		dir = opendir(card);
 		if (!dir)
 			continue;
 
-		while ((snd_hda = readdir(dir))) {
-			char fpath[PATH_MAX];
+		while ((dirent = readdir(dir))) {
+			if (strncmp(dirent->d_name, ELD_PREFIX,
+				    strlen(ELD_PREFIX)) != 0)
+				continue;
+
+			snprintf(path, sizeof(path), "%s/%s", card,
+				 dirent->d_name);
+			if (!eld_parse_entry(path, &eld)) {
+				continue;
+			}
 

Quoted 7 lines

lib/igt_eld.c static bool eld_entry_is_igt(const char *path) · line 120

Quoted 4 lines

lib/igt_eld.c +2 -2

-			if (*snd_hda->d_name == '.' ||
-			    strstr(snd_hda->d_name, "eld") == 0)
+			if (!eld.valid) {
+				igt_debug("Skipping invalid ELD: %s\n", path);

The message is a little confusing. How about "Skipping non-ELD file:
%s\n" instead?

So, this is not exactly a non-ELD file: this happens when the EDID isn't valid (e.g. if some field is malformed).

Is it okay if I change it to: "Skipping ELD entry with invalid=1"?

Quoted 30 lines

Anyway, I like where this is going!

Reviewed-by Martin Peres <[email protected]>

Quoted 25 lines

lib/igt_eld.c +2 -3

 				continue;
+			}
 
-			snprintf(fpath, sizeof(fpath), "%s/%s", cards,
-				 snd_hda->d_name);
-			if (eld_entry_is_igt(fpath)) {
+			if (strcmp(eld.monitor_name, "IGT") == 0) {
 				closedir(dir);
 				return true;
 			}

lib/igt_eld.h +5 -0

index 27f876d9f631..21fe3537acf9 100644
@@ -30,6 +30,11 @@
 
 #include <stdbool.h>
 
+struct eld_entry {
+	bool valid;
+	char monitor_name[16];
+};
+
 bool eld_has_igt(void);
 

Quoted 2 lines

lib/igt_eld.h line 40

Quoted 2 lines

lib/igt_eld.h +0 -0

 #endif

Each valid ELD entry can contain zero, one or more Short Audio Descriptor blocks. These are exposed in sadN_* fields (N being the index of the SAD).

We need to parse them to be able to check that ALSA has properly processed them.

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

---
 lib/igt_edid.h |  2 ++
 lib/igt_eld.c  | 85 +++++++++++++++++++++++++++++++++++++++++++++++++-
 lib/igt_eld.h  | 14 +++++++++
 3 files changed, 100 insertions(+), 1 deletion(-)

lib/igt_edid.h +0 -0

index 7edd7e38f41e..1d0c6aa29578 100644

lib/igt_edid.h +0 -0

 struct est_timings {
 	uint8_t t1;
 	uint8_t t2;

lib/igt_eld.c +70 -0

index a198001a56d7..732bbabd2d7c 100644
@@ -35,6 +35,7 @@
 
 #define ELD_PREFIX "eld#"
 #define ELD_DELIM " \t"
+#define SAD_FMT "sad%d_%ms"
 
 /**
  * EDID-Like Data (ELD) is metadata parsed and exposed by ALSA for HDMI and

@@ -43,13 +44,87 @@
  * on).
  */
 
+static enum cea_sad_format parse_sad_coding_type(const char *value)
+{
+	if (strcmp(value, "LPCM") == 0)
+		return CEA_SAD_FORMAT_PCM;
+	else
+		return 0;
+}
+
+static enum cea_sad_sampling_rate parse_sad_rate(const char *value)
+{
+	switch (atoi(value)) {
+	case 32000:
+		return CEA_SAD_SAMPLING_RATE_32KHZ;
+	case 44100:
+		return CEA_SAD_SAMPLING_RATE_44KHZ;
+	case 48000:
+		return CEA_SAD_SAMPLING_RATE_48KHZ;
+	case 88000:
+		return CEA_SAD_SAMPLING_RATE_88KHZ;
+	case 96000:
+		return CEA_SAD_SAMPLING_RATE_96KHZ;
+	case 176000:
+		return CEA_SAD_SAMPLING_RATE_176KHZ;
+	case 192000:
+		return CEA_SAD_SAMPLING_RATE_192KHZ;
+	default:
+		return 0;
+	}
+}
+
+static enum cea_sad_pcm_sample_size parse_sad_bit(const char *value)
+{
+	switch (atoi(value)) {
+	case 16:
+		return CEA_SAD_SAMPLE_SIZE_16;
+	case 20:
+		return CEA_SAD_SAMPLE_SIZE_20;
+	case 24:
+		return CEA_SAD_SAMPLE_SIZE_24;
+	default:
+		return 0;
+	}
+}
+
+static void parse_sad_field(struct eld_sad *sad, const char *key, char *value)
+{
+	char *tok;
+
+	/* Some fields are prefixed with the raw hex value, strip it */
+	if (value[0] == '[') {
+		value = strchr(value, ' ');
+		igt_assert(value != NULL);
+		value++; /* skip the space */
+	}
+
+	/* Single-value fields */
+	if (strcmp(key, "coding_type") == 0)
+		sad->coding_type = parse_sad_coding_type(value);
+	else if (strcmp(key, "channels") == 0)
+		sad->channels = atoi(value);
+
+	/* Multiple-value fields */
+	tok = strtok(value, " ");
+	while (tok) {
+		if (strcmp(key, "rates") == 0)
+			sad->rates |= parse_sad_rate(tok);
+		else if (strcmp(key, "bits") == 0)
+			sad->bits |= parse_sad_bit(tok);
+

lib/igt_eld.c +1 -0

 	size_t len;
 	bool monitor_present;
+	int sad_index;
 
 	memset(eld, 0, sizeof(*eld));
 

lib/igt_eld.c +3 -0

+			parse_sad_field(&eld->sads[sad_index], sad_key, value);
+			free(sad_key);
+		}
 	}
 
 	if (ferror(f) != 0) {

lib/igt_eld.h +10 -0

index 21fe3537acf9..e16187884d4b 100644
@@ -30,9 +30,23 @@
 
 #include <stdbool.h>
 
+#include "igt_edid.h"
+
+#define ELD_SADS_CAP 4
+
+/** eld_sad: Short Audio Descriptor */
+struct eld_sad {
+	enum cea_sad_format coding_type;
+	int channels;
+	unsigned int rates; /* enum cea_sad_sampling_rate */
+	unsigned int bits; /* enum cea_sad_pcm_sample_size */

Quoted 19 lines · Simon Ser

Each valid ELD entry can contain zero, one or more Short Audio Descriptor blocks. These are exposed in sadN_* fields (N being the index of the SAD).

We need to parse them to be able to check that ALSA has properly processed them.

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

---
 lib/igt_edid.h |  2 ++
 lib/igt_eld.c  | 85 +++++++++++++++++++++++++++++++++++++++++++++++++-
 lib/igt_eld.h  | 14 +++++++++
 3 files changed, 100 insertions(+), 1 deletion(-)

lib/igt_edid.h +0 -0

index 7edd7e38f41e..1d0c6aa29578 100644
@@ -30,6 +30,8 @@
 

lib/igt_edid.h line 34

Quoted 4 lines

lib/igt_edid.h +2 -0

 #include <stdint.h>
 
+#include <xf86drmMode.h>
+

Are you sure this change belongs to this commit? Can't find anything here that would need this header.

Quoted 92 lines

lib/igt_edid.h +0 -0

 struct est_timings {
 	uint8_t t1;
 	uint8_t t2;

lib/igt_eld.c +74 -0

index a198001a56d7..732bbabd2d7c 100644
@@ -35,6 +35,7 @@
 
 #define ELD_PREFIX "eld#"
 #define ELD_DELIM " \t"
+#define SAD_FMT "sad%d_%ms"
 
 /**
  * EDID-Like Data (ELD) is metadata parsed and exposed by ALSA for HDMI and

@@ -43,13 +44,87 @@
  * on).
  */
 
+static enum cea_sad_format parse_sad_coding_type(const char *value)
+{
+	if (strcmp(value, "LPCM") == 0)
+		return CEA_SAD_FORMAT_PCM;
+	else
+		return 0;
+}
+
+static enum cea_sad_sampling_rate parse_sad_rate(const char *value)
+{
+	switch (atoi(value)) {
+	case 32000:
+		return CEA_SAD_SAMPLING_RATE_32KHZ;
+	case 44100:
+		return CEA_SAD_SAMPLING_RATE_44KHZ;
+	case 48000:
+		return CEA_SAD_SAMPLING_RATE_48KHZ;
+	case 88000:
+		return CEA_SAD_SAMPLING_RATE_88KHZ;
+	case 96000:
+		return CEA_SAD_SAMPLING_RATE_96KHZ;
+	case 176000:
+		return CEA_SAD_SAMPLING_RATE_176KHZ;
+	case 192000:
+		return CEA_SAD_SAMPLING_RATE_192KHZ;
+	default:
+		return 0;
+	}
+}
+
+static enum cea_sad_pcm_sample_size parse_sad_bit(const char *value)
+{
+	switch (atoi(value)) {
+	case 16:
+		return CEA_SAD_SAMPLE_SIZE_16;
+	case 20:
+		return CEA_SAD_SAMPLE_SIZE_20;
+	case 24:
+		return CEA_SAD_SAMPLE_SIZE_24;
+	default:
+		return 0;
+	}
+}
+
+static void parse_sad_field(struct eld_sad *sad, const char *key, char *value)
+{
+	char *tok;
+
+	/* Some fields are prefixed with the raw hex value, strip it */
+	if (value[0] == '[') {
+		value = strchr(value, ' ');
+		igt_assert(value != NULL);
+		value++; /* skip the space */
+	}
+
+	/* Single-value fields */
+	if (strcmp(key, "coding_type") == 0)
+		sad->coding_type = parse_sad_coding_type(value);
+	else if (strcmp(key, "channels") == 0)
+		sad->channels = atoi(value);
+
+	/* Multiple-value fields */
+	tok = strtok(value, " ");
+	while (tok) {
+		if (strcmp(key, "rates") == 0)
+			sad->rates |= parse_sad_rate(tok);
+		else if (strcmp(key, "bits") == 0)
+			sad->bits |= parse_sad_bit(tok);
+
+		tok = strtok(NULL, " ");
+	}
+}
+

lib/igt_eld.c line 124

Quoted 6 lines

lib/igt_eld.c +1 -1

 static bool eld_parse_entry(const char *path, struct eld_entry *eld)
 {
 	FILE *f;
 	char buf[1024];
-	char *key, *value;
+	char *key, *value, *sad_key;

Poor key, why are you sad? :D

More seriously, a little documentation in this file about the acronyms would be appreciated!

Quoted 7 lines

lib/igt_eld.c +1 -0

 	size_t len;
 	bool monitor_present;
+	int sad_index;
 
 	memset(eld, 0, sizeof(*eld));
 

lib/igt_eld.c static bool eld_parse_entry(const char *path, struct eld_entry *eld) · line 158

Quoted 8 lines

lib/igt_eld.c +5 -0

@@ -76,6 +151,14 @@ static bool eld_parse_entry(const char *path, struct eld_entry *eld)
 		else if (strcmp(key, "monitor_name") == 0)
 			snprintf(eld->monitor_name, sizeof(eld->monitor_name),
 				 "%s", value);
+		else if (strcmp(key, "sad_count") == 0)
+			eld->sads_len = atoi(value);
+		else if (sscanf(key, "sad%d_%ms", &sad_index, &sad_key) == 2) {
+			igt_assert(sad_index < ELD_SADS_CAP);
+			igt_assert(sad_index < eld->sads_len);

Good that you added that! This patch looks good:

Reviewed-by Martin Peres <[email protected]>

Quoted 33 lines

lib/igt_eld.c +3 -0

+			parse_sad_field(&eld->sads[sad_index], sad_key, value);
+			free(sad_key);
+		}
 	}
 
 	if (ferror(f) != 0) {

lib/igt_eld.h +14 -0

index 21fe3537acf9..e16187884d4b 100644
@@ -30,9 +30,23 @@
 
 #include <stdbool.h>
 
+#include "igt_edid.h"
+
+#define ELD_SADS_CAP 4
+
+/** eld_sad: Short Audio Descriptor */
+struct eld_sad {
+	enum cea_sad_format coding_type;
+	int channels;
+	unsigned int rates; /* enum cea_sad_sampling_rate */
+	unsigned int bits; /* enum cea_sad_pcm_sample_size */
+};
+
 struct eld_entry {
 	bool valid;
 	char monitor_name[16];
+	size_t sads_len;
+	struct eld_sad sads[ELD_SADS_CAP];
 };
 

lib/igt_eld.h line 52

Quoted 2 lines

lib/igt_eld.h +0 -0

 bool eld_has_igt(void);

_______________________________________________
igt-dev mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/igt-dev

The previous function eld_has_igt (1) assumed the monitor name was always "IGT" and (2) didn't provide a way to access the ELD data.

(1) is an issue for EDIDs that don't have "IGT" as the monitor name. This is the case for the Chamelium default EDID, but this will also become an issue with MST where each monitor will need to have a unique name (to be able to tell them apart).

(2) makes it impossible to check ELD audio parameters.

This commit fixes both (1) and (2).

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

---
 lib/igt_eld.c           | 12 +++++-------
 lib/igt_eld.h           |  2 +-
 tests/kms_hdmi_inject.c |  8 +++-----
 3 files changed, 9 insertions(+), 13 deletions(-)

lib/igt_eld.c +5 -7

index 732bbabd2d7c..53655c5e18f1 100644
@@ -171,16 +171,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_from_monitor_name: retrieve an ELD entry from a monitor name */
+bool eld_from_monitor_name(struct eld_entry *eld, const char *monitor_name)
 {
 	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);

@@ -195,16 +193,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, monitor_name) == 0) {
 				closedir(dir);
 				return true;
 			}

lib/igt_eld.h +1 -1

index e16187884d4b..1f34f784749b 100644
@@ -49,6 +49,6 @@ struct eld_entry {
 	struct eld_sad sads[ELD_SADS_CAP];
 };
 
-bool eld_has_igt(void);
+bool eld_from_monitor_name(struct eld_entry *eld, const char *monitor_name);
 
 #endif

tests/kms_hdmi_inject.c +1 -0

index eba25046cead..31ddda731d7b 100644
@@ -146,6 +146,7 @@ hdmi_inject_audio(int drm_fd, drmModeConnector *connector)
 	int fb_id, cid, ret, crtc_mask = -1;
 	struct igt_fb fb;
 	struct kmstest_connector_config config;
+	struct eld_entry eld;
 
 	kmstest_edid_add_audio(igt_kms_get_base_edid(), EDID_LENGTH, &edid,
 			       &length);

@@ -178,11 +179,8 @@ hdmi_inject_audio(int drm_fd, drmModeConnector *connector)
 
 	igt_assert(ret == 0);
 

Quoted 7 lines · Simon Ser

The previous function eld_has_igt (1) assumed the monitor name was always "IGT" and (2) didn't provide a way to access the ELD data.

(1) is an issue for EDIDs that don't have "IGT" as the monitor name. This is the case for the Chamelium default EDID, but this will also become an issue with MST where each monitor will need to have a unique name (to be able to tell them apart).

I dislike the idea of not checking that the EDID has been generated by IGT, but I understand that you might want some freedom there.

How about checking that the monitor name STARTS with IGT.

Given that Arek is also complaining about something like in patch 5, how about merging 1-4, then reviewing/merging your DP patches, then re-spinning the last two patches of this series?

Martin

Quoted 90 lines

(2) makes it impossible to check ELD audio parameters.

This commit fixes both (1) and (2).

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

---
 lib/igt_eld.c           | 12 +++++-------
 lib/igt_eld.h           |  2 +-
 tests/kms_hdmi_inject.c |  8 +++-----
 3 files changed, 9 insertions(+), 13 deletions(-)

lib/igt_eld.c +5 -7

index 732bbabd2d7c..53655c5e18f1 100644
@@ -171,16 +171,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_from_monitor_name: retrieve an ELD entry from a monitor name */
+bool eld_from_monitor_name(struct eld_entry *eld, const char *monitor_name)
 {
 	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);

@@ -195,16 +193,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, monitor_name) == 0) {
 				closedir(dir);
 				return true;
 			}

lib/igt_eld.h +1 -1

index e16187884d4b..1f34f784749b 100644
@@ -49,6 +49,6 @@ struct eld_entry {
 	struct eld_sad sads[ELD_SADS_CAP];
 };
 
-bool eld_has_igt(void);
+bool eld_from_monitor_name(struct eld_entry *eld, const char *monitor_name);
 
 #endif

tests/kms_hdmi_inject.c +3 -5

index eba25046cead..31ddda731d7b 100644
@@ -146,6 +146,7 @@ hdmi_inject_audio(int drm_fd, drmModeConnector *connector)
 	int fb_id, cid, ret, crtc_mask = -1;
 	struct igt_fb fb;
 	struct kmstest_connector_config config;
+	struct eld_entry eld;
 
 	kmstest_edid_add_audio(igt_kms_get_base_edid(), EDID_LENGTH, &edid,
 			       &length);

@@ -178,11 +179,8 @@ hdmi_inject_audio(int drm_fd, drmModeConnector *connector)
 
 	igt_assert(ret == 0);
 
-	/*
-	 * Test if we have /proc/asound/HDMI/eld#0.0 and is its contents are
-	 * valid.
-	 */
-	igt_assert(eld_has_igt());
+	/* Check whether ALSA has properly detected the audio-capable EDID */
+	igt_assert(eld_from_monitor_name(&eld, "IGT"));
 

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

Quoted 3 lines

tests/kms_hdmi_inject.c +0 -0

 	igt_remove_fb(drm_fd, &fb);
 

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.

Remove some high sampling rates from get_hdmi_audio_edid, since the Chamelium doesn't support them anyway. This makes the SAD check uniform for all ports.

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

---
 tests/kms_chamelium.c | 69 +++++++++++++++++++++++++++++++++++++++----
 1 file changed, 64 insertions(+), 5 deletions(-)

tests/kms_chamelium.c +29 -0

index 49c3de0b7beb..16d3a773bb2a 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>

@@ -1426,6 +1427,60 @@ 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;
+	const char *monitor_name;
+	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,

tests/kms_chamelium.c +25 -5

+
+	igt_assert(eld_from_monitor_name(&eld, monitor_name));
+	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));
+
+	igt_remove_fb(data->drm_fd, &fb);
+
+	drmModeFreeConnector(connector);
+}
 
 static void randomize_plane_stride(data_t *data,
 				   uint32_t width, uint32_t height,

@@ -2005,11 +2060,7 @@ static unsigned const char *get_hdmi_audio_edid(void)
 	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;
+			 CEA_SAD_SAMPLING_RATE_48KHZ;
 	sample_sizes = CEA_SAD_SAMPLE_SIZE_16 |
 		       CEA_SAD_SAMPLE_SIZE_20 |
 		       CEA_SAD_SAMPLE_SIZE_24;

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

@@ -2325,6 +2380,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);
 	}
 
 	igt_subtest_group {

Signature · 1 line

2.21.0

Quoted 55 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.

Remove some high sampling rates from get_hdmi_audio_edid, since the Chamelium doesn't support them anyway. This makes the SAD check uniform for all ports.

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

---
 tests/kms_chamelium.c | 69 +++++++++++++++++++++++++++++++++++++++----
 1 file changed, 64 insertions(+), 5 deletions(-)

tests/kms_chamelium.c +29 -0

index 49c3de0b7beb..16d3a773bb2a 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>

@@ -1426,6 +1427,60 @@ 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;
+	const char *monitor_name;
+	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,

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

Quoted 10 lines

tests/kms_chamelium.c +10 -0

+					    DRM_FORMAT_XRGB8888,
+					    LOCAL_DRM_FORMAT_MOD_NONE,
+					    0, 0, 0, &fb);
+	igt_assert(fb_id > 0);
+
+	enable_output(data, port, output, mode, &fb);
+
+	monitor_name = "IGT";
+	if (edid == TEST_EDID_CHAMELIUM_DEFAULT)
+		monitor_name = "AthenaDP";

I am starting to grow less and less fond of using the default Chamelium's EDID. This will just make us build up on simillar checks over time. Let's just define our own copy and then use values from it directly.

"monitor_name" should probably be named manufacturer_id and be obtained by chamelium_get_manufacturer_id(edid_id) or smth.

And since you were talking about MST in one of the other patches, let's talk about MST :-) This would make it more future proof (WIP was defining a copy of EDID for each port using manufacturer id in form of "I-$CHAM_PORT_ID"). chamelium_get_manufacturer_id() can be extended later to also take chamelium_port as parameter.

Other than that the series looks good.

Quoted 53 lines

tests/kms_chamelium.c +25 -5

+
+	igt_assert(eld_from_monitor_name(&eld, monitor_name));
+	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));
+
+	igt_remove_fb(data->drm_fd, &fb);
+
+	drmModeFreeConnector(connector);
+}
 
 static void randomize_plane_stride(data_t *data,
 				   uint32_t width, uint32_t height,

@@ -2005,11 +2060,7 @@ static unsigned const char *get_hdmi_audio_edid(void)
 	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;
+			 CEA_SAD_SAMPLING_RATE_48KHZ;
 	sample_sizes = CEA_SAD_SAMPLE_SIZE_16 |
 		       CEA_SAD_SAMPLE_SIZE_20 |
 		       CEA_SAD_SAMPLE_SIZE_24;

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

@@ -2325,6 +2380,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 4 lines

tests/kms_chamelium.c +0 -0

 	igt_subtest_group {

Signature · 1 line

2.21.0

_______________________________________________
igt-dev mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Quoted 62 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.

Remove some high sampling rates from get_hdmi_audio_edid, since the Chamelium doesn't support them anyway. This makes the SAD check uniform for all ports.

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

---
 tests/kms_chamelium.c | 69 +++++++++++++++++++++++++++++++++++++++----
 1 file changed, 64 insertions(+), 5 deletions(-)

tests/kms_chamelium.c +36 -0

index 49c3de0b7beb..16d3a773bb2a 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>

@@ -1426,6 +1427,60 @@ 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;
+	const char *monitor_name;
+	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 1467

Quoted 3 lines

tests/kms_chamelium.c +3 -0

+	monitor_name = "IGT";
+	if (edid == TEST_EDID_CHAMELIUM_DEFAULT)
+		monitor_name = "AthenaDP";

I am starting to grow less and less fond of using the default Chamelium's EDID. This will just make us build up on simillar checks over time. Let's just define our own copy and then use values from it directly.

"monitor_name" should probably be named manufacturer_id and be obtained by chamelium_get_manufacturer_id(edid_id) or smth.

And since you were talking about MST in one of the other patches, let's talk about MST :-) This would make it more future proof (WIP was defining a copy of EDID for each port using manufacturer id in form of "I-$CHAM_PORT_ID"). chamelium_get_manufacturer_id() can be extended later to also take chamelium_port as parameter.

Other than that the series looks good.

Quoted 53 lines

tests/kms_chamelium.c +25 -5

+
+	igt_assert(eld_from_monitor_name(&eld, monitor_name));
+	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));
+
+	igt_remove_fb(data->drm_fd, &fb);
+
+	drmModeFreeConnector(connector);
+}
 
 static void randomize_plane_stride(data_t *data,
 				   uint32_t width, uint32_t height,

@@ -2005,11 +2060,7 @@ static unsigned const char *get_hdmi_audio_edid(void)
 	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;
+			 CEA_SAD_SAMPLING_RATE_48KHZ;
 	sample_sizes = CEA_SAD_SAMPLE_SIZE_16 |
 		       CEA_SAD_SAMPLE_SIZE_20 |
 		       CEA_SAD_SAMPLE_SIZE_24;

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

@@ -2325,6 +2380,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 4 lines

tests/kms_chamelium.c +0 -0

 	igt_subtest_group {

Signature · 1 line

2.21.0

Quoted 55 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.

Remove some high sampling rates from get_hdmi_audio_edid, since the Chamelium doesn't support them anyway. This makes the SAD check uniform for all ports.

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

---
 tests/kms_chamelium.c | 69 +++++++++++++++++++++++++++++++++++++++----
 1 file changed, 64 insertions(+), 5 deletions(-)

tests/kms_chamelium.c +29 -0

index 49c3de0b7beb..16d3a773bb2a 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>

@@ -1426,6 +1427,60 @@ 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;
+	const char *monitor_name;
+	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,

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

Quoted 10 lines

tests/kms_chamelium.c +10 -0

+					    DRM_FORMAT_XRGB8888,
+					    LOCAL_DRM_FORMAT_MOD_NONE,
+					    0, 0, 0, &fb);
+	igt_assert(fb_id > 0);
+
+	enable_output(data, port, output, mode, &fb);
+
+	monitor_name = "IGT";
+	if (edid == TEST_EDID_CHAMELIUM_DEFAULT)
+		monitor_name = "AthenaDP";

I am starting to grow less and less fond of using the default Chamelium's EDID. This will just make us build up on simillar checks over time. Let's just define our own copy and then use values from it directly.

"monitor_name" should probably be named manufacturer_id and be obtained by chamelium_get_manufacturer_id(edid_id) or smth.

And since you were talking about MST in one of the other patches, let's talk about MST :-) This would make it more future proof (WIP was defining a copy of EDID for each port using manufacturer id in form of "I-$CHAM_PORT_ID"). chamelium_get_manufacturer_id() can be extended later to also take chamelium_port as parameter.

Other than that the series looks good.

Quoted 53 lines

tests/kms_chamelium.c +25 -5

+
+	igt_assert(eld_from_monitor_name(&eld, monitor_name));
+	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));
+
+	igt_remove_fb(data->drm_fd, &fb);
+
+	drmModeFreeConnector(connector);
+}
 
 static void randomize_plane_stride(data_t *data,
 				   uint32_t width, uint32_t height,

@@ -2005,11 +2060,7 @@ static unsigned const char *get_hdmi_audio_edid(void)
 	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;
+			 CEA_SAD_SAMPLING_RATE_48KHZ;
 	sample_sizes = CEA_SAD_SAMPLE_SIZE_16 |
 		       CEA_SAD_SAMPLE_SIZE_20 |
 		       CEA_SAD_SAMPLE_SIZE_24;

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

@@ -2325,6 +2380,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 4 lines

tests/kms_chamelium.c +0 -0

 	igt_subtest_group {

Signature · 1 line

2.21.0

== Series Details ==

Series: Add audio EDID tests to Chamelium
URL : https://patchwork.freedesktop.org/series/61424/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_6175_full -> IGTPW_3090_full
====================================================

Summary
-------

**FAILURE**

Serious unknown changes coming with IGTPW_3090_full absolutely need to be verified manually.

If you think the reported changes have nothing to do with the changes introduced in IGTPW_3090_full, please notify your bug team to allow them to document this new failure mode, which will reduce false positives in CI.

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

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

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

### IGT changes ###

#### Possible regressions ####

* igt@runner@aborted:
- shard-kbl: NOTRUN -> [FAIL][1]
[1]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3090/shard-kbl1/igt@[email protected]

New tests
---------

New tests have been introduced between CI_DRM_6175_full and IGTPW_3090_full:

### New IGT tests (2) ###

* igt@kms_chamelium@dp-audio-edid:
- Statuses : 6 skip(s)
- Exec time: [0.0] s

* igt@kms_chamelium@hdmi-audio-edid:
- Statuses : 6 skip(s)
- Exec time: [0.0] s

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

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

### IGT changes ###

#### Issues hit ####

* igt@gem_linear_blits@interruptible:
- shard-glk: [PASS][2] -> [INCOMPLETE][3] ([fdo#103359] / [k.org#198133])
[2]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6175/shard-glk1/igt@[email protected]
[3]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3090/shard-glk1/igt@[email protected]
- shard-apl: [PASS][4] -> [INCOMPLETE][5] ([fdo#103927])
[4]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6175/shard-apl1/igt@[email protected]
[5]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3090/shard-apl1/igt@[email protected]

* igt@i915_suspend@debugfs-reader:
- shard-apl: [PASS][6] -> [DMESG-WARN][7] ([fdo#108566]) +7 similar issues
[6]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6175/shard-apl7/igt@[email protected]
[7]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3090/shard-apl4/igt@[email protected]

* igt@i915_suspend@sysfs-reader:
- shard-kbl: [PASS][8] -> [DMESG-WARN][9] ([fdo#108566])
[8]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6175/shard-kbl3/igt@[email protected]
[9]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3090/shard-kbl2/igt@[email protected]

* igt@kms_cursor_crc@pipe-a-cursor-64x64-offscreen:
- shard-hsw: [PASS][10] -> [INCOMPLETE][11] ([fdo#103540])
[10]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6175/shard-hsw4/igt@[email protected]
[11]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3090/shard-hsw2/igt@[email protected]

* igt@kms_cursor_legacy@2x-flip-vs-cursor-atomic:
- shard-glk: [PASS][12] -> [FAIL][13] ([fdo#104873])
[12]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6175/shard-glk9/igt@[email protected]
[13]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3090/shard-glk1/igt@[email protected]

* igt@kms_cursor_legacy@flip-vs-cursor-legacy:
- shard-apl: [PASS][14] -> [FAIL][15] ([fdo#102670])
[14]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6175/shard-apl2/igt@[email protected]
[15]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3090/shard-apl1/igt@[email protected]

* igt@kms_frontbuffer_tracking@fbc-1p-offscren-pri-shrfb-draw-blt:
- shard-iclb: [PASS][16] -> [FAIL][17] ([fdo#103167]) +5 similar issues
[16]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6175/shard-iclb7/igt@[email protected]
[17]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3090/shard-iclb1/igt@[email protected]

* igt@kms_plane_lowres@pipe-a-tiling-y:
- shard-iclb: [PASS][18] -> [FAIL][19] ([fdo#103166])
[18]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6175/shard-iclb3/igt@[email protected]
[19]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3090/shard-iclb6/igt@[email protected]

* igt@kms_psr@psr2_sprite_plane_move:
- shard-iclb: [PASS][20] -> [SKIP][21] ([fdo#109441]) +1 similar issue
[20]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6175/shard-iclb2/igt@kms_psr@psr2_sprite_plane_move.html
[21]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3090/shard-iclb5/igt@kms_psr@psr2_sprite_plane_move.html

#### Possible fixes ####

* igt@gem_eio@in-flight-suspend:
- shard-glk: [FAIL][22] ([fdo#110667]) -> [PASS][23]
[22]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6175/shard-glk5/igt@[email protected]
[23]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3090/shard-glk6/igt@[email protected]

* igt@gem_mmap_gtt@forked-medium-copy-xy:
- shard-iclb: [INCOMPLETE][24] ([fdo#107713] / [fdo#109100]) -> [PASS][25] +1 similar issue
[24]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6175/shard-iclb3/igt@[email protected]
[25]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3090/shard-iclb4/igt@[email protected]

* igt@gem_persistent_relocs@forked:
- shard-iclb: [INCOMPLETE][26] ([fdo#107713]) -> [PASS][27]
[26]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6175/shard-iclb7/igt@[email protected]
[27]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3090/shard-iclb5/igt@[email protected]

* igt@gem_softpin@softpin:
- shard-glk: [INCOMPLETE][28] ([fdo#103359] / [k.org#198133]) -> [PASS][29] +1 similar issue
[28]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6175/shard-glk4/igt@[email protected]
[29]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3090/shard-glk5/igt@[email protected]

* igt@gem_tiled_swapping@non-threaded:
- shard-kbl: [FAIL][30] ([fdo#108686]) -> [PASS][31]
[30]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6175/shard-kbl7/igt@[email protected]
[31]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3090/shard-kbl1/igt@[email protected]

* igt@i915_suspend@fence-restore-untiled:
- shard-apl: [DMESG-WARN][32] ([fdo#108566]) -> [PASS][33] +5 similar issues
[32]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6175/shard-apl7/igt@[email protected]
[33]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3090/shard-apl2/igt@[email protected]

* igt@kms_frontbuffer_tracking@fbc-rgb565-draw-pwrite:
- shard-iclb: [FAIL][34] ([fdo#103167]) -> [PASS][35] +3 similar issues
[34]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6175/shard-iclb1/igt@[email protected]
[35]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3090/shard-iclb5/igt@[email protected]

* igt@kms_plane_lowres@pipe-a-tiling-x:
- shard-iclb: [FAIL][36] ([fdo#103166]) -> [PASS][37]
[36]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6175/shard-iclb6/igt@[email protected]
[37]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3090/shard-iclb5/igt@[email protected]

* igt@kms_psr@psr2_cursor_mmap_cpu:
- shard-iclb: [SKIP][38] ([fdo#109441]) -> [PASS][39] +3 similar issues
[38]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6175/shard-iclb3/igt@kms_psr@psr2_cursor_mmap_cpu.html
[39]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3090/shard-iclb2/igt@kms_psr@psr2_cursor_mmap_cpu.html

* igt@perf_pmu@rc6-runtime-pm-long:
- shard-glk: [FAIL][40] ([fdo#105010]) -> [PASS][41]
[40]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6175/shard-glk1/igt@[email protected]
[41]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3090/shard-glk5/igt@[email protected]

[fdo#102670]: https://bugs.freedesktop.org/show_bug.cgi?id=102670
[fdo#103166]: https://bugs.freedesktop.org/show_bug.cgi?id=103166
[fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
[fdo#103359]: https://bugs.freedesktop.org/show_bug.cgi?id=103359
[fdo#103540]: https://bugs.freedesktop.org/show_bug.cgi?id=103540
[fdo#103927]: https://bugs.freedesktop.org/show_bug.cgi?id=103927
[fdo#104873]: https://bugs.freedesktop.org/show_bug.cgi?id=104873
[fdo#105010]: https://bugs.freedesktop.org/show_bug.cgi?id=105010
[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#109441]: https://bugs.freedesktop.org/show_bug.cgi?id=109441
[fdo#110667]: https://bugs.freedesktop.org/show_bug.cgi?id=110667
[k.org#198133]: https://bugzilla.kernel.org/show_bug.cgi?id=198133

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

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

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

* IGT: IGT_5026 -> IGTPW_3090
* Piglit: piglit_4509 -> None

CI_DRM_6175: 0ebe1e37a70176b84d4e2d489f03c3533e0fcd1a @ git://anongit.freedesktop.org/gfx-ci/linux
IGTPW_3090: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3090/
IGT_5026: 4108c74c3b15460de25ab989f4e2031594559dfc @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3090/

Quoted 32 lines · Patchwork

== Series Details ==

Series: Add audio EDID tests to Chamelium
URL : https://patchwork.freedesktop.org/series/61424/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_6175_full -> IGTPW_3090_full
====================================================

Summary
-------

**FAILURE**

Serious unknown changes coming with IGTPW_3090_full absolutely need to be verified manually.

If you think the reported changes have nothing to do with the changes introduced in IGTPW_3090_full, please notify your bug team to allow them to document this new failure mode, which will reduce false positives in CI.

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

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

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

### IGT changes ###

Quoted 5 lines

#### Possible regressions ####

* igt@runner@aborted:
- shard-kbl: NOTRUN -> [FAIL][1]
[1]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3090/shard-kbl1/igt@[email protected]

Cc Martin

Quoted 159 lines

New tests
---------

New tests have been introduced between CI_DRM_6175_full and IGTPW_3090_full:

### New IGT tests (2) ###

* igt@kms_chamelium@dp-audio-edid:
- Statuses : 6 skip(s)
- Exec time: [0.0] s

* igt@kms_chamelium@hdmi-audio-edid:
- Statuses : 6 skip(s)
- Exec time: [0.0] s

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

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

### IGT changes ###

#### Issues hit ####

* igt@gem_linear_blits@interruptible:
- shard-glk: [PASS][2] -> [INCOMPLETE][3] ([fdo#103359] / [k.org#198133])
[2]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6175/shard-glk1/igt@[email protected]
[3]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3090/shard-glk1/igt@[email protected]
- shard-apl: [PASS][4] -> [INCOMPLETE][5] ([fdo#103927])
[4]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6175/shard-apl1/igt@[email protected]
[5]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3090/shard-apl1/igt@[email protected]

* igt@i915_suspend@debugfs-reader:
- shard-apl: [PASS][6] -> [DMESG-WARN][7] ([fdo#108566]) +7 similar issues
[6]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6175/shard-apl7/igt@[email protected]
[7]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3090/shard-apl4/igt@[email protected]

* igt@i915_suspend@sysfs-reader:
- shard-kbl: [PASS][8] -> [DMESG-WARN][9] ([fdo#108566])
[8]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6175/shard-kbl3/igt@[email protected]
[9]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3090/shard-kbl2/igt@[email protected]

* igt@kms_cursor_crc@pipe-a-cursor-64x64-offscreen:
- shard-hsw: [PASS][10] -> [INCOMPLETE][11] ([fdo#103540])
[10]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6175/shard-hsw4/igt@[email protected]
[11]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3090/shard-hsw2/igt@[email protected]

* igt@kms_cursor_legacy@2x-flip-vs-cursor-atomic:
- shard-glk: [PASS][12] -> [FAIL][13] ([fdo#104873])
[12]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6175/shard-glk9/igt@[email protected]
[13]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3090/shard-glk1/igt@[email protected]

* igt@kms_cursor_legacy@flip-vs-cursor-legacy:
- shard-apl: [PASS][14] -> [FAIL][15] ([fdo#102670])
[14]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6175/shard-apl2/igt@[email protected]
[15]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3090/shard-apl1/igt@[email protected]

* igt@kms_frontbuffer_tracking@fbc-1p-offscren-pri-shrfb-draw-blt:
- shard-iclb: [PASS][16] -> [FAIL][17] ([fdo#103167]) +5 similar issues
[16]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6175/shard-iclb7/igt@[email protected]
[17]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3090/shard-iclb1/igt@[email protected]

* igt@kms_plane_lowres@pipe-a-tiling-y:
- shard-iclb: [PASS][18] -> [FAIL][19] ([fdo#103166])
[18]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6175/shard-iclb3/igt@[email protected]
[19]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3090/shard-iclb6/igt@[email protected]

* igt@kms_psr@psr2_sprite_plane_move:
- shard-iclb: [PASS][20] -> [SKIP][21] ([fdo#109441]) +1 similar issue
[20]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6175/shard-iclb2/igt@kms_psr@psr2_sprite_plane_move.html
[21]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3090/shard-iclb5/igt@kms_psr@psr2_sprite_plane_move.html

#### Possible fixes ####

* igt@gem_eio@in-flight-suspend:
- shard-glk: [FAIL][22] ([fdo#110667]) -> [PASS][23]
[22]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6175/shard-glk5/igt@[email protected]
[23]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3090/shard-glk6/igt@[email protected]

* igt@gem_mmap_gtt@forked-medium-copy-xy:
- shard-iclb: [INCOMPLETE][24] ([fdo#107713] / [fdo#109100]) -> [PASS][25] +1 similar issue
[24]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6175/shard-iclb3/igt@[email protected]
[25]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3090/shard-iclb4/igt@[email protected]

* igt@gem_persistent_relocs@forked:
- shard-iclb: [INCOMPLETE][26] ([fdo#107713]) -> [PASS][27]
[26]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6175/shard-iclb7/igt@[email protected]
[27]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3090/shard-iclb5/igt@[email protected]

* igt@gem_softpin@softpin:
- shard-glk: [INCOMPLETE][28] ([fdo#103359] / [k.org#198133]) -> [PASS][29] +1 similar issue
[28]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6175/shard-glk4/igt@[email protected]
[29]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3090/shard-glk5/igt@[email protected]

* igt@gem_tiled_swapping@non-threaded:
- shard-kbl: [FAIL][30] ([fdo#108686]) -> [PASS][31]
[30]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6175/shard-kbl7/igt@[email protected]
[31]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3090/shard-kbl1/igt@[email protected]

* igt@i915_suspend@fence-restore-untiled:
- shard-apl: [DMESG-WARN][32] ([fdo#108566]) -> [PASS][33] +5 similar issues
[32]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6175/shard-apl7/igt@[email protected]
[33]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3090/shard-apl2/igt@[email protected]

* igt@kms_frontbuffer_tracking@fbc-rgb565-draw-pwrite:
- shard-iclb: [FAIL][34] ([fdo#103167]) -> [PASS][35] +3 similar issues
[34]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6175/shard-iclb1/igt@[email protected]
[35]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3090/shard-iclb5/igt@[email protected]

* igt@kms_plane_lowres@pipe-a-tiling-x:
- shard-iclb: [FAIL][36] ([fdo#103166]) -> [PASS][37]
[36]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6175/shard-iclb6/igt@[email protected]
[37]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3090/shard-iclb5/igt@[email protected]

* igt@kms_psr@psr2_cursor_mmap_cpu:
- shard-iclb: [SKIP][38] ([fdo#109441]) -> [PASS][39] +3 similar issues
[38]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6175/shard-iclb3/igt@kms_psr@psr2_cursor_mmap_cpu.html
[39]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3090/shard-iclb2/igt@kms_psr@psr2_cursor_mmap_cpu.html

* igt@perf_pmu@rc6-runtime-pm-long:
- shard-glk: [FAIL][40] ([fdo#105010]) -> [PASS][41]
[40]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6175/shard-glk1/igt@[email protected]
[41]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3090/shard-glk5/igt@[email protected]

[fdo#102670]: https://bugs.freedesktop.org/show_bug.cgi?id=102670
[fdo#103166]: https://bugs.freedesktop.org/show_bug.cgi?id=103166
[fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
[fdo#103359]: https://bugs.freedesktop.org/show_bug.cgi?id=103359
[fdo#103540]: https://bugs.freedesktop.org/show_bug.cgi?id=103540
[fdo#103927]: https://bugs.freedesktop.org/show_bug.cgi?id=103927
[fdo#104873]: https://bugs.freedesktop.org/show_bug.cgi?id=104873
[fdo#105010]: https://bugs.freedesktop.org/show_bug.cgi?id=105010
[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#109441]: https://bugs.freedesktop.org/show_bug.cgi?id=109441
[fdo#110667]: https://bugs.freedesktop.org/show_bug.cgi?id=110667
[k.org#198133]: https://bugzilla.kernel.org/show_bug.cgi?id=198133

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

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

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

* IGT: IGT_5026 -> IGTPW_3090
* Piglit: piglit_4509 -> None

Quoted 8 lines

CI_DRM_6175: 0ebe1e37a70176b84d4e2d489f03c3533e0fcd1a @ git://anongit.freedesktop.org/gfx-ci/linux
IGTPW_3090: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3090/
IGT_5026: 4108c74c3b15460de25ab989f4e2031594559dfc @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3090/