[PATCH i-g-t 0/4 v4] Convert sh scripts to C variants.

[PATCH i-g-t 0/4 v4] Convert sh scripts to C variants.

[Marius Vlad]

TL;DR changes since last version: Use lib/igt_kmod so other users can
make use of it (specifically found that lib/igt_gvt, and covert it).

This series adds some library support to help converting sh scripts to C
version. Based on that I've converted drv_module_reload_basic and
kms_sysfs_edid_timing.  Other tests should follow. drv_module_reload requires
the most boilerplate code.

The reason for so many changes is the fact that some code got moved so other
users can use it. Secondly wrappers around libkmod in lib/igt_kmod and procps
in lib/igt_aux.  Thirdly drv_module_reload has embedded tools/gem_info and
tests/gem_exec_store in it, with minor modifications to allow running them as
subtests. Finally, C is more verbose than sh.

Changes since v3:
- lib/igt_kmod: added libkmod helpers into their own file. There seems to be
another user for it: lib/igt_gvt. Fixed a issue with lib/igt_gvt while at
it and converted to make use of lib/igt_kmod.
- lib/{igt_kmod, igt_aux}: Fixed gtk-doc documentation formatting (Daniel Vetter)
- tests/drv_module_reload: Re-worked reload() method by splitting into
load() and unload() and asserting more.
- replaced SW_FINISH with SET_CACHEING in tests/drv_module_reload (Chris Wilson)

Changes since v2:
- lib/igt_aux: Addressed comments from Chris Wilson
- tests/drv_module_reload: Passed incorrectly boolean instead of uint as flags to
igt_kmod_unload().

Changes since v1:
- lib/igt_aux: Addressed comments from Chris Wilson
- tests/drv_module_reload: Addressed comments from Chris Wilson and Petri Latvala
- tests/kms_sysfs_edid_timing: Addressed comments from Chris Wilson
- (Hopefully): Addressed comments from Jani Nikula.

Marius Vlad (4):
  lib/{igt_sysfs,igt_aux}: Make available to other users kick_fbcon()   
     (unbind_fbcon()), and added helpers to lib/igt_aux, lib/igt_kmod.
  lib/igt_gvt: Make use of libkmod helpers and fix reading gvt
    parameter.
  tests/drv_module_reload: Convert sh script to C version.
  tests/kms_sysfs_edid_timing: Convert sh to C version.

 configure.ac                          |   2 +
 lib/Makefile.am                       |   2 +
 lib/Makefile.sources                  |   2 +
 lib/igt_aux.c                         |  40 ++++
 lib/igt_aux.h                         |   2 +
 lib/igt_gvt.c                         |  79 +++----
 lib/igt_kmod.c                        | 164 ++++++++++++++
 lib/igt_kmod.h                        |  35 +++
 lib/igt_sysfs.c                       |  54 +++++
 lib/igt_sysfs.h                       |   2 +
 tests/Makefile.am                     |   1 -
 tests/Makefile.sources                |   4 +-
 tests/drv_module_reload.c             | 415 ++++++++++++++++++++++++++++++++++
 tests/drv_module_reload_basic         | 105 ---------
 tests/gem_alive.c                     |  35 ---
 tests/gvt_basic.c                     |   2 +-
 tests/intel-ci/fast-feedback.testlist |  13 +-
 tests/kms_sysfs_edid_timing           |  25 --
 tests/kms_sysfs_edid_timing.c         | 107 +++++++++
 tools/Makefile.sources                |   1 +
 tools/intel_gem_info.c                |  35 +++
 21 files changed, 910 insertions(+), 215 deletions(-)
 create mode 100644 lib/igt_kmod.c
 create mode 100644 lib/igt_kmod.h
 create mode 100644 tests/drv_module_reload.c
 delete mode 100755 tests/drv_module_reload_basic
 delete mode 100644 tests/gem_alive.c
 delete mode 100755 tests/kms_sysfs_edid_timing
 create mode 100644 tests/kms_sysfs_edid_timing.c
 create mode 100644 tools/intel_gem_info.c

-- 
2.7.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH i-g-t 1/4 v4] lib/{igt_sysfs, igt_aux}: Make available to other users kick_fbcon() (unbind_fbcon()), and added helpers to lib/igt_aux, lib/igt_kmod.

[Marius Vlad]

lib/igt_aux: Added igt_pkill helper.
lib/igt_kmod: Added load/unload kmod helpers.

v4:
- decided to split libkmod helpers into their own file as there's
another user lib/igt_gvt or tests/gvt_basic.
- fixed some gtk-doc documentation.

v3:
- return -errno (igt_pkill()) in case of failure (Cris Wilson)
- return bool for igt_kmod_is_loaded(), replaced strncasecmp with strncmp
(Chris Wilson)
- use igt_debug() instead of igt_info() for igt_kmod_load()/
igt_kmod_unload() and return -err directly from libkmod (Chris Wilson)

v2:
- Renamed libkmod helpers (Chris Wilson)
- Removed SIGTERM/SIGKILL case where we repeatedly tried to terminate the
process: just call kill(2) once (Chris Wilson)
- Removed redundant check in igt_kmod_unload(), igt_module_in_use() (Chris
Wilson)
- Pass flags to igt_kmod_unload() from the caller (Chris Wilson)
- Removed useless function igt_kill() which acts just as kill(2) (Chris
Wilson)

Signed-off-by: Marius Vlad <marius.c.vlad@intel.com>
---
 configure.ac         |   2 +
 lib/Makefile.am      |   2 +
 lib/Makefile.sources |   2 +
 lib/igt_aux.c        |  40 +++++++++++++
 lib/igt_aux.h        |   2 +
 lib/igt_gvt.c        |  43 +-------------
 lib/igt_kmod.c       | 164 +++++++++++++++++++++++++++++++++++++++++++++++++++
 lib/igt_kmod.h       |  35 +++++++++++
 lib/igt_sysfs.c      |  54 +++++++++++++++++
 lib/igt_sysfs.h      |   2 +
 10 files changed, 305 insertions(+), 41 deletions(-)
 create mode 100644 lib/igt_kmod.c
 create mode 100644 lib/igt_kmod.h

diff --git a/configure.ac b/configure.ac
index 735cfd5..2c6e49d 100644
--- a/configure.ac
+++ b/configure.ac
@@ -121,6 +121,8 @@ AC_SUBST(ASSEMBLER_WARN_CFLAGS)
 
 PKG_CHECK_MODULES(DRM, [libdrm])
 PKG_CHECK_MODULES(PCIACCESS, [pciaccess >= 0.10])
+PKG_CHECK_MODULES(KMOD, [libkmod])
+PKG_CHECK_MODULES(PROCPS, [libprocps])
 
 case "$target_cpu" in
 	x86*|i?86)
diff --git a/lib/Makefile.am b/lib/Makefile.am
index 4c0893d..e1737bd 100644
--- a/lib/Makefile.am
+++ b/lib/Makefile.am
@@ -34,6 +34,8 @@ AM_CFLAGS += $(CAIRO_CFLAGS)
 libintel_tools_la_LIBADD = \
 	$(DRM_LIBS) \
 	$(PCIACCESS_LIBS) \
+	$(PROCPS_LIBS) \
+	$(KMOD_LIBS) \
 	$(CAIRO_LIBS) \
 	$(LIBUDEV_LIBS) \
 	$(LIBUNWIND_LIBS) \
diff --git a/lib/Makefile.sources b/lib/Makefile.sources
index e8e277b..2205c86 100644
--- a/lib/Makefile.sources
+++ b/lib/Makefile.sources
@@ -77,6 +77,8 @@ lib_source_list =	 	\
 	igt_pm.h		\
 	uwildmat/uwildmat.h	\
 	uwildmat/uwildmat.c	\
+	igt_kmod.c		\
+	igt_kmod.h		\
 	$(NULL)
 
 .PHONY: version.h.tmp
diff --git a/lib/igt_aux.c b/lib/igt_aux.c
index 421f6d4..a8c5662 100644
--- a/lib/igt_aux.c
+++ b/lib/igt_aux.c
@@ -51,6 +51,8 @@
 #include <termios.h>
 #include <assert.h>
 
+#include <proc/readproc.h>
+
 #include "drmtest.h"
 #include "i915_drm.h"
 #include "intel_chipset.h"
@@ -1193,6 +1195,44 @@ void igt_set_module_param_int(const char *name, int val)
 	igt_set_module_param(name, str);
 }
 
+/**
+ * igt_pkill:
+ * @sig: Signal to send
+ * @comm: Name of process in the form found in /proc/pid/comm (limited to 15
+ * chars)
+ *
+ * Returns: 0 in case the process is not found running or the signal has been
+ * sent successfully or -errno otherwise.
+ *
+ * This function sends the signal @sig for a process found in process table
+ * with name @comm.
+ */
+int
+igt_pkill(int sig, const char *comm)
+{
+	PROCTAB *proc;
+	proc_t *proc_info;
+	int err = 0;
+
+	proc = openproc(PROC_FILLCOM | PROC_FILLSTAT | PROC_FILLARG);
+	igt_assert(proc != NULL);
+
+	while ((proc_info = readproc(proc, NULL))) {
+		if (!strncasecmp(proc_info->cmd, comm, sizeof(proc_info->cmd))) {
+
+			if (kill(proc_info->tid, sig) < 0)
+				err = -errno;
+
+			freeproc(proc_info);
+			break;
+		}
+		freeproc(proc_info);
+	}
+
+	closeproc(proc);
+	return err;
+}
+
 static struct igt_siglatency {
 	timer_t timer;
 	struct timespec target;
diff --git a/lib/igt_aux.h b/lib/igt_aux.h
index d30196b..b425ed8 100644
--- a/lib/igt_aux.h
+++ b/lib/igt_aux.h
@@ -264,4 +264,6 @@ double igt_stop_siglatency(struct igt_mean *result);
 void igt_set_module_param(const char *name, const char *val);
 void igt_set_module_param_int(const char *name, int val);
 
+int igt_pkill(int sig, const char *comm);
+
 #endif /* IGT_AUX_H */
diff --git a/lib/igt_gvt.c b/lib/igt_gvt.c
index 0f332d1..8bbf9bd 100644
--- a/lib/igt_gvt.c
+++ b/lib/igt_gvt.c
@@ -23,6 +23,7 @@
 
 #include "igt.h"
 #include "igt_gvt.h"
+#include "igt_sysfs.h"
 
 #include <dirent.h>
 #include <unistd.h>
@@ -46,49 +47,9 @@ static bool is_gvt_enabled(void)
 	return enabled;
 }
 
-static void unbind_fbcon(void)
-{
-	char buf[128];
-	const char *path = "/sys/class/vtconsole";
-	DIR *dir;
-	struct dirent *vtcon;
-
-	dir = opendir(path);
-	if (!dir)
-		return;
-
-	while ((vtcon = readdir(dir))) {
-		int fd, len;
-
-		if (strncmp(vtcon->d_name, "vtcon", 5))
-			continue;
-
-		sprintf(buf, "%s/%s/name", path, vtcon->d_name);
-		fd = open(buf, O_RDONLY);
-		if (fd < 0)
-			continue;
-
-		len = read(fd, buf, sizeof(buf) - 1);
-		close(fd);
-		if (len >= 0)
-			buf[len] = '\0';
-
-		if (strstr(buf, "frame buffer device")) {
-			sprintf(buf, "%s/%s/bind", path, vtcon->d_name);
-			fd = open(buf, O_WRONLY);
-			if (fd != -1) {
-				igt_ignore_warn(write(fd, "1\n", 2));
-				close(fd);
-			}
-			break;
-		}
-	}
-	closedir(dir);
-}
-
 static void unload_i915(void)
 {
-	unbind_fbcon();
+	kick_fbcon(false);
 	/* pkill alsact */
 
 	igt_ignore_warn(system("/sbin/modprobe -s -r i915"));
diff --git a/lib/igt_kmod.c b/lib/igt_kmod.c
new file mode 100644
index 0000000..ceb07d9
--- /dev/null
+++ b/lib/igt_kmod.c
@@ -0,0 +1,164 @@
+/*
+ * Copyright © 2016 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ */
+
+#include "igt.h"
+#include "igt_kmod.h"
+
+/**
+ * igt_kmod_is_loaded:
+ * @mod_name: The name of the module.
+ *
+ * Returns: True in case the module has been found or false otherwise.
+ *
+ * Function to check the existance of module @mod_name in list of loaded kernel
+ * modules.
+ *
+ */
+bool
+igt_kmod_is_loaded(const char *mod_name)
+{
+	struct kmod_list *mod, *list;
+	struct kmod_ctx *ctx;
+	bool ret = false;
+
+	ctx = kmod_new(NULL, NULL);
+	igt_assert(ctx != NULL);
+
+	if (kmod_module_new_from_loaded(ctx, &list) < 0) {
+		goto out;
+	}
+
+	kmod_list_foreach(mod, list) {
+		struct kmod_module *kmod = kmod_module_get_module(mod);
+		const char *kmod_name = kmod_module_get_name(kmod);
+
+		if (!strncmp(kmod_name, mod_name, strlen(kmod_name))) {
+			kmod_module_unref(kmod);
+			ret = true;
+			break;
+		}
+		kmod_module_unref(kmod);
+	}
+	kmod_module_unref_list(list);
+out:
+	kmod_unref(ctx);
+
+	return ret;
+}
+
+/**
+ * igt_kmod_load:
+ * @mod_name: The name of the module
+ * @opts: Parameters for the module. NULL in case no parameters
+ * are to be passed, or a '\0' terminated string otherwise.
+ *
+ * Returns: 0 in case of success or -errno in case the module could not
+ * be loaded.
+ *
+ * This function loads a kernel module using the name specified in @mod_name.
+ *
+ * @Note: This functions doesn't automatically resolve other module
+ * dependencies so make make sure you load the dependencies module(s) before
+ * this one.
+ */
+int
+igt_kmod_load(const char *mod_name, const char *opts)
+{
+	struct kmod_ctx *ctx;
+	struct kmod_module *kmod;
+	int err = 0;
+
+	ctx = kmod_new(NULL, NULL);
+	igt_assert(ctx != NULL);
+
+	err = kmod_module_new_from_name(ctx, mod_name, &kmod);
+	if (err < 0) {
+		goto out;
+	}
+
+	err = kmod_module_insert_module(kmod, 0, opts);
+	if (err < 0) {
+		switch (err) {
+		case -EEXIST:
+			igt_debug("Module %s already inserted\n",
+				 kmod_module_get_name(kmod));
+			break;
+		case -ENOENT:
+			igt_debug("Unknown symbol in module %s or "
+				 "unknown parameter\n",
+				 kmod_module_get_name(kmod));
+			break;
+		default:
+			igt_debug("Could not insert %s (%s)\n",
+				 kmod_module_get_name(kmod), strerror(-err));
+			break;
+		}
+	}
+out:
+	kmod_module_unref(kmod);
+	kmod_unref(ctx);
+
+	return -err ? err < 0 : err;
+}
+
+
+/**
+ * igt_kmod_unload:
+ * @mod_name: Module name.
+ * @flags: flags are passed directly to libkmod and can be:
+ * KMOD_REMOVE_FORCE or KMOD_REMOVE_NOWAIT.
+ *
+ * Returns: 0 in case of success or -errno otherwise.
+ *
+ * Removes the module @mod_name.
+ *
+ */
+int
+igt_kmod_unload(const char *mod_name, unsigned int flags)
+{
+	struct kmod_ctx *ctx;
+	struct kmod_module *kmod;
+	int err = 0;
+
+	ctx = kmod_new(NULL, NULL);
+	igt_assert(ctx != NULL);
+
+	err = kmod_module_new_from_name(ctx, mod_name, &kmod);
+	if (err < 0) {
+		igt_debug("Could not use module %s (%s)\n", mod_name,
+				strerror(-err));
+		goto out;
+	}
+
+	err = kmod_module_remove_module(kmod, flags);
+	if (err < 0) {
+		igt_debug("Could not remove module %s (%s)\n", mod_name,
+				strerror(-err));
+	}
+
+out:
+	kmod_module_unref(kmod);
+	kmod_unref(ctx);
+
+	return -err ? err < 0 : err;
+}
diff --git a/lib/igt_kmod.h b/lib/igt_kmod.h
new file mode 100644
index 0000000..798f1e4
--- /dev/null
+++ b/lib/igt_kmod.h
@@ -0,0 +1,35 @@
+/*
+ * Copyright © 2016 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ */
+
+#ifndef IGT_KMOD_H
+#define IGT_KMOD_H
+
+#include <libkmod.h>
+
+bool igt_kmod_is_loaded(const char *mod_name);
+
+int igt_kmod_load(const char *mod_name, const char *opts);
+int igt_kmod_unload(const char *mod_name, unsigned int flags);
+
+
+#endif /* IGT_KMOD_H */
diff --git a/lib/igt_sysfs.c b/lib/igt_sysfs.c
index c19821d..64a95de 100644
--- a/lib/igt_sysfs.c
+++ b/lib/igt_sysfs.c
@@ -34,7 +34,11 @@
 #include <fcntl.h>
 #include <unistd.h>
 #include <i915_drm.h>
+#include <dirent.h>
+#include <unistd.h>
+#include <fcntl.h>
 
+#include "igt_core.h"
 #include "igt_sysfs.h"
 
 /**
@@ -392,3 +396,53 @@ bool igt_sysfs_set_boolean(int dir, const char *attr, bool value)
 {
 	return igt_sysfs_printf(dir, attr, "%d", value) == 1;
 }
+
+/**
+ * kick_fbcon:
+ * @enable: boolean value
+ *
+ * This functions enables/disables the text console running on top of the
+ * framebuffer device.
+ */
+void kick_fbcon(bool enable)
+{
+	char buf[128];
+	const char *path = "/sys/class/vtconsole";
+	DIR *dir;
+	struct dirent *vtcon;
+
+	dir = opendir(path);
+	if (!dir)
+		return;
+
+	while ((vtcon = readdir(dir))) {
+		int fd, len;
+
+		if (strncmp(vtcon->d_name, "vtcon", 5))
+			continue;
+
+		sprintf(buf, "%s/%s/name", path, vtcon->d_name);
+		fd = open(buf, O_RDONLY);
+		if (fd < 0)
+			continue;
+
+		len = read(fd, buf, sizeof(buf) - 1);
+		close(fd);
+		if (len >= 0)
+			buf[len] = '\0';
+
+		if (strstr(buf, "frame buffer device")) {
+			sprintf(buf, "%s/%s/bind", path, vtcon->d_name);
+			fd = open(buf, O_WRONLY);
+			if (fd != -1) {
+				if (enable)
+					igt_ignore_warn(write(fd, "1\n", 2));
+				else
+					igt_ignore_warn(write(fd, "0\n", 2));
+				close(fd);
+			}
+			break;
+		}
+	}
+	closedir(dir);
+}
diff --git a/lib/igt_sysfs.h b/lib/igt_sysfs.h
index 4820066..a141894 100644
--- a/lib/igt_sysfs.h
+++ b/lib/igt_sysfs.h
@@ -43,4 +43,6 @@ bool igt_sysfs_set_u32(int dir, const char *attr, uint32_t value);
 bool igt_sysfs_get_boolean(int dir, const char *attr);
 bool igt_sysfs_set_boolean(int dir, const char *attr, bool value);
 
+void kick_fbcon(bool enable);
+
 #endif /* __IGT_SYSFS_H__ */
-- 
2.7.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH i-g-t 2/4 v4] lib/igt_gvt: Make use of libkmod helpers and fix reading gvt parameter.

[Marius Vlad]

Signed-off-by: Marius Vlad <marius.c.vlad@intel.com>
---
 lib/igt_gvt.c     | 42 +++++++++++++++++++++++++++++++++++-------
 tests/gvt_basic.c |  2 +-
 2 files changed, 36 insertions(+), 8 deletions(-)

diff --git a/lib/igt_gvt.c b/lib/igt_gvt.c
index 8bbf9bd..d868cb3 100644
--- a/lib/igt_gvt.c
+++ b/lib/igt_gvt.c
@@ -24,23 +24,26 @@
 #include "igt.h"
 #include "igt_gvt.h"
 #include "igt_sysfs.h"
+#include "igt_kmod.h"
 
+#include <signal.h>
 #include <dirent.h>
 #include <unistd.h>
 #include <fcntl.h>
+#include <time.h>
 
 static bool is_gvt_enabled(void)
 {
 	FILE *file;
-	int value;
+	char value;
 	bool enabled = false;
 
 	file = fopen("/sys/module/i915/parameters/enable_gvt", "r");
 	if (!file)
 		return false;
 
-	if (fscanf(file, "%d", &value) == 1)
-		enabled = value;
+	if (fscanf(file, "%c", &value) == 1)
+		enabled = (value == 'Y' ? true : false);
 	fclose(file);
 
 	errno = 0;
@@ -50,9 +53,20 @@ static bool is_gvt_enabled(void)
 static void unload_i915(void)
 {
 	kick_fbcon(false);
-	/* pkill alsact */
 
-	igt_ignore_warn(system("/sbin/modprobe -s -r i915"));
+
+	if (igt_kmod_is_loaded("i915")) {
+
+		if (igt_kmod_is_loaded("snd_hda_intel")) {
+			igt_assert(!igt_pkill(SIGTERM, "alsactl"));
+			igt_assert(!igt_kmod_unload("snd_hda_intel", 0));
+		}
+
+		igt_assert(!igt_kmod_unload("i915", 0));
+		igt_assert(!igt_kmod_unload("drm_kms_helper", 0));
+		igt_assert(!igt_kmod_unload("drm", 0));
+	}
+
 }
 
 bool igt_gvt_load_module(void)
@@ -61,8 +75,15 @@ bool igt_gvt_load_module(void)
 		return true;
 
 	unload_i915();
-	igt_ignore_warn(system("/sbin/modprobe -s i915 enable_gvt=1"));
 
+	igt_assert(!igt_kmod_load("drm", NULL));
+	igt_assert(!igt_kmod_load("drm_kms_helper", NULL));
+
+	igt_assert(!igt_kmod_load("i915", "enable_gvt=1"));
+
+	kick_fbcon(true);
+
+	igt_assert(!igt_kmod_load("snd_hda_intel", NULL));
 	return is_gvt_enabled();
 }
 
