On Mon, 2017-07-03 at 15:01 +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.kocialkow...@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/testin
> g/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..76072845 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 == G_KEY_FILE_ERROR_INVALID_VALUE)
That felt very weird to type out and I thought that error was, for some reason,
a pointer used as integer. I double-checked that and it turns out that,
unsurprisingly, it is not.
Please hold off that patch, I will revisit it tomorrow to properly handle the
error.
> + 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 == G_KEY_FILE_ERROR) {
Same thing here.
> + 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
--
Paul Kocialkowski <paul.kocialkow...@linux.intel.com>
Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
