[Intel-gfx] [PATCH i-g-t v2 7/7] Make igtrc configuration common, with configurable suspend/resume delay
Lyude Paul
lyude at redhat.com
Tue Jun 27 21:11:41 UTC 2017
On Tue, 2017-06-27 at 13:53 +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 at 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 | 64
> +++++++++++++++++++++++++++++++++++++++++++++++++++++
> lib/igt_core.h | 2 ++
> tests/Makefile.am | 4 ++--
> tests/chamelium.c | 11 +++++----
> tools/Makefile.am | 4 ++--
> 9 files changed, 112 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 225f98c3..345c44d6 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..b3d1c608 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,20 @@ static void oom_adjust_for_doom(void)
>
> }
>
> +static void config_parse(void)
> +{
> + GError *error = NULL;
> + int rc;
> +
> + if (!igt_key_file)
> + return;
> +
> + rc = g_key_file_get_integer(igt_key_file, "DUT", "SuspendResumeDelay",
> + &error);
We should probably at least throw an error if the key value happens to
be there, but the value is invalid (such as someone specifying "foo" as
the SuspendResumeDelay value). Maybe add a function like
static int
igt_config_get_integer(const char *section, const char *key_name, int *value, int default)
That calls the appropriate glib functions, and fails the current test
if any invalid non-default values are given
> + if (rc != 0)
> + igt_set_autoresume_delay(rc);
> +}
> +
> static int common_init(int *argc, char **argv,
> const char *extra_short_opts,
> const struct option *extra_long_opts,
> @@ -616,6 +650,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 +774,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 (!ret) {
> + g_key_file_free(igt_key_file);
> + igt_key_file = NULL;
> + ret = -1;
> +
> + goto out;
> + }
> +
> + config_parse();
> +
> out:
> + if (!key_file_env && key_file_loc)
> + free(key_file_loc);
> +
> free(short_opts);
> free(combined_opts);
>
> @@ -1345,6 +1406,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 41e0e89b..88b0a94c 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
More information about the Intel-gfx
mailing list