@@ -72,7 +93,14 @@ void igt_gvt_unload_module(void)
 		return;
 
 	unload_i915();
-	igt_ignore_warn(system("/sbin/modprobe -s i915 enable_gvt=0"));
+
+	igt_assert(!igt_kmod_load("drm", NULL));
+	igt_assert(!igt_kmod_load("drm_kms_helper", NULL));
+
+	igt_assert(!igt_kmod_load("i915", "enable_gvt=0"));
 
 	igt_assert(!is_gvt_enabled());
+
+	kick_fbcon(true);
+	igt_assert(!igt_kmod_load("snd_hda_intel", NULL));
 }
diff --git a/tests/gvt_basic.c b/tests/gvt_basic.c
index 48b853a..4e909a5 100644
--- a/tests/gvt_basic.c
+++ b/tests/gvt_basic.c
@@ -32,7 +32,7 @@ igt_main
 
 	igt_fixture {
 		igt_require(igt_gvt_load_module());
-		fd = drm_open_driver(DRIVER_INTEL);
+		fd = __drm_open_driver(DRIVER_INTEL);
 	}
 
 	igt_subtest_f("invalid-placeholder-test");
-- 
2.7.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH i-g-t 3/4 v4] tests/drv_module_reload: Convert sh script to C version.

[Marius Vlad]

v4:
- adjust test to make use of lib/igt_kmod
- replaced SW_FINISH with SET_CACHEING (Chris Wilson)

v3:
- fix passing boolean value as flags to igt_kmod_unload().

v2:
- embedded gem_alive and gem_exec_store into test (Chris Wilson)
- int main() to igt_main (Chris Wilson)
- moved tests/gem_alive -> tools/gem_info (Chris Wilson)
- added to intel-ci/fast-feedback.testlist (Petri Latvala)
- added hda_dynamic_debug() (Petri Latvala)
- renamed from tests/drv_module_reload_basic to tests/drv_module_reload
(all subtests are basic and have been added to fast-feedback.testlist)

Signed-off-by: Marius Vlad <marius.c.vlad@intel.com>
---
 tests/Makefile.am                     |   1 -
 tests/Makefile.sources                |   2 +-
 tests/drv_module_reload.c             | 415 ++++++++++++++++++++++++++++++++++
 tests/drv_module_reload_basic         | 105 ---------
 tests/gem_alive.c                     |  35 ---
 tests/intel-ci/fast-feedback.testlist |  13 +-
 tools/Makefile.sources                |   1 +
 tools/intel_gem_info.c                |  35 +++
 8 files changed, 464 insertions(+), 143 deletions(-)
 create mode 100644 tests/drv_module_reload.c
 delete mode 100755 tests/drv_module_reload_basic
 delete mode 100644 tests/gem_alive.c
 create mode 100644 tools/intel_gem_info.c

diff --git a/tests/Makefile.am b/tests/Makefile.am
index a408126..14a41ae 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -26,7 +26,6 @@ noinst_PROGRAMS = \
 	$(NULL)
 
 pkglibexec_PROGRAMS = \
-	gem_alive \
 	gem_stress \
 	$(TESTS_progs) \
 	$(TESTS_progs_M) \
diff --git a/tests/Makefile.sources b/tests/Makefile.sources
index 6d081c3..b1511c6 100644
--- a/tests/Makefile.sources
+++ b/tests/Makefile.sources
@@ -211,6 +211,7 @@ TESTS_progs = \
 	kms_pwrite_crc \
 	kms_sink_crc_basic \
 	prime_udl \
+	drv_module_reload \
 	$(NULL)
 
 # IMPORTANT: The ZZ_ tests need to be run last!
@@ -221,7 +222,6 @@ TESTS_scripts_M = \
 TESTS_scripts = \
 	debugfs_emon_crash \
 	drv_debugfs_reader \
-	drv_module_reload_basic \
 	kms_sysfs_edid_timing \
 	sysfs_l3_parity \
 	test_rte_check \
