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@xxxxxxxxxxxxxxx> > --- > 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@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx