[igt-dev] [PATCH] tests/i915_pm_rpm: improved strictness and verbosity of i2c subtest

[Oleg Vasilev <oleg.vasilev@intel.com>]

Test checked that the number of valid edids from drm is equal to the
same number from i2c.

Now for each edid the whole blob is compared. In case of mismatch the
whole edid is printed.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=104097
Cc: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
Cc: Imre Deak <imre.deak@intel.com>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Oleg Vasilev <oleg.vasilev@intel.com>
---
 tests/i915/i915_pm_rpm.c | 176 ++++++++++++++++++++++++---------------
 1 file changed, 108 insertions(+), 68 deletions(-)

diff --git a/tests/i915/i915_pm_rpm.c b/tests/i915/i915_pm_rpm.c
index a2c9d0ed..f1530391 100644
--- a/tests/i915/i915_pm_rpm.c
+++ b/tests/i915/i915_pm_rpm.c
@@ -571,33 +571,56 @@ static void assert_drm_infos_equal(struct compare_data *d1,
 		assert_drm_crtcs_equal(d1->crtcs[i], d2->crtcs[i]);
 }
 
-/* We could check the checksum too, but just the header is probably enough. */
-static bool edid_is_valid(const unsigned char *edid)
+static bool find_i2c_path(char *connector_name, char *i2c_path)
 {
-	char edid_header[] = {
-		0x0, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0x0,
-	};
+	struct dirent *dirent;
+	DIR *dir;
+	int sysfs_card_fd = igt_sysfs_open(drm_fd);
+	int connector_fd = -1;
+	bool found_i2c_file = false;
 
-	return (memcmp(edid, edid_header, sizeof(edid_header)) == 0);
-}
+	dir = fdopendir(sysfs_card_fd);
+	igt_assert(dir);
 
-static int count_drm_valid_edids(struct mode_set_data *data)
-{
-	int ret = 0;
+	while ((dirent = readdir(dir))) {
+		/* Skip "cardx-" prefix */
+		char *dirname = dirent->d_name;
+		while(dirname[0] != '\0' && dirname[0] != '-')
+			++dirname;
 
-	if (!data->res)
-		return 0;
+		if(*dirname == '-')
+			++dirname;
 
-	for (int i = 0; i < data->res->count_connectors; i++)
-		if (data->edids[i] && edid_is_valid(data->edids[i]->data))
-			ret++;
-	return ret;
+		if (strcmp(dirname, connector_name) == 0) {
+			connector_fd = openat(sysfs_card_fd, dirent->d_name, O_RDONLY);
+			break;
+		}
+	}
+	closedir(dir);
+
+	if(connector_fd < 0)
+		return false;
+
+	dir = fdopendir(connector_fd);
+	igt_assert(dir);
+
+	while ((dirent = readdir(dir))) {
+		if (strncmp(dirent->d_name, "i2c-", 4) == 0) {
+			sprintf(i2c_path, "/dev/%s", dirent->d_name);
+			found_i2c_file = true;
+		}
+	}
+	closedir(dir);
+	return found_i2c_file;
 }
 
-static bool i2c_edid_is_valid(int fd)
+
+static bool i2c_read_edid(char *connector_name, unsigned char *edid)
 {
-	int rc;
-	unsigned char edid[128] = {};
+	char i2c_path[PATH_MAX];
+	bool result = find_i2c_path(connector_name, i2c_path);
+	int rc, fd;
+
 	struct i2c_msg msgs[] = {
 		{ /* Start at 0. */
 			.addr = 0x50,
@@ -616,69 +639,86 @@ static bool i2c_edid_is_valid(int fd)
 		.nmsgs = 2,
 	};
 
+
+	if (!result)
+		return false;
+
+	fd = open(i2c_path, O_RDWR);
+	igt_assert_neq(fd, -1);
+
 	rc = ioctl(fd, I2C_RDWR, &msgset);
-	return (rc >= 0) ? edid_is_valid(edid) : false;
+
+	close(fd);
+	return rc >= 0;
 }
 
-static int count_i2c_valid_edids(void)
+static void format_hex_string(unsigned char edid[static EDID_LENGTH],
+			      char buf[static EDID_LENGTH * 5 + 1])
 {
-	int fd, ret = 0;
-	DIR *dir;
+	for (size_t i = 0; i < EDID_LENGTH; ++i)
+		sprintf(buf+i*5, "0x%02x ", edid[i]);
+}
 
-	struct dirent *dirent;
-	char full_name[PATH_MAX];
+static void test_i2c(struct mode_set_data *data)
+{
+	bool failed = false;
+	igt_display_t display;
+	igt_display_require(&display, drm_fd);
 
-	dir = opendir("/dev/");
-	igt_assert(dir);
+	for (int i = 0; i < data->res->count_connectors; i++) {
+		unsigned char *drm_edid = data->edids[i] ? data->edids[i]->data : NULL;
+		unsigned char i2c_edid[EDID_LENGTH] = {};
 
-	while ((dirent = readdir(dir))) {
-		if (strncmp(dirent->d_name, "i2c-", 4) == 0) {
-			sprintf(full_name, "/dev/%s", dirent->d_name);
-			fd = open(full_name, O_RDWR);
-			igt_assert_neq(fd, -1);
-			if (i2c_edid_is_valid(fd))
-				ret++;
-			close(fd);
-		}
-	}
+		igt_output_t *output = igt_output_from_connector(&display,
+								 data->connectors[i]);
+		char *connector_name = (char *) igt_output_name(output);
 
-	closedir(dir);
+		bool got_i2c_edid = i2c_read_edid(connector_name, i2c_edid);
+		bool got_drm_edid = drm_edid != NULL;
+		bool is_vga = data->connectors[i]->connector_type == DRM_MODE_CONNECTOR_VGA;
 
-	return ret;
-}
+		bool edids_equal;
 
-static int count_vga_outputs(struct mode_set_data *data)
-{
-	int count = 0;
+		/* We fail to detect some VGA monitors using our i2c method. If you look
+		 * at the dmesg of these cases, you'll see the Kernel complaining about
+		 * the EDID reading mostly FFs and then disabling bit-banging. Since we
+		 * don't want to reimplement everything the Kernel does, let's just
+		 * accept the fact that some VGA outputs won't be properly detected. */
+		if(is_vga)
+			continue;
 
-	if (!data->res)
-		return 0;
+		if(!got_i2c_edid && !got_drm_edid)
+			continue;
+
+		if(got_i2c_edid && got_drm_edid)
+			edids_equal = (0 == memcmp(drm_edid, i2c_edid, EDID_LENGTH));
+		else
+			edids_equal = false;
 
-	for (int i = 0; i < data->res->count_connectors; i++)
-		if (data->connectors[i]->connector_type ==
-		    DRM_MODE_CONNECTOR_VGA)
-			count++;
 
-	return count;
-}
+		if (!edids_equal) {
+			char buf[5 * EDID_LENGTH + 1];
+			igt_critical("For connector %s EDID mismatch!\n",
+				     connector_name);
 
-static void test_i2c(struct mode_set_data *data)
-{
-	int i2c_edids = count_i2c_valid_edids();
-	int drm_edids = count_drm_valid_edids(data);
-	int vga_outputs = count_vga_outputs(data);
-	int diff;
-
-	igt_debug("i2c edids:%d drm edids:%d vga outputs:%d\n",
-		  i2c_edids, drm_edids, vga_outputs);
-
-	/* We fail to detect some VGA monitors using our i2c method. If you look
-	 * at the dmesg of these cases, you'll see the Kernel complaining about
-	 * the EDID reading mostly FFs and then disabling bit-banging. Since we
-	 * don't want to reimplement everything the Kernel does, let's just
-	 * accept the fact that some VGA outputs won't be properly detected. */
-	diff = drm_edids - i2c_edids;
-	igt_assert(diff <= vga_outputs && diff >= 0);
+			if(got_i2c_edid)
+				format_hex_string(i2c_edid, buf);
+			else
+				sprintf(buf, "NULL");
+
+			igt_critical("i2c: %s\n", buf);
+
+			if(got_drm_edid)
+				format_hex_string(drm_edid, buf);
+			else
+				sprintf(buf, "NULL");
+
+			igt_critical("drm: %s\n", buf);
+
+			failed = true;
+		}
+	}
+	igt_fail_on_f(failed, "Mismatch on EDID we got from i2c and DRM!\n");
 }
 
 static void setup_pc8(void)
-- 
2.21.0

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