diff --git a/tests/drv_module_reload.c b/tests/drv_module_reload.c
new file mode 100644
index 0000000..e52f2f7
--- /dev/null
+++ b/tests/drv_module_reload.c
@@ -0,0 +1,415 @@
+/*
+ * Copyright © 2016 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ */
+#include "igt.h"
+#include "igt_debugfs.h"
+#include "igt_aux.h"
+#include "igt_kmod.h"
+#include "igt_sysfs.h"
+#include "igt_core.h"
+
+#include <dirent.h>
+#include <sys/utsname.h>
+#include <linux/limits.h>
+#include <signal.h>
+#include <libgen.h>
+#include <signal.h>
+#include <sys/ioctl.h>
+#include <fcntl.h>
+
+
+#define LOCAL_I915_EXEC_BSD_SHIFT      (13)
+#define LOCAL_I915_EXEC_BSD_MASK       (3 << LOCAL_I915_EXEC_BSD_SHIFT)
+
+#define ENGINE_MASK  (I915_EXEC_RING_MASK | LOCAL_I915_EXEC_BSD_MASK)
+
+static void store_dword(int fd, unsigned ring)
+{
+	const int gen = intel_gen(intel_get_drm_devid(fd));
+	struct drm_i915_gem_exec_object2 obj[2];
+	struct drm_i915_gem_relocation_entry reloc;
+	struct drm_i915_gem_execbuffer2 execbuf;
+	uint32_t batch[16];
+	int i;
+
+	gem_require_ring(fd, ring);
+	igt_skip_on_f(gen == 6 && (ring & ~(3<<13)) == I915_EXEC_BSD,
+		      "MI_STORE_DATA broken on gen6 bsd\n");
+
+	intel_detect_and_clear_missed_interrupts(fd);
+	memset(&execbuf, 0, sizeof(execbuf));
+	execbuf.buffers_ptr = (uintptr_t)obj;
+	execbuf.buffer_count = 2;
+	execbuf.flags = ring;
+	if (gen < 6)
+		execbuf.flags |= I915_EXEC_SECURE;
+
+	memset(obj, 0, sizeof(obj));
+	obj[0].handle = gem_create(fd, 4096);
+	obj[1].handle = gem_create(fd, 4096);
+
+	memset(&reloc, 0, sizeof(reloc));
+	reloc.target_handle = obj[0].handle;
+	reloc.presumed_offset = 0;
+	reloc.offset = sizeof(uint32_t);
+	reloc.delta = 0;
+	reloc.read_domains = I915_GEM_DOMAIN_INSTRUCTION;
+	reloc.write_domain = I915_GEM_DOMAIN_INSTRUCTION;
+	obj[1].relocs_ptr = (uintptr_t)&reloc;
+	obj[1].relocation_count = 1;
+
+	i = 0;
+	batch[i] = MI_STORE_DWORD_IMM | (gen < 6 ? 1 << 22 : 0);
+	if (gen >= 8) {
+		batch[++i] = 0;
+		batch[++i] = 0;
+	} else if (gen >= 4) {
+		batch[++i] = 0;
+		batch[++i] = 0;
+		reloc.offset += sizeof(uint32_t);
+	} else {
+		batch[i]--;
+		batch[++i] = 0;
+	}
+	batch[++i] = 0xc0ffee;
+	batch[++i] = MI_BATCH_BUFFER_END;
+	gem_write(fd, obj[1].handle, 0, batch, sizeof(batch));
+	gem_execbuf(fd, &execbuf);
+	gem_close(fd, obj[1].handle);
+
+	gem_read(fd, obj[0].handle, 0, batch, sizeof(batch));
+	gem_close(fd, obj[0].handle);
+	igt_assert_eq(*batch, 0xc0ffee);
+	igt_assert_eq(intel_detect_and_clear_missed_interrupts(fd), 0);
+}
+
+static void store_all(int fd)
+{
+	const int gen = intel_gen(intel_get_drm_devid(fd));
+	struct drm_i915_gem_exec_object2 obj[2];
+	struct drm_i915_gem_relocation_entry reloc[32];
+	struct drm_i915_gem_execbuffer2 execbuf;
+	unsigned engines[16], permuted[16];
+	uint32_t batch[16];
+	uint64_t offset;
+	unsigned engine, nengine;
+	int value;
+	int i, j;
+
+	memset(&execbuf, 0, sizeof(execbuf));
+	execbuf.buffers_ptr = (uintptr_t)obj;
+	execbuf.buffer_count = 2;
+	if (gen < 6)
+		execbuf.flags |= I915_EXEC_SECURE;
+
+	memset(reloc, 0, sizeof(reloc));
+	memset(obj, 0, sizeof(obj));
+	obj[0].handle = gem_create(fd, 4096);
+	obj[1].handle = gem_create(fd, 4096);
+	obj[1].relocation_count = 1;
+
+	offset = sizeof(uint32_t);
+	i = 0;
+	batch[i] = MI_STORE_DWORD_IMM | (gen < 6 ? 1 << 22 : 0);
+	if (gen >= 8) {
+		batch[++i] = 0;
+		batch[++i] = 0;
+	} else if (gen >= 4) {
+		batch[++i] = 0;
+		batch[++i] = 0;
+		offset += sizeof(uint32_t);
+	} else {
+		batch[i]--;
+		batch[++i] = 0;
+	}
+	batch[value = ++i] = 0xc0ffee;
+	batch[++i] = MI_BATCH_BUFFER_END;
+
+	nengine = 0;
+	intel_detect_and_clear_missed_interrupts(fd);
+	for_each_engine(fd, engine) {
+		if (gen == 6 && (engine & ~(3<<13)) == I915_EXEC_BSD)
+			continue;
+
+		igt_assert(2*(nengine+1)*sizeof(batch) <= 4096);
+
+		execbuf.flags &= ~ENGINE_MASK;
+		execbuf.flags |= engine;
+
+		j = 2*nengine;
+		reloc[j].target_handle = obj[0].handle;
+		reloc[j].presumed_offset = ~0;
+		reloc[j].offset = j*sizeof(batch) + offset;
+		reloc[j].delta = nengine*sizeof(uint32_t);
+		reloc[j].read_domains = I915_GEM_DOMAIN_INSTRUCTION;
+		reloc[j].write_domain = I915_GEM_DOMAIN_INSTRUCTION;
+		obj[1].relocs_ptr = (uintptr_t)&reloc[j];
+
+		batch[value] = 0xdeadbeef;
+		gem_write(fd, obj[1].handle, j*sizeof(batch),
+			  batch, sizeof(batch));
+		execbuf.batch_start_offset = j*sizeof(batch);
+		gem_execbuf(fd, &execbuf);
+
+		j = 2*nengine + 1;
+		reloc[j].target_handle = obj[0].handle;
+		reloc[j].presumed_offset = ~0;
+		reloc[j].offset = j*sizeof(batch) + offset;
+		reloc[j].delta = nengine*sizeof(uint32_t);
+		reloc[j].read_domains = I915_GEM_DOMAIN_INSTRUCTION;
+		reloc[j].write_domain = I915_GEM_DOMAIN_INSTRUCTION;
+		obj[1].relocs_ptr = (uintptr_t)&reloc[j];
+
+		batch[value] = nengine;
+		gem_write(fd, obj[1].handle, j*sizeof(batch),
+			  batch, sizeof(batch));
+		execbuf.batch_start_offset = j*sizeof(batch);
+		gem_execbuf(fd, &execbuf);
+
+		engines[nengine++] = engine;
+	}
+	gem_sync(fd, obj[1].handle);
+
+	for (i = 0; i < nengine; i++) {
+		obj[1].relocs_ptr = (uintptr_t)&reloc[2*i];
+		execbuf.batch_start_offset = 2*i*sizeof(batch);
+		memcpy(permuted, engines, nengine*sizeof(engines[0]));
+		igt_permute_array(permuted, nengine, igt_exchange_int);
+		for (j = 0; j < nengine; j++) {
+			execbuf.flags &= ~ENGINE_MASK;
+			execbuf.flags |= permuted[j];
+			gem_execbuf(fd, &execbuf);
+		}
+		obj[1].relocs_ptr = (uintptr_t)&reloc[2*i+1];
+		execbuf.batch_start_offset = (2*i+1)*sizeof(batch);
+		execbuf.flags &= ~ENGINE_MASK;
+		execbuf.flags |= engines[i];
+		gem_execbuf(fd, &execbuf);
+	}
+	gem_close(fd, obj[1].handle);
+
+	gem_read(fd, obj[0].handle, 0, engines, sizeof(engines));
+	gem_close(fd, obj[0].handle);
+
+	for (i = 0; i < nengine; i++)
+		igt_assert_eq_u32(engines[i], i);
+	igt_assert_eq(intel_detect_and_clear_missed_interrupts(fd), 0);
+}
+
+static int
+unload(void)
+{
+	kick_fbcon(false);
+
+	if (igt_kmod_is_loaded("snd_hda_intel")) {
+		igt_assert(!igt_pkill(SIGTERM, "alsactl"));
+
+		if (igt_kmod_unload("snd_hda_intel", 0)) {
+			igt_info("Could not unload snd_hda_intel\n");
+			return IGT_EXIT_FAILURE;
+		}
+	}
+
+	/* gen5 */
+	if (igt_kmod_is_loaded("intel_ips")) {
+		igt_kmod_unload("intel_ips", 0);
+	}
+
+	if (igt_kmod_is_loaded("i915")) {
+		if (igt_kmod_unload("i915", 0)) {
+			igt_info("Could not unload i915\n");
+			return IGT_EXIT_SKIP;
+		} else {
+			igt_info("i915.ko has been unloaded!\n");
+		}
+	}
+
+	if (igt_kmod_is_loaded("intel-gtt")) {
+		igt_kmod_unload("intel-gtt", 0);
+	}
+
+	igt_assert(!igt_kmod_unload("drm_kms_helper", 0));
+	igt_assert(!igt_kmod_unload("drm", 0));
+
+	if (igt_kmod_is_loaded("i915")) {
+		igt_info("WARNING: i915.ko still loaded!\n");
+		return IGT_EXIT_FAILURE;
+	} else {
+		igt_info("module successfully unloaded\n");
+	}
+
+	return IGT_EXIT_SUCCESS;
+}
+
+static int
+load(const char *opts_i915)
+{
+	if (opts_i915)
+		igt_info("Reloading i915 with %s\n\n", opts_i915);
+
+	/* we do not have automatic loading of dependencies */
+	igt_assert(!igt_kmod_load("drm", NULL));
+	igt_assert(!igt_kmod_load("drm_kms_helper", NULL));
+
+	if (igt_kmod_load("i915", opts_i915)) {
+		igt_info("Could not load i915\n");
+		return IGT_EXIT_FAILURE;
+	}
+
+	kick_fbcon(true);
+	igt_assert(!igt_kmod_load("snd_hda_intel", NULL));
+
+	return IGT_EXIT_SUCCESS;
+}
+
+static int
+reload(const char *opts_i915)
+{
+	int err = IGT_EXIT_SUCCESS;
+
+	if ((err = unload()))
+		return err;
+
+	if ((err = load(opts_i915)))
+		return err;
+
+	return err;
+}
+
+static int
+gem_info(int fd)
+{
+	struct drm_i915_gem_sw_finish arg = { 0 };
+
+	signal(SIGALRM, SIG_IGN);
+
+	alarm(1);
+	if (ioctl(fd, DRM_IOCTL_I915_GEM_SW_FINISH, &arg) == 0) {
+		return IGT_EXIT_SKIP;
+	}
+
+	switch (errno) {
+	case ENOENT:
+		return 0;
+	case EIO:
+		return 1;
+	case EINTR:
+		return 2;
+	default:
+		return 3;
+	}
+}
+
+static int
+gem_exec_store(int fd)
+{
+	const struct intel_execution_engine *e;
+	igt_fixture {
+		igt_fork_hang_detector(fd);
+	}
+
+	for (e = intel_execution_engines; e->name; e++) {
+		igt_subtest_f("basic-exec-store-%s", e->name)
+			store_dword(fd, e->exec_id | e->flags);
+	}
+
+	igt_subtest("basic-exec-store-all")
+		store_all(fd);
+
+	igt_fixture
+		igt_stop_hang_detector();
+
+	return IGT_EXIT_SUCCESS;
+}
+
+static void
+hda_dynamic_debug(bool enable)
+{
+	FILE *fp;
+	const char snd_hda_intel_on[] = "module snd_hda_intel +pf";
+	const char snd_hda_core_on[] = "module snd_hda_core +pf";
+
+	const char snd_hda_intel_off[] = "module snd_hda_core =_";
+	const char snd_hda_core_off[] = "module snd_hda_intel =_";
+
+	fp = fopen("/sys/kernel/debug/dynamic_debug/control", "w");
+	igt_assert(fp != NULL);
+
+	if (enable) {
+		fwrite(snd_hda_intel_on, 1, sizeof(snd_hda_intel_on), fp);
+		fwrite(snd_hda_core_on, 1, sizeof(snd_hda_core_on), fp);
+	} else {
+		fwrite(snd_hda_intel_off, 1, sizeof(snd_hda_intel_off), fp);
+		fwrite(snd_hda_core_off, 1, sizeof(snd_hda_core_off), fp);
+	}
+
+	fclose(fp);
+}
+
+
+igt_main
+{
+	int i, err;
+	char buf[64];
+
+	igt_fixture
+		hda_dynamic_debug(true);
+
+	igt_subtest("basic-reload") {
+		if ((err = reload(NULL)))
+			igt_fail(err);
+	}
+
+	igt_subtest_group {
+		int fd;
+
+		igt_fixture
+			fd = __drm_open_driver(DRIVER_INTEL);
+
+		igt_subtest_group {
+			igt_subtest("basic-gem-info")
+				if ((err = gem_info(fd)))
+					igt_fail(err);
+			igt_subtest_group
+				if ((err = gem_exec_store(fd)))
+					igt_fail(err);
+		}
+
+		igt_fixture
+			close(fd);
+	}
+
+	igt_subtest("basic-reload-inject") {
+		for (i = 0; i < 4; i++) {
+			memset(buf, 0, sizeof(buf));
+			snprintf(buf, sizeof(buf), "inject_load_failure=%d", i);
+			reload(buf);
+		}
+	}
+
+	igt_subtest("basic-reload-final")
+		if ((err = reload(NULL)))
+			igt_fail(err);
+
+	igt_fixture
+		hda_dynamic_debug(false);
+}
diff --git a/tests/drv_module_reload_basic b/tests/drv_module_reload_basic
deleted file mode 100755
index b8cad88..0000000
--- a/tests/drv_module_reload_basic
+++ /dev/null
@@ -1,105 +0,0 @@
-#!/bin/bash
-#
-# Testcase: Reload the drm module
-#
-# ... we've broken this way too often :(
-#
-
-SOURCE_DIR="$( dirname "${BASH_SOURCE[0]}" )"
-. $SOURCE_DIR/drm_lib.sh
-
-# no other drm service should be running, so we can just unbind
-
-# return 0 if module by name $1 is loaded according to lsmod
-function mod_loaded()
-{
-	lsmod | grep -w "^$1" &> /dev/null
-}
-
-function reload() {
-	local snd_hda_intel_unloaded
-
-	echo Reloading i915.ko with $*
-
-	# we must kick away fbcon (but only fbcon)
-	for vtcon in /sys/class/vtconsole/vtcon*/ ; do
-		if grep "frame buffer device" $vtcon/name > /dev/null ; then
-			echo unbinding $vtcon: `cat $vtcon/name`
-			echo 0 > $vtcon/bind
-		fi
-	done
-
-	# The sound driver uses our power well
-	pkill alsactl
-	snd_hda_intel_unloaded=0
-	if mod_loaded snd_hda_intel; then
-		if rmmod snd_hda_intel; then
-			snd_hda_intel_unloaded=1
-		else
-			lsmod >&2
-		fi
-	fi
-
-	# gen5 only
-	if mod_loaded intel_ips; then
-		rmmod intel_ips
-	fi
-
-	if ! rmmod i915; then
-		lsmod >&2
-		return $IGT_EXIT_SKIP
-	fi
-	#ignore errors in intel-gtt, often built-in
-	rmmod intel-gtt &> /dev/null
-	# drm may be used by other devices (nouveau, radeon, udl, etc)
-	rmmod drm_kms_helper &> /dev/null
-	rmmod drm &> /dev/null
-
-	if mod_loaded i915; then
-		echo WARNING: i915.ko still loaded!
-		return $IGT_EXIT_FAILURE
-	else
-		echo module successfully unloaded
-	fi
-
-	modprobe i915 $*
-
-	if [ -f /sys/class/vtconsole/vtcon1/bind ]; then
-		echo 1 > /sys/class/vtconsole/vtcon1/bind
-	fi
-
-	modprobe -q snd_hda_intel || return $snd_hda_intel_unloaded
-}
-
-function finish_load() {
-	# does the device exist?
-	if $SOURCE_DIR/gem_alive > /dev/null ; then
-		echo "module successfully loaded again"
-	else
-		echo "failed to reload module successfully"
-		return $IGT_EXIT_FAILURE
-	fi
-
-	# then try to run something
-	if ! $SOURCE_DIR/gem_exec_store > /dev/null ; then
-		echo "failed to execute a simple batch after reload"
-		return $IGT_EXIT_FAILURE
-	fi
-
-	return $IGT_EXIT_SUCCESS
-}
-
-hda_dynamic_debug_enable
-
-reload || exit $?
-finish_load || exit $?
-
-# Repeat the module reload trying to to generate faults
-for i in $(seq 1 4); do
-	reload inject_load_failure=$i
-done
-
-reload || exit $?
-finish_load
-
-exit $?
diff --git a/tests/gem_alive.c b/tests/gem_alive.c
deleted file mode 100644
index 7544443..0000000
--- a/tests/gem_alive.c
+++ /dev/null
@@ -1,35 +0,0 @@
-#include "igt.h"
-#include <sys/ioctl.h>
-#include <fcntl.h>
-#include <errno.h>
-#include <string.h>
-#include <signal.h>
-#include <i915_drm.h>
-
-
-int main(void)
-{
-	struct drm_i915_gem_sw_finish arg = { 0 };
-	int fd;
-
-	signal(SIGALRM, SIG_IGN);
-
-	fd = __drm_open_driver(DRIVER_INTEL);
-	if (fd < 0)
-		return IGT_EXIT_SKIP;
-
-	alarm(1);
-	if (ioctl(fd, DRM_IOCTL_I915_GEM_SW_FINISH, &arg) == 0)
-		return IGT_EXIT_SKIP;
-
-	switch (errno) {
-	case ENOENT:
-		return 0;
-	case EIO:
-		return 1;
-	case EINTR:
-		return 2;
-	default:
-		return 3;
-	}
-}
diff --git a/tests/intel-ci/fast-feedback.testlist b/tests/intel-ci/fast-feedback.testlist
index f59ec98..3c516ed 100644
--- a/tests/intel-ci/fast-feedback.testlist
+++ b/tests/intel-ci/fast-feedback.testlist
@@ -3,7 +3,18 @@ igt@core_prop_blob@basic
 igt@drv_getparams_basic@basic-eu-total
 igt@drv_getparams_basic@basic-subslice-total
 igt@drv_hangman@error-state-basic
