intel-gfx.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
* [PATCH i-g-t v4 1/3] tests/chamelium: Update connector state without reprobe when possible
@ 2017-07-04 13:33 Paul Kocialkowski
  2017-07-04 13:33 ` [PATCH i-g-t v4 2/3] chamelium: Remove init reset duplicate in favor of per-test reset Paul Kocialkowski
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Paul Kocialkowski @ 2017-07-04 13:33 UTC (permalink / raw)
  To: intel-gfx; +Cc: Lyude

This renames the reprobe_connector function to update_connector and
ensures that full reprobe of the connector is only done when really
necessary (e.g. when changing the EDID).

A full reprobe takes time and is not required for updating the connector
state. Thus, this allows executing tests faster.

Signed-off-by: Paul Kocialkowski <paul.kocialkowski@linux.intel.com>
---
 tests/chamelium.c | 34 ++++++++++++++++++----------------
 1 file changed, 18 insertions(+), 16 deletions(-)

diff --git a/tests/chamelium.c b/tests/chamelium.c
index b412c6a7..346018a5 100644
--- a/tests/chamelium.c
+++ b/tests/chamelium.c
@@ -103,13 +103,14 @@ require_connector_present(data_t *data, unsigned int type)
 }
 
 static drmModeConnection
-reprobe_connector(data_t *data, struct chamelium_port *port)
+update_connector(data_t *data, struct chamelium_port *port, bool reprobe)
 {
 	drmModeConnector *connector;
 	drmModeConnection status;
 
-	igt_debug("Reprobing %s...\n", chamelium_port_get_name(port));
-	connector = chamelium_port_get_connector(data->chamelium, port, true);
+	igt_debug("Updating %s...\n", chamelium_port_get_name(port));
+	connector = chamelium_port_get_connector(data->chamelium, port,
+						 reprobe);
 	igt_assert(connector);
 	status = connector->connection;
 
@@ -119,7 +120,7 @@ reprobe_connector(data_t *data, struct chamelium_port *port)
 
 static void
 wait_for_connector(data_t *data, struct chamelium_port *port,
-		   drmModeConnection status)
+		   drmModeConnection status, bool reprobe)
 {
 	bool finished = false;
 
@@ -132,7 +133,7 @@ wait_for_connector(data_t *data, struct chamelium_port *port,
 	 * that hpd events work in the event that hpd doesn't work on the system
 	 */
 	igt_until_timeout(HOTPLUG_TIMEOUT) {
-		if (reprobe_connector(data, port) == status) {
+		if (update_connector(data, port, reprobe) == status) {
 			finished = true;
 			return;
 		}
@@ -151,11 +152,12 @@ reset_state(data_t *data, struct chamelium_port *port)
 	chamelium_reset(data->chamelium);
 
 	if (port) {
-		wait_for_connector(data, port, DRM_MODE_DISCONNECTED);
+		wait_for_connector(data, port, DRM_MODE_DISCONNECTED, false);
 	} else {
 		for (p = 0; p < data->port_count; p++) {
 			port = data->ports[p];
-			wait_for_connector(data, port, DRM_MODE_DISCONNECTED);
+			wait_for_connector(data, port, DRM_MODE_DISCONNECTED,
+					   false);
 		}
 	}
 }
@@ -175,7 +177,7 @@ test_basic_hotplug(data_t *data, struct chamelium_port *port, int toggle_count)
 		/* Check if we get a sysfs hotplug event */
 		chamelium_plug(data->chamelium, port);
 		igt_assert(igt_hotplug_detected(mon, HOTPLUG_TIMEOUT));
-		igt_assert_eq(reprobe_connector(data, port),
+		igt_assert_eq(update_connector(data, port, false),
 			      DRM_MODE_CONNECTED);
 
 		igt_flush_hotplugs(mon);
@@ -183,7 +185,7 @@ test_basic_hotplug(data_t *data, struct chamelium_port *port, int toggle_count)
 		/* Now check if we get a hotplug from disconnection */
 		chamelium_unplug(data->chamelium, port);
 		igt_assert(igt_hotplug_detected(mon, HOTPLUG_TIMEOUT));
-		igt_assert_eq(reprobe_connector(data, port),
+		igt_assert_eq(update_connector(data, port, false),
 			      DRM_MODE_DISCONNECTED);
 	}
 
@@ -204,7 +206,7 @@ test_edid_read(data_t *data, struct chamelium_port *port,
 
 	chamelium_port_set_edid(data->chamelium, port, edid_id);
 	chamelium_plug(data->chamelium, port);
-	wait_for_connector(data, port, DRM_MODE_CONNECTED);
+	wait_for_connector(data, port, DRM_MODE_CONNECTED, true);
 
 	igt_assert(kmstest_get_property(data->drm_fd, connector->connector_id,
 					DRM_MODE_OBJECT_CONNECTOR, "EDID", NULL,
@@ -247,13 +249,13 @@ try_suspend_resume_hpd(data_t *data, struct chamelium_port *port,
 
 	igt_assert(igt_hotplug_detected(mon, HOTPLUG_TIMEOUT));
 	if (port) {
-		igt_assert_eq(reprobe_connector(data, port), connected ?
+		igt_assert_eq(update_connector(data, port, false), connected ?
 			      DRM_MODE_DISCONNECTED : DRM_MODE_CONNECTED);
 	} else {
 		for (p = 0; p < data->port_count; p++) {
 			port = data->ports[p];
-			igt_assert_eq(reprobe_connector(data, port), connected ?
-				      DRM_MODE_DISCONNECTED :
+			igt_assert_eq(update_connector(data, port, false),
+				      connected ? DRM_MODE_DISCONNECTED :
 				      DRM_MODE_CONNECTED);
 		}
 
@@ -317,7 +319,7 @@ test_suspend_resume_edid_change(data_t *data, struct chamelium_port *port,
 	/* First plug in the port */
 	chamelium_port_set_edid(data->chamelium, port, edid_id);
 	chamelium_plug(data->chamelium, port);
-	wait_for_connector(data, port, DRM_MODE_CONNECTED);
+	wait_for_connector(data, port, DRM_MODE_CONNECTED, true);
 
 	igt_flush_hotplugs(mon);
 
@@ -352,7 +354,7 @@ prepare_output(data_t *data,
 	chamelium_port_set_edid(data->chamelium, port, data->edid_id);
 
 	chamelium_plug(data->chamelium, port);
-	wait_for_connector(data, port, DRM_MODE_CONNECTED);
+	wait_for_connector(data, port, DRM_MODE_CONNECTED, true);
 
 	igt_display_init(display, data->drm_fd);
 	output = igt_output_from_connector(display, connector);
@@ -590,7 +592,7 @@ test_hpd_without_ddc(data_t *data, struct chamelium_port *port)
 	chamelium_plug(data->chamelium, port);
 
 	igt_assert(igt_hotplug_detected(mon, HOTPLUG_TIMEOUT));
-	igt_assert_eq(reprobe_connector(data, port), DRM_MODE_CONNECTED);
+	igt_assert_eq(update_connector(data, port, false), DRM_MODE_CONNECTED);
 
 	igt_cleanup_hotplug(mon);
 }
-- 
2.13.2

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH i-g-t v4 2/3] chamelium: Remove init reset duplicate in favor of per-test reset
  2017-07-04 13:33 [PATCH i-g-t v4 1/3] tests/chamelium: Update connector state without reprobe when possible Paul Kocialkowski
@ 2017-07-04 13:33 ` Paul Kocialkowski
  2017-07-06 15:45   ` Lyude Paul
  2017-07-04 13:33 ` [PATCH i-g-t v4 3/3] Make igtrc configuration common, with configurable suspend/resume delay Paul Kocialkowski
  2017-07-06 16:02 ` [PATCH i-g-t v4 1/3] tests/chamelium: Update connector state without reprobe when possible Lyude Paul
  2 siblings, 1 reply; 6+ messages in thread
From: Paul Kocialkowski @ 2017-07-04 13:33 UTC (permalink / raw)
  To: intel-gfx; +Cc: Lyude

Since most tests are already doing a reset, there is no need to
duplicate it at init time. This removes that duplicate reset and adds
a call to reset_state where in-test resets where not done previously.

Signed-off-by: Paul Kocialkowski <paul.kocialkowski@linux.intel.com>
---
 lib/igt_chamelium.c | 2 --
 tests/chamelium.c   | 6 ++++++
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/lib/igt_chamelium.c b/lib/igt_chamelium.c
index 225f98c3..624f448b 100644
--- a/lib/igt_chamelium.c
+++ b/lib/igt_chamelium.c
@@ -1233,8 +1233,6 @@ struct chamelium *chamelium_init(int drm_fd)
 	if (!chamelium_read_config(chamelium, drm_fd))
 		goto error;
 
-	chamelium_reset(chamelium);
-
 	cleanup_instance = chamelium;
 	igt_install_exit_handler(chamelium_exit_handler);
 
diff --git a/tests/chamelium.c b/tests/chamelium.c
index 346018a5..6247d537 100644
--- a/tests/chamelium.c
+++ b/tests/chamelium.c
@@ -431,6 +431,8 @@ test_display_crc_single(data_t *data, struct chamelium_port *port)
 	drmModeConnector *connector;
 	int fb_id, i;
 
+	reset_state(data, port);
+
 	output = prepare_output(data, &display, port);
 	connector = chamelium_port_get_connector(data->chamelium, port, false);
 	primary = igt_output_get_plane_type(output, DRM_PLANE_TYPE_PRIMARY);
@@ -485,6 +487,8 @@ test_display_crc_multiple(data_t *data, struct chamelium_port *port)
 	drmModeConnector *connector;
 	int fb_id, i, j, captured_frame_count;
 
+	reset_state(data, port);
+
 	output = prepare_output(data, &display, port);
 	connector = chamelium_port_get_connector(data->chamelium, port, false);
 	primary = igt_output_get_plane_type(output, DRM_PLANE_TYPE_PRIMARY);
@@ -544,6 +548,8 @@ test_display_frame_dump(data_t *data, struct chamelium_port *port)
 	drmModeConnector *connector;
 	int fb_id, i, j;
 
+	reset_state(data, port);
+
 	output = prepare_output(data, &display, port);
 	connector = chamelium_port_get_connector(data->chamelium, port, false);
 	primary = igt_output_get_plane_type(output, DRM_PLANE_TYPE_PRIMARY);
-- 
2.13.2

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH i-g-t v4 3/3] Make igtrc configuration common, with configurable suspend/resume delay
  2017-07-04 13:33 [PATCH i-g-t v4 1/3] tests/chamelium: Update connector state without reprobe when possible Paul Kocialkowski
  2017-07-04 13:33 ` [PATCH i-g-t v4 2/3] chamelium: Remove init reset duplicate in favor of per-test reset Paul Kocialkowski
@ 2017-07-04 13:33 ` Paul Kocialkowski
  2017-07-06 15:57   ` Lyude Paul
  2017-07-06 16:02 ` [PATCH i-g-t v4 1/3] tests/chamelium: Update connector state without reprobe when possible Lyude Paul
  2 siblings, 1 reply; 6+ messages in thread
From: Paul Kocialkowski @ 2017-07-04 13:33 UTC (permalink / raw)
  To: intel-gfx; +Cc: Lyude

This adds support for configurable suspend/resume delay and takes the
occasion to move igtrc configuation from igt_chamelium to igt_core.
This way, suspend/resume delay configuration can be used for all tests.

Signed-off-by: Paul Kocialkowski <paul.kocialkowski@linux.intel.com>
---
 lib/Makefile.am     |  2 ++
 lib/igt_aux.c       | 27 +++++++++++++++++----
 lib/igt_aux.h       |  1 +
 lib/igt_chamelium.c | 49 +++++++++----------------------------
 lib/igt_core.c      | 69 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 lib/igt_core.h      |  2 ++
 tests/Makefile.am   |  4 ++--
 tests/chamelium.c   | 11 ++++-----
 tools/Makefile.am   |  4 ++--
 9 files changed, 117 insertions(+), 52 deletions(-)

diff --git a/lib/Makefile.am b/lib/Makefile.am
index 91e72c44..d4f41128 100644
--- a/lib/Makefile.am
+++ b/lib/Makefile.am
@@ -41,6 +41,7 @@ AM_CFLAGS = \
 	    $(XMLRPC_CFLAGS) \
 	    $(LIBUDEV_CFLAGS) \
 	    $(PIXMAN_CFLAGS) \
+	    $(GLIB_CFLAGS) \
 	    $(VALGRIND_CFLAGS) \
 	    -DIGT_SRCDIR=\""$(abs_top_srcdir)/tests"\" \
 	    -DIGT_DATADIR=\""$(pkgdatadir)"\" \
@@ -61,5 +62,6 @@ libintel_tools_la_LIBADD = \
 	$(XMLRPC_LIBS) \
 	$(LIBUDEV_LIBS) \
 	$(PIXMAN_LIBS) \
+	$(GLIB_LIBS) \
 	-lm
 
diff --git a/lib/igt_aux.c b/lib/igt_aux.c
index 882dba06..86a213c2 100644
--- a/lib/igt_aux.c
+++ b/lib/igt_aux.c
@@ -748,10 +748,7 @@ static void suspend_via_rtcwake(enum igt_suspend_state state)
 
 	igt_assert(state < SUSPEND_STATE_NUM);
 
-	if (autoresume_delay)
-		delay = autoresume_delay;
-	else
-		delay = state == SUSPEND_STATE_DISK ? 30 : 15;
+	delay = igt_get_autoresume_delay(state);
 
 	/*
 	 * Skip if rtcwake would fail for a reason not related to the kernel's
@@ -899,6 +896,28 @@ void igt_set_autoresume_delay(int delay_secs)
 }
 
 /**
+ * igt_get_autoresume_delay:
+ * @state: an #igt_suspend_state, the target suspend state
+ *
+ * Retrieves how long we wait to resume the system after suspending it.
+ * This can either be set through igt_set_autoresume_delay or be a default
+ * value that depends on the suspend state.
+ *
+ * Returns: The autoresume delay, in seconds.
+ */
+int igt_get_autoresume_delay(enum igt_suspend_state state)
+{
+	int delay;
+
+	if (autoresume_delay)
+		delay = autoresume_delay;
+	else
+		delay = state == SUSPEND_STATE_DISK ? 30 : 15;
+
+	return delay;
+}
+
+/**
  * igt_drop_root:
  *
  * Drop root privileges and make sure it actually worked. Useful for tests
diff --git a/lib/igt_aux.h b/lib/igt_aux.h
index 54b97164..499a1679 100644
--- a/lib/igt_aux.h
+++ b/lib/igt_aux.h
@@ -192,6 +192,7 @@ enum igt_suspend_test {
 void igt_system_suspend_autoresume(enum igt_suspend_state state,
 				   enum igt_suspend_test test);
 void igt_set_autoresume_delay(int delay_secs);
+int igt_get_autoresume_delay(enum igt_suspend_state state);
 
 /* dropping priviledges */
 void igt_drop_root(void);
diff --git a/lib/igt_chamelium.c b/lib/igt_chamelium.c
index 624f448b..bff08c0e 100644
--- a/lib/igt_chamelium.c
+++ b/lib/igt_chamelium.c
@@ -51,8 +51,8 @@
  * [on the ChromeOS project page](https://www.chromium.org/chromium-os/testing/chamelium).
  *
  * In order to run tests using the Chamelium, a valid configuration file must be
- * present.  The configuration file is a normal Glib keyfile (similar to Windows
- * INI) structured like so:
+ * present. It must contain Chamelium-specific keys as shown with the following
+ * example:
  *
  * |[<!-- language="plain" -->
  *	[Chamelium]
@@ -72,8 +72,6 @@
  *	ChameliumPortID=3
  * ]|
  *
- * By default, this file is expected to exist in ~/.igtrc . The directory for
- * this can be overriden by setting the environment variable %IGT_CONFIG_PATH.
  */
 
 struct chamelium_edid {
@@ -1034,7 +1032,7 @@ static unsigned int chamelium_get_port_type(struct chamelium *chamelium,
 }
 
 static bool chamelium_read_port_mappings(struct chamelium *chamelium,
-					 int drm_fd, GKeyFile *key_file)
+					 int drm_fd)
 {
 	drmModeRes *res;
 	drmModeConnector *connector;
@@ -1045,7 +1043,7 @@ static bool chamelium_read_port_mappings(struct chamelium *chamelium,
 	int port_i, i, j;
 	bool ret = true;
 
-	group_list = g_key_file_get_groups(key_file, NULL);
+	group_list = g_key_file_get_groups(igt_key_file, NULL);
 
 	/* Count how many connector mappings are specified in the config */
 	for (i = 0; group_list[i] != NULL; i++) {
@@ -1068,7 +1066,7 @@ static bool chamelium_read_port_mappings(struct chamelium *chamelium,
 
 		port = &chamelium->ports[port_i++];
 		port->name = strdup(map_name);
-		port->id = g_key_file_get_integer(key_file, group,
+		port->id = g_key_file_get_integer(igt_key_file, group,
 						  "ChameliumPortID",
 						  &error);
 		if (!port->id) {
@@ -1125,47 +1123,22 @@ out:
 
 static bool chamelium_read_config(struct chamelium *chamelium, int drm_fd)
 {
-	GKeyFile *key_file = g_key_file_new();
 	GError *error = NULL;
-	char *key_file_loc, *key_file_env;
-	int rc;
-	bool ret = true;
 
-	key_file_env = getenv("IGT_CONFIG_PATH");
-	if (key_file_env) {
-		key_file_loc = key_file_env;
-	} else {
-		key_file_loc = malloc(100);
-		snprintf(key_file_loc, 100, "%s/.igtrc", g_get_home_dir());
+	if (!igt_key_file) {
+		igt_warn("No configuration file available for chamelium\n");
+		return false;
 	}
 
-	rc = g_key_file_load_from_file(key_file, key_file_loc,
-				       G_KEY_FILE_NONE, &error);
-	if (!rc) {
-		igt_warn("Failed to read chamelium configuration file: %s\n",
-			 error->message);
-		ret = false;
-		goto out;
-	}
-
-	chamelium->url = g_key_file_get_string(key_file, "Chamelium", "URL",
+	chamelium->url = g_key_file_get_string(igt_key_file, "Chamelium", "URL",
 					       &error);
 	if (!chamelium->url) {
 		igt_warn("Couldn't read chamelium URL from config file: %s\n",
 			 error->message);
-		ret = false;
-		goto out;
+		return false;
 	}
 
-	ret = chamelium_read_port_mappings(chamelium, drm_fd, key_file);
-
-out:
-	g_key_file_free(key_file);
-
-	if (!key_file_env)
-		free(key_file_loc);
-
-	return ret;
+	return chamelium_read_port_mappings(chamelium, drm_fd);
 }
 
 /**
diff --git a/lib/igt_core.c b/lib/igt_core.c
index 9a5ed40e..1ba79361 100644
--- a/lib/igt_core.c
+++ b/lib/igt_core.c
@@ -57,6 +57,7 @@
 #include <limits.h>
 #include <locale.h>
 #include <uwildmat/uwildmat.h>
+#include <glib.h>
 
 #include "drmtest.h"
 #include "intel_chipset.h"
@@ -225,6 +226,23 @@
  * - 'basic*,advanced*' match any subtest starting basic or advanced
  * - '*,!basic*' match any subtest not starting basic
  * - 'basic*,!basic-render*' match any subtest starting basic but not starting basic-render
+ *
+ * # Configuration
+ *
+ * Some of IGT's behavior can be configured through a configuration file.
+ * By default, this file is expected to exist in ~/.igtrc . The directory for
+ * this can be overriden by setting the environment variable %IGT_CONFIG_PATH.
+ * An example configuration follows:
+ *
+ * |[<!-- language="plain" -->
+ *	# The following section is used for configuring the Device Under Test.
+ *	# It is not mandatory and allows overriding default values.
+ *	[DUT]
+ *	SuspendResumeDelay=10
+ * ]|
+ *
+ * Some specific configuration options may be used by specific parts of IGT,
+ * such as those related to Chamelium support.
  */
 
 static unsigned int exit_handler_count;
@@ -271,6 +289,8 @@ static struct {
 } log_buffer;
 static pthread_mutex_t log_buffer_mutex = PTHREAD_MUTEX_INITIALIZER;
 
+GKeyFile *igt_key_file;
+
 const char *igt_test_name(void)
 {
 	return command_str;
@@ -593,6 +613,25 @@ static void oom_adjust_for_doom(void)
 
 }
 
+static int config_parse(void)
+{
+	GError *error = NULL;
+	int rc;
+
+	if (!igt_key_file)
+		return 0;
+
+	rc = g_key_file_get_integer(igt_key_file, "DUT", "SuspendResumeDelay",
+				    &error);
+	if (error && error->code == G_KEY_FILE_ERROR_INVALID_VALUE)
+		return -2;
+
+	if (rc != 0)
+		igt_set_autoresume_delay(rc);
+
+	return 0;
+}
+
 static int common_init(int *argc, char **argv,
 		       const char *extra_short_opts,
 		       const struct option *extra_long_opts,
@@ -616,6 +655,9 @@ static int common_init(int *argc, char **argv,
 	int extra_opt_count;
 	int all_opt_count;
 	int ret = 0;
+	char *key_file_loc = NULL;
+	char *key_file_env = NULL;
+	GError *error = NULL;
 	const char *env;
 
 	if (!isatty(STDOUT_FILENO) || getenv("IGT_PLAIN_OUTPUT"))
@@ -737,7 +779,31 @@ static int common_init(int *argc, char **argv,
 		}
 	}
 
+	key_file_env = getenv("IGT_CONFIG_PATH");
+	if (key_file_env) {
+		key_file_loc = key_file_env;
+	} else {
+		key_file_loc = malloc(100);
+		snprintf(key_file_loc, 100, "%s/.igtrc", g_get_home_dir());
+	}
+
+	igt_key_file = g_key_file_new();
+	ret = g_key_file_load_from_file(igt_key_file, key_file_loc,
+					G_KEY_FILE_NONE, &error);
+	if (error && error->code == G_KEY_FILE_ERROR) {
+		g_key_file_free(igt_key_file);
+		igt_key_file = NULL;
+		ret = -2;
+
+		goto out;
+	}
+
+	ret = config_parse();
+
 out:
+	if (!key_file_env && key_file_loc)
+		free(key_file_loc);
+
 	free(short_opts);
 	free(combined_opts);
 
@@ -1345,6 +1411,9 @@ void igt_exit(void)
 {
 	igt_exit_called = true;
 
+	if (igt_key_file)
+		g_key_file_free(igt_key_file);
+
 	if (run_single_subtest && !run_single_subtest_found) {
 		igt_warn("Unknown subtest: %s\n", run_single_subtest);
 		exit(IGT_EXIT_INVALID);
diff --git a/lib/igt_core.h b/lib/igt_core.h
index a2ed9720..0739ca83 100644
--- a/lib/igt_core.h
+++ b/lib/igt_core.h
@@ -40,6 +40,7 @@
 #include <stdarg.h>
 #include <getopt.h>
 #include <unistd.h>
+#include <glib.h>
 
 #ifndef IGT_LOG_DOMAIN
 #define IGT_LOG_DOMAIN (NULL)
@@ -48,6 +49,7 @@
 
 extern const char* __igt_test_description __attribute__((weak));
 extern bool __igt_plain_output;
+extern GKeyFile *igt_key_file;
 
 
 /**
diff --git a/tests/Makefile.am b/tests/Makefile.am
index b051364c..f9d11e6c 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -72,9 +72,9 @@ AM_CFLAGS = $(DRM_CFLAGS) $(CWARNFLAGS) -Wno-unused-result $(DEBUG_CFLAGS)\
 	$(LIBUNWIND_CFLAGS) $(WERROR_CFLAGS) \
 	$(NULL)
 
-LDADD = ../lib/libintel_tools.la $(GLIB_LIBS) $(XMLRPC_LIBS)
+LDADD = ../lib/libintel_tools.la $(XMLRPC_LIBS)
 
-AM_CFLAGS += $(CAIRO_CFLAGS) $(LIBUDEV_CFLAGS) $(GLIB_CFLAGS)
+AM_CFLAGS += $(CAIRO_CFLAGS) $(LIBUDEV_CFLAGS)
 AM_LDFLAGS = -Wl,--as-needed
 
 drm_import_export_CFLAGS = $(AM_CFLAGS) $(THREAD_CFLAGS)
diff --git a/tests/chamelium.c b/tests/chamelium.c
index 6247d537..e3067664 100644
--- a/tests/chamelium.c
+++ b/tests/chamelium.c
@@ -42,7 +42,6 @@ typedef struct {
 } data_t;
 
 #define HOTPLUG_TIMEOUT 20 /* seconds */
-#define SUSPEND_RESUME_DELAY 20 /* seconds */
 
 #define HPD_STORM_PULSE_INTERVAL_DP 100 /* ms */
 #define HPD_STORM_PULSE_INTERVAL_HDMI 200 /* ms */
@@ -225,21 +224,21 @@ try_suspend_resume_hpd(data_t *data, struct chamelium_port *port,
 		       enum igt_suspend_state state, enum igt_suspend_test test,
 		       struct udev_monitor *mon, bool connected)
 {
+	int delay;
 	int p;
 
-	igt_set_autoresume_delay(SUSPEND_RESUME_DELAY);
 	igt_flush_hotplugs(mon);
 
+	delay = igt_get_autoresume_delay(state) * 1000 / 2;
+
 	if (port) {
-		chamelium_schedule_hpd_toggle(data->chamelium, port,
-					      SUSPEND_RESUME_DELAY * 1000 / 2,
+		chamelium_schedule_hpd_toggle(data->chamelium, port, delay,
 					      !connected);
 	} else {
 		for (p = 0; p < data->port_count; p++) {
 			port = data->ports[p];
 			chamelium_schedule_hpd_toggle(data->chamelium, port,
-						      SUSPEND_RESUME_DELAY * 1000 / 2,
-						      !connected);
+						      delay, !connected);
 		}
 
 		port = NULL;
diff --git a/tools/Makefile.am b/tools/Makefile.am
index 18a67efb..c40e75c7 100644
--- a/tools/Makefile.am
+++ b/tools/Makefile.am
@@ -9,8 +9,8 @@ endif
 
 if HAVE_UDEV
 bin_PROGRAMS += intel_dp_compliance
-intel_dp_compliance_CFLAGS = $(AM_CFLAGS) $(GLIB_CFLAGS)
-intel_dp_compliance_LDADD = $(top_builddir)/lib/libintel_tools.la $(GLIB_LIBS)
+intel_dp_compliance_CFLAGS = $(AM_CFLAGS)
+intel_dp_compliance_LDADD = $(top_builddir)/lib/libintel_tools.la
 endif
 
 SUBDIRS = null_state_gen registers
-- 
2.13.2

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH i-g-t v4 2/3] chamelium: Remove init reset duplicate in favor of per-test reset
  2017-07-04 13:33 ` [PATCH i-g-t v4 2/3] chamelium: Remove init reset duplicate in favor of per-test reset Paul Kocialkowski
@ 2017-07-06 15:45   ` Lyude Paul
  0 siblings, 0 replies; 6+ messages in thread
From: Lyude Paul @ 2017-07-06 15:45 UTC (permalink / raw)
  To: Paul Kocialkowski, intel-gfx

RB'd and pushed

On Tue, 2017-07-04 at 16:33 +0300, Paul Kocialkowski wrote:
> Since most tests are already doing a reset, there is no need to
> duplicate it at init time. This removes that duplicate reset and adds
> a call to reset_state where in-test resets where not done previously.
> 
> Signed-off-by: Paul Kocialkowski <paul.kocialkowski@linux.intel.com>
> ---
>  lib/igt_chamelium.c | 2 --
>  tests/chamelium.c   | 6 ++++++
>  2 files changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/lib/igt_chamelium.c b/lib/igt_chamelium.c
> index 225f98c3..624f448b 100644
> --- a/lib/igt_chamelium.c
> +++ b/lib/igt_chamelium.c
> @@ -1233,8 +1233,6 @@ struct chamelium *chamelium_init(int drm_fd)
>  	if (!chamelium_read_config(chamelium, drm_fd))
>  		goto error;
>  
> -	chamelium_reset(chamelium);
> -
>  	cleanup_instance = chamelium;
>  	igt_install_exit_handler(chamelium_exit_handler);
>  
> diff --git a/tests/chamelium.c b/tests/chamelium.c
> index 346018a5..6247d537 100644
> --- a/tests/chamelium.c
> +++ b/tests/chamelium.c
> @@ -431,6 +431,8 @@ test_display_crc_single(data_t *data, struct
> chamelium_port *port)
>  	drmModeConnector *connector;
>  	int fb_id, i;
>  
> +	reset_state(data, port);
> +
>  	output = prepare_output(data, &display, port);
>  	connector = chamelium_port_get_connector(data->chamelium,
> port, false);
>  	primary = igt_output_get_plane_type(output,
> DRM_PLANE_TYPE_PRIMARY);
> @@ -485,6 +487,8 @@ test_display_crc_multiple(data_t *data, struct
> chamelium_port *port)
>  	drmModeConnector *connector;
>  	int fb_id, i, j, captured_frame_count;
>  
> +	reset_state(data, port);
> +
>  	output = prepare_output(data, &display, port);
>  	connector = chamelium_port_get_connector(data->chamelium,
> port, false);
>  	primary = igt_output_get_plane_type(output,
> DRM_PLANE_TYPE_PRIMARY);
> @@ -544,6 +548,8 @@ test_display_frame_dump(data_t *data, struct
> chamelium_port *port)
>  	drmModeConnector *connector;
>  	int fb_id, i, j;
>  
> +	reset_state(data, port);
> +
>  	output = prepare_output(data, &display, port);
>  	connector = chamelium_port_get_connector(data->chamelium,
> port, false);
>  	primary = igt_output_get_plane_type(output,
> DRM_PLANE_TYPE_PRIMARY);
-- 
Cheers,
	Lyude
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH i-g-t v4 3/3] Make igtrc configuration common, with configurable suspend/resume delay
  2017-07-04 13:33 ` [PATCH i-g-t v4 3/3] Make igtrc configuration common, with configurable suspend/resume delay Paul Kocialkowski
@ 2017-07-06 15:57   ` Lyude Paul
  0 siblings, 0 replies; 6+ messages in thread
From: Lyude Paul @ 2017-07-06 15:57 UTC (permalink / raw)
  To: Paul Kocialkowski, intel-gfx

RB'd and pushed, thanks!

On Tue, 2017-07-04 at 16:33 +0300, Paul Kocialkowski wrote:
> This adds support for configurable suspend/resume delay and takes the
> occasion to move igtrc configuation from igt_chamelium to igt_core.
> This way, suspend/resume delay configuration can be used for all
> tests.
> 
> Signed-off-by: Paul Kocialkowski <paul.kocialkowski@linux.intel.com>
> ---
>  lib/Makefile.am     |  2 ++
>  lib/igt_aux.c       | 27 +++++++++++++++++----
>  lib/igt_aux.h       |  1 +
>  lib/igt_chamelium.c | 49 +++++++++----------------------------
>  lib/igt_core.c      | 69
> +++++++++++++++++++++++++++++++++++++++++++++++++++++
>  lib/igt_core.h      |  2 ++
>  tests/Makefile.am   |  4 ++--
>  tests/chamelium.c   | 11 ++++-----
>  tools/Makefile.am   |  4 ++--
>  9 files changed, 117 insertions(+), 52 deletions(-)
> 
> diff --git a/lib/Makefile.am b/lib/Makefile.am
> index 91e72c44..d4f41128 100644
> --- a/lib/Makefile.am
> +++ b/lib/Makefile.am
> @@ -41,6 +41,7 @@ AM_CFLAGS = \
>  	    $(XMLRPC_CFLAGS) \
>  	    $(LIBUDEV_CFLAGS) \
>  	    $(PIXMAN_CFLAGS) \
> +	    $(GLIB_CFLAGS) \
>  	    $(VALGRIND_CFLAGS) \
>  	    -DIGT_SRCDIR=\""$(abs_top_srcdir)/tests"\" \
>  	    -DIGT_DATADIR=\""$(pkgdatadir)"\" \
> @@ -61,5 +62,6 @@ libintel_tools_la_LIBADD = \
>  	$(XMLRPC_LIBS) \
>  	$(LIBUDEV_LIBS) \
>  	$(PIXMAN_LIBS) \
> +	$(GLIB_LIBS) \
>  	-lm
>  
> diff --git a/lib/igt_aux.c b/lib/igt_aux.c
> index 882dba06..86a213c2 100644
> --- a/lib/igt_aux.c
> +++ b/lib/igt_aux.c
> @@ -748,10 +748,7 @@ static void suspend_via_rtcwake(enum
> igt_suspend_state state)
>  
>  	igt_assert(state < SUSPEND_STATE_NUM);
>  
> -	if (autoresume_delay)
> -		delay = autoresume_delay;
> -	else
> -		delay = state == SUSPEND_STATE_DISK ? 30 : 15;
> +	delay = igt_get_autoresume_delay(state);
>  
>  	/*
>  	 * Skip if rtcwake would fail for a reason not related to
> the kernel's
> @@ -899,6 +896,28 @@ void igt_set_autoresume_delay(int delay_secs)
>  }
>  
>  /**
> + * igt_get_autoresume_delay:
> + * @state: an #igt_suspend_state, the target suspend state
> + *
> + * Retrieves how long we wait to resume the system after suspending
> it.
> + * This can either be set through igt_set_autoresume_delay or be a
> default
> + * value that depends on the suspend state.
> + *
> + * Returns: The autoresume delay, in seconds.
> + */
> +int igt_get_autoresume_delay(enum igt_suspend_state state)
> +{
> +	int delay;
> +
> +	if (autoresume_delay)
> +		delay = autoresume_delay;
> +	else
> +		delay = state == SUSPEND_STATE_DISK ? 30 : 15;
> +
> +	return delay;
> +}
> +
> +/**
>   * igt_drop_root:
>   *
>   * Drop root privileges and make sure it actually worked. Useful for
> tests
> diff --git a/lib/igt_aux.h b/lib/igt_aux.h
> index 54b97164..499a1679 100644
> --- a/lib/igt_aux.h
> +++ b/lib/igt_aux.h
> @@ -192,6 +192,7 @@ enum igt_suspend_test {
>  void igt_system_suspend_autoresume(enum igt_suspend_state state,
>  				   enum igt_suspend_test test);
>  void igt_set_autoresume_delay(int delay_secs);
> +int igt_get_autoresume_delay(enum igt_suspend_state state);
>  
>  /* dropping priviledges */
>  void igt_drop_root(void);
> diff --git a/lib/igt_chamelium.c b/lib/igt_chamelium.c
> index 624f448b..bff08c0e 100644
> --- a/lib/igt_chamelium.c
> +++ b/lib/igt_chamelium.c
> @@ -51,8 +51,8 @@
>   * [on the ChromeOS project page](https://www.chromium.org/chromium-
> os/testing/chamelium).
>   *
>   * In order to run tests using the Chamelium, a valid configuration
> file must be
> - * present.  The configuration file is a normal Glib keyfile
> (similar to Windows
> - * INI) structured like so:
> + * present. It must contain Chamelium-specific keys as shown with
> the following
> + * example:
>   *
>   * |[<!-- language="plain" -->
>   *	[Chamelium]
> @@ -72,8 +72,6 @@
>   *	ChameliumPortID=3
>   * ]|
>   *
> - * By default, this file is expected to exist in ~/.igtrc . The
> directory for
> - * this can be overriden by setting the environment variable
> %IGT_CONFIG_PATH.
>   */
>  
>  struct chamelium_edid {
> @@ -1034,7 +1032,7 @@ static unsigned int
> chamelium_get_port_type(struct chamelium *chamelium,
>  }
>  
>  static bool chamelium_read_port_mappings(struct chamelium
> *chamelium,
> -					 int drm_fd, GKeyFile
> *key_file)
> +					 int drm_fd)
>  {
>  	drmModeRes *res;
>  	drmModeConnector *connector;
> @@ -1045,7 +1043,7 @@ static bool chamelium_read_port_mappings(struct
> chamelium *chamelium,
>  	int port_i, i, j;
>  	bool ret = true;
>  
> -	group_list = g_key_file_get_groups(key_file, NULL);
> +	group_list = g_key_file_get_groups(igt_key_file, NULL);
>  
>  	/* Count how many connector mappings are specified in the
> config */
>  	for (i = 0; group_list[i] != NULL; i++) {
> @@ -1068,7 +1066,7 @@ static bool chamelium_read_port_mappings(struct
> chamelium *chamelium,
>  
>  		port = &chamelium->ports[port_i++];
>  		port->name = strdup(map_name);
> -		port->id = g_key_file_get_integer(key_file, group,
> +		port->id = g_key_file_get_integer(igt_key_file,
> group,
>  						  "ChameliumPortID",
>  						  &error);
>  		if (!port->id) {
> @@ -1125,47 +1123,22 @@ out:
>  
>  static bool chamelium_read_config(struct chamelium *chamelium, int
> drm_fd)
>  {
> -	GKeyFile *key_file = g_key_file_new();
>  	GError *error = NULL;
> -	char *key_file_loc, *key_file_env;
> -	int rc;
> -	bool ret = true;
>  
> -	key_file_env = getenv("IGT_CONFIG_PATH");
> -	if (key_file_env) {
> -		key_file_loc = key_file_env;
> -	} else {
> -		key_file_loc = malloc(100);
> -		snprintf(key_file_loc, 100, "%s/.igtrc",
> g_get_home_dir());
> +	if (!igt_key_file) {
> +		igt_warn("No configuration file available for
> chamelium\n");
> +		return false;
>  	}
>  
> -	rc = g_key_file_load_from_file(key_file, key_file_loc,
> -				       G_KEY_FILE_NONE, &error);
> -	if (!rc) {
> -		igt_warn("Failed to read chamelium configuration
> file: %s\n",
> -			 error->message);
> -		ret = false;
> -		goto out;
> -	}
> -
> -	chamelium->url = g_key_file_get_string(key_file,
> "Chamelium", "URL",
> +	chamelium->url = g_key_file_get_string(igt_key_file,
> "Chamelium", "URL",
>  					       &error);
>  	if (!chamelium->url) {
>  		igt_warn("Couldn't read chamelium URL from config
> file: %s\n",
>  			 error->message);
> -		ret = false;
> -		goto out;
> +		return false;
>  	}
>  
> -	ret = chamelium_read_port_mappings(chamelium, drm_fd,
> key_file);
> -
> -out:
> -	g_key_file_free(key_file);
> -
> -	if (!key_file_env)
> -		free(key_file_loc);
> -
> -	return ret;
> +	return chamelium_read_port_mappings(chamelium, drm_fd);
>  }
>  
>  /**
> diff --git a/lib/igt_core.c b/lib/igt_core.c
> index 9a5ed40e..1ba79361 100644
> --- a/lib/igt_core.c
> +++ b/lib/igt_core.c
> @@ -57,6 +57,7 @@
>  #include <limits.h>
>  #include <locale.h>
>  #include <uwildmat/uwildmat.h>
> +#include <glib.h>
>  
>  #include "drmtest.h"
>  #include "intel_chipset.h"
> @@ -225,6 +226,23 @@
>   * - 'basic*,advanced*' match any subtest starting basic or advanced
>   * - '*,!basic*' match any subtest not starting basic
>   * - 'basic*,!basic-render*' match any subtest starting basic but
> not starting basic-render
> + *
> + * # Configuration
> + *
> + * Some of IGT's behavior can be configured through a configuration
> file.
> + * By default, this file is expected to exist in ~/.igtrc . The
> directory for
> + * this can be overriden by setting the environment variable
> %IGT_CONFIG_PATH.
> + * An example configuration follows:
> + *
> + * |[<!-- language="plain" -->
> + *	# The following section is used for configuring the Device
> Under Test.
> + *	# It is not mandatory and allows overriding default
> values.
> + *	[DUT]
> + *	SuspendResumeDelay=10
> + * ]|
> + *
> + * Some specific configuration options may be used by specific parts
> of IGT,
> + * such as those related to Chamelium support.
>   */
>  
>  static unsigned int exit_handler_count;
> @@ -271,6 +289,8 @@ static struct {
>  } log_buffer;
>  static pthread_mutex_t log_buffer_mutex = PTHREAD_MUTEX_INITIALIZER;
>  
> +GKeyFile *igt_key_file;
> +
>  const char *igt_test_name(void)
>  {
>  	return command_str;
> @@ -593,6 +613,25 @@ static void oom_adjust_for_doom(void)
>  
>  }
>  
> +static int config_parse(void)
> +{
> +	GError *error = NULL;
> +	int rc;
> +
> +	if (!igt_key_file)
> +		return 0;
> +
> +	rc = g_key_file_get_integer(igt_key_file, "DUT",
> "SuspendResumeDelay",
> +				    &error);
> +	if (error && error->code == G_KEY_FILE_ERROR_INVALID_VALUE)
> +		return -2;
> +
> +	if (rc != 0)
> +		igt_set_autoresume_delay(rc);
> +
> +	return 0;
> +}
> +
>  static int common_init(int *argc, char **argv,
>  		       const char *extra_short_opts,
>  		       const struct option *extra_long_opts,
> @@ -616,6 +655,9 @@ static int common_init(int *argc, char **argv,
>  	int extra_opt_count;
>  	int all_opt_count;
>  	int ret = 0;
> +	char *key_file_loc = NULL;
> +	char *key_file_env = NULL;
> +	GError *error = NULL;
>  	const char *env;
>  
>  	if (!isatty(STDOUT_FILENO) || getenv("IGT_PLAIN_OUTPUT"))
> @@ -737,7 +779,31 @@ static int common_init(int *argc, char **argv,
>  		}
>  	}
>  
> +	key_file_env = getenv("IGT_CONFIG_PATH");
> +	if (key_file_env) {
> +		key_file_loc = key_file_env;
> +	} else {
> +		key_file_loc = malloc(100);
> +		snprintf(key_file_loc, 100, "%s/.igtrc",
> g_get_home_dir());
> +	}
> +
> +	igt_key_file = g_key_file_new();
> +	ret = g_key_file_load_from_file(igt_key_file, key_file_loc,
> +					G_KEY_FILE_NONE, &error);
> +	if (error && error->code == G_KEY_FILE_ERROR) {
> +		g_key_file_free(igt_key_file);
> +		igt_key_file = NULL;
> +		ret = -2;
> +
> +		goto out;
> +	}
> +
> +	ret = config_parse();
> +
>  out:
> +	if (!key_file_env && key_file_loc)
> +		free(key_file_loc);
> +
>  	free(short_opts);
>  	free(combined_opts);
>  
> @@ -1345,6 +1411,9 @@ void igt_exit(void)
>  {
>  	igt_exit_called = true;
>  
> +	if (igt_key_file)
> +		g_key_file_free(igt_key_file);
> +
>  	if (run_single_subtest && !run_single_subtest_found) {
>  		igt_warn("Unknown subtest: %s\n",
> run_single_subtest);
>  		exit(IGT_EXIT_INVALID);
> diff --git a/lib/igt_core.h b/lib/igt_core.h
> index a2ed9720..0739ca83 100644
> --- a/lib/igt_core.h
> +++ b/lib/igt_core.h
> @@ -40,6 +40,7 @@
>  #include <stdarg.h>
>  #include <getopt.h>
>  #include <unistd.h>
> +#include <glib.h>
>  
>  #ifndef IGT_LOG_DOMAIN
>  #define IGT_LOG_DOMAIN (NULL)
> @@ -48,6 +49,7 @@
>  
>  extern const char* __igt_test_description __attribute__((weak));
>  extern bool __igt_plain_output;
> +extern GKeyFile *igt_key_file;
>  
>  
>  /**
> diff --git a/tests/Makefile.am b/tests/Makefile.am
> index b051364c..f9d11e6c 100644
> --- a/tests/Makefile.am
> +++ b/tests/Makefile.am
> @@ -72,9 +72,9 @@ AM_CFLAGS = $(DRM_CFLAGS) $(CWARNFLAGS) -Wno-
> unused-result $(DEBUG_CFLAGS)\
>  	$(LIBUNWIND_CFLAGS) $(WERROR_CFLAGS) \
>  	$(NULL)
>  
> -LDADD = ../lib/libintel_tools.la $(GLIB_LIBS) $(XMLRPC_LIBS)
> +LDADD = ../lib/libintel_tools.la $(XMLRPC_LIBS)
>  
> -AM_CFLAGS += $(CAIRO_CFLAGS) $(LIBUDEV_CFLAGS) $(GLIB_CFLAGS)
> +AM_CFLAGS += $(CAIRO_CFLAGS) $(LIBUDEV_CFLAGS)
>  AM_LDFLAGS = -Wl,--as-needed
>  
>  drm_import_export_CFLAGS = $(AM_CFLAGS) $(THREAD_CFLAGS)
> diff --git a/tests/chamelium.c b/tests/chamelium.c
> index 6247d537..e3067664 100644
> --- a/tests/chamelium.c
> +++ b/tests/chamelium.c
> @@ -42,7 +42,6 @@ typedef struct {
>  } data_t;
>  
>  #define HOTPLUG_TIMEOUT 20 /* seconds */
> -#define SUSPEND_RESUME_DELAY 20 /* seconds */
>  
>  #define HPD_STORM_PULSE_INTERVAL_DP 100 /* ms */
>  #define HPD_STORM_PULSE_INTERVAL_HDMI 200 /* ms */
> @@ -225,21 +224,21 @@ try_suspend_resume_hpd(data_t *data, struct
> chamelium_port *port,
>  		       enum igt_suspend_state state, enum
> igt_suspend_test test,
>  		       struct udev_monitor *mon, bool connected)
>  {
> +	int delay;
>  	int p;
>  
> -	igt_set_autoresume_delay(SUSPEND_RESUME_DELAY);
>  	igt_flush_hotplugs(mon);
>  
> +	delay = igt_get_autoresume_delay(state) * 1000 / 2;
> +
>  	if (port) {
> -		chamelium_schedule_hpd_toggle(data->chamelium, port,
> -					      SUSPEND_RESUME_DELAY *
> 1000 / 2,
> +		chamelium_schedule_hpd_toggle(data->chamelium, port,
> delay,
>  					      !connected);
>  	} else {
>  		for (p = 0; p < data->port_count; p++) {
>  			port = data->ports[p];
>  			chamelium_schedule_hpd_toggle(data-
> >chamelium, port,
> -						      SUSPEND_RESUME
> _DELAY * 1000 / 2,
> -						      !connected);
> +						      delay,
> !connected);
>  		}
>  
>  		port = NULL;
> diff --git a/tools/Makefile.am b/tools/Makefile.am
> index 18a67efb..c40e75c7 100644
> --- a/tools/Makefile.am
> +++ b/tools/Makefile.am
> @@ -9,8 +9,8 @@ endif
>  
>  if HAVE_UDEV
>  bin_PROGRAMS += intel_dp_compliance
> -intel_dp_compliance_CFLAGS = $(AM_CFLAGS) $(GLIB_CFLAGS)
> -intel_dp_compliance_LDADD = $(top_builddir)/lib/libintel_tools.la
> $(GLIB_LIBS)
> +intel_dp_compliance_CFLAGS = $(AM_CFLAGS)
> +intel_dp_compliance_LDADD = $(top_builddir)/lib/libintel_tools.la
>  endif
>  
>  SUBDIRS = null_state_gen registers
-- 
Cheers,
	Lyude
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH i-g-t v4 1/3] tests/chamelium: Update connector state without reprobe when possible
  2017-07-04 13:33 [PATCH i-g-t v4 1/3] tests/chamelium: Update connector state without reprobe when possible Paul Kocialkowski
  2017-07-04 13:33 ` [PATCH i-g-t v4 2/3] chamelium: Remove init reset duplicate in favor of per-test reset Paul Kocialkowski
  2017-07-04 13:33 ` [PATCH i-g-t v4 3/3] Make igtrc configuration common, with configurable suspend/resume delay Paul Kocialkowski
@ 2017-07-06 16:02 ` Lyude Paul
  2 siblings, 0 replies; 6+ messages in thread
From: Lyude Paul @ 2017-07-06 16:02 UTC (permalink / raw)
  To: Paul Kocialkowski, intel-gfx

On Tue, 2017-07-04 at 16:33 +0300, Paul Kocialkowski wrote:
> This renames the reprobe_connector function to update_connector and
> ensures that full reprobe of the connector is only done when really
> necessary (e.g. when changing the EDID).
> 
> A full reprobe takes time and is not required for updating the
> connector
> state. Thus, this allows executing tests faster.
After thinking on this for a while I still have to NAK it unfortunately
:(. While this stuff will work for i915 since it's not really following
the expected DRM HPD behavior there's not much guarantee it'll work for
anything else. I think we need to have a more well-defined hotplug
event model in DRM before we consider adding patches like this.
> 
> Signed-off-by: Paul Kocialkowski <paul.kocialkowski@linux.intel.com>
> ---
>  tests/chamelium.c | 34 ++++++++++++++++++----------------
>  1 file changed, 18 insertions(+), 16 deletions(-)
> 
> diff --git a/tests/chamelium.c b/tests/chamelium.c
> index b412c6a7..346018a5 100644
> --- a/tests/chamelium.c
> +++ b/tests/chamelium.c
> @@ -103,13 +103,14 @@ require_connector_present(data_t *data,
> unsigned int type)
>  }
>  
>  static drmModeConnection
> -reprobe_connector(data_t *data, struct chamelium_port *port)
> +update_connector(data_t *data, struct chamelium_port *port, bool
> reprobe)
>  {
>  	drmModeConnector *connector;
>  	drmModeConnection status;
>  
> -	igt_debug("Reprobing %s...\n",
> chamelium_port_get_name(port));
> -	connector = chamelium_port_get_connector(data->chamelium,
> port, true);
> +	igt_debug("Updating %s...\n",
> chamelium_port_get_name(port));
> +	connector = chamelium_port_get_connector(data->chamelium,
> port,
> +						 reprobe);
>  	igt_assert(connector);
>  	status = connector->connection;
>  
> @@ -119,7 +120,7 @@ reprobe_connector(data_t *data, struct
> chamelium_port *port)
>  
>  static void
>  wait_for_connector(data_t *data, struct chamelium_port *port,
> -		   drmModeConnection status)
> +		   drmModeConnection status, bool reprobe)
>  {
>  	bool finished = false;
>  
> @@ -132,7 +133,7 @@ wait_for_connector(data_t *data, struct
> chamelium_port *port,
>  	 * that hpd events work in the event that hpd doesn't work
> on the system
>  	 */
>  	igt_until_timeout(HOTPLUG_TIMEOUT) {
> -		if (reprobe_connector(data, port) == status) {
> +		if (update_connector(data, port, reprobe) == status)
> {
>  			finished = true;
>  			return;
>  		}
> @@ -151,11 +152,12 @@ reset_state(data_t *data, struct chamelium_port
> *port)
>  	chamelium_reset(data->chamelium);
>  
>  	if (port) {
> -		wait_for_connector(data, port,
> DRM_MODE_DISCONNECTED);
> +		wait_for_connector(data, port,
> DRM_MODE_DISCONNECTED, false);
>  	} else {
>  		for (p = 0; p < data->port_count; p++) {
>  			port = data->ports[p];
> -			wait_for_connector(data, port,
> DRM_MODE_DISCONNECTED);
> +			wait_for_connector(data, port,
> DRM_MODE_DISCONNECTED,
> +					   false);
>  		}
>  	}
>  }
> @@ -175,7 +177,7 @@ test_basic_hotplug(data_t *data, struct
> chamelium_port *port, int toggle_count)
>  		/* Check if we get a sysfs hotplug event */
>  		chamelium_plug(data->chamelium, port);
>  		igt_assert(igt_hotplug_detected(mon,
> HOTPLUG_TIMEOUT));
> -		igt_assert_eq(reprobe_connector(data, port),
> +		igt_assert_eq(update_connector(data, port, false),
>  			      DRM_MODE_CONNECTED);
>  
>  		igt_flush_hotplugs(mon);
> @@ -183,7 +185,7 @@ test_basic_hotplug(data_t *data, struct
> chamelium_port *port, int toggle_count)
>  		/* Now check if we get a hotplug from disconnection
> */
>  		chamelium_unplug(data->chamelium, port);
>  		igt_assert(igt_hotplug_detected(mon,
> HOTPLUG_TIMEOUT));
> -		igt_assert_eq(reprobe_connector(data, port),
> +		igt_assert_eq(update_connector(data, port, false),
>  			      DRM_MODE_DISCONNECTED);
>  	}
>  
> @@ -204,7 +206,7 @@ test_edid_read(data_t *data, struct
> chamelium_port *port,
>  
>  	chamelium_port_set_edid(data->chamelium, port, edid_id);
>  	chamelium_plug(data->chamelium, port);
> -	wait_for_connector(data, port, DRM_MODE_CONNECTED);
> +	wait_for_connector(data, port, DRM_MODE_CONNECTED, true);
>  
>  	igt_assert(kmstest_get_property(data->drm_fd, connector-
> >connector_id,
>  					DRM_MODE_OBJECT_CONNECTOR,
> "EDID", NULL,
> @@ -247,13 +249,13 @@ try_suspend_resume_hpd(data_t *data, struct
> chamelium_port *port,
>  
>  	igt_assert(igt_hotplug_detected(mon, HOTPLUG_TIMEOUT));
>  	if (port) {
> -		igt_assert_eq(reprobe_connector(data, port),
> connected ?
> +		igt_assert_eq(update_connector(data, port, false),
> connected ?
>  			      DRM_MODE_DISCONNECTED :
> DRM_MODE_CONNECTED);
>  	} else {
>  		for (p = 0; p < data->port_count; p++) {
>  			port = data->ports[p];
> -			igt_assert_eq(reprobe_connector(data, port),
> connected ?
> -				      DRM_MODE_DISCONNECTED :
> +			igt_assert_eq(update_connector(data, port,
> false),
> +				      connected ?
> DRM_MODE_DISCONNECTED :
>  				      DRM_MODE_CONNECTED);
>  		}
>  
> @@ -317,7 +319,7 @@ test_suspend_resume_edid_change(data_t *data,
> struct chamelium_port *port,
>  	/* First plug in the port */
>  	chamelium_port_set_edid(data->chamelium, port, edid_id);
>  	chamelium_plug(data->chamelium, port);
> -	wait_for_connector(data, port, DRM_MODE_CONNECTED);
> +	wait_for_connector(data, port, DRM_MODE_CONNECTED, true);
>  
>  	igt_flush_hotplugs(mon);
>  
> @@ -352,7 +354,7 @@ prepare_output(data_t *data,
>  	chamelium_port_set_edid(data->chamelium, port, data-
> >edid_id);
>  
>  	chamelium_plug(data->chamelium, port);
> -	wait_for_connector(data, port, DRM_MODE_CONNECTED);
> +	wait_for_connector(data, port, DRM_MODE_CONNECTED, true);
>  
>  	igt_display_init(display, data->drm_fd);
>  	output = igt_output_from_connector(display, connector);
> @@ -590,7 +592,7 @@ test_hpd_without_ddc(data_t *data, struct
> chamelium_port *port)
>  	chamelium_plug(data->chamelium, port);
>  
>  	igt_assert(igt_hotplug_detected(mon, HOTPLUG_TIMEOUT));
> -	igt_assert_eq(reprobe_connector(data, port),
> DRM_MODE_CONNECTED);
> +	igt_assert_eq(update_connector(data, port, false),
> DRM_MODE_CONNECTED);
>  
>  	igt_cleanup_hotplug(mon);
>  }
-- 
Cheers,
	Lyude
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2017-07-06 16:02 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-07-04 13:33 [PATCH i-g-t v4 1/3] tests/chamelium: Update connector state without reprobe when possible Paul Kocialkowski
2017-07-04 13:33 ` [PATCH i-g-t v4 2/3] chamelium: Remove init reset duplicate in favor of per-test reset Paul Kocialkowski
2017-07-06 15:45   ` Lyude Paul
2017-07-04 13:33 ` [PATCH i-g-t v4 3/3] Make igtrc configuration common, with configurable suspend/resume delay Paul Kocialkowski
2017-07-06 15:57   ` Lyude Paul
2017-07-06 16:02 ` [PATCH i-g-t v4 1/3] tests/chamelium: Update connector state without reprobe when possible Lyude Paul

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).