All of lore.kernel.org
 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.