-igt@drv_module_reload_basic
+igt@drv_module_reload@basic-reload
+igt@drv_module_reload@basic-gem-info
+igt@drv_module_reload@basic-exec-store-default
+igt@drv_module_reload@basic-exec-store-render
+igt@drv_module_reload@basic-exec-store-bsd
+igt@drv_module_reload@basic-exec-store-bsd1
+igt@drv_module_reload@basic-exec-store-bsd2
+igt@drv_module_reload@basic-exec-store-blt
+igt@drv_module_reload@basic-exec-store-vebox
+igt@drv_module_reload@basic-exec-store-all
+igt@drv_module_reload@basic-reload-inject
+igt@drv_module_reload@basic-reload-final
 igt@gem_basic@bad-close
 igt@gem_basic@create-close
 igt@gem_basic@create-fd-close
diff --git a/tools/Makefile.sources b/tools/Makefile.sources
index be58871..e2451ea 100644
--- a/tools/Makefile.sources
+++ b/tools/Makefile.sources
@@ -29,6 +29,7 @@ tools_prog_lists =		\
 	intel_residency		\
 	intel_stepping		\
 	intel_watermark		\
+	intel_gem_info		\
 	$(NULL)
 
 dist_bin_SCRIPTS = intel_gpu_abrt
diff --git a/tools/intel_gem_info.c b/tools/intel_gem_info.c
new file mode 100644
index 0000000..7544443
--- /dev/null
+++ b/tools/intel_gem_info.c
@@ -0,0 +1,35 @@
+#include "igt.h"
+#include <sys/ioctl.h>
+#include <fcntl.h>
+#include <errno.h>
+#include <string.h>
+#include <signal.h>
+#include <i915_drm.h>
+
+
+int main(void)
+{
+	struct drm_i915_gem_sw_finish arg = { 0 };
+	int fd;
+
+	signal(SIGALRM, SIG_IGN);
+
+	fd = __drm_open_driver(DRIVER_INTEL);
+	if (fd < 0)
+		return IGT_EXIT_SKIP;
+
+	alarm(1);
+	if (ioctl(fd, DRM_IOCTL_I915_GEM_SW_FINISH, &arg) == 0)
+		return IGT_EXIT_SKIP;
+
+	switch (errno) {
+	case ENOENT:
+		return 0;
+	case EIO:
+		return 1;
+	case EINTR:
+		return 2;
+	default:
+		return 3;
+	}
+}
-- 
2.7.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH i-g-t 4/4 v4] tests/kms_sysfs_edid_timing: Convert sh to C version.

[Marius Vlad]

v2:
- don't read cached values (Chris Wilson)
- warn on per connector, and fail per mean (Chris Wilson)

These are synthetic: 5us per connector, and 600us for all (as
threshold).

Signed-off-by: Marius Vlad <marius.c.vlad@intel.com>
---
 tests/Makefile.sources        |   2 +-
 tests/kms_sysfs_edid_timing   |  25 ----------
 tests/kms_sysfs_edid_timing.c | 107 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 108 insertions(+), 26 deletions(-)
 delete mode 100755 tests/kms_sysfs_edid_timing
 create mode 100644 tests/kms_sysfs_edid_timing.c

diff --git a/tests/Makefile.sources b/tests/Makefile.sources
index b1511c6..8af455a 100644
--- a/tests/Makefile.sources
+++ b/tests/Makefile.sources
@@ -212,6 +212,7 @@ TESTS_progs = \
 	kms_sink_crc_basic \
 	prime_udl \
 	drv_module_reload \
+	kms_sysfs_edid_timing \
 	$(NULL)
 
 # IMPORTANT: The ZZ_ tests need to be run last!
@@ -222,7 +223,6 @@ TESTS_scripts_M = \
 TESTS_scripts = \
 	debugfs_emon_crash \
 	drv_debugfs_reader \
-	kms_sysfs_edid_timing \
 	sysfs_l3_parity \
 	test_rte_check \
 	tools_test \
diff --git a/tests/kms_sysfs_edid_timing b/tests/kms_sysfs_edid_timing
deleted file mode 100755
index 46ea540..0000000
--- a/tests/kms_sysfs_edid_timing
+++ /dev/null
@@ -1,25 +0,0 @@
-#!/bin/bash
-#
-# This check the time we take to read the content of all the possible connectors.
-# Without the edid -ENXIO patch (http://permalink.gmane.org/gmane.comp.video.dri.devel/62083),
-# we sometimes take a *really* long time. So let's just check for some reasonable timing here
-#
-
-DRM_LIB_ALLOW_NO_MASTER=1
-
-SOURCE_DIR="$( dirname "${BASH_SOURCE[0]}" )"
-. $SOURCE_DIR/drm_lib.sh
-
-TIME1=$(date +%s%N)
-cat $(find /sys/devices/|grep drm | grep /status) > /dev/null
-TIME2=$(date +%s%N)
-
-# time in ms
-RES=$(((TIME2 - TIME1) / 1000000))
-
-if [ $RES -gt 600 ]; then
-	echo "Talking to outputs took ${RES}ms, something is wrong"
-	exit $IGT_EXIT_FAILURE
-fi
-
-exit $IGT_EXIT_SUCCESS
diff --git a/tests/kms_sysfs_edid_timing.c b/tests/kms_sysfs_edid_timing.c
new file mode 100644
index 0000000..3b8bca0
--- /dev/null
+++ b/tests/kms_sysfs_edid_timing.c
@@ -0,0 +1,107 @@
+/*
+ * Copyright © 2016 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ */
+#include "igt.h"
+#include "igt_debugfs.h"
+#include "igt_sysfs.h"
+#include "igt_core.h"
+
+#include <dirent.h>
+#include <fcntl.h>
+#include <sys/stat.h>
+#include <time.h>
+
+#define USEC_PER_SEC		(1000*1000)
+#define THRESHOLD_PER_CONNECTOR	5
+#define THRESHOLD_TOTAL		600
+
+IGT_TEST_DESCRIPTION("This check the time we take to read the content of all "
+		     "the possible connectors. Without the edid -ENXIO patch "
+		     "(http://permalink.gmane.org/gmane.comp.video.dri.devel/62083), "
+		     "we sometimes take a *really* long time. "
+		     "So let's just check for some reasonable timing here");
+
+static uint64_t
+mean(uint64_t *a, size_t a_len)
+{
+	int p;
+	uint64_t __sum;
+
+	for (p = 0, __sum = 0; p < a_len; p++) {
+		__sum += a[p];
+	}
+
+	return (__sum / a_len);
+}
+
+igt_simple_main
+{
+	int dir = igt_sysfs_open(-1, NULL);
+	DIR *dirp = fdopendir(dir);
+	struct dirent *de;
+
+	int i, fd = 0;
+	int fds[32];
+
+	uint64_t elapsed, elapsed_mean;
+	uint64_t a[32];
+
+	struct stat st;
+	struct timespec start = {};
+
+	memset(fds, -1, sizeof(fds));
+	memset(a, -1, sizeof(a));
+
+	while ((de = readdir(dirp))) {
+
+		if (*de->d_name == '.')
+			continue;
+
+		if (fstatat(dir, de->d_name, &st, 0))
+			continue;
+
+		if (S_ISDIR(st.st_mode) && !strncmp(de->d_name, "card0-", 6)) {
+			int dfd = openat(dir, de->d_name, O_RDONLY);
+			igt_assert(dfd > 0);
+			fds[fd++] = dfd;
+		}
+	}
+	closedir(dirp);
+
+	for (i = 0; i < fd; i++) {
+		igt_sysfs_set(fds[i], "status", "detect");
+
+		elapsed = igt_nsec_elapsed(&start);
+		igt_sysfs_get(fds[i], "status");
+		elapsed = igt_nsec_elapsed(&start);
+
+		igt_warn_on_f(elapsed > (THRESHOLD_PER_CONNECTOR * USEC_PER_SEC),
+		"Threshold per connector exceeded (elapsed %lu ns)\n", elapsed);
+
+		a[i] = elapsed;
+		close(fds[i]);
+	}
+
+	elapsed_mean = mean(a, i);
+	igt_fail_on_f(elapsed_mean > THRESHOLD_TOTAL * USEC_PER_SEC,
+		     "Threshold exceeded (mean %lu ns)\n", elapsed_mean);
+}
-- 
2.7.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH i-g-t 3/4 v4] tests/drv_module_reload: Convert sh script to C version.

[Chris Wilson]

On Fri, Oct 28, 2016 at 12:31:28PM +0300, Marius Vlad wrote:
> +	igt_subtest_group {
> +		int fd;
> +
> +		igt_fixture
> +			fd = __drm_open_driver(DRIVER_INTEL);
> +
> +		igt_subtest_group {
> +			igt_subtest("basic-gem-info")
> +				if ((err = gem_info(fd)))
> +					igt_fail(err);
> +			igt_subtest_group
> +				if ((err = gem_exec_store(fd)))
> +					igt_fail(err);

This is a misinterpretation of the existing test. These are not
subtests, but a sanity check that reload() works. A call to
gem_exec_store() can just whither away, it doesn't reveal much about the
general sanity of the driver, but a quick call to an ioctl such as
gem_info() is enough to be sure the driver is there (i.e. the module load
worked as adverticed) and not hung upon load.

One more thing about gem_info(), to be used in process it needs to be
something like:

/* Check the driver load completed by seeing if it responds to an ioctl
 * correctly. We pick an ioctl that calls i915_mutex_interruptible()
 * first (before failing on the invalid arg) so that we can inspect its
 * driver error detection (-EIO if already wedged, or -EINTR if stuck).
 */
void i915_gem_sanitycheck(void)
{
	struct set_cacheing arg = {};
	int fd, err;

	fd = __drm_driver_open(DRIVER_INTEL);
	igt_set_timeout(1, "module reload failed");

	err = 0;
	if (ioctl(fd, SET_CACHEING, &arg))
		err = -errno;

	igt_set_timeout(0, NULL);
	close(fd);

	igt_assert_eq(err, -ENOENT);

}
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH i-g-t 4/4 v4] tests/kms_sysfs_edid_timing: Convert sh to C version.

[Chris Wilson]

On Fri, Oct 28, 2016 at 12:31:29PM +0300, Marius Vlad wrote:
> v2:
> - don't read cached values (Chris Wilson)
> - warn on per connector, and fail per mean (Chris Wilson)
> 
> These are synthetic: 5us per connector, and 600us for all (as
> threshold).
> 
> Signed-off-by: Marius Vlad <marius.c.vlad@intel.com>
> ---
>  tests/Makefile.sources        |   2 +-
>  tests/kms_sysfs_edid_timing   |  25 ----------
>  tests/kms_sysfs_edid_timing.c | 107 ++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 108 insertions(+), 26 deletions(-)
>  delete mode 100755 tests/kms_sysfs_edid_timing
>  create mode 100644 tests/kms_sysfs_edid_timing.c
> 
> diff --git a/tests/Makefile.sources b/tests/Makefile.sources
> index b1511c6..8af455a 100644
> --- a/tests/Makefile.sources
> +++ b/tests/Makefile.sources
> @@ -212,6 +212,7 @@ TESTS_progs = \
>  	kms_sink_crc_basic \
>  	prime_udl \
>  	drv_module_reload \
> +	kms_sysfs_edid_timing \
>  	$(NULL)
>  
>  # IMPORTANT: The ZZ_ tests need to be run last!
> @@ -222,7 +223,6 @@ TESTS_scripts_M = \
>  TESTS_scripts = \
>  	debugfs_emon_crash \
>  	drv_debugfs_reader \
> -	kms_sysfs_edid_timing \
>  	sysfs_l3_parity \
>  	test_rte_check \
>  	tools_test \
> diff --git a/tests/kms_sysfs_edid_timing b/tests/kms_sysfs_edid_timing
> deleted file mode 100755
> index 46ea540..0000000
> --- a/tests/kms_sysfs_edid_timing
> +++ /dev/null
> @@ -1,25 +0,0 @@
> -#!/bin/bash
> -#
> -# This check the time we take to read the content of all the possible connectors.
> -# Without the edid -ENXIO patch (http://permalink.gmane.org/gmane.comp.video.dri.devel/62083),
> -# we sometimes take a *really* long time. So let's just check for some reasonable timing here
> -#
> -
> -DRM_LIB_ALLOW_NO_MASTER=1
> -
> -SOURCE_DIR="$( dirname "${BASH_SOURCE[0]}" )"
> -. $SOURCE_DIR/drm_lib.sh
> -
> -TIME1=$(date +%s%N)
> -cat $(find /sys/devices/|grep drm | grep /status) > /dev/null
> -TIME2=$(date +%s%N)
> -
> -# time in ms
> -RES=$(((TIME2 - TIME1) / 1000000))
> -
> -if [ $RES -gt 600 ]; then
> -	echo "Talking to outputs took ${RES}ms, something is wrong"
> -	exit $IGT_EXIT_FAILURE
> -fi
> -
> -exit $IGT_EXIT_SUCCESS
> diff --git a/tests/kms_sysfs_edid_timing.c b/tests/kms_sysfs_edid_timing.c
> new file mode 100644
> index 0000000..3b8bca0
> --- /dev/null
> +++ b/tests/kms_sysfs_edid_timing.c
> @@ -0,0 +1,107 @@
> +/*
> + * Copyright © 2016 Intel Corporation
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the next
> + * paragraph) shall be included in all copies or substantial portions of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
> + * IN THE SOFTWARE.
> + */
> +#include "igt.h"
> +#include "igt_debugfs.h"
> +#include "igt_sysfs.h"
> +#include "igt_core.h"
> +
> +#include <dirent.h>
> +#include <fcntl.h>
> +#include <sys/stat.h>
> +#include <time.h>
> +
> +#define USEC_PER_SEC		(1000*1000)
> +#define THRESHOLD_PER_CONNECTOR	5
> +#define THRESHOLD_TOTAL		600
> +
> +IGT_TEST_DESCRIPTION("This check the time we take to read the content of all "
> +		     "the possible connectors. Without the edid -ENXIO patch "
> +		     "(http://permalink.gmane.org/gmane.comp.video.dri.devel/62083), "
> +		     "we sometimes take a *really* long time. "
> +		     "So let's just check for some reasonable timing here");
> +
> +static uint64_t
> +mean(uint64_t *a, size_t a_len)
> +{
> +	int p;
> +	uint64_t __sum;
> +
> +	for (p = 0, __sum = 0; p < a_len; p++) {
> +		__sum += a[p];
> +	}
> +
> +	return (__sum / a_len);
> +}

See igt_stats.h: igt_mean

> +igt_simple_main
> +{
> +	int dir = igt_sysfs_open(-1, NULL);

Hmm, that will try to find intel - I was hoping to be a little more
generic and do every cardN-connectorM in /sys/class/drm (not
/sys/class/drm/card0 as sysfs_open will return)

> +	DIR *dirp = fdopendir(dir);
> +	struct dirent *de;
> +
> +	int i, fd = 0;
> +	int fds[32];
> +
> +	uint64_t elapsed, elapsed_mean;
> +	uint64_t a[32];
> +
> +	struct stat st;
> +	struct timespec start = {};
> +
> +	memset(fds, -1, sizeof(fds));
> +	memset(a, -1, sizeof(a));
> +
> +	while ((de = readdir(dirp))) {
> +
> +		if (*de->d_name == '.')
> +			continue;
> +
> +		if (fstatat(dir, de->d_name, &st, 0))
> +			continue;
> +
> +		if (S_ISDIR(st.st_mode) && !strncmp(de->d_name, "card0-", 6)) {

sysfs_open would have reported the index as assuming card0 fails if vgem
was loaded first, for example.

> +			int dfd = openat(dir, de->d_name, O_RDONLY);
> +			igt_assert(dfd > 0);
> +			fds[fd++] = dfd;
> +		}
> +	}
> +	closedir(dirp);
> +
> +	for (i = 0; i < fd; i++) {
> +		igt_sysfs_set(fds[i], "status", "detect");
> +
> +		elapsed = igt_nsec_elapsed(&start);
> +		igt_sysfs_get(fds[i], "status");
> +		elapsed = igt_nsec_elapsed(&start);
> +
> +		igt_warn_on_f(elapsed > (THRESHOLD_PER_CONNECTOR * USEC_PER_SEC),
> +		"Threshold per connector exceeded (elapsed %lu ns)\n", elapsed);

I was suggesting we repeat the probe of each connector several times.

DIR *dir = opendir("/sys/class/drm");
while ((de = readdir(dir)) {
	struct igt_mean mean = {};
	struct stat st;
	char path[128];

	if (*de->d_name == '.') continue;
	snprintf(path, sizeof(path), "/sys/class/drm/%s/status", de->d_name);
	if (stat(path, &st)
		continue;

	for (int i = 0; i < 15; i++) {
		struct timespec tv;
		int fd;

		fd = open(path, O_WRONLY);

		igt_nsec_elapsed(&tv);
		write(fd, "detect\n", 7);
		igt_mean_add(&mean, igt_nsec_elapsed(&tv));

		close(fd);
	}

	if (mean.max > 10e6) /* 10 milisecond */
		igt_warn("%s: probe time exceed 10ms, max=%.2fms, avg=%.2fms\n",
			de->d_name, mean.max / 1e6, mean.mean / 1e6);
	igt_assert_f(mean.mean < 50e6,
		     "%s: average probe time exceeded 50ms, max=%.2fms, avg=%.2fms\n",
		     de->d_name, mean.max / 1e6, mean.mean / 1e6);
}
closedir(dir);

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH i-g-t 2/4 v4] lib/igt_gvt: Make use of libkmod helpers and fix reading gvt parameter.

[Chris Wilson]

On Fri, Oct 28, 2016 at 12:31:27PM +0300, Marius Vlad wrote:
> Signed-off-by: Marius Vlad <marius.c.vlad@intel.com>
> ---
>  lib/igt_gvt.c     | 42 +++++++++++++++++++++++++++++++++++-------
>  tests/gvt_basic.c |  2 +-
>  2 files changed, 36 insertions(+), 8 deletions(-)
> 
> diff --git a/lib/igt_gvt.c b/lib/igt_gvt.c
> index 8bbf9bd..d868cb3 100644
> --- a/lib/igt_gvt.c
> +++ b/lib/igt_gvt.c
> @@ -24,23 +24,26 @@
>  #include "igt.h"
>  #include "igt_gvt.h"
>  #include "igt_sysfs.h"
> +#include "igt_kmod.h"
>  
> +#include <signal.h>
>  #include <dirent.h>
>  #include <unistd.h>
>  #include <fcntl.h>
> +#include <time.h>
>  
>  static bool is_gvt_enabled(void)
>  {
>  	FILE *file;
> -	int value;
> +	char value;
>  	bool enabled = false;
>  
>  	file = fopen("/sys/module/i915/parameters/enable_gvt", "r");
>  	if (!file)
>  		return false;
>  
> -	if (fscanf(file, "%d", &value) == 1)
> -		enabled = value;
> +	if (fscanf(file, "%c", &value) == 1)
> +		enabled = (value == 'Y' ? true : false);
	enabled = value == 'Y';
else if (fscanf(file, "%d", &value) == 1)
	enabled = value;

Do I see a igt_kmod_parameter_get_boolean() in the future, I think I do!

>  	fclose(file);
>  
>  	errno = 0;
> @@ -50,9 +53,20 @@ static bool is_gvt_enabled(void)
>  static void unload_i915(void)
>  {
>  	kick_fbcon(false);
> -	/* pkill alsact */
>  
> -	igt_ignore_warn(system("/sbin/modprobe -s -r i915"));
> +
> +	if (igt_kmod_is_loaded("i915")) {
> +
> +		if (igt_kmod_is_loaded("snd_hda_intel")) {
> +			igt_assert(!igt_pkill(SIGTERM, "alsactl"));
> +			igt_assert(!igt_kmod_unload("snd_hda_intel", 0));
> +		}
> +
> +		igt_assert(!igt_kmod_unload("i915", 0));
> +		igt_assert(!igt_kmod_unload("drm_kms_helper", 0));
> +		igt_assert(!igt_kmod_unload("drm", 0));

But don't we already have this routine...

For this test, we shouldn't fail if we can't setup the environment as we
need, but skip the test.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH i-g-t 3/4 v4] tests/drv_module_reload: Convert sh script to C version.

[Marius Vlad]

[-- Attachment #1.1: Type: text/plain, Size: 2224 bytes --]

On Fri, Oct 28, 2016 at 10:48:33AM +0100, Chris Wilson wrote:
> On Fri, Oct 28, 2016 at 12:31:28PM +0300, Marius Vlad wrote:
> > +	igt_subtest_group {
> > +		int fd;
> > +
> > +		igt_fixture
> > +			fd = __drm_open_driver(DRIVER_INTEL);
> > +
> > +		igt_subtest_group {
> > +			igt_subtest("basic-gem-info")
> > +				if ((err = gem_info(fd)))
> > +					igt_fail(err);
> > +			igt_subtest_group
> > +				if ((err = gem_exec_store(fd)))
> > +					igt_fail(err);
> 
> This is a misinterpretation of the existing test. These are not
> subtests, but a sanity check that reload() works. A call to
> gem_exec_store() can just whither away, it doesn't reveal much about the
> general sanity of the driver, but a quick call to an ioctl such as
> gem_info() is enough to be sure the driver is there (i.e. the module load
> worked as adverticed) and not hung upon load.
Indeed, that's exactly what they are: sanity check that reloading of the
driver works, just that grouping them as subtests/subgroups made
embedding them easier and I thought that worked better given igt is
centered around tests/subtests (gem_exec_store has a igt_skip_on_f()
thus the subgroup)

I guess that I can call them directly in "basic-reload" subtest. If
either gem_info and gem_exec_store failed than reloading failed. Is that a
reasonable approach and it is OK with you?

> 
> One more thing about gem_info(), to be used in process it needs to be
> something like:
> 
> /* Check the driver load completed by seeing if it responds to an ioctl
>  * correctly. We pick an ioctl that calls i915_mutex_interruptible()
>  * first (before failing on the invalid arg) so that we can inspect its
>  * driver error detection (-EIO if already wedged, or -EINTR if stuck).
>  */
> void i915_gem_sanitycheck(void)
> {
> 	struct set_cacheing arg = {};
> 	int fd, err;
> 
> 	fd = __drm_driver_open(DRIVER_INTEL);
> 	igt_set_timeout(1, "module reload failed");
> 
> 	err = 0;
> 	if (ioctl(fd, SET_CACHEING, &arg))
> 		err = -errno;
> 
> 	igt_set_timeout(0, NULL);
> 	close(fd);
> 
> 	igt_assert_eq(err, -ENOENT);
> 
> }
Alright will do.
> -Chris
> 
> -- 
> Chris Wilson, Intel Open Source Technology Centre

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH i-g-t 4/4 v4] tests/kms_sysfs_edid_timing: Convert sh to C version.

[Marius Vlad]

[-- Attachment #1.1: Type: text/plain, Size: 7535 bytes --]

On Fri, Oct 28, 2016 at 11:03:44AM +0100, Chris Wilson wrote:
> On Fri, Oct 28, 2016 at 12:31:29PM +0300, Marius Vlad wrote:
> > v2:
> > - don't read cached values (Chris Wilson)
> > - warn on per connector, and fail per mean (Chris Wilson)
> > 
> > These are synthetic: 5us per connector, and 600us for all (as
> > threshold).
> > 
> > Signed-off-by: Marius Vlad <marius.c.vlad@intel.com>
> > ---
> >  tests/Makefile.sources        |   2 +-
> >  tests/kms_sysfs_edid_timing   |  25 ----------
> >  tests/kms_sysfs_edid_timing.c | 107 ++++++++++++++++++++++++++++++++++++++++++
> >  3 files changed, 108 insertions(+), 26 deletions(-)
> >  delete mode 100755 tests/kms_sysfs_edid_timing
> >  create mode 100644 tests/kms_sysfs_edid_timing.c
> > 
> > diff --git a/tests/Makefile.sources b/tests/Makefile.sources
> > index b1511c6..8af455a 100644
> > --- a/tests/Makefile.sources
> > +++ b/tests/Makefile.sources
> > @@ -212,6 +212,7 @@ TESTS_progs = \
> >  	kms_sink_crc_basic \
> >  	prime_udl \
> >  	drv_module_reload \
> > +	kms_sysfs_edid_timing \
> >  	$(NULL)
> >  
> >  # IMPORTANT: The ZZ_ tests need to be run last!
> > @@ -222,7 +223,6 @@ TESTS_scripts_M = \
> >  TESTS_scripts = \
> >  	debugfs_emon_crash \
> >  	drv_debugfs_reader \
> > -	kms_sysfs_edid_timing \
> >  	sysfs_l3_parity \
> >  	test_rte_check \
> >  	tools_test \
> > diff --git a/tests/kms_sysfs_edid_timing b/tests/kms_sysfs_edid_timing
> > deleted file mode 100755
> > index 46ea540..0000000
> > --- a/tests/kms_sysfs_edid_timing
> > +++ /dev/null
> > @@ -1,25 +0,0 @@
> > -#!/bin/bash
> > -#
> > -# This check the time we take to read the content of all the possible connectors.
> > -# Without the edid -ENXIO patch (http://permalink.gmane.org/gmane.comp.video.dri.devel/62083),
> > -# we sometimes take a *really* long time. So let's just check for some reasonable timing here
> > -#
> > -
> > -DRM_LIB_ALLOW_NO_MASTER=1
> > -
> > -SOURCE_DIR="$( dirname "${BASH_SOURCE[0]}" )"
> > -. $SOURCE_DIR/drm_lib.sh
> > -
> > -TIME1=$(date +%s%N)
> > -cat $(find /sys/devices/|grep drm | grep /status) > /dev/null
> > -TIME2=$(date +%s%N)
> > -
> > -# time in ms
> > -RES=$(((TIME2 - TIME1) / 1000000))
> > -
> > -if [ $RES -gt 600 ]; then
> > -	echo "Talking to outputs took ${RES}ms, something is wrong"
> > -	exit $IGT_EXIT_FAILURE
> > -fi
> > -
> > -exit $IGT_EXIT_SUCCESS
> > diff --git a/tests/kms_sysfs_edid_timing.c b/tests/kms_sysfs_edid_timing.c
> > new file mode 100644
> > index 0000000..3b8bca0
> > --- /dev/null
> > +++ b/tests/kms_sysfs_edid_timing.c
> > @@ -0,0 +1,107 @@
> > +/*
> > + * Copyright © 2016 Intel Corporation
> > + *
> > + * Permission is hereby granted, free of charge, to any person obtaining a
> > + * copy of this software and associated documentation files (the "Software"),
> > + * to deal in the Software without restriction, including without limitation
> > + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> > + * and/or sell copies of the Software, and to permit persons to whom the
> > + * Software is furnished to do so, subject to the following conditions:
> > + *
> > + * The above copyright notice and this permission notice (including the next
> > + * paragraph) shall be included in all copies or substantial portions of the
> > + * Software.
> > + *
> > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> > + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> > + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> > + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> > + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
> > + * IN THE SOFTWARE.
> > + */
> > +#include "igt.h"
> > +#include "igt_debugfs.h"
> > +#include "igt_sysfs.h"
> > +#include "igt_core.h"
> > +
> > +#include <dirent.h>
> > +#include <fcntl.h>
> > +#include <sys/stat.h>
> > +#include <time.h>
> > +
> > +#define USEC_PER_SEC		(1000*1000)
> > +#define THRESHOLD_PER_CONNECTOR	5
> > +#define THRESHOLD_TOTAL		600
> > +
> > +IGT_TEST_DESCRIPTION("This check the time we take to read the content of all "
> > +		     "the possible connectors. Without the edid -ENXIO patch "
> > +		     "(http://permalink.gmane.org/gmane.comp.video.dri.devel/62083), "
> > +		     "we sometimes take a *really* long time. "
> > +		     "So let's just check for some reasonable timing here");
> > +
> > +static uint64_t
> > +mean(uint64_t *a, size_t a_len)
> > +{
> > +	int p;
> > +	uint64_t __sum;
> > +
> > +	for (p = 0, __sum = 0; p < a_len; p++) {
> > +		__sum += a[p];
> > +	}
> > +
> > +	return (__sum / a_len);
> > +}
> 
> See igt_stats.h: igt_mean
Alright will use that.
> 
> > +igt_simple_main
> > +{
> > +	int dir = igt_sysfs_open(-1, NULL);
> 
> Hmm, that will try to find intel - I was hoping to be a little more
> generic and do every cardN-connectorM in /sys/class/drm (not
> /sys/class/drm/card0 as sysfs_open will return)
Right, misunderstood you the first time.
> 
> > +	DIR *dirp = fdopendir(dir);
> > +	struct dirent *de;
> > +
> > +	int i, fd = 0;
> > +	int fds[32];
> > +
> > +	uint64_t elapsed, elapsed_mean;
> > +	uint64_t a[32];
> > +
> > +	struct stat st;
> > +	struct timespec start = {};
> > +
> > +	memset(fds, -1, sizeof(fds));
> > +	memset(a, -1, sizeof(a));
> > +
> > +	while ((de = readdir(dirp))) {
> > +
> > +		if (*de->d_name == '.')
> > +			continue;
> > +
> > +		if (fstatat(dir, de->d_name, &st, 0))
> > +			continue;
> > +
> > +		if (S_ISDIR(st.st_mode) && !strncmp(de->d_name, "card0-", 6)) {
> 
> sysfs_open would have reported the index as assuming card0 fails if vgem
> was loaded first, for example.
> 
> > +			int dfd = openat(dir, de->d_name, O_RDONLY);
> > +			igt_assert(dfd > 0);
> > +			fds[fd++] = dfd;
> > +		}
> > +	}
> > +	closedir(dirp);
> > +
> > +	for (i = 0; i < fd; i++) {
> > +		igt_sysfs_set(fds[i], "status", "detect");
> > +
> > +		elapsed = igt_nsec_elapsed(&start);
> > +		igt_sysfs_get(fds[i], "status");
> > +		elapsed = igt_nsec_elapsed(&start);
> > +
> > +		igt_warn_on_f(elapsed > (THRESHOLD_PER_CONNECTOR * USEC_PER_SEC),
> > +		"Threshold per connector exceeded (elapsed %lu ns)\n", elapsed);
> 
> I was suggesting we repeat the probe of each connector several times.
> 
> DIR *dir = opendir("/sys/class/drm");
> while ((de = readdir(dir)) {
> 	struct igt_mean mean = {};
> 	struct stat st;
> 	char path[128];
> 
> 	if (*de->d_name == '.') continue;
> 	snprintf(path, sizeof(path), "/sys/class/drm/%s/status", de->d_name);
> 	if (stat(path, &st)
> 		continue;
> 
> 	for (int i = 0; i < 15; i++) {
> 		struct timespec tv;
> 		int fd;
> 
> 		fd = open(path, O_WRONLY);
> 
> 		igt_nsec_elapsed(&tv);
> 		write(fd, "detect\n", 7);
> 		igt_mean_add(&mean, igt_nsec_elapsed(&tv));
> 
> 		close(fd);
> 	}
> 
> 	if (mean.max > 10e6) /* 10 milisecond */
> 		igt_warn("%s: probe time exceed 10ms, max=%.2fms, avg=%.2fms\n",
> 			de->d_name, mean.max / 1e6, mean.mean / 1e6);
> 	igt_assert_f(mean.mean < 50e6,
> 		     "%s: average probe time exceeded 50ms, max=%.2fms, avg=%.2fms\n",
> 		     de->d_name, mean.max / 1e6, mean.mean / 1e6);
> }
Okay will do that.
> 
> -- 
> Chris Wilson, Intel Open Source Technology Centre

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH i-g-t 2/4 v4] lib/igt_gvt: Make use of libkmod helpers and fix reading gvt parameter.

[Marius Vlad]

[-- Attachment #1.1: Type: text/plain, Size: 2699 bytes --]

On Fri, Oct 28, 2016 at 11:08:19AM +0100, Chris Wilson wrote:
> On Fri, Oct 28, 2016 at 12:31:27PM +0300, Marius Vlad wrote:
> > Signed-off-by: Marius Vlad <marius.c.vlad@intel.com>
> > ---
> >  lib/igt_gvt.c     | 42 +++++++++++++++++++++++++++++++++++-------
> >  tests/gvt_basic.c |  2 +-
> >  2 files changed, 36 insertions(+), 8 deletions(-)
> > 
> > diff --git a/lib/igt_gvt.c b/lib/igt_gvt.c
> > index 8bbf9bd..d868cb3 100644
> > --- a/lib/igt_gvt.c
> > +++ b/lib/igt_gvt.c
> > @@ -24,23 +24,26 @@
> >  #include "igt.h"
> >  #include "igt_gvt.h"
> >  #include "igt_sysfs.h"
> > +#include "igt_kmod.h"
> >  
> > +#include <signal.h>
> >  #include <dirent.h>
> >  #include <unistd.h>
> >  #include <fcntl.h>
> > +#include <time.h>
> >  
> >  static bool is_gvt_enabled(void)
> >  {
> >  	FILE *file;
> > -	int value;
> > +	char value;
> >  	bool enabled = false;
> >  
> >  	file = fopen("/sys/module/i915/parameters/enable_gvt", "r");
> >  	if (!file)
> >  		return false;
> >  
> > -	if (fscanf(file, "%d", &value) == 1)
> > -		enabled = value;
> > +	if (fscanf(file, "%c", &value) == 1)
> > +		enabled = (value == 'Y' ? true : false);
> 	enabled = value == 'Y';
> else if (fscanf(file, "%d", &value) == 1)
> 	enabled = value;
> 
> Do I see a igt_kmod_parameter_get_boolean() in the future, I think I do!
> 
> >  	fclose(file);
> >  
> >  	errno = 0;
> > @@ -50,9 +53,20 @@ static bool is_gvt_enabled(void)
> >  static void unload_i915(void)
> >  {
> >  	kick_fbcon(false);
> > -	/* pkill alsact */
> >  
> > -	igt_ignore_warn(system("/sbin/modprobe -s -r i915"));
> > +
> > +	if (igt_kmod_is_loaded("i915")) {
> > +
> > +		if (igt_kmod_is_loaded("snd_hda_intel")) {
> > +			igt_assert(!igt_pkill(SIGTERM, "alsactl"));
> > +			igt_assert(!igt_kmod_unload("snd_hda_intel", 0));
> > +		}
> > +
> > +		igt_assert(!igt_kmod_unload("i915", 0));
> > +		igt_assert(!igt_kmod_unload("drm_kms_helper", 0));
> > +		igt_assert(!igt_kmod_unload("drm", 0));
> 
> But don't we already have this routine...

No. But I guess I can add a driver_load()/driver_unload() in
lib/igt_kmod which does that, if that is what you meant.

> 
> For this test, we shouldn't fail if we can't setup the environment as we
> need, but skip the test.

Right, maybe I got it wrong, but you call
igt_require(igt_gvt_load_module()) which in turn calls is_gvt_enabled().
igt_require will skip if igt_gvt_load_module() is false. That is why I
kept like that. Besides that, I haven't checked what happens when the
modules are built-in, so the assert might fail in that case.

> -Chris
> 
> -- 
> Chris Wilson, Intel Open Source Technology Centre

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH i-g-t 2/4 v4] lib/igt_gvt: Make use of libkmod helpers and fix reading gvt parameter.

[Chris Wilson]

On Sun, Oct 30, 2016 at 02:47:56PM +0200, Marius Vlad wrote:
> On Fri, Oct 28, 2016 at 11:08:19AM +0100, Chris Wilson wrote:
> > On Fri, Oct 28, 2016 at 12:31:27PM +0300, Marius Vlad wrote:
> > > Signed-off-by: Marius Vlad <marius.c.vlad@intel.com>
> > > ---
> > >  lib/igt_gvt.c     | 42 +++++++++++++++++++++++++++++++++++-------
> > >  tests/gvt_basic.c |  2 +-
> > >  2 files changed, 36 insertions(+), 8 deletions(-)
> > > 
> > > diff --git a/lib/igt_gvt.c b/lib/igt_gvt.c
> > > index 8bbf9bd..d868cb3 100644
> > > --- a/lib/igt_gvt.c
> > > +++ b/lib/igt_gvt.c
> > > @@ -24,23 +24,26 @@
> > >  #include "igt.h"
> > >  #include "igt_gvt.h"
> > >  #include "igt_sysfs.h"
> > > +#include "igt_kmod.h"
> > >  
> > > +#include <signal.h>
> > >  #include <dirent.h>
> > >  #include <unistd.h>
> > >  #include <fcntl.h>
> > > +#include <time.h>
> > >  
> > >  static bool is_gvt_enabled(void)
> > >  {
> > >  	FILE *file;
> > > -	int value;
> > > +	char value;
> > >  	bool enabled = false;
> > >  
> > >  	file = fopen("/sys/module/i915/parameters/enable_gvt", "r");
> > >  	if (!file)
> > >  		return false;
> > >  
> > > -	if (fscanf(file, "%d", &value) == 1)
> > > -		enabled = value;
> > > +	if (fscanf(file, "%c", &value) == 1)
> > > +		enabled = (value == 'Y' ? true : false);
> > 	enabled = value == 'Y';
> > else if (fscanf(file, "%d", &value) == 1)
> > 	enabled = value;
> > 
> > Do I see a igt_kmod_parameter_get_boolean() in the future, I think I do!
> > 
> > >  	fclose(file);
> > >  
> > >  	errno = 0;
> > > @@ -50,9 +53,20 @@ static bool is_gvt_enabled(void)
> > >  static void unload_i915(void)
> > >  {
> > >  	kick_fbcon(false);
> > > -	/* pkill alsact */
> > >  
> > > -	igt_ignore_warn(system("/sbin/modprobe -s -r i915"));
> > > +
> > > +	if (igt_kmod_is_loaded("i915")) {
> > > +
> > > +		if (igt_kmod_is_loaded("snd_hda_intel")) {
> > > +			igt_assert(!igt_pkill(SIGTERM, "alsactl"));
> > > +			igt_assert(!igt_kmod_unload("snd_hda_intel", 0));
> > > +		}
> > > +
> > > +		igt_assert(!igt_kmod_unload("i915", 0));
> > > +		igt_assert(!igt_kmod_unload("drm_kms_helper", 0));
> > > +		igt_assert(!igt_kmod_unload("drm", 0));
> > 
> > But don't we already have this routine...
> 
> No. But I guess I can add a driver_load()/driver_unload() in
> lib/igt_kmod which does that, if that is what you meant.

This is the same sequence as required for reloading any kms driver.
However, note here we only require i915 to be reloaded, we can avoid
taking further risk in reloading the drm layer. So perhaps we don't want
the same code here as drv_module_reload. (Or a more complete interface
that gives finer control.)

> > 
> > For this test, we shouldn't fail if we can't setup the environment as we
> > need, but skip the test.
> 
> Right, maybe I got it wrong, but you call
> igt_require(igt_gvt_load_module()) which in turn calls is_gvt_enabled().
> igt_require will skip if igt_gvt_load_module() is false. That is why I
> kept like that. Besides that, I haven't checked what happens when the
> modules are built-in, so the assert might fail in that case.

The asserts are indeed testing something unrelated to the test, only the
setup of its environment. If we can't set up the environment for the
test, we skip.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH i-g-t 1/4 v4] lib/{igt_sysfs, igt_aux}: Make available to other users kick_fbcon() (unbind_fbcon()), and added helpers to lib/igt_aux, lib/igt_kmod.

[Chris Wilson]

On Fri, Oct 28, 2016 at 12:31:26PM +0300, Marius Vlad wrote:
> diff --git a/lib/Makefile.am b/lib/Makefile.am
> index 4c0893d..e1737bd 100644
> --- a/lib/Makefile.am
> +++ b/lib/Makefile.am
> @@ -34,6 +34,8 @@ AM_CFLAGS += $(CAIRO_CFLAGS)

Missed adding the CFLAGS for KMOD and PROCPS.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx