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

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

[Simon Ser]

The first patch has already been submitted, but the two other patches depend on
it.

v2: do not use the Chamelium default EDID, use the one we generate

Simon Ser (3):
  lib/igt_kms: generate an EDID suitable for DP audio
  lib/igt_eld: add eld_get_igt
  tests/kms_chamelium: add an audio EDID test

 lib/igt_eld.c           | 20 +++++++-----
 lib/igt_eld.h           |  1 +
 lib/igt_kms.c           | 46 ++++++++++++++++++++++------
 lib/igt_kms.h           |  3 +-
 tests/kms_chamelium.c   | 68 +++++++++++++++++++++++++++++++++++++++--
 tests/kms_hdmi_inject.c |  2 +-
 6 files changed, 119 insertions(+), 21 deletions(-)

--
2.21.0

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

[igt-dev] [PATCH i-g-t 1/3] lib/igt_kms: generate an EDID suitable for DP audio

[Simon Ser]

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

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

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

Signed-off-by: Simon Ser <simon.ser@intel.com>
---
 lib/igt_kms.c           | 46 +++++++++++++++++++++++++++++++++--------
 lib/igt_kms.h           |  3 ++-
 tests/kms_chamelium.c   | 10 ++++++---
 tests/kms_hdmi_inject.c |  2 +-
 4 files changed, 47 insertions(+), 14 deletions(-)

diff --git a/lib/igt_kms.c b/lib/igt_kms.c
index d7d711a72d27..011b064579a8 100644
--- a/lib/igt_kms.c
+++ b/lib/igt_kms.c
@@ -183,9 +183,9 @@ const unsigned char *igt_kms_get_alt_edid(void)
 }
 
 static void
-generate_hdmi_audio_edid(unsigned char raw_edid[static HDMI_AUDIO_EDID_LENGTH],
-			 struct cea_sad *sad,
-			 struct cea_speaker_alloc *speaker_alloc)
+generate_audio_edid(unsigned char raw_edid[static AUDIO_EDID_LENGTH],
+		    bool with_vsd, struct cea_sad *sad,
+		    struct cea_speaker_alloc *speaker_alloc)
 {
 	struct edid *edid;
 	struct edid_ext *edid_ext;
@@ -210,10 +210,12 @@ generate_hdmi_audio_edid(unsigned char raw_edid[static HDMI_AUDIO_EDID_LENGTH],
 	cea_data_size += edid_cea_data_block_set_sad(block, sad, 1);
 
 	/* A Vendor Specific Data block is needed for HDMI audio */
-	block = (struct edid_cea_data_block *) &cea_data[cea_data_size];
-	vsd = cea_vsd_get_hdmi_default(&vsd_size);
-	cea_data_size += edid_cea_data_block_set_vsd(block, vsd,
-						     vsd_size);
+	if (with_vsd) {
+		block = (struct edid_cea_data_block *) &cea_data[cea_data_size];
+		vsd = cea_vsd_get_hdmi_default(&vsd_size);
+		cea_data_size += edid_cea_data_block_set_vsd(block, vsd,
+							     vsd_size);
+	}
 
 	/* Speaker Allocation Data block */
 	block = (struct edid_cea_data_block *) &cea_data[cea_data_size];
@@ -233,7 +235,33 @@ const unsigned char *igt_kms_get_hdmi_audio_edid(void)
 {
 	int channels;
 	uint8_t sampling_rates, sample_sizes;
-	static unsigned char raw_edid[HDMI_AUDIO_EDID_LENGTH] = {0};
+	static unsigned char raw_edid[AUDIO_EDID_LENGTH] = {0};
+	struct cea_sad sad = {0};
+	struct cea_speaker_alloc speaker_alloc = {0};
+
+	/* Initialize the Short Audio Descriptor for PCM */
+	channels = 2;
+	sampling_rates = CEA_SAD_SAMPLING_RATE_32KHZ |
+			 CEA_SAD_SAMPLING_RATE_44KHZ |
+			 CEA_SAD_SAMPLING_RATE_48KHZ;
+	sample_sizes = CEA_SAD_SAMPLE_SIZE_16 |
+		       CEA_SAD_SAMPLE_SIZE_20 |
+		       CEA_SAD_SAMPLE_SIZE_24;
+	cea_sad_init_pcm(&sad, channels, sampling_rates, sample_sizes);
+
+	/* Initialize the Speaker Allocation Data */
+	speaker_alloc.speakers = CEA_SPEAKER_FRONT_LEFT_RIGHT_CENTER;
+
+	generate_audio_edid(raw_edid, true, &sad, &speaker_alloc);
+
+	return raw_edid;
+}
+
+const unsigned char *igt_kms_get_dp_audio_edid(void)
+{
+	int channels;
+	uint8_t sampling_rates, sample_sizes;
+	static unsigned char raw_edid[AUDIO_EDID_LENGTH] = {0};
 	struct cea_sad sad = {0};
 	struct cea_speaker_alloc speaker_alloc = {0};
 
@@ -250,7 +278,7 @@ const unsigned char *igt_kms_get_hdmi_audio_edid(void)
 	/* Initialize the Speaker Allocation Data */
 	speaker_alloc.speakers = CEA_SPEAKER_FRONT_LEFT_RIGHT_CENTER;
 
-	generate_hdmi_audio_edid(raw_edid, &sad, &speaker_alloc);
+	generate_audio_edid(raw_edid, false, &sad, &speaker_alloc);
 
 	return raw_edid;
 }
diff --git a/lib/igt_kms.h b/lib/igt_kms.h
index 4ac28131b6d9..5df9f18c0bd1 100644
--- a/lib/igt_kms.h
+++ b/lib/igt_kms.h
@@ -757,10 +757,11 @@ struct cea_sad;
 struct cea_speaker_alloc;
 
 #define EDID_LENGTH 128
-#define HDMI_AUDIO_EDID_LENGTH (2 * EDID_LENGTH)
+#define AUDIO_EDID_LENGTH (2 * EDID_LENGTH)
 const unsigned char *igt_kms_get_base_edid(void);
 const unsigned char *igt_kms_get_alt_edid(void);
 const unsigned char *igt_kms_get_hdmi_audio_edid(void);
+const unsigned char *igt_kms_get_dp_audio_edid(void);
 
 struct udev_monitor *igt_watch_hotplug(void);
 bool igt_hotplug_detected(struct udev_monitor *mon,
diff --git a/tests/kms_chamelium.c b/tests/kms_chamelium.c
index 52e3f0953621..3beea0c623bd 100644
--- a/tests/kms_chamelium.c
+++ b/tests/kms_chamelium.c
@@ -39,7 +39,9 @@ enum test_edid {
 	TEST_EDID_BASE,
 	TEST_EDID_ALT,
 	TEST_EDID_HDMI_AUDIO,
+	TEST_EDID_DP_AUDIO,
 };
+#define TEST_EDID_COUNT 5
 
 typedef struct {
 	struct chamelium *chamelium;
@@ -49,7 +51,7 @@ typedef struct {
 
 	int drm_fd;
 
-	int edids[4];
+	int edids[TEST_EDID_COUNT];
 } data_t;
 
 #define HOTPLUG_TIMEOUT 20 /* seconds */
@@ -1997,6 +1999,8 @@ static const unsigned char *get_edid(enum test_edid edid)
 		return igt_kms_get_alt_edid();
 	case TEST_EDID_HDMI_AUDIO:
 		return igt_kms_get_hdmi_audio_edid();
+	case TEST_EDID_DP_AUDIO:
+		return igt_kms_get_dp_audio_edid();
 	}
 	assert(0); /* unreachable */
 }
@@ -2031,7 +2035,7 @@ igt_main
 						 &data.port_count);
 
 		data.edids[TEST_EDID_CHAMELIUM_DEFAULT] = CHAMELIUM_DEFAULT_EDID;
-		for (i = 1; i < sizeof(data.edids) / sizeof(data.edids[0]); ++i) {
+		for (i = 1; i < TEST_EDID_COUNT; ++i) {
 			data.edids[i] = chamelium_new_edid(data.chamelium,
 							   get_edid(i));
 		}
@@ -2113,7 +2117,7 @@ igt_main
 		 * Use the Chamelium's default EDID for DP audio. */
 		connector_subtest("dp-audio", DisplayPort)
 			test_display_audio(&data, port, "HDMI",
-					   TEST_EDID_CHAMELIUM_DEFAULT);
+					   TEST_EDID_DP_AUDIO);
 	}
 
 	igt_subtest_group {
diff --git a/tests/kms_hdmi_inject.c b/tests/kms_hdmi_inject.c
index 754a7407b441..8c0d1333db19 100644
--- a/tests/kms_hdmi_inject.c
+++ b/tests/kms_hdmi_inject.c
@@ -149,7 +149,7 @@ hdmi_inject_audio(int drm_fd, drmModeConnector *connector)
 	struct kmstest_connector_config config;
 
 	edid = igt_kms_get_hdmi_audio_edid();
-	length = HDMI_AUDIO_EDID_LENGTH;
+	length = AUDIO_EDID_LENGTH;
 
 	kmstest_force_edid(drm_fd, connector, edid, length);
 
-- 
2.21.0

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

[igt-dev] [PATCH i-g-t 2/3] lib/igt_eld: add eld_get_igt

[Simon Ser]

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

Signed-off-by: Simon Ser <simon.ser@intel.com>
---
 lib/igt_eld.c | 20 +++++++++++++-------
 lib/igt_eld.h |  1 +
 2 files changed, 14 insertions(+), 7 deletions(-)

diff --git a/lib/igt_eld.c b/lib/igt_eld.c
index 1bb294e3fb2a..7217962ee5ca 100644
--- a/lib/igt_eld.c
+++ b/lib/igt_eld.c
@@ -200,16 +200,14 @@ static bool eld_parse_entry(const char *path, struct eld_entry *eld)
 	return monitor_present;
 }
 
-/** eld_has_igt: check whether ALSA has detected the audio-capable IGT EDID by
- * parsing ELD entries */
-bool eld_has_igt(void)
+/** eld_get_igt: retrieve the ALSA ELD entry matching the IGT EDID */
+bool eld_get_igt(struct eld_entry *eld)
 {
 	DIR *dir;
 	struct dirent *dirent;
 	int i;
 	char card[64];
 	char path[PATH_MAX];
-	struct eld_entry eld;
 
 	for (i = 0; i < 8; i++) {
 		snprintf(card, sizeof(card), "/proc/asound/card%d", i);
@@ -224,16 +222,16 @@ bool eld_has_igt(void)
 
 			snprintf(path, sizeof(path), "%s/%s", card,
 				 dirent->d_name);
-			if (!eld_parse_entry(path, &eld)) {
+			if (!eld_parse_entry(path, eld)) {
 				continue;
 			}
 
-			if (!eld.valid) {
+			if (!eld->valid) {
 				igt_debug("Skipping invalid ELD: %s\n", path);
 				continue;
 			}
 
-			if (strcmp(eld.monitor_name, "IGT") == 0) {
+			if (strcmp(eld->monitor_name, "IGT") == 0) {
 				closedir(dir);
 				return true;
 			}
@@ -243,3 +241,11 @@ bool eld_has_igt(void)
 
 	return false;
 }
+
+/** eld_has_igt: check whether ALSA has detected the audio-capable IGT EDID by
+ * parsing ELD entries */
+bool eld_has_igt(void)
+{
+	struct eld_entry eld;
+	return eld_get_igt(&eld);
+}
diff --git a/lib/igt_eld.h b/lib/igt_eld.h
index e16187884d4b..7c4489f054f1 100644
--- a/lib/igt_eld.h
+++ b/lib/igt_eld.h
@@ -49,6 +49,7 @@ struct eld_entry {
 	struct eld_sad sads[ELD_SADS_CAP];
 };
 
+bool eld_get_igt(struct eld_entry *eld);
 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/3] 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.

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

diff --git a/tests/kms_chamelium.c b/tests/kms_chamelium.c
index 3beea0c623bd..1a5271bb770f 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>
@@ -1428,6 +1429,55 @@ test_display_audio(data_t *data, struct chamelium_port *port,
 	free(alsa);
 }
 
+static void
+test_display_audio_edid(data_t *data, struct chamelium_port *port,
+			enum test_edid edid)
+{
+	igt_output_t *output;
+	igt_plane_t *primary;
+	struct igt_fb fb;
+	drmModeModeInfo *mode;
+	drmModeConnector *connector;
+	int fb_id;
+	struct eld_entry eld;
+	struct eld_sad *sad;
+
+	reset_state(data, port);
+
+	output = prepare_output(data, port, edid);
+	connector = chamelium_port_get_connector(data->chamelium, port, false);
+	primary = igt_output_get_plane_type(output, DRM_PLANE_TYPE_PRIMARY);
+	igt_assert(primary);
+
+	/* Enable the output because audio cannot be played on inactive
+	 * connectors. */
+	igt_assert(connector->count_modes > 0);
+	mode = &connector->modes[0];
+
+	fb_id = igt_create_color_pattern_fb(data->drm_fd,
+					    mode->hdisplay, mode->vdisplay,
+					    DRM_FORMAT_XRGB8888,
+					    LOCAL_DRM_FORMAT_MOD_NONE,
+					    0, 0, 0, &fb);
+	igt_assert(fb_id > 0);
+
+	enable_output(data, port, output, mode, &fb);
+
+	igt_assert(eld_get_igt(&eld));
+	igt_assert(eld.sads_len == 1);
+
+	sad = &eld.sads[0];
+	igt_assert(sad->coding_type == CEA_SAD_FORMAT_PCM);
+	igt_assert(sad->channels == 2);
+	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,
@@ -2118,6 +2168,10 @@ igt_main
 		connector_subtest("dp-audio", DisplayPort)
 			test_display_audio(&data, port, "HDMI",
 					   TEST_EDID_DP_AUDIO);
+
+		connector_subtest("dp-audio-edid", DisplayPort)
+			test_display_audio_edid(&data, port,
+						TEST_EDID_DP_AUDIO);
 	}
 
 	igt_subtest_group {
@@ -2269,6 +2323,10 @@ igt_main
 		connector_subtest("hdmi-audio", HDMIA)
 			test_display_audio(&data, port, "HDMI",
 					   TEST_EDID_HDMI_AUDIO);
+
+		connector_subtest("hdmi-audio-edid", HDMIA)
+			test_display_audio_edid(&data, port,
+						TEST_EDID_HDMI_AUDIO);
 	}
 
 	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 (rev3)

[Patchwork]

== Series Details ==

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

== Summary ==

CI Bug Log - changes from CI_DRM_6206 -> IGTPW_3127
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

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

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

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

### IGT changes ###

#### Issues hit ####

  * igt@kms_cursor_legacy@basic-flip-after-cursor-legacy:
    - fi-icl-u3:          [PASS][1] -> [DMESG-WARN][2] ([fdo#107724]) +1 similar issue
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6206/fi-icl-u3/igt@kms_cursor_legacy@basic-flip-after-cursor-legacy.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3127/fi-icl-u3/igt@kms_cursor_legacy@basic-flip-after-cursor-legacy.html

  
#### Possible fixes ####

  * igt@kms_frontbuffer_tracking@basic:
    - fi-icl-u3:          [FAIL][3] ([fdo#103167]) -> [PASS][4]
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6206/fi-icl-u3/igt@kms_frontbuffer_tracking@basic.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3127/fi-icl-u3/igt@kms_frontbuffer_tracking@basic.html

  * igt@vgem_basic@unload:
    - fi-icl-u3:          [DMESG-WARN][5] ([fdo#107724]) -> [PASS][6]
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6206/fi-icl-u3/igt@vgem_basic@unload.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3127/fi-icl-u3/igt@vgem_basic@unload.html

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


Participating hosts (55 -> 47)
------------------------------

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


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

  * IGT: IGT_5043 -> IGTPW_3127

  CI_DRM_6206: 14ef563cbee376503d3551992d71f2f075e7462c @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_3127: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3127/
  IGT_5043: 3e2b20817b68ab41377c1b86207a1e7309fc3779 @ 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_3127/
_______________________________________________
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 (rev3)

[Patchwork]

== Series Details ==

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

== Summary ==

CI Bug Log - changes from CI_DRM_6206_full -> IGTPW_3127_full
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

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

New tests
---------

  New tests have been introduced between CI_DRM_6206_full and IGTPW_3127_full:

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

  * igt@kms_chamelium@dp-audio-edid:
    - Statuses : 5 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_3127_full that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@gem_eio@in-flight-suspend:
    - shard-kbl:          [PASS][1] -> [FAIL][2] ([fdo#110667])
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6206/shard-kbl7/igt@gem_eio@in-flight-suspend.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3127/shard-kbl7/igt@gem_eio@in-flight-suspend.html

  * igt@gem_mmap_gtt@forked-big-copy:
    - shard-iclb:         [PASS][3] -> [TIMEOUT][4] ([fdo#109673])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6206/shard-iclb1/igt@gem_mmap_gtt@forked-big-copy.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3127/shard-iclb5/igt@gem_mmap_gtt@forked-big-copy.html

  * igt@gem_tiled_swapping@non-threaded:
    - shard-apl:          [PASS][5] -> [DMESG-WARN][6] ([fdo#108686])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6206/shard-apl1/igt@gem_tiled_swapping@non-threaded.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3127/shard-apl5/igt@gem_tiled_swapping@non-threaded.html

  * igt@kms_color@pipe-a-ctm-blue-to-red:
    - shard-kbl:          [PASS][7] -> [FAIL][8] ([fdo#107201])
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6206/shard-kbl1/igt@kms_color@pipe-a-ctm-blue-to-red.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3127/shard-kbl4/igt@kms_color@pipe-a-ctm-blue-to-red.html
    - shard-apl:          [PASS][9] -> [FAIL][10] ([fdo#107201])
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6206/shard-apl7/igt@kms_color@pipe-a-ctm-blue-to-red.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3127/shard-apl1/igt@kms_color@pipe-a-ctm-blue-to-red.html

  * igt@kms_cursor_edge_walk@pipe-b-256x256-left-edge:
    - shard-snb:          [PASS][11] -> [SKIP][12] ([fdo#109271] / [fdo#109278])
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6206/shard-snb6/igt@kms_cursor_edge_walk@pipe-b-256x256-left-edge.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3127/shard-snb5/igt@kms_cursor_edge_walk@pipe-b-256x256-left-edge.html

  * igt@kms_flip@flip-vs-suspend-interruptible:
    - shard-apl:          [PASS][13] -> [DMESG-WARN][14] ([fdo#108566]) +2 similar issues
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6206/shard-apl1/igt@kms_flip@flip-vs-suspend-interruptible.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3127/shard-apl7/igt@kms_flip@flip-vs-suspend-interruptible.html

  * igt@kms_frontbuffer_tracking@fbc-1p-offscren-pri-indfb-draw-blt:
    - shard-snb:          [PASS][15] -> [SKIP][16] ([fdo#109271]) +1 similar issue
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6206/shard-snb4/igt@kms_frontbuffer_tracking@fbc-1p-offscren-pri-indfb-draw-blt.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3127/shard-snb5/igt@kms_frontbuffer_tracking@fbc-1p-offscren-pri-indfb-draw-blt.html

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-draw-render:
    - shard-apl:          [PASS][17] -> [FAIL][18] ([fdo#103167])
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6206/shard-apl8/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-draw-render.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3127/shard-apl1/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-draw-render.html
    - shard-kbl:          [PASS][19] -> [FAIL][20] ([fdo#103167])
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6206/shard-kbl6/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-draw-render.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3127/shard-kbl4/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-draw-render.html
    - shard-glk:          [PASS][21] -> [FAIL][22] ([fdo#103167])
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6206/shard-glk6/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-draw-render.html
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3127/shard-glk1/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-draw-render.html

  * igt@kms_frontbuffer_tracking@fbc-tilingchange:
    - shard-iclb:         [PASS][23] -> [INCOMPLETE][24] ([fdo#107713])
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6206/shard-iclb1/igt@kms_frontbuffer_tracking@fbc-tilingchange.html
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3127/shard-iclb1/igt@kms_frontbuffer_tracking@fbc-tilingchange.html

  * igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-pri-indfb-draw-blt:
    - shard-iclb:         [PASS][25] -> [FAIL][26] ([fdo#103167]) +6 similar issues
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6206/shard-iclb2/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-pri-indfb-draw-blt.html
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3127/shard-iclb2/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-pri-indfb-draw-blt.html

  * igt@kms_setmode@basic:
    - shard-apl:          [PASS][27] -> [FAIL][28] ([fdo#99912])
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6206/shard-apl1/igt@kms_setmode@basic.html
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3127/shard-apl8/igt@kms_setmode@basic.html

  
#### Possible fixes ####

  * {igt@gem_exec_balancer@bonded-imm}:
    - shard-iclb:         [FAIL][29] ([fdo#110851]) -> [PASS][30] +1 similar issue
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6206/shard-iclb1/igt@gem_exec_balancer@bonded-imm.html
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3127/shard-iclb6/igt@gem_exec_balancer@bonded-imm.html

  * igt@gem_mmap_gtt@forked-basic-small-copy-odd:
    - shard-iclb:         [INCOMPLETE][31] ([fdo#107713]) -> [PASS][32]
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6206/shard-iclb7/igt@gem_mmap_gtt@forked-basic-small-copy-odd.html
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3127/shard-iclb2/igt@gem_mmap_gtt@forked-basic-small-copy-odd.html

  * igt@gem_tiled_swapping@non-threaded:
    - shard-iclb:         [FAIL][33] ([fdo#108686]) -> [PASS][34]
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6206/shard-iclb4/igt@gem_tiled_swapping@non-threaded.html
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3127/shard-iclb3/igt@gem_tiled_swapping@non-threaded.html
    - shard-kbl:          [DMESG-WARN][35] ([fdo#108686]) -> [PASS][36]
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6206/shard-kbl3/igt@gem_tiled_swapping@non-threaded.html
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3127/shard-kbl6/igt@gem_tiled_swapping@non-threaded.html

  * igt@i915_pm_rpm@universal-planes:
    - shard-iclb:         [INCOMPLETE][37] ([fdo#107713] / [fdo#108840]) -> [PASS][38]
   [37]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6206/shard-iclb5/igt@i915_pm_rpm@universal-planes.html
   [38]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3127/shard-iclb2/igt@i915_pm_rpm@universal-planes.html

  * igt@i915_suspend@fence-restore-tiled2untiled:
    - shard-apl:          [DMESG-WARN][39] ([fdo#108566]) -> [PASS][40] +3 similar issues
   [39]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6206/shard-apl5/igt@i915_suspend@fence-restore-tiled2untiled.html
   [40]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3127/shard-apl1/igt@i915_suspend@fence-restore-tiled2untiled.html

  * igt@kms_cursor_crc@pipe-b-cursor-128x128-random:
    - shard-kbl:          [FAIL][41] ([fdo#103232]) -> [PASS][42]
   [41]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6206/shard-kbl2/igt@kms_cursor_crc@pipe-b-cursor-128x128-random.html
   [42]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3127/shard-kbl2/igt@kms_cursor_crc@pipe-b-cursor-128x128-random.html
    - shard-apl:          [FAIL][43] ([fdo#103232]) -> [PASS][44]
   [43]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6206/shard-apl3/igt@kms_cursor_crc@pipe-b-cursor-128x128-random.html
   [44]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3127/shard-apl8/igt@kms_cursor_crc@pipe-b-cursor-128x128-random.html

  * igt@kms_frontbuffer_tracking@fbc-rgb565-draw-pwrite:
    - shard-iclb:         [FAIL][45] ([fdo#103167]) -> [PASS][46] +3 similar issues
   [45]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6206/shard-iclb6/igt@kms_frontbuffer_tracking@fbc-rgb565-draw-pwrite.html
   [46]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3127/shard-iclb7/igt@kms_frontbuffer_tracking@fbc-rgb565-draw-pwrite.html

  * igt@kms_psr@psr2_cursor_mmap_gtt:
    - shard-iclb:         [SKIP][47] ([fdo#109441]) -> [PASS][48]
   [47]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6206/shard-iclb4/igt@kms_psr@psr2_cursor_mmap_gtt.html
   [48]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3127/shard-iclb2/igt@kms_psr@psr2_cursor_mmap_gtt.html

  * igt@kms_setmode@basic:
    - shard-kbl:          [FAIL][49] ([fdo#99912]) -> [PASS][50]
   [49]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6206/shard-kbl7/igt@kms_setmode@basic.html
   [50]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3127/shard-kbl6/igt@kms_setmode@basic.html

  * igt@perf@polling:
    - shard-iclb:         [FAIL][51] ([fdo#110728]) -> [PASS][52]
   [51]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6206/shard-iclb7/igt@perf@polling.html
   [52]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3127/shard-iclb7/igt@perf@polling.html

  
#### Warnings ####

  * igt@gem_mmap_gtt@forked-big-copy-xy:
    - shard-iclb:         [INCOMPLETE][53] ([fdo#107713] / [fdo#109100]) -> [TIMEOUT][54] ([fdo#109673])
   [53]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6206/shard-iclb6/igt@gem_mmap_gtt@forked-big-copy-xy.html
   [54]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3127/shard-iclb2/igt@gem_mmap_gtt@forked-big-copy-xy.html

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

  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#103232]: https://bugs.freedesktop.org/show_bug.cgi?id=103232
  [fdo#107201]: https://bugs.freedesktop.org/show_bug.cgi?id=107201
  [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#108840]: https://bugs.freedesktop.org/show_bug.cgi?id=108840
  [fdo#109100]: https://bugs.freedesktop.org/show_bug.cgi?id=109100
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109278]: https://bugs.freedesktop.org/show_bug.cgi?id=109278
  [fdo#109441]: https://bugs.freedesktop.org/show_bug.cgi?id=109441
  [fdo#109673]: https://bugs.freedesktop.org/show_bug.cgi?id=109673
  [fdo#110667]: https://bugs.freedesktop.org/show_bug.cgi?id=110667
  [fdo#110728]: https://bugs.freedesktop.org/show_bug.cgi?id=110728
  [fdo#110851]: https://bugs.freedesktop.org/show_bug.cgi?id=110851
  [fdo#99912]: https://bugs.freedesktop.org/show_bug.cgi?id=99912


Participating hosts (9 -> 5)
------------------------------

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


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

  * IGT: IGT_5043 -> IGTPW_3127
  * Piglit: piglit_4509 -> None

  CI_DRM_6206: 14ef563cbee376503d3551992d71f2f075e7462c @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_3127: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_3127/
  IGT_5043: 3e2b20817b68ab41377c1b86207a1e7309fc3779 @ 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_3127/
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t 1/3] lib/igt_kms: generate an EDID suitable for DP audio

[Martin Peres]

On 06/06/2019 14:48, Simon Ser wrote:
> Prior to this commit, we were using the Chamelium's default EDID for DP audio.
> This relies on the fact that this EDID supports audio and has correct audio
> paremeters for our testing.
> 
> Generating our own EDID is less error-prone and will allow us to test different
> audio parameters.
> 
> v2:
> - Replace {HDMI,DP}_AUDIO_EDID_LENGTH with AUDIO_EDID_LENGTH (Arek)
> - Don't hardcode EDIDs array length (Arek)
> 
> Signed-off-by: Simon Ser <simon.ser@intel.com>
> ---
>  lib/igt_kms.c           | 46 +++++++++++++++++++++++++++++++++--------
>  lib/igt_kms.h           |  3 ++-
>  tests/kms_chamelium.c   | 10 ++++++---
>  tests/kms_hdmi_inject.c |  2 +-
>  4 files changed, 47 insertions(+), 14 deletions(-)
> 
> diff --git a/lib/igt_kms.c b/lib/igt_kms.c
> index d7d711a72d27..011b064579a8 100644
> --- a/lib/igt_kms.c
> +++ b/lib/igt_kms.c
> @@ -183,9 +183,9 @@ const unsigned char *igt_kms_get_alt_edid(void)
>  }
>  
>  static void
> -generate_hdmi_audio_edid(unsigned char raw_edid[static HDMI_AUDIO_EDID_LENGTH],
> -			 struct cea_sad *sad,
> -			 struct cea_speaker_alloc *speaker_alloc)
> +generate_audio_edid(unsigned char raw_edid[static AUDIO_EDID_LENGTH],
> +		    bool with_vsd, struct cea_sad *sad,

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

> +		    struct cea_speaker_alloc *speaker_alloc)
>  {
>  	struct edid *edid;
>  	struct edid_ext *edid_ext;
> @@ -210,10 +210,12 @@ generate_hdmi_audio_edid(unsigned char raw_edid[static HDMI_AUDIO_EDID_LENGTH],
>  	cea_data_size += edid_cea_data_block_set_sad(block, sad, 1);
>  
>  	/* A Vendor Specific Data block is needed for HDMI audio */
> -	block = (struct edid_cea_data_block *) &cea_data[cea_data_size];
> -	vsd = cea_vsd_get_hdmi_default(&vsd_size);
> -	cea_data_size += edid_cea_data_block_set_vsd(block, vsd,
> -						     vsd_size);
> +	if (with_vsd) {
> +		block = (struct edid_cea_data_block *) &cea_data[cea_data_size];
> +		vsd = cea_vsd_get_hdmi_default(&vsd_size);
> +		cea_data_size += edid_cea_data_block_set_vsd(block, vsd,
> +							     vsd_size);
> +	}
>  
>  	/* Speaker Allocation Data block */
>  	block = (struct edid_cea_data_block *) &cea_data[cea_data_size];
> @@ -233,7 +235,33 @@ const unsigned char *igt_kms_get_hdmi_audio_edid(void)
>  {
>  	int channels;
>  	uint8_t sampling_rates, sample_sizes;
> -	static unsigned char raw_edid[HDMI_AUDIO_EDID_LENGTH] = {0};
> +	static unsigned char raw_edid[AUDIO_EDID_LENGTH] = {0};
> +	struct cea_sad sad = {0};
> +	struct cea_speaker_alloc speaker_alloc = {0};
> +
> +	/* Initialize the Short Audio Descriptor for PCM */
> +	channels = 2;
> +	sampling_rates = CEA_SAD_SAMPLING_RATE_32KHZ |
> +			 CEA_SAD_SAMPLING_RATE_44KHZ |
> +			 CEA_SAD_SAMPLING_RATE_48KHZ;
> +	sample_sizes = CEA_SAD_SAMPLE_SIZE_16 |
> +		       CEA_SAD_SAMPLE_SIZE_20 |
> +		       CEA_SAD_SAMPLE_SIZE_24;
> +	cea_sad_init_pcm(&sad, channels, sampling_rates, sample_sizes);
> +
> +	/* Initialize the Speaker Allocation Data */
> +	speaker_alloc.speakers = CEA_SPEAKER_FRONT_LEFT_RIGHT_CENTER;
> +
> +	generate_audio_edid(raw_edid, true, &sad, &speaker_alloc);
> +
> +	return raw_edid;
> +}
> +
> +const unsigned char *igt_kms_get_dp_audio_edid(void)
> +{
> +	int channels;
> +	uint8_t sampling_rates, sample_sizes;
> +	static unsigned char raw_edid[AUDIO_EDID_LENGTH] = {0};
>  	struct cea_sad sad = {0};
>  	struct cea_speaker_alloc speaker_alloc = {0};
>  
> @@ -250,7 +278,7 @@ const unsigned char *igt_kms_get_hdmi_audio_edid(void)
>  	/* Initialize the Speaker Allocation Data */
>  	speaker_alloc.speakers = CEA_SPEAKER_FRONT_LEFT_RIGHT_CENTER;
>  
> -	generate_hdmi_audio_edid(raw_edid, &sad, &speaker_alloc);
> +	generate_audio_edid(raw_edid, false, &sad, &speaker_alloc);
>  
>  	return raw_edid;
>  }
> diff --git a/lib/igt_kms.h b/lib/igt_kms.h
> index 4ac28131b6d9..5df9f18c0bd1 100644
> --- a/lib/igt_kms.h
> +++ b/lib/igt_kms.h
> @@ -757,10 +757,11 @@ struct cea_sad;
>  struct cea_speaker_alloc;
>  
>  #define EDID_LENGTH 128
> -#define HDMI_AUDIO_EDID_LENGTH (2 * EDID_LENGTH)
> +#define AUDIO_EDID_LENGTH (2 * EDID_LENGTH)
>  const unsigned char *igt_kms_get_base_edid(void);
>  const unsigned char *igt_kms_get_alt_edid(void);
>  const unsigned char *igt_kms_get_hdmi_audio_edid(void);
> +const unsigned char *igt_kms_get_dp_audio_edid(void);
>  
>  struct udev_monitor *igt_watch_hotplug(void);
>  bool igt_hotplug_detected(struct udev_monitor *mon,
> diff --git a/tests/kms_chamelium.c b/tests/kms_chamelium.c
> index 52e3f0953621..3beea0c623bd 100644
> --- a/tests/kms_chamelium.c
> +++ b/tests/kms_chamelium.c
> @@ -39,7 +39,9 @@ enum test_edid {
>  	TEST_EDID_BASE,
>  	TEST_EDID_ALT,
>  	TEST_EDID_HDMI_AUDIO,
> +	TEST_EDID_DP_AUDIO,

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

With this particular change done, this is :
Reviewed-by: Martin Peres <martin.peres@linux.intel.com>

The rename is up to you.

>  };
> +#define TEST_EDID_COUNT 5
>  
>  typedef struct {
>  	struct chamelium *chamelium;
> @@ -49,7 +51,7 @@ typedef struct {
>  
>  	int drm_fd;
>  
> -	int edids[4];
> +	int edids[TEST_EDID_COUNT];
>  } data_t;
>  
>  #define HOTPLUG_TIMEOUT 20 /* seconds */
> @@ -1997,6 +1999,8 @@ static const unsigned char *get_edid(enum test_edid edid)
>  		return igt_kms_get_alt_edid();
>  	case TEST_EDID_HDMI_AUDIO:
>  		return igt_kms_get_hdmi_audio_edid();
> +	case TEST_EDID_DP_AUDIO:
> +		return igt_kms_get_dp_audio_edid();
>  	}
>  	assert(0); /* unreachable */
>  }
> @@ -2031,7 +2035,7 @@ igt_main
>  						 &data.port_count);
>  
>  		data.edids[TEST_EDID_CHAMELIUM_DEFAULT] = CHAMELIUM_DEFAULT_EDID;
> -		for (i = 1; i < sizeof(data.edids) / sizeof(data.edids[0]); ++i) {
> +		for (i = 1; i < TEST_EDID_COUNT; ++i) {
>  			data.edids[i] = chamelium_new_edid(data.chamelium,
>  							   get_edid(i));
>  		}
> @@ -2113,7 +2117,7 @@ igt_main
>  		 * Use the Chamelium's default EDID for DP audio. */
>  		connector_subtest("dp-audio", DisplayPort)
>  			test_display_audio(&data, port, "HDMI",
> -					   TEST_EDID_CHAMELIUM_DEFAULT);
> +					   TEST_EDID_DP_AUDIO);
>  	}
>  
>  	igt_subtest_group {
> diff --git a/tests/kms_hdmi_inject.c b/tests/kms_hdmi_inject.c
> index 754a7407b441..8c0d1333db19 100644
> --- a/tests/kms_hdmi_inject.c
> +++ b/tests/kms_hdmi_inject.c
> @@ -149,7 +149,7 @@ hdmi_inject_audio(int drm_fd, drmModeConnector *connector)
>  	struct kmstest_connector_config config;
>  
>  	edid = igt_kms_get_hdmi_audio_edid();
> -	length = HDMI_AUDIO_EDID_LENGTH;
> +	length = AUDIO_EDID_LENGTH;
>  
>  	kmstest_force_edid(drm_fd, connector, edid, length);
>  
> 
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t 2/3] lib/igt_eld: add eld_get_igt

[Martin Peres]

On 06/06/2019 14:48, Simon Ser wrote:
> The existing eld_has_igt function doesn't allow the caller to retrieve the
> parsed ELD and check audio parameters.
> 
> Signed-off-by: Simon Ser <simon.ser@intel.com>

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

> ---
>  lib/igt_eld.c | 20 +++++++++++++-------
>  lib/igt_eld.h |  1 +
>  2 files changed, 14 insertions(+), 7 deletions(-)
> 
> diff --git a/lib/igt_eld.c b/lib/igt_eld.c
> index 1bb294e3fb2a..7217962ee5ca 100644
> --- a/lib/igt_eld.c
> +++ b/lib/igt_eld.c
> @@ -200,16 +200,14 @@ static bool eld_parse_entry(const char *path, struct eld_entry *eld)
>  	return monitor_present;
>  }
>  
> -/** eld_has_igt: check whether ALSA has detected the audio-capable IGT EDID by
> - * parsing ELD entries */
> -bool eld_has_igt(void)
> +/** eld_get_igt: retrieve the ALSA ELD entry matching the IGT EDID */
> +bool eld_get_igt(struct eld_entry *eld)
>  {
>  	DIR *dir;
>  	struct dirent *dirent;
>  	int i;
>  	char card[64];
>  	char path[PATH_MAX];
> -	struct eld_entry eld;
>  
>  	for (i = 0; i < 8; i++) {
>  		snprintf(card, sizeof(card), "/proc/asound/card%d", i);
> @@ -224,16 +222,16 @@ bool eld_has_igt(void)
>  
>  			snprintf(path, sizeof(path), "%s/%s", card,
>  				 dirent->d_name);
> -			if (!eld_parse_entry(path, &eld)) {
> +			if (!eld_parse_entry(path, eld)) {
>  				continue;
>  			}
>  
> -			if (!eld.valid) {
> +			if (!eld->valid) {
>  				igt_debug("Skipping invalid ELD: %s\n", path);
>  				continue;
>  			}
>  
> -			if (strcmp(eld.monitor_name, "IGT") == 0) {
> +			if (strcmp(eld->monitor_name, "IGT") == 0) {
>  				closedir(dir);
>  				return true;
>  			}
> @@ -243,3 +241,11 @@ bool eld_has_igt(void)
>  
>  	return false;
>  }
> +
> +/** eld_has_igt: check whether ALSA has detected the audio-capable IGT EDID by
> + * parsing ELD entries */
> +bool eld_has_igt(void)
> +{
> +	struct eld_entry eld;
> +	return eld_get_igt(&eld);
> +}
> diff --git a/lib/igt_eld.h b/lib/igt_eld.h
> index e16187884d4b..7c4489f054f1 100644
> --- a/lib/igt_eld.h
> +++ b/lib/igt_eld.h
> @@ -49,6 +49,7 @@ struct eld_entry {
>  	struct eld_sad sads[ELD_SADS_CAP];
>  };
>  
> +bool eld_get_igt(struct eld_entry *eld);
>  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/3] tests/kms_chamelium: add an audio EDID test

[Martin Peres]

On 06/06/2019 14:48, 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.


> 
> Signed-off-by: Simon Ser <simon.ser@intel.com>
> ---
>  tests/kms_chamelium.c | 58 +++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 58 insertions(+)
> 
> diff --git a/tests/kms_chamelium.c b/tests/kms_chamelium.c
> index 3beea0c623bd..1a5271bb770f 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>
> @@ -1428,6 +1429,55 @@ test_display_audio(data_t *data, struct chamelium_port *port,
>  	free(alsa);
>  }
>  
> +static void
> +test_display_audio_edid(data_t *data, struct chamelium_port *port,
> +			enum test_edid edid)
> +{
> +	igt_output_t *output;
> +	igt_plane_t *primary;
> +	struct igt_fb fb;
> +	drmModeModeInfo *mode;
> +	drmModeConnector *connector;
> +	int fb_id;
> +	struct eld_entry eld;
> +	struct eld_sad *sad;
> +
> +	reset_state(data, port);
> +
> +	output = prepare_output(data, port, edid);
> +	connector = chamelium_port_get_connector(data->chamelium, port, false);
> +	primary = igt_output_get_plane_type(output, DRM_PLANE_TYPE_PRIMARY);
> +	igt_assert(primary);
> +
> +	/* Enable the output because audio cannot be played on inactive
> +	 * connectors. */
> +	igt_assert(connector->count_modes > 0);
> +	mode = &connector->modes[0];
> +
> +	fb_id = igt_create_color_pattern_fb(data->drm_fd,
> +					    mode->hdisplay, mode->vdisplay,
> +					    DRM_FORMAT_XRGB8888,
> +					    LOCAL_DRM_FORMAT_MOD_NONE,
> +					    0, 0, 0, &fb);
> +	igt_assert(fb_id > 0);
> +
> +	enable_output(data, port, output, mode, &fb);
> +
> +	igt_assert(eld_get_igt(&eld));
> +	igt_assert(eld.sads_len == 1);
> +
> +	sad = &eld.sads[0];
> +	igt_assert(sad->coding_type == CEA_SAD_FORMAT_PCM);
> +	igt_assert(sad->channels == 2);
> +	igt_assert(sad->rates == (CEA_SAD_SAMPLING_RATE_32KHZ |
> +		   CEA_SAD_SAMPLING_RATE_44KHZ | CEA_SAD_SAMPLING_RATE_48KHZ));
> +	igt_assert(sad->bits == (CEA_SAD_SAMPLE_SIZE_16 |
> +		   CEA_SAD_SAMPLE_SIZE_20 | CEA_SAD_SAMPLE_SIZE_24));

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

With channels = 8 and a couple of other coding_types, the patch is:
Reviewed-by: Martin Peres <martin.peres@linux.intel.com>

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

Martin
> +
> +	igt_remove_fb(data->drm_fd, &fb);
> +
> +	drmModeFreeConnector(connector);
> +}
>  
>  static void randomize_plane_stride(data_t *data,
>  				   uint32_t width, uint32_t height,
> @@ -2118,6 +2168,10 @@ igt_main
>  		connector_subtest("dp-audio", DisplayPort)
>  			test_display_audio(&data, port, "HDMI",
>  					   TEST_EDID_DP_AUDIO);
> +
> +		connector_subtest("dp-audio-edid", DisplayPort)
> +			test_display_audio_edid(&data, port,
> +						TEST_EDID_DP_AUDIO);
>  	}
>  
>  	igt_subtest_group {
> @@ -2269,6 +2323,10 @@ igt_main
>  		connector_subtest("hdmi-audio", HDMIA)
>  			test_display_audio(&data, port, "HDMI",
>  					   TEST_EDID_HDMI_AUDIO);
> +
> +		connector_subtest("hdmi-audio-edid", HDMIA)
> +			test_display_audio_edid(&data, port,
> +						TEST_EDID_HDMI_AUDIO);
>  	}
>  
>  	igt_subtest_group {
> 
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t 1/3] lib/igt_kms: generate an EDID suitable for DP audio

[Ser, Simon]

On Wed, 2019-06-12 at 13:31 +0300, Martin Peres wrote:
> On 06/06/2019 14:48, Simon Ser wrote:
> > Prior to this commit, we were using the Chamelium's default EDID for DP audio.
> > This relies on the fact that this EDID supports audio and has correct audio
> > paremeters for our testing.
> > 
> > Generating our own EDID is less error-prone and will allow us to test different
> > audio parameters.
> > 
> > v2:
> > - Replace {HDMI,DP}_AUDIO_EDID_LENGTH with AUDIO_EDID_LENGTH (Arek)
> > - Don't hardcode EDIDs array length (Arek)
> > 
> > Signed-off-by: Simon Ser <simon.ser@intel.com>
> > ---
> >  lib/igt_kms.c           | 46 +++++++++++++++++++++++++++++++++--------
> >  lib/igt_kms.h           |  3 ++-
> >  tests/kms_chamelium.c   | 10 ++++++---
> >  tests/kms_hdmi_inject.c |  2 +-
> >  4 files changed, 47 insertions(+), 14 deletions(-)
> > 
> > diff --git a/lib/igt_kms.c b/lib/igt_kms.c
> > index d7d711a72d27..011b064579a8 100644
> > --- a/lib/igt_kms.c
> > +++ b/lib/igt_kms.c
> > @@ -183,9 +183,9 @@ const unsigned char *igt_kms_get_alt_edid(void)
> >  }
> >  
> >  static void
> > -generate_hdmi_audio_edid(unsigned char raw_edid[static HDMI_AUDIO_EDID_LENGTH],
> > -			 struct cea_sad *sad,
> > -			 struct cea_speaker_alloc *speaker_alloc)
> > +generate_audio_edid(unsigned char raw_edid[static AUDIO_EDID_LENGTH],
> > +		    bool with_vsd, struct cea_sad *sad,
> 
> with_vsd is not exactly super clear as to what it does, right? How about
> with_hdmi_audio_vsd?

Makes sense.

> > +		    struct cea_speaker_alloc *speaker_alloc)
> >  {
> >  	struct edid *edid;
> >  	struct edid_ext *edid_ext;
> > @@ -210,10 +210,12 @@ generate_hdmi_audio_edid(unsigned char raw_edid[static HDMI_AUDIO_EDID_LENGTH],
> >  	cea_data_size += edid_cea_data_block_set_sad(block, sad, 1);
> >  
> >  	/* A Vendor Specific Data block is needed for HDMI audio */
> > -	block = (struct edid_cea_data_block *) &cea_data[cea_data_size];
> > -	vsd = cea_vsd_get_hdmi_default(&vsd_size);
> > -	cea_data_size += edid_cea_data_block_set_vsd(block, vsd,
> > -						     vsd_size);
> > +	if (with_vsd) {
> > +		block = (struct edid_cea_data_block *) &cea_data[cea_data_size];
> > +		vsd = cea_vsd_get_hdmi_default(&vsd_size);
> > +		cea_data_size += edid_cea_data_block_set_vsd(block, vsd,
> > +							     vsd_size);
> > +	}
> >  
> >  	/* Speaker Allocation Data block */
> >  	block = (struct edid_cea_data_block *) &cea_data[cea_data_size];
> > @@ -233,7 +235,33 @@ const unsigned char *igt_kms_get_hdmi_audio_edid(void)
> >  {
> >  	int channels;
> >  	uint8_t sampling_rates, sample_sizes;
> > -	static unsigned char raw_edid[HDMI_AUDIO_EDID_LENGTH] = {0};
> > +	static unsigned char raw_edid[AUDIO_EDID_LENGTH] = {0};
> > +	struct cea_sad sad = {0};
> > +	struct cea_speaker_alloc speaker_alloc = {0};
> > +
> > +	/* Initialize the Short Audio Descriptor for PCM */
> > +	channels = 2;
> > +	sampling_rates = CEA_SAD_SAMPLING_RATE_32KHZ |
> > +			 CEA_SAD_SAMPLING_RATE_44KHZ |
> > +			 CEA_SAD_SAMPLING_RATE_48KHZ;
> > +	sample_sizes = CEA_SAD_SAMPLE_SIZE_16 |
> > +		       CEA_SAD_SAMPLE_SIZE_20 |
> > +		       CEA_SAD_SAMPLE_SIZE_24;
> > +	cea_sad_init_pcm(&sad, channels, sampling_rates, sample_sizes);
> > +
> > +	/* Initialize the Speaker Allocation Data */
> > +	speaker_alloc.speakers = CEA_SPEAKER_FRONT_LEFT_RIGHT_CENTER;
> > +
> > +	generate_audio_edid(raw_edid, true, &sad, &speaker_alloc);
> > +
> > +	return raw_edid;
> > +}
> > +
> > +const unsigned char *igt_kms_get_dp_audio_edid(void)
> > +{
> > +	int channels;
> > +	uint8_t sampling_rates, sample_sizes;
> > +	static unsigned char raw_edid[AUDIO_EDID_LENGTH] = {0};
> >  	struct cea_sad sad = {0};
> >  	struct cea_speaker_alloc speaker_alloc = {0};
> >  
> > @@ -250,7 +278,7 @@ const unsigned char *igt_kms_get_hdmi_audio_edid(void)
> >  	/* Initialize the Speaker Allocation Data */
> >  	speaker_alloc.speakers = CEA_SPEAKER_FRONT_LEFT_RIGHT_CENTER;
> >  
> > -	generate_hdmi_audio_edid(raw_edid, &sad, &speaker_alloc);
> > +	generate_audio_edid(raw_edid, false, &sad, &speaker_alloc);
> >  
> >  	return raw_edid;
> >  }
> > diff --git a/lib/igt_kms.h b/lib/igt_kms.h
> > index 4ac28131b6d9..5df9f18c0bd1 100644
> > --- a/lib/igt_kms.h
> > +++ b/lib/igt_kms.h
> > @@ -757,10 +757,11 @@ struct cea_sad;
> >  struct cea_speaker_alloc;
> >  
> >  #define EDID_LENGTH 128
> > -#define HDMI_AUDIO_EDID_LENGTH (2 * EDID_LENGTH)
> > +#define AUDIO_EDID_LENGTH (2 * EDID_LENGTH)
> >  const unsigned char *igt_kms_get_base_edid(void);
> >  const unsigned char *igt_kms_get_alt_edid(void);
> >  const unsigned char *igt_kms_get_hdmi_audio_edid(void);
> > +const unsigned char *igt_kms_get_dp_audio_edid(void);
> >  
> >  struct udev_monitor *igt_watch_hotplug(void);
> >  bool igt_hotplug_detected(struct udev_monitor *mon,
> > diff --git a/tests/kms_chamelium.c b/tests/kms_chamelium.c
> > index 52e3f0953621..3beea0c623bd 100644
> > --- a/tests/kms_chamelium.c
> > +++ b/tests/kms_chamelium.c
> > @@ -39,7 +39,9 @@ enum test_edid {
> >  	TEST_EDID_BASE,
> >  	TEST_EDID_ALT,
> >  	TEST_EDID_HDMI_AUDIO,
> > +	TEST_EDID_DP_AUDIO,
> 
> Move TEST_EDID_COUNT here, so as it will automatically get the right
> value when you change the amount of tests.

The issue with this approach is that I'll then need to handle the
invalid TEST_EDID_COUNT enum item in each switch using this enum. Also
from a semantic point-of-view, the count is not a valid enum item.

I also agree that having to keep the count in sync is annoying. Both
solutions are annoying. :(

I haven't found a perfect solution for this issue yet. Suggestions
welcome, if you have any.

> With this particular change done, this is :
> Reviewed-by: Martin Peres <martin.peres@linux.intel.com>
> 
> The rename is up to you.
> 
> >  };
> > +#define TEST_EDID_COUNT 5
> >  
> >  typedef struct {
> >  	struct chamelium *chamelium;
> > @@ -49,7 +51,7 @@ typedef struct {
> >  
> >  	int drm_fd;
> >  
> > -	int edids[4];
> > +	int edids[TEST_EDID_COUNT];
> >  } data_t;
> >  
> >  #define HOTPLUG_TIMEOUT 20 /* seconds */
> > @@ -1997,6 +1999,8 @@ static const unsigned char *get_edid(enum test_edid edid)
> >  		return igt_kms_get_alt_edid();
> >  	case TEST_EDID_HDMI_AUDIO:
> >  		return igt_kms_get_hdmi_audio_edid();
> > +	case TEST_EDID_DP_AUDIO:
> > +		return igt_kms_get_dp_audio_edid();
> >  	}
> >  	assert(0); /* unreachable */
> >  }
> > @@ -2031,7 +2035,7 @@ igt_main
> >  						 &data.port_count);
> >  
> >  		data.edids[TEST_EDID_CHAMELIUM_DEFAULT] = CHAMELIUM_DEFAULT_EDID;
> > -		for (i = 1; i < sizeof(data.edids) / sizeof(data.edids[0]); ++i) {
> > +		for (i = 1; i < TEST_EDID_COUNT; ++i) {
> >  			data.edids[i] = chamelium_new_edid(data.chamelium,
> >  							   get_edid(i));
> >  		}
> > @@ -2113,7 +2117,7 @@ igt_main
> >  		 * Use the Chamelium's default EDID for DP audio. */
> >  		connector_subtest("dp-audio", DisplayPort)
> >  			test_display_audio(&data, port, "HDMI",
> > -					   TEST_EDID_CHAMELIUM_DEFAULT);
> > +					   TEST_EDID_DP_AUDIO);
> >  	}
> >  
> >  	igt_subtest_group {
> > diff --git a/tests/kms_hdmi_inject.c b/tests/kms_hdmi_inject.c
> > index 754a7407b441..8c0d1333db19 100644
> > --- a/tests/kms_hdmi_inject.c
> > +++ b/tests/kms_hdmi_inject.c
> > @@ -149,7 +149,7 @@ hdmi_inject_audio(int drm_fd, drmModeConnector *connector)
> >  	struct kmstest_connector_config config;
> >  
> >  	edid = igt_kms_get_hdmi_audio_edid();
> > -	length = HDMI_AUDIO_EDID_LENGTH;
> > +	length = AUDIO_EDID_LENGTH;
> >  
> >  	kmstest_force_edid(drm_fd, connector, edid, length);
> >  
> > 
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

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

[Ser, Simon]

On Wed, 2019-06-12 at 13:47 +0300, Martin Peres wrote:
> On 06/06/2019 14:48, 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.
> > Signed-off-by: Simon Ser <simon.ser@intel.com>
> > ---
> >  tests/kms_chamelium.c | 58 +++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 58 insertions(+)
> > 
> > diff --git a/tests/kms_chamelium.c b/tests/kms_chamelium.c
> > index 3beea0c623bd..1a5271bb770f 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>
> > @@ -1428,6 +1429,55 @@ test_display_audio(data_t *data, struct chamelium_port *port,
> >  	free(alsa);
> >  }
> >  
> > +static void
> > +test_display_audio_edid(data_t *data, struct chamelium_port *port,
> > +			enum test_edid edid)
> > +{
> > +	igt_output_t *output;
> > +	igt_plane_t *primary;
> > +	struct igt_fb fb;
> > +	drmModeModeInfo *mode;
> > +	drmModeConnector *connector;
> > +	int fb_id;
> > +	struct eld_entry eld;
> > +	struct eld_sad *sad;
> > +
> > +	reset_state(data, port);
> > +
> > +	output = prepare_output(data, port, edid);
> > +	connector = chamelium_port_get_connector(data->chamelium, port, false);
> > +	primary = igt_output_get_plane_type(output, DRM_PLANE_TYPE_PRIMARY);
> > +	igt_assert(primary);
> > +
> > +	/* Enable the output because audio cannot be played on inactive
> > +	 * connectors. */
> > +	igt_assert(connector->count_modes > 0);
> > +	mode = &connector->modes[0];
> > +
> > +	fb_id = igt_create_color_pattern_fb(data->drm_fd,
> > +					    mode->hdisplay, mode->vdisplay,
> > +					    DRM_FORMAT_XRGB8888,
> > +					    LOCAL_DRM_FORMAT_MOD_NONE,
> > +					    0, 0, 0, &fb);
> > +	igt_assert(fb_id > 0);
> > +
> > +	enable_output(data, port, output, mode, &fb);
> > +
> > +	igt_assert(eld_get_igt(&eld));
> > +	igt_assert(eld.sads_len == 1);
> > +
> > +	sad = &eld.sads[0];
> > +	igt_assert(sad->coding_type == CEA_SAD_FORMAT_PCM);
> > +	igt_assert(sad->channels == 2);
> > +	igt_assert(sad->rates == (CEA_SAD_SAMPLING_RATE_32KHZ |
> > +		   CEA_SAD_SAMPLING_RATE_44KHZ | CEA_SAD_SAMPLING_RATE_48KHZ));
> > +	igt_assert(sad->bits == (CEA_SAD_SAMPLE_SIZE_16 |
> > +		   CEA_SAD_SAMPLE_SIZE_20 | CEA_SAD_SAMPLE_SIZE_24));
> 
> If we have only one tests for one EDID, I would suggest trying more
> coding_types and channels to make it less typical (we want to detect
> hardcoded values).

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

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

> With channels = 8 and a couple of other coding_types, the patch is:
> Reviewed-by: Martin Peres <martin.peres@linux.intel.com>
> 
> Any plans on generating broken ELDs and seeing how Alsa parses that?

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

> Martin
> > +
> > +	igt_remove_fb(data->drm_fd, &fb);
> > +
> > +	drmModeFreeConnector(connector);
> > +}
> >  
> >  static void randomize_plane_stride(data_t *data,
> >  				   uint32_t width, uint32_t height,
> > @@ -2118,6 +2168,10 @@ igt_main
> >  		connector_subtest("dp-audio", DisplayPort)
> >  			test_display_audio(&data, port, "HDMI",
> >  					   TEST_EDID_DP_AUDIO);
> > +
> > +		connector_subtest("dp-audio-edid", DisplayPort)
> > +			test_display_audio_edid(&data, port,
> > +						TEST_EDID_DP_AUDIO);
> >  	}
> >  
> >  	igt_subtest_group {
> > @@ -2269,6 +2323,10 @@ igt_main
> >  		connector_subtest("hdmi-audio", HDMIA)
> >  			test_display_audio(&data, port, "HDMI",
> >  					   TEST_EDID_HDMI_AUDIO);
> > +
> > +		connector_subtest("hdmi-audio-edid", HDMIA)
> > +			test_display_audio_edid(&data, port,
> > +						TEST_EDID_HDMI_AUDIO);
> >  	}
> >  
> >  	igt_subtest_group {
> > 
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

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

[Martin Peres]

On 12/06/2019 14:35, Ser, Simon wrote:
> On Wed, 2019-06-12 at 13:47 +0300, Martin Peres wrote:
>> On 06/06/2019 14:48, 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.
>>> Signed-off-by: Simon Ser <simon.ser@intel.com>
>>> ---
>>>  tests/kms_chamelium.c | 58 +++++++++++++++++++++++++++++++++++++++++++
>>>  1 file changed, 58 insertions(+)
>>>
>>> diff --git a/tests/kms_chamelium.c b/tests/kms_chamelium.c
>>> index 3beea0c623bd..1a5271bb770f 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>
>>> @@ -1428,6 +1429,55 @@ test_display_audio(data_t *data, struct chamelium_port *port,
>>>  	free(alsa);
>>>  }
>>>  
>>> +static void
>>> +test_display_audio_edid(data_t *data, struct chamelium_port *port,
>>> +			enum test_edid edid)
>>> +{
>>> +	igt_output_t *output;
>>> +	igt_plane_t *primary;
>>> +	struct igt_fb fb;
>>> +	drmModeModeInfo *mode;
>>> +	drmModeConnector *connector;
>>> +	int fb_id;
>>> +	struct eld_entry eld;
>>> +	struct eld_sad *sad;
>>> +
>>> +	reset_state(data, port);
>>> +
>>> +	output = prepare_output(data, port, edid);
>>> +	connector = chamelium_port_get_connector(data->chamelium, port, false);
>>> +	primary = igt_output_get_plane_type(output, DRM_PLANE_TYPE_PRIMARY);
>>> +	igt_assert(primary);
>>> +
>>> +	/* Enable the output because audio cannot be played on inactive
>>> +	 * connectors. */
>>> +	igt_assert(connector->count_modes > 0);
>>> +	mode = &connector->modes[0];
>>> +
>>> +	fb_id = igt_create_color_pattern_fb(data->drm_fd,
>>> +					    mode->hdisplay, mode->vdisplay,
>>> +					    DRM_FORMAT_XRGB8888,
>>> +					    LOCAL_DRM_FORMAT_MOD_NONE,
>>> +					    0, 0, 0, &fb);
>>> +	igt_assert(fb_id > 0);
>>> +
>>> +	enable_output(data, port, output, mode, &fb);
>>> +
>>> +	igt_assert(eld_get_igt(&eld));
>>> +	igt_assert(eld.sads_len == 1);
>>> +
>>> +	sad = &eld.sads[0];
>>> +	igt_assert(sad->coding_type == CEA_SAD_FORMAT_PCM);
>>> +	igt_assert(sad->channels == 2);
>>> +	igt_assert(sad->rates == (CEA_SAD_SAMPLING_RATE_32KHZ |
>>> +		   CEA_SAD_SAMPLING_RATE_44KHZ | CEA_SAD_SAMPLING_RATE_48KHZ));
>>> +	igt_assert(sad->bits == (CEA_SAD_SAMPLE_SIZE_16 |
>>> +		   CEA_SAD_SAMPLE_SIZE_20 | CEA_SAD_SAMPLE_SIZE_24));
>>
>> If we have only one tests for one EDID, I would suggest trying more
>> coding_types and channels to make it less typical (we want to detect
>> hardcoded values).
> 
> So, the issue is that right now the EDID matches the capabilities of
> the Chamelium receiver. I'm not too comfortable with advertising an
> EDID with more than what the Chamelium supports for audio tests.

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

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

OK!

> 
>> With channels = 8 and a couple of other coding_types, the patch is:
>> Reviewed-by: Martin Peres <martin.peres@linux.intel.com>
>>
>> Any plans on generating broken ELDs and seeing how Alsa parses that?
> 
> Yeah, broken EDIDs/ELDs would be interesting, at least to make sure it
> doesn't freak out completely.

Exactly! :)

> 
>> Martin
>>> +
>>> +	igt_remove_fb(data->drm_fd, &fb);
>>> +
>>> +	drmModeFreeConnector(connector);
>>> +}
>>>  
>>>  static void randomize_plane_stride(data_t *data,
>>>  				   uint32_t width, uint32_t height,
>>> @@ -2118,6 +2168,10 @@ igt_main
>>>  		connector_subtest("dp-audio", DisplayPort)
>>>  			test_display_audio(&data, port, "HDMI",
>>>  					   TEST_EDID_DP_AUDIO);
>>> +
>>> +		connector_subtest("dp-audio-edid", DisplayPort)
>>> +			test_display_audio_edid(&data, port,
>>> +						TEST_EDID_DP_AUDIO);
>>>  	}
>>>  
>>>  	igt_subtest_group {
>>> @@ -2269,6 +2323,10 @@ igt_main
>>>  		connector_subtest("hdmi-audio", HDMIA)
>>>  			test_display_audio(&data, port, "HDMI",
>>>  					   TEST_EDID_HDMI_AUDIO);
>>> +
>>> +		connector_subtest("hdmi-audio-edid", HDMIA)
>>> +			test_display_audio_edid(&data, port,
>>> +						TEST_EDID_HDMI_AUDIO);
>>>  	}
>>>  
>>>  	igt_subtest_group {
>>>
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

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

[Ser, Simon]

On Wed, 2019-06-12 at 15:24 +0300, Martin Peres wrote:
> On 12/06/2019 14:35, Ser, Simon wrote:
> > On Wed, 2019-06-12 at 13:47 +0300, Martin Peres wrote:
> > > On 06/06/2019 14:48, 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.
> > > > Signed-off-by: Simon Ser <simon.ser@intel.com>
> > > > ---
> > > >  tests/kms_chamelium.c | 58 +++++++++++++++++++++++++++++++++++++++++++
> > > >  1 file changed, 58 insertions(+)
> > > > 
> > > > diff --git a/tests/kms_chamelium.c b/tests/kms_chamelium.c
> > > > index 3beea0c623bd..1a5271bb770f 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>
> > > > @@ -1428,6 +1429,55 @@ test_display_audio(data_t *data, struct chamelium_port *port,
> > > >  	free(alsa);
> > > >  }
> > > >  
> > > > +static void
> > > > +test_display_audio_edid(data_t *data, struct chamelium_port *port,
> > > > +			enum test_edid edid)
> > > > +{
> > > > +	igt_output_t *output;
> > > > +	igt_plane_t *primary;
> > > > +	struct igt_fb fb;
> > > > +	drmModeModeInfo *mode;
> > > > +	drmModeConnector *connector;
> > > > +	int fb_id;
> > > > +	struct eld_entry eld;
> > > > +	struct eld_sad *sad;
> > > > +
> > > > +	reset_state(data, port);
> > > > +
> > > > +	output = prepare_output(data, port, edid);
> > > > +	connector = chamelium_port_get_connector(data->chamelium, port, false);
> > > > +	primary = igt_output_get_plane_type(output, DRM_PLANE_TYPE_PRIMARY);
> > > > +	igt_assert(primary);
> > > > +
> > > > +	/* Enable the output because audio cannot be played on inactive
> > > > +	 * connectors. */
> > > > +	igt_assert(connector->count_modes > 0);
> > > > +	mode = &connector->modes[0];
> > > > +
> > > > +	fb_id = igt_create_color_pattern_fb(data->drm_fd,
> > > > +					    mode->hdisplay, mode->vdisplay,
> > > > +					    DRM_FORMAT_XRGB8888,
> > > > +					    LOCAL_DRM_FORMAT_MOD_NONE,
> > > > +					    0, 0, 0, &fb);
> > > > +	igt_assert(fb_id > 0);
> > > > +
> > > > +	enable_output(data, port, output, mode, &fb);
> > > > +
> > > > +	igt_assert(eld_get_igt(&eld));
> > > > +	igt_assert(eld.sads_len == 1);
> > > > +
> > > > +	sad = &eld.sads[0];
> > > > +	igt_assert(sad->coding_type == CEA_SAD_FORMAT_PCM);
> > > > +	igt_assert(sad->channels == 2);
> > > > +	igt_assert(sad->rates == (CEA_SAD_SAMPLING_RATE_32KHZ |
> > > > +		   CEA_SAD_SAMPLING_RATE_44KHZ | CEA_SAD_SAMPLING_RATE_48KHZ));
> > > > +	igt_assert(sad->bits == (CEA_SAD_SAMPLE_SIZE_16 |
> > > > +		   CEA_SAD_SAMPLE_SIZE_20 | CEA_SAD_SAMPLE_SIZE_24));
> > > 
> > > If we have only one tests for one EDID, I would suggest trying more
> > > coding_types and channels to make it less typical (we want to detect
> > > hardcoded values).
> > 
> > So, the issue is that right now the EDID matches the capabilities of
> > the Chamelium receiver. I'm not too comfortable with advertising an
> > EDID with more than what the Chamelium supports for audio tests.
> 
> Well, we are not going to send anything to the chamelium, so not sure
> why you are afraid of anything :s

The same EDID is used for audio tests.

> > I agree that we need more EDIDs with different params. I have plans to
> > send a patch for this.
> 
> OK!
> 
> > > With channels = 8 and a couple of other coding_types, the patch is:
> > > Reviewed-by: Martin Peres <martin.peres@linux.intel.com>
> > > 
> > > Any plans on generating broken ELDs and seeing how Alsa parses that?
> > 
> > Yeah, broken EDIDs/ELDs would be interesting, at least to make sure it
> > doesn't freak out completely.
> 
> Exactly! :)
> 
> > > Martin
> > > > +
> > > > +	igt_remove_fb(data->drm_fd, &fb);
> > > > +
> > > > +	drmModeFreeConnector(connector);
> > > > +}
> > > >  
> > > >  static void randomize_plane_stride(data_t *data,
> > > >  				   uint32_t width, uint32_t height,
> > > > @@ -2118,6 +2168,10 @@ igt_main
> > > >  		connector_subtest("dp-audio", DisplayPort)
> > > >  			test_display_audio(&data, port, "HDMI",
> > > >  					   TEST_EDID_DP_AUDIO);
> > > > +
> > > > +		connector_subtest("dp-audio-edid", DisplayPort)
> > > > +			test_display_audio_edid(&data, port,
> > > > +						TEST_EDID_DP_AUDIO);
> > > >  	}
> > > >  
> > > >  	igt_subtest_group {
> > > > @@ -2269,6 +2323,10 @@ igt_main
> > > >  		connector_subtest("hdmi-audio", HDMIA)
> > > >  			test_display_audio(&data, port, "HDMI",
> > > >  					   TEST_EDID_HDMI_AUDIO);
> > > > +
> > > > +		connector_subtest("hdmi-audio-edid", HDMIA)
> > > > +			test_display_audio_edid(&data, port,
> > > > +						TEST_EDID_HDMI_AUDIO);
> > > >  	}
> > > >  
> > > >  	igt_subtest_group {
> > > > 
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

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

[Martin Peres]

On 12/06/2019 15:52, Ser, Simon wrote:
> On Wed, 2019-06-12 at 15:24 +0300, Martin Peres wrote:
>> On 12/06/2019 14:35, Ser, Simon wrote:
>>> On Wed, 2019-06-12 at 13:47 +0300, Martin Peres wrote:
>>>> On 06/06/2019 14:48, 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.
>>>>> Signed-off-by: Simon Ser <simon.ser@intel.com>
>>>>> ---
>>>>>  tests/kms_chamelium.c | 58 +++++++++++++++++++++++++++++++++++++++++++
>>>>>  1 file changed, 58 insertions(+)
>>>>>
>>>>> diff --git a/tests/kms_chamelium.c b/tests/kms_chamelium.c
>>>>> index 3beea0c623bd..1a5271bb770f 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>
>>>>> @@ -1428,6 +1429,55 @@ test_display_audio(data_t *data, struct chamelium_port *port,
>>>>>  	free(alsa);
>>>>>  }
>>>>>  
>>>>> +static void
>>>>> +test_display_audio_edid(data_t *data, struct chamelium_port *port,
>>>>> +			enum test_edid edid)
>>>>> +{
>>>>> +	igt_output_t *output;
>>>>> +	igt_plane_t *primary;
>>>>> +	struct igt_fb fb;
>>>>> +	drmModeModeInfo *mode;
>>>>> +	drmModeConnector *connector;
>>>>> +	int fb_id;
>>>>> +	struct eld_entry eld;
>>>>> +	struct eld_sad *sad;
>>>>> +
>>>>> +	reset_state(data, port);
>>>>> +
>>>>> +	output = prepare_output(data, port, edid);
>>>>> +	connector = chamelium_port_get_connector(data->chamelium, port, false);
>>>>> +	primary = igt_output_get_plane_type(output, DRM_PLANE_TYPE_PRIMARY);
>>>>> +	igt_assert(primary);
>>>>> +
>>>>> +	/* Enable the output because audio cannot be played on inactive
>>>>> +	 * connectors. */
>>>>> +	igt_assert(connector->count_modes > 0);
>>>>> +	mode = &connector->modes[0];
>>>>> +
>>>>> +	fb_id = igt_create_color_pattern_fb(data->drm_fd,
>>>>> +					    mode->hdisplay, mode->vdisplay,
>>>>> +					    DRM_FORMAT_XRGB8888,
>>>>> +					    LOCAL_DRM_FORMAT_MOD_NONE,
>>>>> +					    0, 0, 0, &fb);
>>>>> +	igt_assert(fb_id > 0);
>>>>> +
>>>>> +	enable_output(data, port, output, mode, &fb);
>>>>> +
>>>>> +	igt_assert(eld_get_igt(&eld));
>>>>> +	igt_assert(eld.sads_len == 1);
>>>>> +
>>>>> +	sad = &eld.sads[0];
>>>>> +	igt_assert(sad->coding_type == CEA_SAD_FORMAT_PCM);
>>>>> +	igt_assert(sad->channels == 2);
>>>>> +	igt_assert(sad->rates == (CEA_SAD_SAMPLING_RATE_32KHZ |
>>>>> +		   CEA_SAD_SAMPLING_RATE_44KHZ | CEA_SAD_SAMPLING_RATE_48KHZ));
>>>>> +	igt_assert(sad->bits == (CEA_SAD_SAMPLE_SIZE_16 |
>>>>> +		   CEA_SAD_SAMPLE_SIZE_20 | CEA_SAD_SAMPLE_SIZE_24));
>>>>
>>>> If we have only one tests for one EDID, I would suggest trying more
>>>> coding_types and channels to make it less typical (we want to detect
>>>> hardcoded values).
>>>
>>> So, the issue is that right now the EDID matches the capabilities of
>>> the Chamelium receiver. I'm not too comfortable with advertising an
>>> EDID with more than what the Chamelium supports for audio tests.
>>
>> Well, we are not going to send anything to the chamelium, so not sure
>> why you are afraid of anything :s
> 
> The same EDID is used for audio tests.

That's OK to test the parsing with it, but I was arguing for creating
custom EDIDs in this tests ;)

Anyway, we can land this patch already and build on it later!

Martin
> 
>>> I agree that we need more EDIDs with different params. I have plans to
>>> send a patch for this.
>>
>> OK!
>>
>>>> With channels = 8 and a couple of other coding_types, the patch is:
>>>> Reviewed-by: Martin Peres <martin.peres@linux.intel.com>
>>>>
>>>> Any plans on generating broken ELDs and seeing how Alsa parses that?
>>>
>>> Yeah, broken EDIDs/ELDs would be interesting, at least to make sure it
>>> doesn't freak out completely.
>>
>> Exactly! :)
>>
>>>> Martin
>>>>> +
>>>>> +	igt_remove_fb(data->drm_fd, &fb);
>>>>> +
>>>>> +	drmModeFreeConnector(connector);
>>>>> +}
>>>>>  
>>>>>  static void randomize_plane_stride(data_t *data,
>>>>>  				   uint32_t width, uint32_t height,
>>>>> @@ -2118,6 +2168,10 @@ igt_main
>>>>>  		connector_subtest("dp-audio", DisplayPort)
>>>>>  			test_display_audio(&data, port, "HDMI",
>>>>>  					   TEST_EDID_DP_AUDIO);
>>>>> +
>>>>> +		connector_subtest("dp-audio-edid", DisplayPort)
>>>>> +			test_display_audio_edid(&data, port,
>>>>> +						TEST_EDID_DP_AUDIO);
>>>>>  	}
>>>>>  
>>>>>  	igt_subtest_group {
>>>>> @@ -2269,6 +2323,10 @@ igt_main
>>>>>  		connector_subtest("hdmi-audio", HDMIA)
>>>>>  			test_display_audio(&data, port, "HDMI",
>>>>>  					   TEST_EDID_HDMI_AUDIO);
>>>>> +
>>>>> +		connector_subtest("hdmi-audio-edid", HDMIA)
>>>>> +			test_display_audio_edid(&data, port,
>>>>> +						TEST_EDID_HDMI_AUDIO);
>>>>>  	}
>>>>>  
>>>>>  	igt_subtest_group {
>>>>>
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev