[igt-dev] [PATCH i-g-t 0/5] Add audio EDID tests to Chamelium

[igt-dev] [PATCH i-g-t 0/5] Add audio EDID tests to Chamelium

[Simon Ser]

This series isolates the EDID-Like Data procfs parsing library and uses it from
Chamelium tests.

This series superseedes the previous one beginning with "lib/igt_eld: introduce
an ELD library".

Simon Ser (5):
  lib/igt_eld: introduce an ELD library
  lib/igt_eld: consolidate ELD parsing
  lib/igt_eld: parse Short Audio Descriptors
  lib/igt_eld: allow retrieving an ELD entry per monitor name
  tests/kms_chamelium: add an audio EDID test

 lib/igt_edid.h          |   2 +
 lib/igt_eld.c           | 214 ++++++++++++++++++++++++++++++++++++++++
 lib/igt_eld.h           |  54 ++++++++++
 lib/meson.build         |   1 +
 tests/kms_chamelium.c   |  69 ++++++++++++-
 tests/kms_hdmi_inject.c |  86 ++--------------
 6 files changed, 342 insertions(+), 84 deletions(-)
 create mode 100644 lib/igt_eld.c
 create mode 100644 lib/igt_eld.h

--
2.21.0

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

[igt-dev] [PATCH i-g-t 1/5] lib/igt_eld: introduce an ELD library

[Simon Ser]

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 <simon.ser@intel.com>
---
 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

diff --git a/lib/igt_eld.c b/lib/igt_eld.c
new file mode 100644
index 000000000000..8e0dcc306e85
--- /dev/null
+++ b/lib/igt_eld.c
@@ -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 <simon.ser@intel.com>
+ */
+
+#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)
+		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_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;
+}
diff --git a/lib/igt_eld.h b/lib/igt_eld.h
new file mode 100644
index 000000000000..27f876d9f631
--- /dev/null
+++ b/lib/igt_eld.h
@@ -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 <simon.ser@intel.com>
+ */
+
+#ifndef IGT_ELD_H
+#define IGT_ELD_H
+
+#include "config.h"
+
+#include <stdbool.h>
+
+bool eld_has_igt(void);
+
+#endif
diff --git a/lib/meson.build b/lib/meson.build
index cdb450e1e762..844e0abcd919 100644
--- a/lib/meson.build
+++ b/lib/meson.build
@@ -59,6 +59,7 @@ lib_sources = [
 	'igt_psr.c',
 	'igt_amd.c',
 	'igt_edid.c',
+	'igt_eld.c',
 ]
 
 lib_deps = [
diff --git a/tests/kms_hdmi_inject.c b/tests/kms_hdmi_inject.c
index a24061042c20..eba25046cead 100644
--- a/tests/kms_hdmi_inject.c
+++ b/tests/kms_hdmi_inject.c
@@ -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)
 {
@@ -252,7 +182,7 @@ hdmi_inject_audio(int drm_fd, drmModeConnector *connector)
 	 * Test if we have /proc/asound/HDMI/eld#0.0 and is its contents are
 	 * valid.
 	 */
-	igt_assert(eld_is_valid());
+	igt_assert(eld_has_igt());
 
 	igt_remove_fb(drm_fd, &fb);
 
-- 
2.21.0

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

[igt-dev] [PATCH i-g-t 2/5] lib/igt_eld: consolidate ELD parsing

[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 <simon.ser@intel.com>
---
 lib/igt_eld.c | 94 +++++++++++++++++++++++++++++++--------------------
 lib/igt_eld.h |  5 +++
 2 files changed, 63 insertions(+), 36 deletions(-)

diff --git a/lib/igt_eld.c b/lib/igt_eld.c
index 8e0dcc306e85..a198001a56d7 100644
--- a/lib/igt_eld.c
+++ b/lib/igt_eld.c
@@ -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
@@ -39,39 +43,49 @@
  * 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)
+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;
+	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++;
-		}
+	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, "");
+		/* Skip whitespace at the beginning */
+		value += strspn(value, ELD_DELIM);
+
+		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;
+			}
 
-			if (*snd_hda->d_name == '.' ||
-			    strstr(snd_hda->d_name, "eld") == 0)
+			if (!eld.valid) {
+				igt_debug("Skipping invalid ELD: %s\n", path);
 				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;
 			}
diff --git a/lib/igt_eld.h b/lib/igt_eld.h
index 27f876d9f631..21fe3537acf9 100644
--- a/lib/igt_eld.h
+++ b/lib/igt_eld.h
@@ -30,6 +30,11 @@
 
 #include <stdbool.h>
 
+struct eld_entry {
+	bool valid;
+	char monitor_name[16];
+};
+
 bool eld_has_igt(void);
 
 #endif
-- 
2.21.0

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

[igt-dev] [PATCH i-g-t 3/5] lib/igt_eld: parse Short Audio Descriptors

[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 <simon.ser@intel.com>
---
 lib/igt_edid.h |  2 ++
 lib/igt_eld.c  | 85 +++++++++++++++++++++++++++++++++++++++++++++++++-
 lib/igt_eld.h  | 14 +++++++++
 3 files changed, 100 insertions(+), 1 deletion(-)

diff --git a/lib/igt_edid.h b/lib/igt_edid.h
index 7edd7e38f41e..1d0c6aa29578 100644
--- a/lib/igt_edid.h
+++ b/lib/igt_edid.h
@@ -30,6 +30,8 @@
 
 #include <stdint.h>
 
+#include <xf86drmMode.h>
+
 struct est_timings {
 	uint8_t t1;
 	uint8_t t2;
diff --git a/lib/igt_eld.c b/lib/igt_eld.c
index a198001a56d7..732bbabd2d7c 100644
--- a/lib/igt_eld.c
+++ b/lib/igt_eld.c
@@ -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, " ");
+	}
+}
+
 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;
 	size_t len;
 	bool monitor_present;
+	int sad_index;
 
 	memset(eld, 0, sizeof(*eld));
 
@@ -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);
+			parse_sad_field(&eld->sads[sad_index], sad_key, value);
+			free(sad_key);
+		}
 	}
 
 	if (ferror(f) != 0) {
diff --git a/lib/igt_eld.h b/lib/igt_eld.h
index 21fe3537acf9..e16187884d4b 100644
--- a/lib/igt_eld.h
+++ b/lib/igt_eld.h
@@ -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];
 };
 
 bool eld_has_igt(void);
-- 
2.21.0

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

[igt-dev] [PATCH i-g-t 4/5] lib/igt_eld: allow retrieving an ELD entry per monitor name

[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).

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

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

Signed-off-by: Simon Ser <simon.ser@intel.com>
---
 lib/igt_eld.c           | 12 +++++-------
 lib/igt_eld.h           |  2 +-
 tests/kms_hdmi_inject.c |  8 +++-----
 3 files changed, 9 insertions(+), 13 deletions(-)

diff --git a/lib/igt_eld.c b/lib/igt_eld.c
index 732bbabd2d7c..53655c5e18f1 100644
--- a/lib/igt_eld.c
+++ b/lib/igt_eld.c
@@ -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;
 			}
diff --git a/lib/igt_eld.h b/lib/igt_eld.h
index e16187884d4b..1f34f784749b 100644
--- a/lib/igt_eld.h
+++ b/lib/igt_eld.h
@@ -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
diff --git a/tests/kms_hdmi_inject.c b/tests/kms_hdmi_inject.c
index eba25046cead..31ddda731d7b 100644
--- a/tests/kms_hdmi_inject.c
+++ b/tests/kms_hdmi_inject.c
@@ -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"));
 
 	igt_remove_fb(drm_fd, &fb);
 
-- 
2.21.0

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

[igt-dev] [PATCH i-g-t 5/5] tests/kms_chamelium: add an audio EDID test

[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 <simon.ser@intel.com>
---
 tests/kms_chamelium.c | 69 +++++++++++++++++++++++++++++++++++++++----
 1 file changed, 64 insertions(+), 5 deletions(-)

diff --git a/tests/kms_chamelium.c b/tests/kms_chamelium.c
index 49c3de0b7beb..16d3a773bb2a 100644
--- a/tests/kms_chamelium.c
+++ b/tests/kms_chamelium.c
@@ -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);
+
+	monitor_name = "IGT";
+	if (edid == TEST_EDID_CHAMELIUM_DEFAULT)
+		monitor_name = "AthenaDP";
+
+	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 {
-- 
2.21.0

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

Re: [igt-dev] [PATCH i-g-t 5/5] tests/kms_chamelium: add an audio EDID test

[Arkadiusz Hiler]

On Fri, May 31, 2019 at 02:22:00PM +0300, Simon Ser wrote:
> 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 <simon.ser@intel.com>
> ---
>  tests/kms_chamelium.c | 69 +++++++++++++++++++++++++++++++++++++++----
>  1 file changed, 64 insertions(+), 5 deletions(-)
> 
> diff --git a/tests/kms_chamelium.c b/tests/kms_chamelium.c
> index 49c3de0b7beb..16d3a773bb2a 100644
> --- a/tests/kms_chamelium.c
> +++ b/tests/kms_chamelium.c
> @@ -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);
> +
> +	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.

> +
> +	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 {
> -- 
> 2.21.0
> 
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t 5/5] tests/kms_chamelium: add an audio EDID test

[Arkadiusz Hiler]

On Fri, May 31, 2019 at 04:18:21PM +0300, Arkadiusz Hiler wrote:
> On Fri, May 31, 2019 at 02:22:00PM +0300, Simon Ser wrote:
> > 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 <simon.ser@intel.com>
> > ---
> >  tests/kms_chamelium.c | 69 +++++++++++++++++++++++++++++++++++++++----
> >  1 file changed, 64 insertions(+), 5 deletions(-)
> > 
> > diff --git a/tests/kms_chamelium.c b/tests/kms_chamelium.c
> > index 49c3de0b7beb..16d3a773bb2a 100644
> > --- a/tests/kms_chamelium.c
> > +++ b/tests/kms_chamelium.c
> > @@ -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);
> > +
> > +	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.

Scratch the part about manufacturer_id, as AthenaDP is definately more
than 3 letters and we are using here the actual monitor name. My bad.

> 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.
> 
> > +
> > +	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 {
> > -- 
> > 2.21.0
> > 
> _______________________________________________
> igt-dev mailing list
> igt-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/igt-dev
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t 5/5] tests/kms_chamelium: add an audio EDID test

[Ser, Simon]

On Fri, 2019-05-31 at 16:18 +0300, Arkadiusz Hiler wrote:
> On Fri, May 31, 2019 at 02:22:00PM +0300, Simon Ser wrote:
> > 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 <simon.ser@intel.com>
> > ---
> >  tests/kms_chamelium.c | 69 +++++++++++++++++++++++++++++++++++++++----
> >  1 file changed, 64 insertions(+), 5 deletions(-)
> > 
> > diff --git a/tests/kms_chamelium.c b/tests/kms_chamelium.c
> > index 49c3de0b7beb..16d3a773bb2a 100644
> > --- a/tests/kms_chamelium.c
> > +++ b/tests/kms_chamelium.c
> > @@ -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);
> > +
> > +	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.

I agree. I tried to add a DP audio EDID when implementing the HDMI
audio EDID code, however this requires:

1. Comparing our generated EDID with the default Chamelium one
2. Trying to figure out what field in the diff could make audio work
3. Adding support for that field in igt_edid, praying that only this
   part was misisng
4. Goto 1

It took several rounds to get HDMI to work. At some point I just gave
up for DP because we had a simple way to do it with Chamelium's default
EDID.

I still think it would be better to generate all EDIDs. One of my
recent patch series adds support for Speaker Allocation Data blocks
("the other SAD"). I'll try again and see if that works.

> "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.

One other option would be to keep "IGT" for the manufacturer and use
the serial number to tell EDIDs apart.

Note that this mechanism would also be useful for configuration-less
Chamelium setup.

> Other than that the series looks good.

Thanks for the review!

> > +
> > +	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 {
> > -- 
> > 2.21.0
> > 
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] ✓ Fi.CI.BAT: success for Add audio EDID tests to Chamelium

[Patchwork]

== Series Details ==

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

== Summary ==

CI Bug Log - changes from CI_DRM_6175 -> IGTPW_3090
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

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

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

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

### IGT changes ###

#### Issues hit ####

  * igt@i915_selftest@live_evict:
    - fi-bsw-kefka:       [PASS][1] -> [DMESG-WARN][2] ([fdo#107709])
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6175/fi-bsw-kefka/igt@i915_selftest@live_evict.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3090/fi-bsw-kefka/igt@i915_selftest@live_evict.html

  * igt@prime_vgem@basic-fence-wait-default:
    - fi-icl-u3:          [PASS][3] -> [DMESG-WARN][4] ([fdo#107724]) +1 similar issue
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6175/fi-icl-u3/igt@prime_vgem@basic-fence-wait-default.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3090/fi-icl-u3/igt@prime_vgem@basic-fence-wait-default.html

  
#### Possible fixes ####

  * igt@gem_exec_fence@basic-busy-default:
    - {fi-icl-guc}:       [INCOMPLETE][5] ([fdo#107713]) -> [PASS][6]
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6175/fi-icl-guc/igt@gem_exec_fence@basic-busy-default.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3090/fi-icl-guc/igt@gem_exec_fence@basic-busy-default.html

  * igt@i915_pm_rpm@module-reload:
    - fi-icl-u3:          [INCOMPLETE][7] ([fdo#107713] / [fdo#108840]) -> [PASS][8]
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6175/fi-icl-u3/igt@i915_pm_rpm@module-reload.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3090/fi-icl-u3/igt@i915_pm_rpm@module-reload.html

  * {igt@i915_selftest@live_mman}:
    - fi-icl-y:           [TIMEOUT][9] -> [PASS][10]
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6175/fi-icl-y/igt@i915_selftest@live_mman.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3090/fi-icl-y/igt@i915_selftest@live_mman.html

  * igt@kms_addfb_basic@invalid-get-prop-any:
    - fi-icl-u3:          [DMESG-WARN][11] ([fdo#107724]) -> [PASS][12]
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6175/fi-icl-u3/igt@kms_addfb_basic@invalid-get-prop-any.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3090/fi-icl-u3/igt@kms_addfb_basic@invalid-get-prop-any.html

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

  [fdo#107709]: https://bugs.freedesktop.org/show_bug.cgi?id=107709
  [fdo#107713]: https://bugs.freedesktop.org/show_bug.cgi?id=107713
  [fdo#107724]: https://bugs.freedesktop.org/show_bug.cgi?id=107724
  [fdo#108840]: https://bugs.freedesktop.org/show_bug.cgi?id=108840


Participating hosts (54 -> 46)
------------------------------

  Missing    (8): fi-ilk-m540 fi-hsw-4200u fi-bsw-n3050 fi-byt-squawks fi-bsw-cyan fi-kbl-7560u fi-byt-clapper fi-bdw-samus 


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

  * IGT: IGT_5026 -> IGTPW_3090

  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



== Testlist changes ==

+igt@kms_chamelium@dp-audio-edid
+igt@kms_chamelium@hdmi-audio-edid

== Logs ==

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

[igt-dev] ✗ Fi.CI.IGT: failure for Add audio EDID tests to Chamelium

[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 ###

#### 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@runner@aborted.html

  
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@gem_linear_blits@interruptible.html
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3090/shard-glk1/igt@gem_linear_blits@interruptible.html
    - shard-apl:          [PASS][4] -> [INCOMPLETE][5] ([fdo#103927])
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6175/shard-apl1/igt@gem_linear_blits@interruptible.html
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3090/shard-apl1/igt@gem_linear_blits@interruptible.html

  * 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@i915_suspend@debugfs-reader.html
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3090/shard-apl4/igt@i915_suspend@debugfs-reader.html

  * 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@i915_suspend@sysfs-reader.html
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3090/shard-kbl2/igt@i915_suspend@sysfs-reader.html

  * 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@kms_cursor_crc@pipe-a-cursor-64x64-offscreen.html
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3090/shard-hsw2/igt@kms_cursor_crc@pipe-a-cursor-64x64-offscreen.html

  * 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@kms_cursor_legacy@2x-flip-vs-cursor-atomic.html
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3090/shard-glk1/igt@kms_cursor_legacy@2x-flip-vs-cursor-atomic.html

  * 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@kms_cursor_legacy@flip-vs-cursor-legacy.html
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3090/shard-apl1/igt@kms_cursor_legacy@flip-vs-cursor-legacy.html

  * 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@kms_frontbuffer_tracking@fbc-1p-offscren-pri-shrfb-draw-blt.html
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3090/shard-iclb1/igt@kms_frontbuffer_tracking@fbc-1p-offscren-pri-shrfb-draw-blt.html

  * 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@kms_plane_lowres@pipe-a-tiling-y.html
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3090/shard-iclb6/igt@kms_plane_lowres@pipe-a-tiling-y.html

  * 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@gem_eio@in-flight-suspend.html
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3090/shard-glk6/igt@gem_eio@in-flight-suspend.html

  * 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@gem_mmap_gtt@forked-medium-copy-xy.html
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3090/shard-iclb4/igt@gem_mmap_gtt@forked-medium-copy-xy.html

  * 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@gem_persistent_relocs@forked.html
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3090/shard-iclb5/igt@gem_persistent_relocs@forked.html

  * 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@gem_softpin@softpin.html
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3090/shard-glk5/igt@gem_softpin@softpin.html

  * 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@gem_tiled_swapping@non-threaded.html
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3090/shard-kbl1/igt@gem_tiled_swapping@non-threaded.html

  * 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@i915_suspend@fence-restore-untiled.html
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3090/shard-apl2/igt@i915_suspend@fence-restore-untiled.html

  * 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@kms_frontbuffer_tracking@fbc-rgb565-draw-pwrite.html
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3090/shard-iclb5/igt@kms_frontbuffer_tracking@fbc-rgb565-draw-pwrite.html

  * 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@kms_plane_lowres@pipe-a-tiling-x.html
   [37]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3090/shard-iclb5/igt@kms_plane_lowres@pipe-a-tiling-x.html

  * 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@perf_pmu@rc6-runtime-pm-long.html
   [41]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3090/shard-glk5/igt@perf_pmu@rc6-runtime-pm-long.html

  
  [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/
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] ✗ Fi.CI.IGT: failure for Add audio EDID tests to Chamelium

[Ser, Simon]

On Sat, 2019-06-01 at 18:54 +0000, Patchwork wrote:
> == 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@runner@aborted.html

Cc Martin

>   
> 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@gem_linear_blits@interruptible.html
>    [3]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3090/shard-glk1/igt@gem_linear_blits@interruptible.html
>     - shard-apl:          [PASS][4] -> [INCOMPLETE][5] ([fdo#103927])
>    [4]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6175/shard-apl1/igt@gem_linear_blits@interruptible.html
>    [5]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3090/shard-apl1/igt@gem_linear_blits@interruptible.html
> 
>   * 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@i915_suspend@debugfs-reader.html
>    [7]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3090/shard-apl4/igt@i915_suspend@debugfs-reader.html
> 
>   * 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@i915_suspend@sysfs-reader.html
>    [9]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3090/shard-kbl2/igt@i915_suspend@sysfs-reader.html
> 
>   * 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@kms_cursor_crc@pipe-a-cursor-64x64-offscreen.html
>    [11]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3090/shard-hsw2/igt@kms_cursor_crc@pipe-a-cursor-64x64-offscreen.html
> 
>   * 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@kms_cursor_legacy@2x-flip-vs-cursor-atomic.html
>    [13]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3090/shard-glk1/igt@kms_cursor_legacy@2x-flip-vs-cursor-atomic.html
> 
>   * 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@kms_cursor_legacy@flip-vs-cursor-legacy.html
>    [15]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3090/shard-apl1/igt@kms_cursor_legacy@flip-vs-cursor-legacy.html
> 
>   * 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@kms_frontbuffer_tracking@fbc-1p-offscren-pri-shrfb-draw-blt.html
>    [17]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3090/shard-iclb1/igt@kms_frontbuffer_tracking@fbc-1p-offscren-pri-shrfb-draw-blt.html
> 
>   * 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@kms_plane_lowres@pipe-a-tiling-y.html
>    [19]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3090/shard-iclb6/igt@kms_plane_lowres@pipe-a-tiling-y.html
> 
>   * 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@gem_eio@in-flight-suspend.html
>    [23]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3090/shard-glk6/igt@gem_eio@in-flight-suspend.html
> 
>   * 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@gem_mmap_gtt@forked-medium-copy-xy.html
>    [25]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3090/shard-iclb4/igt@gem_mmap_gtt@forked-medium-copy-xy.html
> 
>   * 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@gem_persistent_relocs@forked.html
>    [27]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3090/shard-iclb5/igt@gem_persistent_relocs@forked.html
> 
>   * 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@gem_softpin@softpin.html
>    [29]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3090/shard-glk5/igt@gem_softpin@softpin.html
> 
>   * 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@gem_tiled_swapping@non-threaded.html
>    [31]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3090/shard-kbl1/igt@gem_tiled_swapping@non-threaded.html
> 
>   * 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@i915_suspend@fence-restore-untiled.html
>    [33]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3090/shard-apl2/igt@i915_suspend@fence-restore-untiled.html
> 
>   * 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@kms_frontbuffer_tracking@fbc-rgb565-draw-pwrite.html
>    [35]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3090/shard-iclb5/igt@kms_frontbuffer_tracking@fbc-rgb565-draw-pwrite.html
> 
>   * 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@kms_plane_lowres@pipe-a-tiling-x.html
>    [37]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3090/shard-iclb5/igt@kms_plane_lowres@pipe-a-tiling-x.html
> 
>   * 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@perf_pmu@rc6-runtime-pm-long.html
>    [41]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3090/shard-glk5/igt@perf_pmu@rc6-runtime-pm-long.html
> 
>   
>   [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/
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] ✗ Fi.CI.IGT: failure for Add audio EDID tests to Chamelium

[Peres, Martin]

On 03/06/2019 09:38, Ser, Simon wrote:
> On Sat, 2019-06-01 at 18:54 +0000, Patchwork wrote:
>> == 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@runner@aborted.html
> 
> Cc Martin

Filed and re-reported!

https://bugs.freedesktop.org/show_bug.cgi?id=110827

Thanks,
Martin

> 
>>   
>> 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@gem_linear_blits@interruptible.html
>>    [3]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3090/shard-glk1/igt@gem_linear_blits@interruptible.html
>>     - shard-apl:          [PASS][4] -> [INCOMPLETE][5] ([fdo#103927])
>>    [4]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6175/shard-apl1/igt@gem_linear_blits@interruptible.html
>>    [5]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3090/shard-apl1/igt@gem_linear_blits@interruptible.html
>>
>>   * 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@i915_suspend@debugfs-reader.html
>>    [7]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3090/shard-apl4/igt@i915_suspend@debugfs-reader.html
>>
>>   * 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@i915_suspend@sysfs-reader.html
>>    [9]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3090/shard-kbl2/igt@i915_suspend@sysfs-reader.html
>>
>>   * 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@kms_cursor_crc@pipe-a-cursor-64x64-offscreen.html
>>    [11]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3090/shard-hsw2/igt@kms_cursor_crc@pipe-a-cursor-64x64-offscreen.html
>>
>>   * 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@kms_cursor_legacy@2x-flip-vs-cursor-atomic.html
>>    [13]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3090/shard-glk1/igt@kms_cursor_legacy@2x-flip-vs-cursor-atomic.html
>>
>>   * 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@kms_cursor_legacy@flip-vs-cursor-legacy.html
>>    [15]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3090/shard-apl1/igt@kms_cursor_legacy@flip-vs-cursor-legacy.html
>>
>>   * 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@kms_frontbuffer_tracking@fbc-1p-offscren-pri-shrfb-draw-blt.html
>>    [17]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3090/shard-iclb1/igt@kms_frontbuffer_tracking@fbc-1p-offscren-pri-shrfb-draw-blt.html
>>
>>   * 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@kms_plane_lowres@pipe-a-tiling-y.html
>>    [19]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3090/shard-iclb6/igt@kms_plane_lowres@pipe-a-tiling-y.html
>>
>>   * 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@gem_eio@in-flight-suspend.html
>>    [23]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3090/shard-glk6/igt@gem_eio@in-flight-suspend.html
>>
>>   * 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@gem_mmap_gtt@forked-medium-copy-xy.html
>>    [25]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3090/shard-iclb4/igt@gem_mmap_gtt@forked-medium-copy-xy.html
>>
>>   * 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@gem_persistent_relocs@forked.html
>>    [27]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3090/shard-iclb5/igt@gem_persistent_relocs@forked.html
>>
>>   * 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@gem_softpin@softpin.html
>>    [29]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3090/shard-glk5/igt@gem_softpin@softpin.html
>>
>>   * 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@gem_tiled_swapping@non-threaded.html
>>    [31]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3090/shard-kbl1/igt@gem_tiled_swapping@non-threaded.html
>>
>>   * 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@i915_suspend@fence-restore-untiled.html
>>    [33]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3090/shard-apl2/igt@i915_suspend@fence-restore-untiled.html
>>
>>   * 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@kms_frontbuffer_tracking@fbc-rgb565-draw-pwrite.html
>>    [35]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3090/shard-iclb5/igt@kms_frontbuffer_tracking@fbc-rgb565-draw-pwrite.html
>>
>>   * 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@kms_plane_lowres@pipe-a-tiling-x.html
>>    [37]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3090/shard-iclb5/igt@kms_plane_lowres@pipe-a-tiling-x.html
>>
>>   * 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@perf_pmu@rc6-runtime-pm-long.html
>>    [41]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3090/shard-glk5/igt@perf_pmu@rc6-runtime-pm-long.html
>>
>>   
>>   [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/
> 

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

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] ✓ Fi.CI.IGT: success for Add audio EDID tests to Chamelium

[Patchwork]

== Series Details ==

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

== Summary ==

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

Summary
-------

  **SUCCESS**

  No regressions found.

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

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][1] -> [INCOMPLETE][2] ([fdo#103359] / [k.org#198133])
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6175/shard-glk1/igt@gem_linear_blits@interruptible.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3090/shard-glk1/igt@gem_linear_blits@interruptible.html
    - shard-apl:          [PASS][3] -> [INCOMPLETE][4] ([fdo#103927])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6175/shard-apl1/igt@gem_linear_blits@interruptible.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3090/shard-apl1/igt@gem_linear_blits@interruptible.html

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

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

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

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

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

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

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

  * igt@kms_psr@psr2_sprite_plane_move:
    - shard-iclb:         [PASS][19] -> [SKIP][20] ([fdo#109441]) +1 similar issue
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6175/shard-iclb2/igt@kms_psr@psr2_sprite_plane_move.html
   [20]: 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][21] ([fdo#110667]) -> [PASS][22]
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6175/shard-glk5/igt@gem_eio@in-flight-suspend.html
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3090/shard-glk6/igt@gem_eio@in-flight-suspend.html

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

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

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

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

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

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

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

  * igt@kms_psr@psr2_cursor_mmap_cpu:
    - shard-iclb:         [SKIP][37] ([fdo#109441]) -> [PASS][38] +3 similar issues
   [37]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6175/shard-iclb3/igt@kms_psr@psr2_cursor_mmap_cpu.html
   [38]: 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][39] ([fdo#105010]) -> [PASS][40]
   [39]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6175/shard-glk1/igt@perf_pmu@rc6-runtime-pm-long.html
   [40]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3090/shard-glk5/igt@perf_pmu@rc6-runtime-pm-long.html

  
  [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/
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t 1/5] lib/igt_eld: introduce an ELD library

[Martin Peres]

On 31/05/2019 14:21, Simon Ser wrote:
> 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 <simon.ser@intel.com>
> ---
>  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
> 
> diff --git a/lib/igt_eld.c b/lib/igt_eld.c
> new file mode 100644
> index 000000000000..8e0dcc306e85
> --- /dev/null
> +++ b/lib/igt_eld.c
> @@ -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 <simon.ser@intel.com>
> + */
> +
> +#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)
> +		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++;

I understand that you are just copy-pasting the previous implementation,
but this is not exactly the nicest way of making a boolean. How about
just setting it to 1, to make it clear that we are not counting anything?

This could be done in another patch or merged in this one.

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

> +		}
> +
> +		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;
> +}
> diff --git a/lib/igt_eld.h b/lib/igt_eld.h
> new file mode 100644
> index 000000000000..27f876d9f631
> --- /dev/null
> +++ b/lib/igt_eld.h
> @@ -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 <simon.ser@intel.com>
> + */
> +
> +#ifndef IGT_ELD_H
> +#define IGT_ELD_H
> +
> +#include "config.h"
> +
> +#include <stdbool.h>
> +
> +bool eld_has_igt(void);
> +
> +#endif
> diff --git a/lib/meson.build b/lib/meson.build
> index cdb450e1e762..844e0abcd919 100644
> --- a/lib/meson.build
> +++ b/lib/meson.build
> @@ -59,6 +59,7 @@ lib_sources = [
>  	'igt_psr.c',
>  	'igt_amd.c',
>  	'igt_edid.c',
> +	'igt_eld.c',
>  ]
>  
>  lib_deps = [
> diff --git a/tests/kms_hdmi_inject.c b/tests/kms_hdmi_inject.c
> index a24061042c20..eba25046cead 100644
> --- a/tests/kms_hdmi_inject.c
> +++ b/tests/kms_hdmi_inject.c
> @@ -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)
>  {
> @@ -252,7 +182,7 @@ hdmi_inject_audio(int drm_fd, drmModeConnector *connector)
>  	 * Test if we have /proc/asound/HDMI/eld#0.0 and is its contents are
>  	 * valid.
>  	 */
> -	igt_assert(eld_is_valid());
> +	igt_assert(eld_has_igt());
>  
>  	igt_remove_fb(drm_fd, &fb);
>  
> 
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t 2/5] lib/igt_eld: consolidate ELD parsing

[Martin Peres]

On 31/05/2019 14:21, Simon Ser wrote:
> 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 <simon.ser@intel.com>
> ---
>  lib/igt_eld.c | 94 +++++++++++++++++++++++++++++++--------------------
>  lib/igt_eld.h |  5 +++
>  2 files changed, 63 insertions(+), 36 deletions(-)
> 
> diff --git a/lib/igt_eld.c b/lib/igt_eld.c
> index 8e0dcc306e85..a198001a56d7 100644
> --- a/lib/igt_eld.c
> +++ b/lib/igt_eld.c
> @@ -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
> @@ -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

> -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;
> +	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!

> +	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!

> +		/* 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!

> +
> +		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;
> +			}
>  
> -			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 <martin.peres@linux.intel.com>

>  				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;
>  			}
> diff --git a/lib/igt_eld.h b/lib/igt_eld.h
> index 27f876d9f631..21fe3537acf9 100644
> --- a/lib/igt_eld.h
> +++ b/lib/igt_eld.h
> @@ -30,6 +30,11 @@
>  
>  #include <stdbool.h>
>  
> +struct eld_entry {
> +	bool valid;
> +	char monitor_name[16];
> +};
> +
>  bool eld_has_igt(void);
>  
>  #endif
> 
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t 5/5] tests/kms_chamelium: add an audio EDID test

[Ser, Simon]

On Fri, 2019-05-31 at 16:18 +0300, Arkadiusz Hiler wrote:
> On Fri, May 31, 2019 at 02:22:00PM +0300, Simon Ser wrote:
> > 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 <simon.ser@intel.com>
> > ---
> >  tests/kms_chamelium.c | 69 +++++++++++++++++++++++++++++++++++++++----
> >  1 file changed, 64 insertions(+), 5 deletions(-)
> > 
> > diff --git a/tests/kms_chamelium.c b/tests/kms_chamelium.c
> > index 49c3de0b7beb..16d3a773bb2a 100644
> > --- a/tests/kms_chamelium.c
> > +++ b/tests/kms_chamelium.c
> > @@ -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);
> > +
> > +	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.

I now have a patch for another series that adds a DisplayPort EDID
(depends on Speaker Allocation Data blocks, so can't be added to this
one).

> Other than that the series looks good.
> 
> > +
> > +	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 {
> > -- 
> > 2.21.0
> > 
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t 3/5] lib/igt_eld: parse Short Audio Descriptors

[Martin Peres]

On 31/05/2019 14:21, Simon Ser wrote:
> 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 <simon.ser@intel.com>
> ---
>  lib/igt_edid.h |  2 ++
>  lib/igt_eld.c  | 85 +++++++++++++++++++++++++++++++++++++++++++++++++-
>  lib/igt_eld.h  | 14 +++++++++
>  3 files changed, 100 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/igt_edid.h b/lib/igt_edid.h
> index 7edd7e38f41e..1d0c6aa29578 100644
> --- a/lib/igt_edid.h
> +++ b/lib/igt_edid.h
> @@ -30,6 +30,8 @@
>  
>  #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.

>  struct est_timings {
>  	uint8_t t1;
>  	uint8_t t2;
> diff --git a/lib/igt_eld.c b/lib/igt_eld.c
> index a198001a56d7..732bbabd2d7c 100644
> --- a/lib/igt_eld.c
> +++ b/lib/igt_eld.c
> @@ -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, " ");
> +	}
> +}
> +
>  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!

>  	size_t len;
>  	bool monitor_present;
> +	int sad_index;
>  
>  	memset(eld, 0, sizeof(*eld));
>  
> @@ -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 <martin.peres@linux.intel.com>

> +			parse_sad_field(&eld->sads[sad_index], sad_key, value);
> +			free(sad_key);
> +		}
>  	}
>  
>  	if (ferror(f) != 0) {
> diff --git a/lib/igt_eld.h b/lib/igt_eld.h
> index 21fe3537acf9..e16187884d4b 100644
> --- a/lib/igt_eld.h
> +++ b/lib/igt_eld.h
> @@ -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];
>  };
>  
>  bool eld_has_igt(void);
> 
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t 4/5] lib/igt_eld: allow retrieving an ELD entry per monitor name

[Martin Peres]

On 31/05/2019 14:21, Simon Ser wrote:
> 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

> 
> (2) makes it impossible to check ELD audio parameters.
> 
> This commit fixes both (1) and (2).
> 
> Signed-off-by: Simon Ser <simon.ser@intel.com>
> ---
>  lib/igt_eld.c           | 12 +++++-------
>  lib/igt_eld.h           |  2 +-
>  tests/kms_hdmi_inject.c |  8 +++-----
>  3 files changed, 9 insertions(+), 13 deletions(-)
> 
> diff --git a/lib/igt_eld.c b/lib/igt_eld.c
> index 732bbabd2d7c..53655c5e18f1 100644
> --- a/lib/igt_eld.c
> +++ b/lib/igt_eld.c
> @@ -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;
>  			}
> diff --git a/lib/igt_eld.h b/lib/igt_eld.h
> index e16187884d4b..1f34f784749b 100644
> --- a/lib/igt_eld.h
> +++ b/lib/igt_eld.h
> @@ -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
> diff --git a/tests/kms_hdmi_inject.c b/tests/kms_hdmi_inject.c
> index eba25046cead..31ddda731d7b 100644
> --- a/tests/kms_hdmi_inject.c
> +++ b/tests/kms_hdmi_inject.c
> @@ -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"));
>  
>  	igt_remove_fb(drm_fd, &fb);
>  
> 
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] ✓ Fi.CI.BAT: success for Add audio EDID tests to Chamelium (rev2)

[Patchwork]

== Series Details ==

Series: Add audio EDID tests to Chamelium (rev2)
URL   : https://patchwork.freedesktop.org/series/61424/
State : success

== Summary ==

CI Bug Log - changes from IGT_5028 -> IGTPW_3095
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

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

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

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

### IGT changes ###

#### Issues hit ####

  * igt@kms_addfb_basic@unused-offsets:
    - fi-icl-u3:          [PASS][1] -> [DMESG-WARN][2] ([fdo#107724]) +1 similar issue
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5028/fi-icl-u3/igt@kms_addfb_basic@unused-offsets.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3095/fi-icl-u3/igt@kms_addfb_basic@unused-offsets.html

  * igt@prime_vgem@basic-fence-read:
    - fi-icl-dsi:         [PASS][3] -> [INCOMPLETE][4] ([fdo#107713])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5028/fi-icl-dsi/igt@prime_vgem@basic-fence-read.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3095/fi-icl-dsi/igt@prime_vgem@basic-fence-read.html

  
#### Possible fixes ####

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

  * igt@gem_exec_suspend@basic-s4-devices:
    - fi-kbl-7500u:       [DMESG-WARN][7] ([fdo#105128] / [fdo#107139]) -> [PASS][8]
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5028/fi-kbl-7500u/igt@gem_exec_suspend@basic-s4-devices.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3095/fi-kbl-7500u/igt@gem_exec_suspend@basic-s4-devices.html

  * {igt@i915_selftest@live_mman}:
    - fi-icl-u3:          [TIMEOUT][9] ([fdo#110818 ]) -> [PASS][10]
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5028/fi-icl-u3/igt@i915_selftest@live_mman.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3095/fi-icl-u3/igt@i915_selftest@live_mman.html

  * igt@kms_chamelium@dp-crc-fast:
    - fi-cml-u2:          [FAIL][11] ([fdo#110627]) -> [PASS][12]
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5028/fi-cml-u2/igt@kms_chamelium@dp-crc-fast.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3095/fi-cml-u2/igt@kms_chamelium@dp-crc-fast.html

  * igt@kms_cursor_legacy@basic-flip-after-cursor-legacy:
    - fi-icl-u3:          [DMESG-WARN][13] ([fdo#107724]) -> [PASS][14]
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5028/fi-icl-u3/igt@kms_cursor_legacy@basic-flip-after-cursor-legacy.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3095/fi-icl-u3/igt@kms_cursor_legacy@basic-flip-after-cursor-legacy.html

  * igt@kms_frontbuffer_tracking@basic:
    - fi-hsw-peppy:       [DMESG-WARN][15] ([fdo#102614]) -> [PASS][16]
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5028/fi-hsw-peppy/igt@kms_frontbuffer_tracking@basic.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3095/fi-hsw-peppy/igt@kms_frontbuffer_tracking@basic.html

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

  [fdo#102614]: https://bugs.freedesktop.org/show_bug.cgi?id=102614
  [fdo#105128]: https://bugs.freedesktop.org/show_bug.cgi?id=105128
  [fdo#107139]: https://bugs.freedesktop.org/show_bug.cgi?id=107139
  [fdo#107713]: https://bugs.freedesktop.org/show_bug.cgi?id=107713
  [fdo#107718]: https://bugs.freedesktop.org/show_bug.cgi?id=107718
  [fdo#107724]: https://bugs.freedesktop.org/show_bug.cgi?id=107724
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#110627]: https://bugs.freedesktop.org/show_bug.cgi?id=110627
  [fdo#110818 ]: https://bugs.freedesktop.org/show_bug.cgi?id=110818 


Participating hosts (51 -> 45)
------------------------------

  Additional (1): fi-icl-y 
  Missing    (7): fi-ilk-m540 fi-hsw-4200u fi-bsw-cyan fi-pnv-d510 fi-kbl-7560u fi-byt-clapper fi-bdw-samus 


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

  * IGT: IGT_5028 -> IGTPW_3095

  CI_DRM_6180: e724dd1cacd9da18b18808880a13b0cb33ddd3d7 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_3095: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3095/
  IGT_5028: 2d5c26f844cf1851e5097680d30c951978291c29 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools



== Testlist changes ==

+igt@kms_chamelium@dp-audio-edid
+igt@kms_chamelium@hdmi-audio-edid

== Logs ==

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

Re: [igt-dev] [PATCH i-g-t 2/5] lib/igt_eld: consolidate ELD parsing

[Ser, Simon]

On Mon, 2019-06-03 at 15:42 +0300, Martin Peres wrote:
> On 31/05/2019 14:21, Simon Ser wrote:
> > 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 <simon.ser@intel.com>
> > ---
> >  lib/igt_eld.c | 94 +++++++++++++++++++++++++++++++--------------------
> >  lib/igt_eld.h |  5 +++
> >  2 files changed, 63 insertions(+), 36 deletions(-)
> > 
> > diff --git a/lib/igt_eld.c b/lib/igt_eld.c
> > index 8e0dcc306e85..a198001a56d7 100644
> > --- a/lib/igt_eld.c
> > +++ b/lib/igt_eld.c
> > @@ -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
> > @@ -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
> 
> > -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;
> > +	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!
> 
> > +	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!
> 
> > +		/* 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

> > +
> > +		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;
> > +			}
> >  
> > -			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"?

> Anyway, I like where this is going!
> 
> Reviewed-by: Martin Peres <martin.peres@linux.intel.com>
> 
> >  				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;
> >  			}
> > diff --git a/lib/igt_eld.h b/lib/igt_eld.h
> > index 27f876d9f631..21fe3537acf9 100644
> > --- a/lib/igt_eld.h
> > +++ b/lib/igt_eld.h
> > @@ -30,6 +30,11 @@
> >  
> >  #include <stdbool.h>
> >  
> > +struct eld_entry {
> > +	bool valid;
> > +	char monitor_name[16];
> > +};
> > +
> >  bool eld_has_igt(void);
> >  
> >  #endif
> > 
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t 3/5] lib/igt_eld: parse Short Audio Descriptors

[Ser, Simon]

On Mon, 2019-06-03 at 15:57 +0300, Martin Peres wrote:
> On 31/05/2019 14:21, Simon Ser wrote:
> > 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 <simon.ser@intel.com>
> > ---
> >  lib/igt_edid.h |  2 ++
> >  lib/igt_eld.c  | 85 +++++++++++++++++++++++++++++++++++++++++++++++++-
> >  lib/igt_eld.h  | 14 +++++++++
> >  3 files changed, 100 insertions(+), 1 deletion(-)
> > 
> > diff --git a/lib/igt_edid.h b/lib/igt_edid.h
> > index 7edd7e38f41e..1d0c6aa29578 100644
> > --- a/lib/igt_edid.h
> > +++ b/lib/igt_edid.h
> > @@ -30,6 +30,8 @@
> >  
> >  #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.

Fair, I've split it into another commit

> >  struct est_timings {
> >  	uint8_t t1;
> >  	uint8_t t2;
> > diff --git a/lib/igt_eld.c b/lib/igt_eld.c
> > index a198001a56d7..732bbabd2d7c 100644
> > --- a/lib/igt_eld.c
> > +++ b/lib/igt_eld.c
> > @@ -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, " ");
> > +	}
> > +}
> > +
> >  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

So many sad symbols! It's a shame EDID doesn't include any HAPPY
fields…

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

Agreed. I'll add a note about this. And I'll send a patch for igt_edid
docs.

> >  	size_t len;
> >  	bool monitor_present;
> > +	int sad_index;
> >  
> >  	memset(eld, 0, sizeof(*eld));
> >  
> > @@ -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 <martin.peres@linux.intel.com>
> 
> > +			parse_sad_field(&eld->sads[sad_index], sad_key, value);
> > +			free(sad_key);
> > +		}
> >  	}
> >  
> >  	if (ferror(f) != 0) {
> > diff --git a/lib/igt_eld.h b/lib/igt_eld.h
> > index 21fe3537acf9..e16187884d4b 100644
> > --- a/lib/igt_eld.h
> > +++ b/lib/igt_eld.h
> > @@ -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];
> >  };
> >  
> >  bool eld_has_igt(void);
> > 
> _______________________________________________
> igt-dev mailing list
> igt-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/igt-dev
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t 4/5] lib/igt_eld: allow retrieving an ELD entry per monitor name

[Ser, Simon]

On Mon, 2019-06-03 at 16:04 +0300, Martin Peres wrote:
> On 31/05/2019 14:21, Simon Ser wrote:
> > 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?

Good idea!

> Martin
> 
> > (2) makes it impossible to check ELD audio parameters.
> > 
> > This commit fixes both (1) and (2).
> > 
> > Signed-off-by: Simon Ser <simon.ser@intel.com>
> > ---
> >  lib/igt_eld.c           | 12 +++++-------
> >  lib/igt_eld.h           |  2 +-
> >  tests/kms_hdmi_inject.c |  8 +++-----
> >  3 files changed, 9 insertions(+), 13 deletions(-)
> > 
> > diff --git a/lib/igt_eld.c b/lib/igt_eld.c
> > index 732bbabd2d7c..53655c5e18f1 100644
> > --- a/lib/igt_eld.c
> > +++ b/lib/igt_eld.c
> > @@ -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;
> >  			}
> > diff --git a/lib/igt_eld.h b/lib/igt_eld.h
> > index e16187884d4b..1f34f784749b 100644
> > --- a/lib/igt_eld.h
> > +++ b/lib/igt_eld.h
> > @@ -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
> > diff --git a/tests/kms_hdmi_inject.c b/tests/kms_hdmi_inject.c
> > index eba25046cead..31ddda731d7b 100644
> > --- a/tests/kms_hdmi_inject.c
> > +++ b/tests/kms_hdmi_inject.c
> > @@ -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"));
> >  
> >  	igt_remove_fb(drm_fd, &fb);
> >  
> > 
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] ✓ Fi.CI.IGT: success for Add audio EDID tests to Chamelium (rev2)

[Patchwork]

== Series Details ==

Series: Add audio EDID tests to Chamelium (rev2)
URL   : https://patchwork.freedesktop.org/series/61424/
State : success

== Summary ==

CI Bug Log - changes from IGT_5028_full -> IGTPW_3095_full
====================================================

Summary
-------

  **WARNING**

  Minor unknown changes coming with IGTPW_3095_full need to be verified
  manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in IGTPW_3095_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/2/mbox/

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

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

### IGT changes ###

#### Warnings ####

  * igt@kms_hdmi_inject@inject-audio:
    - shard-iclb:         [FAIL][1] ([fdo#102370]) -> [FAIL][2]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5028/shard-iclb7/igt@kms_hdmi_inject@inject-audio.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3095/shard-iclb3/igt@kms_hdmi_inject@inject-audio.html

  
New tests
---------

  New tests have been introduced between IGT_5028_full and IGTPW_3095_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 : 5 skip(s)
    - Exec time: [0.0] s

  

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

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

### IGT changes ###

#### Issues hit ####

  * igt@gem_eio@in-flight-suspend:
    - shard-kbl:          [PASS][3] -> [INCOMPLETE][4] ([fdo#103665]) +1 similar issue
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5028/shard-kbl6/igt@gem_eio@in-flight-suspend.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3095/shard-kbl2/igt@gem_eio@in-flight-suspend.html
    - shard-apl:          [PASS][5] -> [DMESG-WARN][6] ([fdo#108566]) +4 similar issues
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5028/shard-apl7/igt@gem_eio@in-flight-suspend.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3095/shard-apl8/igt@gem_eio@in-flight-suspend.html

  * igt@gem_eio@unwedge-stress:
    - shard-snb:          [PASS][7] -> [FAIL][8] ([fdo#109661])
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5028/shard-snb2/igt@gem_eio@unwedge-stress.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3095/shard-snb6/igt@gem_eio@unwedge-stress.html

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

  * igt@kms_frontbuffer_tracking@fbc-rgb565-draw-render:
    - shard-iclb:         [PASS][11] -> [FAIL][12] ([fdo#103167]) +1 similar issue
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5028/shard-iclb5/igt@kms_frontbuffer_tracking@fbc-rgb565-draw-render.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3095/shard-iclb2/igt@kms_frontbuffer_tracking@fbc-rgb565-draw-render.html

  * igt@kms_psr2_su@page_flip:
    - shard-iclb:         [PASS][13] -> [SKIP][14] ([fdo#109642])
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5028/shard-iclb2/igt@kms_psr2_su@page_flip.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3095/shard-iclb6/igt@kms_psr2_su@page_flip.html

  * igt@kms_psr@psr2_primary_page_flip:
    - shard-iclb:         [PASS][15] -> [SKIP][16] ([fdo#109441]) +3 similar issues
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5028/shard-iclb2/igt@kms_psr@psr2_primary_page_flip.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3095/shard-iclb6/igt@kms_psr@psr2_primary_page_flip.html

  
#### Possible fixes ####

  * igt@gem_mmap_gtt@forked-medium-copy-xy:
    - shard-iclb:         [TIMEOUT][17] ([fdo#109673]) -> [PASS][18]
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5028/shard-iclb1/igt@gem_mmap_gtt@forked-medium-copy-xy.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3095/shard-iclb8/igt@gem_mmap_gtt@forked-medium-copy-xy.html

  * igt@gem_tiled_swapping@non-threaded:
    - shard-iclb:         [FAIL][19] ([fdo#108686]) -> [PASS][20]
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5028/shard-iclb8/igt@gem_tiled_swapping@non-threaded.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3095/shard-iclb4/igt@gem_tiled_swapping@non-threaded.html

  * igt@gem_workarounds@suspend-resume:
    - shard-apl:          [DMESG-WARN][21] ([fdo#108566]) -> [PASS][22] +7 similar issues
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5028/shard-apl5/igt@gem_workarounds@suspend-resume.html
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3095/shard-apl7/igt@gem_workarounds@suspend-resume.html

  * igt@kms_cursor_crc@pipe-b-cursor-alpha-opaque:
    - shard-apl:          [FAIL][23] ([fdo#103232]) -> [PASS][24] +1 similar issue
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5028/shard-apl6/igt@kms_cursor_crc@pipe-b-cursor-alpha-opaque.html
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3095/shard-apl4/igt@kms_cursor_crc@pipe-b-cursor-alpha-opaque.html
    - shard-kbl:          [FAIL][25] ([fdo#103232]) -> [PASS][26] +1 similar issue
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5028/shard-kbl7/igt@kms_cursor_crc@pipe-b-cursor-alpha-opaque.html
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3095/shard-kbl2/igt@kms_cursor_crc@pipe-b-cursor-alpha-opaque.html

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-draw-blt:
    - shard-iclb:         [FAIL][27] ([fdo#103167]) -> [PASS][28] +4 similar issues
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5028/shard-iclb7/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-draw-blt.html
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3095/shard-iclb7/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-draw-blt.html

  * igt@kms_plane@plane-position-covered-pipe-a-planes:
    - shard-snb:          [SKIP][29] ([fdo#109271]) -> [PASS][30] +1 similar issue
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5028/shard-snb6/igt@kms_plane@plane-position-covered-pipe-a-planes.html
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3095/shard-snb4/igt@kms_plane@plane-position-covered-pipe-a-planes.html

  * igt@kms_plane_lowres@pipe-a-tiling-y:
    - shard-iclb:         [FAIL][31] ([fdo#103166]) -> [PASS][32]
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5028/shard-iclb1/igt@kms_plane_lowres@pipe-a-tiling-y.html
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3095/shard-iclb8/igt@kms_plane_lowres@pipe-a-tiling-y.html

  * igt@kms_psr@psr2_basic:
    - shard-iclb:         [SKIP][33] ([fdo#109441]) -> [PASS][34]
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5028/shard-iclb3/igt@kms_psr@psr2_basic.html
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3095/shard-iclb2/igt@kms_psr@psr2_basic.html

  
#### Warnings ####

  * igt@gem_mmap_gtt@forked-big-copy-odd:
    - shard-iclb:         [TIMEOUT][35] ([fdo#109673]) -> [INCOMPLETE][36] ([fdo#107713] / [fdo#109100])
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5028/shard-iclb8/igt@gem_mmap_gtt@forked-big-copy-odd.html
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3095/shard-iclb5/igt@gem_mmap_gtt@forked-big-copy-odd.html

  * igt@gem_tiled_swapping@non-threaded:
    - shard-hsw:          [FAIL][37] ([fdo#108686]) -> [INCOMPLETE][38] ([fdo#103540])
   [37]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5028/shard-hsw5/igt@gem_tiled_swapping@non-threaded.html
   [38]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3095/shard-hsw5/igt@gem_tiled_swapping@non-threaded.html

  
  [fdo#102370]: https://bugs.freedesktop.org/show_bug.cgi?id=102370
  [fdo#103166]: https://bugs.freedesktop.org/show_bug.cgi?id=103166
  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#103232]: https://bugs.freedesktop.org/show_bug.cgi?id=103232
  [fdo#103540]: https://bugs.freedesktop.org/show_bug.cgi?id=103540
  [fdo#103665]: https://bugs.freedesktop.org/show_bug.cgi?id=103665
  [fdo#105363]: https://bugs.freedesktop.org/show_bug.cgi?id=105363
  [fdo#107713]: https://bugs.freedesktop.org/show_bug.cgi?id=107713
  [fdo#108566]: https://bugs.freedesktop.org/show_bug.cgi?id=108566
  [fdo#108686]: https://bugs.freedesktop.org/show_bug.cgi?id=108686
  [fdo#109100]: https://bugs.freedesktop.org/show_bug.cgi?id=109100
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109441]: https://bugs.freedesktop.org/show_bug.cgi?id=109441
  [fdo#109642]: https://bugs.freedesktop.org/show_bug.cgi?id=109642
  [fdo#109661]: https://bugs.freedesktop.org/show_bug.cgi?id=109661
  [fdo#109673]: https://bugs.freedesktop.org/show_bug.cgi?id=109673


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

  Missing    (1): shard-skl 


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

  * IGT: IGT_5028 -> IGTPW_3095

  CI_DRM_6180: e724dd1cacd9da18b18808880a13b0cb33ddd3d7 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_3095: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3095/
  IGT_5028: 2d5c26f844cf1851e5097680d30c951978291c29 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools

== Logs ==

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

Re: [igt-dev] [PATCH i-g-t 2/5] lib/igt_eld: consolidate ELD parsing

[Peres, Martin]

On 03/06/2019 17:34, Ser, Simon wrote:
> On Mon, 2019-06-03 at 15:42 +0300, Martin Peres wrote:
>> On 31/05/2019 14:21, Simon Ser wrote:
>>> 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 <simon.ser@intel.com>
>>> ---
>>>  lib/igt_eld.c | 94 +++++++++++++++++++++++++++++++--------------------
>>>  lib/igt_eld.h |  5 +++
>>>  2 files changed, 63 insertions(+), 36 deletions(-)
>>>
>>> diff --git a/lib/igt_eld.c b/lib/igt_eld.c
>>> index 8e0dcc306e85..a198001a56d7 100644
>>> --- a/lib/igt_eld.c
>>> +++ b/lib/igt_eld.c
>>> @@ -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
>>> @@ -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
>>
>>> -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;
>>> +	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!
>>
>>> +	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!
>>
>>> +		/* 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
> 
>>> +
>>> +		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;
>>> +			}
>>>  
>>> -			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).

Right, that was the previous code that was checking that, but you moved
it to the top with the strcmp.

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

I think your original message was better. Sorry for the noise! Feel free
to merge!

> 
>> Anyway, I like where this is going!
>>
>> Reviewed-by: Martin Peres <martin.peres@linux.intel.com>
>>
>>>  				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;
>>>  			}
>>> diff --git a/lib/igt_eld.h b/lib/igt_eld.h
>>> index 27f876d9f631..21fe3537acf9 100644
>>> --- a/lib/igt_eld.h
>>> +++ b/lib/igt_eld.h
>>> @@ -30,6 +30,11 @@
>>>  
>>>  #include <stdbool.h>
>>>  
>>> +struct eld_entry {
>>> +	bool valid;
>>> +	char monitor_name[16];
>>> +};
>>> +
>>>  bool eld_has_igt(void);
>>>  
>>>  #endif
>>>
> 

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

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

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