On Thu, Oct 27, 2016 at 12:02:30AM +0300, Marius Vlad wrote:
> On Mon, Oct 24, 2016 at 10:40:37AM +0200, Daniel Vetter wrote:
> > On Thu, Oct 20, 2016 at 10:36:47PM +0300, Marius Vlad wrote:
> > > Previously under unbind_fbcon(), disable/enable framebuffer console.
> > >
> > > lib/igt_aux: Added helpers to help convert sh scripts to C version. libkmod and
> > > procps interface.
> > >
> > > Signed-off-by: Marius Vlad <marius.c.vlad@xxxxxxxxx>
> > > ---
> > > configure.ac | 2 +
> > > lib/Makefile.am | 2 +
> > > lib/igt_aux.c | 278 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> > > lib/igt_aux.h | 7 ++
> > > lib/igt_gvt.c | 43 +--------
> > > lib/igt_sysfs.c | 54 +++++++++++
> > > lib/igt_sysfs.h | 2 +
> > > 7 files changed, 347 insertions(+), 41 deletions(-)
> >
> > If you go with extracting stuff into helpers I'd go with a new helper
> > library like igt_kmod.c. Or just keep it in-line with tests, extracting
> > code is imo overrated ;-)
> Have no issue putting kmod helpers into their own file. Can't really
> tell which is the best approach though: the only user is drv_module_reload
> yet the test already got ``fatten'' up with the gem subtests (see v3).
tbh helper libraries only used by one testcase aren't all that useful. If
it's only used by this testcase then I'd say don't bother with the
library: It's a lot more work, and with just 1 user you're pretty much
guaranteed to come up with a sub-par library interface. It takes a few
different users of the same feature to see what's really needed in a good
library.
-Daniel
> >
> > Also pls make sure the docs are correct and look good, there's a bunch of
> > issues with them (like non-matching function names between comment and
> > actual code).
> Sorry, I totally missed your input, will double check on this.
> > -Daniel
> >
> > >
> > > 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/igt_aux.c b/lib/igt_aux.c
> > > index 421f6d4..d150818 100644
> > > --- a/lib/igt_aux.c
> > > +++ b/lib/igt_aux.c
> > > @@ -51,6 +51,9 @@
> > > #include <termios.h>
> > > #include <assert.h>
> > >
> > > +#include <proc/readproc.h>
> > > +#include <libkmod.h>
> > > +
> > > #include "drmtest.h"
> > > #include "i915_drm.h"
> > > #include "intel_chipset.h"
> > > @@ -1193,6 +1196,281 @@ void igt_set_module_param_int(const char *name, int val)
> > > igt_set_module_param(name, str);
> > > }
> > >
> > > +/**
> > > + * igt_pkill:
> > > + * @sig: Signal to send
> > > + * @name: 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 -1 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)
> > > +{
> > > + int err = 0, try = 5;
> > > + PROCTAB *proc;
> > > + proc_t *proc_info;
> > > +
> > > + proc = openproc(PROC_FILLCOM | PROC_FILLSTAT | PROC_FILLARG);
> > > + igt_assert(proc != NULL);
> > > +
> > > + while ((proc_info = readproc(proc, NULL))) {
> > > + if (proc_info &&
> > > + !strncasecmp(proc_info->cmd, comm, sizeof(proc_info->cmd))) {
> > > + switch (sig) {
> > > + case SIGTERM:
> > > + case SIGKILL:
> > > + do {
> > > + kill(proc_info->tid, sig);
> > > + } while (kill(proc_info->tid, 0) < 0 && try--);
> > > +
> > > + if (kill(proc_info->tid, 0) < 0)
> > > + err = -1;
> > > + break;
> > > + default:
> > > + if (kill(proc_info->tid, sig) < 0)
> > > + err = -1;
> > > + break;
> > > + }
> > > +
> > > + freeproc(proc_info);
> > > + break;
> > > + }
> > > + freeproc(proc_info);
> > > + }
> > > +
> > > + closeproc(proc);
> > > + return err;
> > > +}
> > > +
> > > +/**
> > > + * igt_kill:
> > > + * @sig: Signal to send.
> > > + * @pid: Process pid to send.
> > > + * @returns: 0 in case of success or -1 otherwise.
> > > + *
> > > + * This function is identical to igt_pkill() only that it searches the process
> > > + * table using @pid instead of comm name.
> > > + *
> > > + */
> > > +int
> > > +igt_kill(int sig, pid_t pid)
> > > +{
> > > + int err = 0, try = 5;
> > > + PROCTAB *proc;
> > > + proc_t *proc_info;
> > > +
> > > + proc = openproc(PROC_PID | PROC_FILLSTAT | PROC_FILLARG);
> > > + igt_assert(proc != NULL);
> > > +
> > > + while ((proc_info = readproc(proc, NULL))) {
> > > + if (proc_info && proc_info->tid == pid) {
> > > + switch (sig) {
> > > + case SIGTERM:
> > > + case SIGKILL:
> > > + do {
> > > + kill(proc_info->tid, sig);
> > > + } while (kill(proc_info->tid, 0) < 0 && try--);
> > > +
> > > + if (kill(proc_info->tid, 0) < 0)
> > > + err = -1;
> > > + break;
> > > + default:
> > > + if (kill(proc_info->tid, sig) < 0)
> > > + err = -1;
> > > + break;
> > > + }
> > > + freeproc(proc_info);
> > > + break;
> > > + }
> > > + freeproc(proc_info);
> > > + }
> > > +
> > > + closeproc(proc);
> > > + return err;
> > > +}
> > > +
> > > +static bool
> > > +igt_module_in_use(struct kmod_module *kmod)
> > > +{
> > > + int err;
> > > +
> > > + err = kmod_module_get_initstate(kmod);
> > > +
> > > + /* if compiled builtin or does not exist */
> > > + if (err == KMOD_MODULE_BUILTIN || err < 0)
> > > + return false;
> > > +
> > > + if (kmod_module_get_refcnt(kmod) != 0 ||
> > > + kmod_module_get_holders(kmod) != NULL)
> > > + return true;
> > > +
> > > +
> > > + return false;
> > > +}
> > > +
> > > +/**
> > > + * igt_lsmod:
> > > + * @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_lsmod_has_module(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) {
> > > + kmod_unref(ctx);
> > > + return ret;
> > > + }
> > > +
> > > + kmod_list_foreach(mod, list) {
> > > + struct kmod_module *kmod = kmod_module_get_module(mod);
> > > + const char *kmod_name = kmod_module_get_name(kmod);
> > > +
> > > + if (!strncasecmp(kmod_name, mod_name, strlen(kmod_name))) {
> > > + kmod_module_unref(kmod);
> > > + ret = true;
> > > + break;
> > > + }
> > > + kmod_module_unref(kmod);
> > > + }
> > > +
> > > + kmod_module_unref_list(list);
> > > + kmod_unref(ctx);
> > > +
> > > + return ret;
> > > +}
> > > +
> > > +
> > > +/**
> > > + * igt_insmod:
> > > + * @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 -1 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_insmod(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_info("Module %s already inserted\n",
> > > + kmod_module_get_name(kmod));
> > > + err = -1;
> > > + break;
> > > + case -ENOENT:
> > > + igt_info("Unknown symbol in module %s or "
> > > + "unknown parameter\n",
> > > + kmod_module_get_name(kmod));
> > > + err = -1;
> > > + break;
> > > + default:
> > > + igt_info("Could not insert %s (%s)\n",
> > > + kmod_module_get_name(kmod), strerror(-err));
> > > + err = -1;
> > > + break;
> > > + }
> > > + }
> > > +out:
> > > + kmod_module_unref(kmod);
> > > + kmod_unref(ctx);
> > > +
> > > + return err;
> > > +}
> > > +
> > > +
> > > +/**
> > > + * igt_rmmod:
> > > + * @mod_name: Module name.
> > > + * @force: A bool value to force removal. Note that his flag does not remove
> > > + * the module(s) that the module specified by @mod_name depends on. In other
> > > + * words you must igt_rmmod() the module(s) dependencies before calling it with
> > > + * @mod_name.
> > > + * @returns: 0 in case of success or -1 otherwise.
> > > + *
> > > + * Removes the module @mod_name.
> > > + *
> > > + */
> > > +int
> > > +igt_rmmod(const char *mod_name, bool force)
> > > +{
> > > + struct kmod_ctx *ctx;
> > > + struct kmod_module *kmod;
> > > + int err, flags = 0;
> > > +
> > > + ctx = kmod_new(NULL, NULL);
> > > + igt_assert(ctx != NULL);
> > > +
> > > + err = kmod_module_new_from_name(ctx, mod_name, &kmod);
> > > + if (err < 0) {
> > > + igt_info("Could not use module %s (%s)\n", mod_name,
> > > + strerror(-err));
> > > + err = -1;
> > > + goto out;
> > > + }
> > > +
> > > + if (igt_module_in_use(kmod)) {
> > > + igt_info("Module %s is in use\n", mod_name);
> > > + err = -1;
> > > + goto out;
> > > + }
> > > +
> > > + if (force) {
> > > + flags |= KMOD_REMOVE_FORCE;
> > > + }
> > > +
> > > + err = kmod_module_remove_module(kmod, flags);
> > > + if (err < 0) {
> > > + igt_info("Could not remove module %s (%s)\n", mod_name,
> > > + strerror(-err));
> > > + err = -1;
> > > + }
> > > +
> > > +out:
> > > + kmod_module_unref(kmod);
> > > + kmod_unref(ctx);
> > > +
> > > + 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..7946923 100644
> > > --- a/lib/igt_aux.h
> > > +++ b/lib/igt_aux.h
> > > @@ -264,4 +264,11 @@ 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);
> > > +int igt_kill(int sig, pid_t pid);
> > > +
> > > +bool igt_lsmod_has_module(const char *mod_name);
> > > +int igt_insmod(const char *mod_name, const char *opts);
> > > +int igt_rmmod(const char *mod_name, bool force);
> > > +
> > > #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_sysfs.c b/lib/igt_sysfs.c
> > > index 612de75..0803659 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"
> > >
> > > /**
> > > @@ -391,3 +395,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@xxxxxxxxxxxxxxxxxxxxx
> > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> >
> > --
> > Daniel Vetter
> > Software Engineer, Intel Corporation
> > http://blog.ffwll.ch
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
