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