* [igt-dev] [PATCH i-g-t 1/8] lib/params: add igt_params.c for module parameter access
2020-04-21 16:17 [igt-dev] [PATCH i-g-t 0/8] Use device dependant module parameters Juha-Pekka Heikkila
@ 2020-04-21 16:17 ` Juha-Pekka Heikkila
2020-04-22 8:02 ` Petri Latvala
2020-04-21 16:17 ` [igt-dev] [PATCH i-g-t 2/8] lib/params: start renaming functions igt_params_* Juha-Pekka Heikkila
` (9 subsequent siblings)
10 siblings, 1 reply; 35+ messages in thread
From: Juha-Pekka Heikkila @ 2020-04-21 16:17 UTC (permalink / raw)
To: igt-dev; +Cc: Jani Nikula
From: Jani Nikula <jani.nikula@intel.com>
We have generic helpers for sysfs access in igt_sysfs.c, but we also
have a number of module parameter access specific helpers scattered here
and there. Start gathering the latter into a file of its own.
For i915, the long-term goal is to migrate from module parameters to
device specific debugfs parameters. With all igt module param access
centralized in one place, we can make the transition much easier.
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
Signed-off-by: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>
---
lib/Makefile.sources | 2 +
lib/i915/gem_submission.c | 1 +
lib/igt.h | 1 +
lib/igt_aux.c | 143 +-----------------
lib/igt_aux.h | 3 -
lib/igt_gt.c | 1 +
lib/igt_params.c | 244 +++++++++++++++++++++++++++++++
lib/igt_params.h | 38 +++++
lib/igt_psr.c | 1 +
lib/igt_sysfs.c | 68 ---------
lib/igt_sysfs.h | 5 -
lib/meson.build | 1 +
tests/i915/gem_ctx_persistence.c | 1 +
13 files changed, 291 insertions(+), 218 deletions(-)
create mode 100644 lib/igt_params.c
create mode 100644 lib/igt_params.h
diff --git a/lib/Makefile.sources b/lib/Makefile.sources
index 1e2c88ae..2ead9d02 100644
--- a/lib/Makefile.sources
+++ b/lib/Makefile.sources
@@ -47,6 +47,8 @@ lib_source_list = \
igt_list.h \
igt_matrix.c \
igt_matrix.h \
+ igt_params.c \
+ igt_params.h \
igt_primes.c \
igt_primes.h \
igt_rand.c \
diff --git a/lib/i915/gem_submission.c b/lib/i915/gem_submission.c
index 72de0c22..bb58a207 100644
--- a/lib/i915/gem_submission.c
+++ b/lib/i915/gem_submission.c
@@ -32,6 +32,7 @@
#include "igt_core.h"
#include "igt_gt.h"
+#include "igt_params.h"
#include "igt_sysfs.h"
#include "intel_chipset.h"
#include "intel_reg.h"
diff --git a/lib/igt.h b/lib/igt.h
index a6c4e44d..0bf98aa5 100644
--- a/lib/igt.h
+++ b/lib/igt.h
@@ -37,6 +37,7 @@
#include "igt_frame.h"
#include "igt_gt.h"
#include "igt_kms.h"
+#include "igt_params.h"
#include "igt_pm.h"
#include "igt_stats.h"
#ifdef HAVE_CHAMELIUM
diff --git a/lib/igt_aux.c b/lib/igt_aux.c
index ecab5d99..c55b2916 100644
--- a/lib/igt_aux.c
+++ b/lib/igt_aux.c
@@ -60,6 +60,7 @@
#include "igt_aux.h"
#include "igt_debugfs.h"
#include "igt_gt.h"
+#include "igt_params.h"
#include "igt_rand.h"
#include "igt_sysfs.h"
#include "config.h"
@@ -1116,148 +1117,6 @@ void igt_unlock_mem(void)
locked_mem = NULL;
}
-
-#define MODULE_PARAM_DIR "/sys/module/i915/parameters/"
-#define PARAM_NAME_MAX_SZ 32
-#define PARAM_VALUE_MAX_SZ 16
-#define PARAM_FILE_PATH_MAX_SZ (strlen(MODULE_PARAM_DIR) + PARAM_NAME_MAX_SZ)
-
-struct module_param_data {
- char name[PARAM_NAME_MAX_SZ];
- char original_value[PARAM_VALUE_MAX_SZ];
-
- struct module_param_data *next;
-};
-struct module_param_data *module_params = NULL;
-
-static void igt_module_param_exit_handler(int sig)
-{
- const size_t dir_len = strlen(MODULE_PARAM_DIR);
- char file_path[PARAM_FILE_PATH_MAX_SZ];
- struct module_param_data *data;
- int fd;
-
- /* We don't need to assert string sizes on this function since they were
- * already checked before being stored on the lists. Besides,
- * igt_assert() is not AS-Safe. */
- strcpy(file_path, MODULE_PARAM_DIR);
-
- for (data = module_params; data != NULL; data = data->next) {
- strcpy(file_path + dir_len, data->name);
-
- fd = open(file_path, O_RDWR);
- if (fd >= 0) {
- int size = strlen (data->original_value);
-
- if (size != write(fd, data->original_value, size)) {
- const char msg[] = "WARNING: Module parameters "
- "may not have been reset to their "
- "original values\n";
- assert(write(STDERR_FILENO, msg, sizeof(msg))
- == sizeof(msg));
- }
-
- close(fd);
- }
- }
- /* free() is not AS-Safe, so we can't call it here. */
-}
-
-/**
- * igt_save_module_param:
- * @name: name of the i915.ko module parameter
- * @file_path: full sysfs file path for the parameter
- *
- * Reads the current value of an i915.ko module parameter, saves it on an array,
- * then installs an exit handler to restore it when the program exits.
- *
- * It is safe to call this function multiple times for the same parameter.
- *
- * Notice that this function is called by igt_set_module_param(), so that one -
- * or one of its wrappers - is the only function the test programs need to call.
- */
-static void igt_save_module_param(const char *name, const char *file_path)
-{
- struct module_param_data *data;
- size_t n;
- int fd;
-
- /* Check if this parameter is already saved. */
- for (data = module_params; data != NULL; data = data->next)
- if (strncmp(data->name, name, PARAM_NAME_MAX_SZ) == 0)
- return;
-
- if (!module_params)
- igt_install_exit_handler(igt_module_param_exit_handler);
-
- data = calloc(1, sizeof (*data));
- igt_assert(data);
-
- strncpy(data->name, name, PARAM_NAME_MAX_SZ - 1);
-
- fd = open(file_path, O_RDONLY);
- igt_assert(fd >= 0);
-
- n = read(fd, data->original_value, PARAM_VALUE_MAX_SZ);
- igt_assert_f(n > 0 && n < PARAM_VALUE_MAX_SZ,
- "Need to increase PARAM_VALUE_MAX_SZ\n");
-
- igt_assert(close(fd) == 0);
-
- data->next = module_params;
- module_params = data;
-}
-
-/**
- * igt_set_module_param:
- * @name: i915.ko parameter name
- * @val: i915.ko parameter value
- *
- * This function sets the desired value for the given i915.ko parameter. It also
- * takes care of saving and restoring the values that were already set before
- * the test was run.
- *
- * Please consider using igt_set_module_param_int() for the integer and bool
- * parameters.
- */
-void igt_set_module_param(const char *name, const char *val)
-{
- char file_path[PARAM_FILE_PATH_MAX_SZ];
- size_t len = strlen(val);
- int fd;
-
- igt_assert_f(strlen(name) < PARAM_NAME_MAX_SZ,
- "Need to increase PARAM_NAME_MAX_SZ\n");
- strcpy(file_path, MODULE_PARAM_DIR);
- strcpy(file_path + strlen(MODULE_PARAM_DIR), name);
-
- igt_save_module_param(name, file_path);
-
- fd = open(file_path, O_RDWR);
- igt_assert(write(fd, val, len) == len);
- igt_assert(close(fd) == 0);
-}
-
-/**
- * igt_set_module_param_int:
- * @name: i915.ko parameter name
- * @val: i915.ko parameter value
- *
- * This is a wrapper for igt_set_module_param() that takes an integer instead of
- * a string. Please see igt_set_module_param().
- */
-void igt_set_module_param_int(const char *name, int val)
-{
- char str[PARAM_VALUE_MAX_SZ];
- int n;
-
- n = snprintf(str, PARAM_VALUE_MAX_SZ, "%d\n", val);
- igt_assert_f(n < PARAM_VALUE_MAX_SZ,
- "Need to increase PARAM_VALUE_MAX_SZ\n");
-
- igt_set_module_param(name, str);
-}
-
/**
* igt_is_process_running:
* @comm: Name of process in the form found in /proc/pid/comm (limited to 15
diff --git a/lib/igt_aux.h b/lib/igt_aux.h
index 04d22904..bf57ccf5 100644
--- a/lib/igt_aux.h
+++ b/lib/igt_aux.h
@@ -285,9 +285,6 @@ double igt_stop_siglatency(struct igt_mean *result);
bool igt_allow_unlimited_files(void);
-void igt_set_module_param(const char *name, const char *val);
-void igt_set_module_param_int(const char *name, int val);
-
int igt_is_process_running(const char *comm);
int igt_terminate_process(int sig, const char *comm);
void igt_lsof(const char *dpath);
diff --git a/lib/igt_gt.c b/lib/igt_gt.c
index 256c7cbc..22340a3d 100644
--- a/lib/igt_gt.c
+++ b/lib/igt_gt.c
@@ -35,6 +35,7 @@
#include "igt_aux.h"
#include "igt_core.h"
#include "igt_gt.h"
+#include "igt_params.h"
#include "igt_sysfs.h"
#include "igt_debugfs.h"
#include "ioctl_wrappers.h"
diff --git a/lib/igt_params.c b/lib/igt_params.c
new file mode 100644
index 00000000..4bd2b1f2
--- /dev/null
+++ b/lib/igt_params.c
@@ -0,0 +1,244 @@
+/*
+ * Copyright © 2019 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 <assert.h>
+#include <fcntl.h>
+#include <limits.h>
+#include <stdbool.h>
+#include <stdio.h>
+#include <sys/ioctl.h>
+
+#include <i915_drm.h>
+
+#include "igt_core.h"
+#include "igt_params.h"
+#include "igt_sysfs.h"
+
+#define MODULE_PARAM_DIR "/sys/module/i915/parameters/"
+#define PARAM_NAME_MAX_SZ 32
+#define PARAM_VALUE_MAX_SZ 16
+#define PARAM_FILE_PATH_MAX_SZ (strlen(MODULE_PARAM_DIR) + PARAM_NAME_MAX_SZ)
+
+struct module_param_data {
+ char name[PARAM_NAME_MAX_SZ];
+ char original_value[PARAM_VALUE_MAX_SZ];
+
+ struct module_param_data *next;
+};
+struct module_param_data *module_params = NULL;
+
+static void igt_module_param_exit_handler(int sig)
+{
+ const size_t dir_len = strlen(MODULE_PARAM_DIR);
+ char file_path[PARAM_FILE_PATH_MAX_SZ];
+ struct module_param_data *data;
+ int fd;
+
+ /* We don't need to assert string sizes on this function since they were
+ * already checked before being stored on the lists. Besides,
+ * igt_assert() is not AS-Safe. */
+ strcpy(file_path, MODULE_PARAM_DIR);
+
+ for (data = module_params; data != NULL; data = data->next) {
+ strcpy(file_path + dir_len, data->name);
+
+ fd = open(file_path, O_RDWR);
+ if (fd >= 0) {
+ int size = strlen (data->original_value);
+
+ if (size != write(fd, data->original_value, size)) {
+ const char msg[] = "WARNING: Module parameters "
+ "may not have been reset to their "
+ "original values\n";
+ assert(write(STDERR_FILENO, msg, sizeof(msg))
+ == sizeof(msg));
+ }
+
+ close(fd);
+ }
+ }
+ /* free() is not AS-Safe, so we can't call it here. */
+}
+
+/**
+ * igt_save_module_param:
+ * @name: name of the i915.ko module parameter
+ * @file_path: full sysfs file path for the parameter
+ *
+ * Reads the current value of an i915.ko module parameter, saves it on an array,
+ * then installs an exit handler to restore it when the program exits.
+ *
+ * It is safe to call this function multiple times for the same parameter.
+ *
+ * Notice that this function is called by igt_set_module_param(), so that one -
+ * or one of its wrappers - is the only function the test programs need to call.
+ */
+static void igt_save_module_param(const char *name, const char *file_path)
+{
+ struct module_param_data *data;
+ size_t n;
+ int fd;
+
+ /* Check if this parameter is already saved. */
+ for (data = module_params; data != NULL; data = data->next)
+ if (strncmp(data->name, name, PARAM_NAME_MAX_SZ) == 0)
+ return;
+
+ if (!module_params)
+ igt_install_exit_handler(igt_module_param_exit_handler);
+
+ data = calloc(1, sizeof (*data));
+ igt_assert(data);
+
+ strncpy(data->name, name, PARAM_NAME_MAX_SZ - 1);
+
+ fd = open(file_path, O_RDONLY);
+ igt_assert(fd >= 0);
+
+ n = read(fd, data->original_value, PARAM_VALUE_MAX_SZ);
+ igt_assert_f(n > 0 && n < PARAM_VALUE_MAX_SZ,
+ "Need to increase PARAM_VALUE_MAX_SZ\n");
+
+ igt_assert(close(fd) == 0);
+
+ data->next = module_params;
+ module_params = data;
+}
+
+/**
+ * igt_sysfs_open_parameters:
+ * @device: fd of the device
+ *
+ * This opens the module parameters directory (under sysfs) corresponding
+ * to the device for use with igt_sysfs_set() and igt_sysfs_get().
+ *
+ * Returns:
+ * The directory fd, or -1 on failure.
+ */
+int igt_sysfs_open_parameters(int device)
+{
+ int dir, params = -1;
+
+ dir = igt_sysfs_open(device);
+ if (dir >= 0) {
+ params = openat(dir,
+ "device/driver/module/parameters",
+ O_RDONLY);
+ close(dir);
+ }
+
+ if (params < 0) { /* builtin? */
+ drm_version_t version;
+ char name[32] = "";
+ char path[PATH_MAX];
+
+ memset(&version, 0, sizeof(version));
+ version.name_len = sizeof(name);
+ version.name = name;
+ ioctl(device, DRM_IOCTL_VERSION, &version);
+
+ sprintf(path, "/sys/module/%s/parameters", name);
+ params = open(path, O_RDONLY);
+ }
+
+ return params;
+}
+
+/**
+ * igt_sysfs_set_parameters:
+ * @device: fd of the device
+ * @parameter: the name of the parameter to set
+ * @fmt: printf-esque format string
+ *
+ * Returns true on success
+ */
+bool igt_sysfs_set_parameter(int device,
+ const char *parameter,
+ const char *fmt, ...)
+{
+ va_list ap;
+ int dir;
+ int ret;
+
+ dir = igt_sysfs_open_parameters(device);
+ if (dir < 0)
+ return false;
+
+ va_start(ap, fmt);
+ ret = igt_sysfs_vprintf(dir, parameter, fmt, ap);
+ va_end(ap);
+
+ close(dir);
+
+ return ret > 0;
+}
+
+/**
+ * igt_set_module_param:
+ * @name: i915.ko parameter name
+ * @val: i915.ko parameter value
+ *
+ * This function sets the desired value for the given i915.ko parameter. It also
+ * takes care of saving and restoring the values that were already set before
+ * the test was run.
+ *
+ * Please consider using igt_set_module_param_int() for the integer and bool
+ * parameters.
+ */
+void igt_set_module_param(const char *name, const char *val)
+{
+ char file_path[PARAM_FILE_PATH_MAX_SZ];
+ size_t len = strlen(val);
+ int fd;
+
+ igt_assert_f(strlen(name) < PARAM_NAME_MAX_SZ,
+ "Need to increase PARAM_NAME_MAX_SZ\n");
+ strcpy(file_path, MODULE_PARAM_DIR);
+ strcpy(file_path + strlen(MODULE_PARAM_DIR), name);
+
+ igt_save_module_param(name, file_path);
+
+ fd = open(file_path, O_RDWR);
+ igt_assert(write(fd, val, len) == len);
+ igt_assert(close(fd) == 0);
+}
+
+/**
+ * igt_set_module_param_int:
+ * @name: i915.ko parameter name
+ * @val: i915.ko parameter value
+ *
+ * This is a wrapper for igt_set_module_param() that takes an integer instead of
+ * a string. Please see igt_set_module_param().
+ */
+void igt_set_module_param_int(const char *name, int val)
+{
+ char str[PARAM_VALUE_MAX_SZ];
+ int n;
+
+ n = snprintf(str, PARAM_VALUE_MAX_SZ, "%d\n", val);
+ igt_assert_f(n < PARAM_VALUE_MAX_SZ,
+ "Need to increase PARAM_VALUE_MAX_SZ\n");
+
+ igt_set_module_param(name, str);
+}
diff --git a/lib/igt_params.h b/lib/igt_params.h
new file mode 100644
index 00000000..cf0fd18f
--- /dev/null
+++ b/lib/igt_params.h
@@ -0,0 +1,38 @@
+/*
+ * Copyright © 2019 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_PARAMS_H__
+#define __IGT_PARAMS_H__
+
+#include <stdbool.h>
+
+int igt_sysfs_open_parameters(int device);
+
+__attribute__((format(printf, 3, 4)))
+bool igt_sysfs_set_parameter(int device, const char *parameter,
+ const char *fmt, ...);
+
+void igt_set_module_param(const char *name, const char *val);
+void igt_set_module_param_int(const char *name, int val);
+
+#endif /* __IGT_PARAMS_H__ */
diff --git a/lib/igt_psr.c b/lib/igt_psr.c
index 83ccacdd..2bcf14ea 100644
--- a/lib/igt_psr.c
+++ b/lib/igt_psr.c
@@ -21,6 +21,7 @@
* IN THE SOFTWARE.
*/
+#include "igt_params.h"
#include "igt_psr.h"
#include "igt_sysfs.h"
#include <errno.h>
diff --git a/lib/igt_sysfs.c b/lib/igt_sysfs.c
index 7528c3bd..6aafe534 100644
--- a/lib/igt_sysfs.c
+++ b/lib/igt_sysfs.c
@@ -135,74 +135,6 @@ int igt_sysfs_open(int device)
return open(path, O_RDONLY);
}
-/**
- * igt_sysfs_set_parameters:
- * @device: fd of the device
- * @parameter: the name of the parameter to set
- * @fmt: printf-esque format string
- *
- * Returns true on success
- */
-bool igt_sysfs_set_parameter(int device,
- const char *parameter,
- const char *fmt, ...)
-{
- va_list ap;
- int dir;
- int ret;
-
- dir = igt_sysfs_open_parameters(device);
- if (dir < 0)
- return false;
-
- va_start(ap, fmt);
- ret = igt_sysfs_vprintf(dir, parameter, fmt, ap);
- va_end(ap);
-
- close(dir);
-
- return ret > 0;
-}
-
-/**
- * igt_sysfs_open_parameters:
- * @device: fd of the device
- *
- * This opens the module parameters directory (under sysfs) corresponding
- * to the device for use with igt_sysfs_set() and igt_sysfs_get().
- *
- * Returns:
- * The directory fd, or -1 on failure.
- */
-int igt_sysfs_open_parameters(int device)
-{
- int dir, params = -1;
-
- dir = igt_sysfs_open(device);
- if (dir >= 0) {
- params = openat(dir,
- "device/driver/module/parameters",
- O_RDONLY);
- close(dir);
- }
-
- if (params < 0) { /* builtin? */
- drm_version_t version;
- char name[32] = "";
- char path[PATH_MAX];
-
- memset(&version, 0, sizeof(version));
- version.name_len = sizeof(name);
- version.name = name;
- ioctl(device, DRM_IOCTL_VERSION, &version);
-
- sprintf(path, "/sys/module/%s/parameters", name);
- params = open(path, O_RDONLY);
- }
-
- return params;
-}
-
/**
* igt_sysfs_write:
* @dir: directory for the device from igt_sysfs_open()
diff --git a/lib/igt_sysfs.h b/lib/igt_sysfs.h
index b646df30..64935a5c 100644
--- a/lib/igt_sysfs.h
+++ b/lib/igt_sysfs.h
@@ -30,11 +30,6 @@
char *igt_sysfs_path(int device, char *path, int pathlen);
int igt_sysfs_open(int device);
-int igt_sysfs_open_parameters(int device);
-bool igt_sysfs_set_parameter(int device,
- const char *parameter,
- const char *fmt, ...)
- __attribute__((format(printf,3,4)));
int igt_sysfs_read(int dir, const char *attr, void *data, int len);
int igt_sysfs_write(int dir, const char *attr, const void *data, int len);
diff --git a/lib/meson.build b/lib/meson.build
index e2060430..c9af0403 100644
--- a/lib/meson.build
+++ b/lib/meson.build
@@ -17,6 +17,7 @@ lib_sources = [
'igt_halffloat.c',
'igt_matrix.c',
'igt_perf.c',
+ 'igt_params.c',
'igt_primes.c',
'igt_rand.c',
'igt_rapl.c',
diff --git a/tests/i915/gem_ctx_persistence.c b/tests/i915/gem_ctx_persistence.c
index 3d52987d..965b788b 100644
--- a/tests/i915/gem_ctx_persistence.c
+++ b/tests/i915/gem_ctx_persistence.c
@@ -38,6 +38,7 @@
#include "igt_dummyload.h"
#include "igt_gt.h"
#include "igt_sysfs.h"
+#include "igt_params.h"
#include "ioctl_wrappers.h" /* gem_wait()! */
#include "sw_sync.h"
--
2.17.1
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply related [flat|nested] 35+ messages in thread* Re: [igt-dev] [PATCH i-g-t 1/8] lib/params: add igt_params.c for module parameter access
2020-04-21 16:17 ` [igt-dev] [PATCH i-g-t 1/8] lib/params: add igt_params.c for module parameter access Juha-Pekka Heikkila
@ 2020-04-22 8:02 ` Petri Latvala
2020-04-22 8:13 ` Juha-Pekka Heikkila
0 siblings, 1 reply; 35+ messages in thread
From: Petri Latvala @ 2020-04-22 8:02 UTC (permalink / raw)
To: Juha-Pekka Heikkila; +Cc: igt-dev, Jani Nikula
On Tue, Apr 21, 2020 at 07:17:18PM +0300, Juha-Pekka Heikkila wrote:
> From: Jani Nikula <jani.nikula@intel.com>
>
> We have generic helpers for sysfs access in igt_sysfs.c, but we also
> have a number of module parameter access specific helpers scattered here
> and there. Start gathering the latter into a file of its own.
>
> For i915, the long-term goal is to migrate from module parameters to
> device specific debugfs parameters. With all igt module param access
> centralized in one place, we can make the transition much easier.
>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> Signed-off-by: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>
This patch is just moving code around without functional changes,
correct?
> diff --git a/lib/meson.build b/lib/meson.build
> index e2060430..c9af0403 100644
> --- a/lib/meson.build
> +++ b/lib/meson.build
> @@ -17,6 +17,7 @@ lib_sources = [
> 'igt_halffloat.c',
> 'igt_matrix.c',
> 'igt_perf.c',
> + 'igt_params.c',
> 'igt_primes.c',
> 'igt_rand.c',
> 'igt_rapl.c',
Alphabetical ordering is off-by-one here.
--
Petri Latvala
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [igt-dev] [PATCH i-g-t 1/8] lib/params: add igt_params.c for module parameter access
2020-04-22 8:02 ` Petri Latvala
@ 2020-04-22 8:13 ` Juha-Pekka Heikkila
2020-04-22 8:19 ` Petri Latvala
0 siblings, 1 reply; 35+ messages in thread
From: Juha-Pekka Heikkila @ 2020-04-22 8:13 UTC (permalink / raw)
To: Petri Latvala; +Cc: igt-dev, Jani Nikula
On 22.4.2020 11.02, Petri Latvala wrote:
> On Tue, Apr 21, 2020 at 07:17:18PM +0300, Juha-Pekka Heikkila wrote:
>> From: Jani Nikula <jani.nikula@intel.com>
>>
>> We have generic helpers for sysfs access in igt_sysfs.c, but we also
>> have a number of module parameter access specific helpers scattered here
>> and there. Start gathering the latter into a file of its own.
>>
>> For i915, the long-term goal is to migrate from module parameters to
>> device specific debugfs parameters. With all igt module param access
>> centralized in one place, we can make the transition much easier.
>>
>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>> Signed-off-by: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>
>
>
> This patch is just moving code around without functional changes,
> correct?
Yes, it's correct. It's just buildup for those changes that follow in
those other patches.
>
>
>> diff --git a/lib/meson.build b/lib/meson.build
>> index e2060430..c9af0403 100644
>> --- a/lib/meson.build
>> +++ b/lib/meson.build
>> @@ -17,6 +17,7 @@ lib_sources = [
>> 'igt_halffloat.c',
>> 'igt_matrix.c',
>> 'igt_perf.c',
>> + 'igt_params.c',
>> 'igt_primes.c',
>> 'igt_rand.c',
>> 'igt_rapl.c',
>
> Alphabetical ordering is off-by-one here.
I'll rearrange.
>
>
>
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [igt-dev] [PATCH i-g-t 1/8] lib/params: add igt_params.c for module parameter access
2020-04-22 8:13 ` Juha-Pekka Heikkila
@ 2020-04-22 8:19 ` Petri Latvala
0 siblings, 0 replies; 35+ messages in thread
From: Petri Latvala @ 2020-04-22 8:19 UTC (permalink / raw)
To: Juha-Pekka Heikkila; +Cc: igt-dev, Jani Nikula
On Wed, Apr 22, 2020 at 11:13:43AM +0300, Juha-Pekka Heikkila wrote:
> On 22.4.2020 11.02, Petri Latvala wrote:
> > On Tue, Apr 21, 2020 at 07:17:18PM +0300, Juha-Pekka Heikkila wrote:
> > > From: Jani Nikula <jani.nikula@intel.com>
> > >
> > > We have generic helpers for sysfs access in igt_sysfs.c, but we also
> > > have a number of module parameter access specific helpers scattered here
> > > and there. Start gathering the latter into a file of its own.
> > >
> > > For i915, the long-term goal is to migrate from module parameters to
> > > device specific debugfs parameters. With all igt module param access
> > > centralized in one place, we can make the transition much easier.
> > >
> > > Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> > > Signed-off-by: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>
> >
> >
> > This patch is just moving code around without functional changes,
> > correct?
>
> Yes, it's correct. It's just buildup for those changes that follow in those
> other patches.
Alright. Code motion patches are impossible to determine to be just
code motion without magic eyes. :P
>
> >
> >
> > > diff --git a/lib/meson.build b/lib/meson.build
> > > index e2060430..c9af0403 100644
> > > --- a/lib/meson.build
> > > +++ b/lib/meson.build
> > > @@ -17,6 +17,7 @@ lib_sources = [
> > > 'igt_halffloat.c',
> > > 'igt_matrix.c',
> > > 'igt_perf.c',
> > > + 'igt_params.c',
> > > 'igt_primes.c',
> > > 'igt_rand.c',
> > > 'igt_rapl.c',
> >
> > Alphabetical ordering is off-by-one here.
>
> I'll rearrange.
With that done, slap in a
Reviewed-by: Petri Latvala <petri.latvala@intel.com>
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply [flat|nested] 35+ messages in thread
* [igt-dev] [PATCH i-g-t 2/8] lib/params: start renaming functions igt_params_*
2020-04-21 16:17 [igt-dev] [PATCH i-g-t 0/8] Use device dependant module parameters Juha-Pekka Heikkila
2020-04-21 16:17 ` [igt-dev] [PATCH i-g-t 1/8] lib/params: add igt_params.c for module parameter access Juha-Pekka Heikkila
@ 2020-04-21 16:17 ` Juha-Pekka Heikkila
2020-04-22 8:28 ` Petri Latvala
2020-04-21 16:17 ` [igt-dev] [PATCH i-g-t 3/8] lib/params: overhaul param saving Juha-Pekka Heikkila
` (8 subsequent siblings)
10 siblings, 1 reply; 35+ messages in thread
From: Juha-Pekka Heikkila @ 2020-04-21 16:17 UTC (permalink / raw)
To: igt-dev; +Cc: Jani Nikula
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
Signed-off-by: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>
---
lib/drmtest.c | 2 +-
lib/i915/gem_submission.c | 2 +-
lib/igt_aux.c | 3 +--
lib/igt_gt.c | 2 +-
lib/igt_params.c | 12 +++++-------
lib/igt_params.h | 5 ++---
tests/i915/gem_ctx_exec.c | 2 +-
tests/i915/gem_ctx_persistence.c | 9 ++++-----
tests/i915/gem_mmap_gtt.c | 2 +-
tests/i915/gem_reset_stats.c | 6 ++----
tests/i915/sysfs_heartbeat_interval.c | 3 ++-
tests/i915/sysfs_preempt_timeout.c | 3 ++-
tests/i915/sysfs_timeslice_duration.c | 3 ++-
13 files changed, 25 insertions(+), 29 deletions(-)
diff --git a/lib/drmtest.c b/lib/drmtest.c
index 1fc39925..17067843 100644
--- a/lib/drmtest.c
+++ b/lib/drmtest.c
@@ -393,7 +393,7 @@ static void __cancel_work_at_exit(int fd)
{
igt_terminate_spins(); /* for older kernels */
- igt_sysfs_set_parameter(fd, "reset", "%x", -1u /* any method */);
+ igt_params_set(fd, "reset", "%x", -1u /* any method */);
igt_drop_caches_set(fd,
/* cancel everything */
DROP_RESET_ACTIVE | DROP_RESET_SEQNO |
diff --git a/lib/i915/gem_submission.c b/lib/i915/gem_submission.c
index bb58a207..4d80eb6a 100644
--- a/lib/i915/gem_submission.c
+++ b/lib/i915/gem_submission.c
@@ -78,7 +78,7 @@ unsigned gem_submission_method(int fd)
int dir;
- dir = igt_sysfs_open_parameters(fd);
+ dir = igt_params_open(fd);
if (dir < 0)
return 0;
diff --git a/lib/igt_aux.c b/lib/igt_aux.c
index c55b2916..fba34933 100644
--- a/lib/igt_aux.c
+++ b/lib/igt_aux.c
@@ -514,8 +514,7 @@ void igt_fork_hang_detector(int fd)
* they are a test failure!) and so the loss of per-engine reset
* functionality is not an issue.
*/
- igt_assert(igt_sysfs_set_parameter
- (fd, "reset", "%d", 1 /* only global reset */));
+ igt_assert(igt_params_set(fd, "reset", "%d", 1 /* only global reset */));
signal(SIGIO, sig_abort);
igt_fork_helper(&hang_detector)
diff --git a/lib/igt_gt.c b/lib/igt_gt.c
index 22340a3d..cedb142d 100644
--- a/lib/igt_gt.c
+++ b/lib/igt_gt.c
@@ -188,7 +188,7 @@ igt_hang_t igt_allow_hang(int fd, unsigned ctx, unsigned flags)
__gem_context_set_param(fd, ¶m);
allow_reset = INT_MAX; /* any reset method */
}
- igt_require(igt_sysfs_set_parameter(fd, "reset", "%d", allow_reset));
+ igt_require(igt_params_set(fd, "reset", "%d", allow_reset));
if (!igt_check_boolean_env_var("IGT_HANG_WITHOUT_RESET", false))
igt_require(has_gpu_reset(fd));
diff --git a/lib/igt_params.c b/lib/igt_params.c
index 4bd2b1f2..f220e73b 100644
--- a/lib/igt_params.c
+++ b/lib/igt_params.c
@@ -126,7 +126,7 @@ static void igt_save_module_param(const char *name, const char *file_path)
}
/**
- * igt_sysfs_open_parameters:
+ * igt_params_open:
* @device: fd of the device
*
* This opens the module parameters directory (under sysfs) corresponding
@@ -135,7 +135,7 @@ static void igt_save_module_param(const char *name, const char *file_path)
* Returns:
* The directory fd, or -1 on failure.
*/
-int igt_sysfs_open_parameters(int device)
+int igt_params_open(int device)
{
int dir, params = -1;
@@ -165,22 +165,20 @@ int igt_sysfs_open_parameters(int device)
}
/**
- * igt_sysfs_set_parameters:
+ * igt_params_set:
* @device: fd of the device
* @parameter: the name of the parameter to set
* @fmt: printf-esque format string
*
* Returns true on success
*/
-bool igt_sysfs_set_parameter(int device,
- const char *parameter,
- const char *fmt, ...)
+bool igt_params_set(int device, const char *parameter, const char *fmt, ...)
{
va_list ap;
int dir;
int ret;
- dir = igt_sysfs_open_parameters(device);
+ dir = igt_params_open(device);
if (dir < 0)
return false;
diff --git a/lib/igt_params.h b/lib/igt_params.h
index cf0fd18f..52eed77f 100644
--- a/lib/igt_params.h
+++ b/lib/igt_params.h
@@ -26,11 +26,10 @@
#include <stdbool.h>
-int igt_sysfs_open_parameters(int device);
+int igt_params_open(int device);
__attribute__((format(printf, 3, 4)))
-bool igt_sysfs_set_parameter(int device, const char *parameter,
- const char *fmt, ...);
+bool igt_params_set(int device, const char *parameter, const char *fmt, ...);
void igt_set_module_param(const char *name, const char *val);
void igt_set_module_param_int(const char *name, int val);
diff --git a/tests/i915/gem_ctx_exec.c b/tests/i915/gem_ctx_exec.c
index ad2f9e54..bc2a352c 100644
--- a/tests/i915/gem_ctx_exec.c
+++ b/tests/i915/gem_ctx_exec.c
@@ -276,7 +276,7 @@ static void nohangcheck_hostile(int i915)
i915 = gem_reopen_driver(i915);
- dir = igt_sysfs_open_parameters(i915);
+ dir = igt_params_open(i915);
igt_require(dir != -1);
ctx = gem_context_create(i915);
diff --git a/tests/i915/gem_ctx_persistence.c b/tests/i915/gem_ctx_persistence.c
index 965b788b..cdaa87aa 100644
--- a/tests/i915/gem_ctx_persistence.c
+++ b/tests/i915/gem_ctx_persistence.c
@@ -95,7 +95,7 @@ static void enable_hangcheck(int i915)
{
int dir;
- dir = igt_sysfs_open_parameters(i915);
+ dir = igt_params_open(i915);
if (dir < 0) /* no parameters, must be default! */
return;
@@ -361,7 +361,7 @@ static void test_nohangcheck_hostile(int i915)
* we forcibly terminate that context.
*/
- dir = igt_sysfs_open_parameters(i915);
+ dir = igt_params_open(i915);
igt_require(dir != -1);
igt_require(__enable_hangcheck(dir, false));
@@ -398,7 +398,7 @@ static void test_nohangcheck_hang(int i915)
igt_require(!gem_has_cmdparser(i915, ALL_ENGINES));
- dir = igt_sysfs_open_parameters(i915);
+ dir = igt_params_open(i915);
igt_require(dir != -1);
igt_require(__enable_hangcheck(dir, false));
@@ -1090,8 +1090,7 @@ igt_main
igt_require_gem(i915);
/* Restore the reset modparam if left clobbered */
- igt_assert(igt_sysfs_set_parameter
- (i915, "reset", "%d", -1 /* any [default] reset */));
+ igt_assert(igt_params_set(i915, "reset", "%d", -1));
enable_hangcheck(i915);
igt_install_exit_handler(exit_handler);
diff --git a/tests/i915/gem_mmap_gtt.c b/tests/i915/gem_mmap_gtt.c
index 1f4655af..32c92287 100644
--- a/tests/i915/gem_mmap_gtt.c
+++ b/tests/i915/gem_mmap_gtt.c
@@ -580,7 +580,7 @@ test_hang(int fd)
int dir;
hang = igt_allow_hang(fd, 0, 0);
- igt_require(igt_sysfs_set_parameter(fd, "reset", "1")); /* global */
+ igt_require(igt_params_set(fd, "reset", "1")); /* global */
control = mmap(NULL, 4096, PROT_WRITE, MAP_SHARED | MAP_ANON, -1, 0);
igt_assert(control != MAP_FAILED);
diff --git a/tests/i915/gem_reset_stats.c b/tests/i915/gem_reset_stats.c
index 9c7b6645..8ea090d8 100644
--- a/tests/i915/gem_reset_stats.c
+++ b/tests/i915/gem_reset_stats.c
@@ -784,8 +784,7 @@ igt_main
has_reset_stats = gem_has_reset_stats(fd);
- igt_assert(igt_sysfs_set_parameter
- (fd, "reset", "%d", 1 /* only global reset */));
+ igt_assert(igt_params_set(fd, "reset", "%d", 1 /* only global reset */));
using_full_reset = !gem_engine_reset_enabled(fd) &&
gem_gpu_reset_enabled(fd);
@@ -846,8 +845,7 @@ igt_main
int fd;
fd = drm_open_driver(DRIVER_INTEL);
- igt_assert(igt_sysfs_set_parameter
- (fd, "reset", "%d", INT_MAX /* any reset method */));
+ igt_assert(igt_params_set(fd, "reset", "%d", INT_MAX /* any reset method */));
close(fd);
}
}
diff --git a/tests/i915/sysfs_heartbeat_interval.c b/tests/i915/sysfs_heartbeat_interval.c
index 662f4865..d1924f95 100644
--- a/tests/i915/sysfs_heartbeat_interval.c
+++ b/tests/i915/sysfs_heartbeat_interval.c
@@ -31,6 +31,7 @@
#include <sys/wait.h>
#include <unistd.h>
+#include "igt_params.h"
#include "drmtest.h" /* gem_quiescent_gpu()! */
#include "i915/gem_engine_topology.h"
#include "igt_dummyload.h"
@@ -52,7 +53,7 @@ static void enable_hangcheck(int i915, bool state)
{
int dir;
- dir = igt_sysfs_open_parameters(i915);
+ dir = igt_params_open(i915);
if (dir < 0) /* no parameters, must be default! */
return;
diff --git a/tests/i915/sysfs_preempt_timeout.c b/tests/i915/sysfs_preempt_timeout.c
index a7e93a4d..8a52dfb9 100644
--- a/tests/i915/sysfs_preempt_timeout.c
+++ b/tests/i915/sysfs_preempt_timeout.c
@@ -29,6 +29,7 @@
#include <sys/types.h>
#include <unistd.h>
+#include "igt_params.h"
#include "drmtest.h" /* gem_quiescent_gpu()! */
#include "i915/gem_engine_topology.h"
#include "igt_dummyload.h"
@@ -51,7 +52,7 @@ static bool enable_hangcheck(int i915, bool state)
bool success;
int dir;
- dir = igt_sysfs_open_parameters(i915);
+ dir = igt_params_open(i915);
if (dir < 0) /* no parameters, must be default! */
return false;
diff --git a/tests/i915/sysfs_timeslice_duration.c b/tests/i915/sysfs_timeslice_duration.c
index b983df43..123b9e19 100644
--- a/tests/i915/sysfs_timeslice_duration.c
+++ b/tests/i915/sysfs_timeslice_duration.c
@@ -29,6 +29,7 @@
#include <sys/types.h>
#include <unistd.h>
+#include "igt_params.h"
#include "drmtest.h" /* gem_quiescent_gpu()! */
#include "i915/gem_engine_topology.h"
#include "i915/gem_mman.h"
@@ -61,7 +62,7 @@ static bool enable_hangcheck(int i915, bool state)
bool success;
int dir;
- dir = igt_sysfs_open_parameters(i915);
+ dir = igt_params_open(i915);
if (dir < 0) /* no parameters, must be default! */
return false;
--
2.17.1
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply related [flat|nested] 35+ messages in thread* Re: [igt-dev] [PATCH i-g-t 2/8] lib/params: start renaming functions igt_params_*
2020-04-21 16:17 ` [igt-dev] [PATCH i-g-t 2/8] lib/params: start renaming functions igt_params_* Juha-Pekka Heikkila
@ 2020-04-22 8:28 ` Petri Latvala
2020-04-22 8:34 ` Jani Nikula
0 siblings, 1 reply; 35+ messages in thread
From: Petri Latvala @ 2020-04-22 8:28 UTC (permalink / raw)
To: Juha-Pekka Heikkila; +Cc: igt-dev, Jani Nikula
On Tue, Apr 21, 2020 at 07:17:19PM +0300, Juha-Pekka Heikkila wrote:
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> Signed-off-by: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>
> ---
> lib/drmtest.c | 2 +-
> lib/i915/gem_submission.c | 2 +-
> lib/igt_aux.c | 3 +--
> lib/igt_gt.c | 2 +-
> lib/igt_params.c | 12 +++++-------
> lib/igt_params.h | 5 ++---
> tests/i915/gem_ctx_exec.c | 2 +-
> tests/i915/gem_ctx_persistence.c | 9 ++++-----
> tests/i915/gem_mmap_gtt.c | 2 +-
> tests/i915/gem_reset_stats.c | 6 ++----
> tests/i915/sysfs_heartbeat_interval.c | 3 ++-
> tests/i915/sysfs_preempt_timeout.c | 3 ++-
> tests/i915/sysfs_timeslice_duration.c | 3 ++-
> 13 files changed, 25 insertions(+), 29 deletions(-)
>
> diff --git a/lib/drmtest.c b/lib/drmtest.c
> index 1fc39925..17067843 100644
> --- a/lib/drmtest.c
> +++ b/lib/drmtest.c
> @@ -393,7 +393,7 @@ static void __cancel_work_at_exit(int fd)
> {
> igt_terminate_spins(); /* for older kernels */
>
> - igt_sysfs_set_parameter(fd, "reset", "%x", -1u /* any method */);
> + igt_params_set(fd, "reset", "%x", -1u /* any method */);
Hmm. Now that we're renaming anyway, we have a good chance to consider
if the name is good.
igt_module_params_set()? No, the point of this series is to make the
params not module-specific. igt_device_params_set()?
I don't have a strong opinion on this and I can live with
igt_params_set() as well. Just calling out for opinions! Arek?
As for the patch, whatever the name, straight up renames all around.
Reviewed-by: Petri Latvala <petri.latvala@intel.com>
--
Petri Latvala
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply [flat|nested] 35+ messages in thread* Re: [igt-dev] [PATCH i-g-t 2/8] lib/params: start renaming functions igt_params_*
2020-04-22 8:28 ` Petri Latvala
@ 2020-04-22 8:34 ` Jani Nikula
2020-04-22 8:43 ` Petri Latvala
0 siblings, 1 reply; 35+ messages in thread
From: Jani Nikula @ 2020-04-22 8:34 UTC (permalink / raw)
To: Petri Latvala, Juha-Pekka Heikkila; +Cc: igt-dev
On Wed, 22 Apr 2020, Petri Latvala <petri.latvala@intel.com> wrote:
> On Tue, Apr 21, 2020 at 07:17:19PM +0300, Juha-Pekka Heikkila wrote:
>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>> Signed-off-by: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>
>> ---
>> lib/drmtest.c | 2 +-
>> lib/i915/gem_submission.c | 2 +-
>> lib/igt_aux.c | 3 +--
>> lib/igt_gt.c | 2 +-
>> lib/igt_params.c | 12 +++++-------
>> lib/igt_params.h | 5 ++---
>> tests/i915/gem_ctx_exec.c | 2 +-
>> tests/i915/gem_ctx_persistence.c | 9 ++++-----
>> tests/i915/gem_mmap_gtt.c | 2 +-
>> tests/i915/gem_reset_stats.c | 6 ++----
>> tests/i915/sysfs_heartbeat_interval.c | 3 ++-
>> tests/i915/sysfs_preempt_timeout.c | 3 ++-
>> tests/i915/sysfs_timeslice_duration.c | 3 ++-
>> 13 files changed, 25 insertions(+), 29 deletions(-)
>>
>> diff --git a/lib/drmtest.c b/lib/drmtest.c
>> index 1fc39925..17067843 100644
>> --- a/lib/drmtest.c
>> +++ b/lib/drmtest.c
>> @@ -393,7 +393,7 @@ static void __cancel_work_at_exit(int fd)
>> {
>> igt_terminate_spins(); /* for older kernels */
>>
>> - igt_sysfs_set_parameter(fd, "reset", "%x", -1u /* any method */);
>> + igt_params_set(fd, "reset", "%x", -1u /* any method */);
>
>
> Hmm. Now that we're renaming anyway, we have a good chance to consider
> if the name is good.
>
> igt_module_params_set()? No, the point of this series is to make the
> params not module-specific. igt_device_params_set()?
>
> I don't have a strong opinion on this and I can live with
> igt_params_set() as well. Just calling out for opinions! Arek?
>
> As for the patch, whatever the name, straight up renames all around.
> Reviewed-by: Petri Latvala <petri.latvala@intel.com>
The way I envisioned it is that it would look at the debugfs for device
params, and if they don't exist, it should fall back to the module
params for older kernels. Hence just igt_params_set to avoid being
specific about device vs. module. Also, shorter.
BR,
Jani.
--
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply [flat|nested] 35+ messages in thread* Re: [igt-dev] [PATCH i-g-t 2/8] lib/params: start renaming functions igt_params_*
2020-04-22 8:34 ` Jani Nikula
@ 2020-04-22 8:43 ` Petri Latvala
2020-04-22 8:48 ` Jani Nikula
0 siblings, 1 reply; 35+ messages in thread
From: Petri Latvala @ 2020-04-22 8:43 UTC (permalink / raw)
To: Jani Nikula; +Cc: igt-dev
On Wed, Apr 22, 2020 at 11:34:44AM +0300, Jani Nikula wrote:
> On Wed, 22 Apr 2020, Petri Latvala <petri.latvala@intel.com> wrote:
> > On Tue, Apr 21, 2020 at 07:17:19PM +0300, Juha-Pekka Heikkila wrote:
> >> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> >> Signed-off-by: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>
> >> ---
> >> lib/drmtest.c | 2 +-
> >> lib/i915/gem_submission.c | 2 +-
> >> lib/igt_aux.c | 3 +--
> >> lib/igt_gt.c | 2 +-
> >> lib/igt_params.c | 12 +++++-------
> >> lib/igt_params.h | 5 ++---
> >> tests/i915/gem_ctx_exec.c | 2 +-
> >> tests/i915/gem_ctx_persistence.c | 9 ++++-----
> >> tests/i915/gem_mmap_gtt.c | 2 +-
> >> tests/i915/gem_reset_stats.c | 6 ++----
> >> tests/i915/sysfs_heartbeat_interval.c | 3 ++-
> >> tests/i915/sysfs_preempt_timeout.c | 3 ++-
> >> tests/i915/sysfs_timeslice_duration.c | 3 ++-
> >> 13 files changed, 25 insertions(+), 29 deletions(-)
> >>
> >> diff --git a/lib/drmtest.c b/lib/drmtest.c
> >> index 1fc39925..17067843 100644
> >> --- a/lib/drmtest.c
> >> +++ b/lib/drmtest.c
> >> @@ -393,7 +393,7 @@ static void __cancel_work_at_exit(int fd)
> >> {
> >> igt_terminate_spins(); /* for older kernels */
> >>
> >> - igt_sysfs_set_parameter(fd, "reset", "%x", -1u /* any method */);
> >> + igt_params_set(fd, "reset", "%x", -1u /* any method */);
> >
> >
> > Hmm. Now that we're renaming anyway, we have a good chance to consider
> > if the name is good.
> >
> > igt_module_params_set()? No, the point of this series is to make the
> > params not module-specific. igt_device_params_set()?
> >
> > I don't have a strong opinion on this and I can live with
> > igt_params_set() as well. Just calling out for opinions! Arek?
> >
> > As for the patch, whatever the name, straight up renames all around.
> > Reviewed-by: Petri Latvala <petri.latvala@intel.com>
>
> The way I envisioned it is that it would look at the debugfs for device
> params, and if they don't exist, it should fall back to the module
> params for older kernels. Hence just igt_params_set to avoid being
> specific about device vs. module. Also, shorter.
I was more concerned with the confusion between params for the kernel
vs params for other operations. But I'm well prepared to accept that
the confusion exists only in my own head.
--
Petri Latvala
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply [flat|nested] 35+ messages in thread* Re: [igt-dev] [PATCH i-g-t 2/8] lib/params: start renaming functions igt_params_*
2020-04-22 8:43 ` Petri Latvala
@ 2020-04-22 8:48 ` Jani Nikula
2020-04-22 9:04 ` Arkadiusz Hiler
0 siblings, 1 reply; 35+ messages in thread
From: Jani Nikula @ 2020-04-22 8:48 UTC (permalink / raw)
To: Petri Latvala; +Cc: igt-dev
On Wed, 22 Apr 2020, Petri Latvala <petri.latvala@intel.com> wrote:
> On Wed, Apr 22, 2020 at 11:34:44AM +0300, Jani Nikula wrote:
>> On Wed, 22 Apr 2020, Petri Latvala <petri.latvala@intel.com> wrote:
>> > On Tue, Apr 21, 2020 at 07:17:19PM +0300, Juha-Pekka Heikkila wrote:
>> >> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>> >> Signed-off-by: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>
>> >> ---
>> >> lib/drmtest.c | 2 +-
>> >> lib/i915/gem_submission.c | 2 +-
>> >> lib/igt_aux.c | 3 +--
>> >> lib/igt_gt.c | 2 +-
>> >> lib/igt_params.c | 12 +++++-------
>> >> lib/igt_params.h | 5 ++---
>> >> tests/i915/gem_ctx_exec.c | 2 +-
>> >> tests/i915/gem_ctx_persistence.c | 9 ++++-----
>> >> tests/i915/gem_mmap_gtt.c | 2 +-
>> >> tests/i915/gem_reset_stats.c | 6 ++----
>> >> tests/i915/sysfs_heartbeat_interval.c | 3 ++-
>> >> tests/i915/sysfs_preempt_timeout.c | 3 ++-
>> >> tests/i915/sysfs_timeslice_duration.c | 3 ++-
>> >> 13 files changed, 25 insertions(+), 29 deletions(-)
>> >>
>> >> diff --git a/lib/drmtest.c b/lib/drmtest.c
>> >> index 1fc39925..17067843 100644
>> >> --- a/lib/drmtest.c
>> >> +++ b/lib/drmtest.c
>> >> @@ -393,7 +393,7 @@ static void __cancel_work_at_exit(int fd)
>> >> {
>> >> igt_terminate_spins(); /* for older kernels */
>> >>
>> >> - igt_sysfs_set_parameter(fd, "reset", "%x", -1u /* any method */);
>> >> + igt_params_set(fd, "reset", "%x", -1u /* any method */);
>> >
>> >
>> > Hmm. Now that we're renaming anyway, we have a good chance to consider
>> > if the name is good.
>> >
>> > igt_module_params_set()? No, the point of this series is to make the
>> > params not module-specific. igt_device_params_set()?
>> >
>> > I don't have a strong opinion on this and I can live with
>> > igt_params_set() as well. Just calling out for opinions! Arek?
>> >
>> > As for the patch, whatever the name, straight up renames all around.
>> > Reviewed-by: Petri Latvala <petri.latvala@intel.com>
>>
>> The way I envisioned it is that it would look at the debugfs for device
>> params, and if they don't exist, it should fall back to the module
>> params for older kernels. Hence just igt_params_set to avoid being
>> specific about device vs. module. Also, shorter.
>
> I was more concerned with the confusion between params for the kernel
> vs params for other operations. But I'm well prepared to accept that
> the confusion exists only in my own head.
Ah. I'll leave it up to your discretion if the confusion is bad
enough. At least I'm not hung up on the naming, so long as we unify the
naming, something this series tries to do as a side effect.
BR,
Jani.
--
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply [flat|nested] 35+ messages in thread* Re: [igt-dev] [PATCH i-g-t 2/8] lib/params: start renaming functions igt_params_*
2020-04-22 8:48 ` Jani Nikula
@ 2020-04-22 9:04 ` Arkadiusz Hiler
0 siblings, 0 replies; 35+ messages in thread
From: Arkadiusz Hiler @ 2020-04-22 9:04 UTC (permalink / raw)
To: Jani Nikula; +Cc: igt-dev, Petri Latvala
On Wed, Apr 22, 2020 at 11:48:53AM +0300, Jani Nikula wrote:
> On Wed, 22 Apr 2020, Petri Latvala <petri.latvala@intel.com> wrote:
> > On Wed, Apr 22, 2020 at 11:34:44AM +0300, Jani Nikula wrote:
> >> On Wed, 22 Apr 2020, Petri Latvala <petri.latvala@intel.com> wrote:
> >> > On Tue, Apr 21, 2020 at 07:17:19PM +0300, Juha-Pekka Heikkila wrote:
> >> >> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> >> >> Signed-off-by: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>
> >> >> ---
> >> >> lib/drmtest.c | 2 +-
> >> >> lib/i915/gem_submission.c | 2 +-
> >> >> lib/igt_aux.c | 3 +--
> >> >> lib/igt_gt.c | 2 +-
> >> >> lib/igt_params.c | 12 +++++-------
> >> >> lib/igt_params.h | 5 ++---
> >> >> tests/i915/gem_ctx_exec.c | 2 +-
> >> >> tests/i915/gem_ctx_persistence.c | 9 ++++-----
> >> >> tests/i915/gem_mmap_gtt.c | 2 +-
> >> >> tests/i915/gem_reset_stats.c | 6 ++----
> >> >> tests/i915/sysfs_heartbeat_interval.c | 3 ++-
> >> >> tests/i915/sysfs_preempt_timeout.c | 3 ++-
> >> >> tests/i915/sysfs_timeslice_duration.c | 3 ++-
> >> >> 13 files changed, 25 insertions(+), 29 deletions(-)
> >> >>
> >> >> diff --git a/lib/drmtest.c b/lib/drmtest.c
> >> >> index 1fc39925..17067843 100644
> >> >> --- a/lib/drmtest.c
> >> >> +++ b/lib/drmtest.c
> >> >> @@ -393,7 +393,7 @@ static void __cancel_work_at_exit(int fd)
> >> >> {
> >> >> igt_terminate_spins(); /* for older kernels */
> >> >>
> >> >> - igt_sysfs_set_parameter(fd, "reset", "%x", -1u /* any method */);
> >> >> + igt_params_set(fd, "reset", "%x", -1u /* any method */);
> >> >
> >> >
> >> > Hmm. Now that we're renaming anyway, we have a good chance to consider
> >> > if the name is good.
> >> >
> >> > igt_module_params_set()? No, the point of this series is to make the
> >> > params not module-specific. igt_device_params_set()?
> >> >
> >> > I don't have a strong opinion on this and I can live with
> >> > igt_params_set() as well. Just calling out for opinions! Arek?
> >> >
> >> > As for the patch, whatever the name, straight up renames all around.
> >> > Reviewed-by: Petri Latvala <petri.latvala@intel.com>
> >>
> >> The way I envisioned it is that it would look at the debugfs for device
> >> params, and if they don't exist, it should fall back to the module
> >> params for older kernels. Hence just igt_params_set to avoid being
> >> specific about device vs. module. Also, shorter.
> >
> > I was more concerned with the confusion between params for the kernel
> > vs params for other operations. But I'm well prepared to accept that
> > the confusion exists only in my own head.
>
> Ah. I'll leave it up to your discretion if the confusion is bad
> enough. At least I'm not hung up on the naming, so long as we unify the
> naming, something this series tries to do as a side effect.
igt_params() looks a bit like IGT itself is parametrizable in some
way... But the confusion did not last long, and I think this should be
easy enough to internalize. I would prefer using something like
igt_dev_params to both keep it short and make it a bit more obvious as
of what it does, but no strong opinions here.
--
Cheers,
Arek
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply [flat|nested] 35+ messages in thread
* [igt-dev] [PATCH i-g-t 3/8] lib/params: overhaul param saving
2020-04-21 16:17 [igt-dev] [PATCH i-g-t 0/8] Use device dependant module parameters Juha-Pekka Heikkila
2020-04-21 16:17 ` [igt-dev] [PATCH i-g-t 1/8] lib/params: add igt_params.c for module parameter access Juha-Pekka Heikkila
2020-04-21 16:17 ` [igt-dev] [PATCH i-g-t 2/8] lib/params: start renaming functions igt_params_* Juha-Pekka Heikkila
@ 2020-04-21 16:17 ` Juha-Pekka Heikkila
2020-04-28 12:26 ` Petri Latvala
2020-04-21 16:17 ` [igt-dev] [PATCH i-g-t 4/8] params open with path return Juha-Pekka Heikkila
` (7 subsequent siblings)
10 siblings, 1 reply; 35+ messages in thread
From: Juha-Pekka Heikkila @ 2020-04-21 16:17 UTC (permalink / raw)
To: igt-dev; +Cc: Jani Nikula
From: Jani Nikula <jani.nikula@intel.com>
More generic, use existing functions.
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
Signed-off-by: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>
---
lib/igt_params.c | 101 +++++++++++++++++++----------------------------
1 file changed, 41 insertions(+), 60 deletions(-)
diff --git a/lib/igt_params.c b/lib/igt_params.c
index f220e73b..b5ac1266 100644
--- a/lib/igt_params.c
+++ b/lib/igt_params.c
@@ -34,92 +34,74 @@
#include "igt_params.h"
#include "igt_sysfs.h"
-#define MODULE_PARAM_DIR "/sys/module/i915/parameters/"
-#define PARAM_NAME_MAX_SZ 32
-#define PARAM_VALUE_MAX_SZ 16
-#define PARAM_FILE_PATH_MAX_SZ (strlen(MODULE_PARAM_DIR) + PARAM_NAME_MAX_SZ)
-
struct module_param_data {
- char name[PARAM_NAME_MAX_SZ];
- char original_value[PARAM_VALUE_MAX_SZ];
+ char *path;
+ char *name;
+ char *original_value;
struct module_param_data *next;
};
struct module_param_data *module_params = NULL;
-static void igt_module_param_exit_handler(int sig)
+static void igt_params_exit_handler(int sig)
{
- const size_t dir_len = strlen(MODULE_PARAM_DIR);
- char file_path[PARAM_FILE_PATH_MAX_SZ];
struct module_param_data *data;
- int fd;
-
- /* We don't need to assert string sizes on this function since they were
- * already checked before being stored on the lists. Besides,
- * igt_assert() is not AS-Safe. */
- strcpy(file_path, MODULE_PARAM_DIR);
+ int dir;
for (data = module_params; data != NULL; data = data->next) {
- strcpy(file_path + dir_len, data->name);
-
- fd = open(file_path, O_RDWR);
- if (fd >= 0) {
- int size = strlen (data->original_value);
-
- if (size != write(fd, data->original_value, size)) {
- const char msg[] = "WARNING: Module parameters "
- "may not have been reset to their "
- "original values\n";
- assert(write(STDERR_FILENO, msg, sizeof(msg))
- == sizeof(msg));
- }
-
- close(fd);
+ dir = open(data->path, O_RDONLY);
+
+ if (!igt_sysfs_set(dir, data->name, data->original_value)) {
+ const char msg[] = "WARNING: Module parameters "
+ "may not have been reset to their "
+ "original values\n";
+ assert(write(STDERR_FILENO, msg, sizeof(msg))
+ == sizeof(msg));
}
+
+ close(dir);
}
/* free() is not AS-Safe, so we can't call it here. */
}
/**
- * igt_save_module_param:
- * @name: name of the i915.ko module parameter
- * @file_path: full sysfs file path for the parameter
+ * igt_params_save:
+ * @dir: file handle for path
+ * @path: full path to the sysfs directory
+ * @name: name of the sysfs attribute
*
- * Reads the current value of an i915.ko module parameter, saves it on an array,
- * then installs an exit handler to restore it when the program exits.
+ * Reads the current value of a sysfs attribute, saves it on an array, then
+ * installs an exit handler to restore it when the program exits.
*
* It is safe to call this function multiple times for the same parameter.
*
* Notice that this function is called by igt_set_module_param(), so that one -
* or one of its wrappers - is the only function the test programs need to call.
*/
-static void igt_save_module_param(const char *name, const char *file_path)
+static void igt_params_save(int dir, const char *path, const char *name)
{
struct module_param_data *data;
- size_t n;
- int fd;
/* Check if this parameter is already saved. */
for (data = module_params; data != NULL; data = data->next)
- if (strncmp(data->name, name, PARAM_NAME_MAX_SZ) == 0)
+ if (strcmp(data->path, path) == 0 &&
+ strcmp(data->name, name) == 0)
return;
if (!module_params)
- igt_install_exit_handler(igt_module_param_exit_handler);
+ igt_install_exit_handler(igt_params_exit_handler);
data = calloc(1, sizeof (*data));
igt_assert(data);
- strncpy(data->name, name, PARAM_NAME_MAX_SZ - 1);
-
- fd = open(file_path, O_RDONLY);
- igt_assert(fd >= 0);
+ data->path = strdup(path);
+ igt_assert(data->path);
- n = read(fd, data->original_value, PARAM_VALUE_MAX_SZ);
- igt_assert_f(n > 0 && n < PARAM_VALUE_MAX_SZ,
- "Need to increase PARAM_VALUE_MAX_SZ\n");
+ data->name = strdup(name);
+ igt_assert(data->name);
- igt_assert(close(fd) == 0);
+ data->original_value = igt_sysfs_get(dir, name);
+ igt_assert(data->original_value);
data->next = module_params;
module_params = data;
@@ -205,22 +187,21 @@ bool igt_params_set(int device, const char *parameter, const char *fmt, ...)
*/
void igt_set_module_param(const char *name, const char *val)
{
- char file_path[PARAM_FILE_PATH_MAX_SZ];
- size_t len = strlen(val);
- int fd;
+ const char *path = "/sys/module/i915/parameters";
+ int dir;
- igt_assert_f(strlen(name) < PARAM_NAME_MAX_SZ,
- "Need to increase PARAM_NAME_MAX_SZ\n");
- strcpy(file_path, MODULE_PARAM_DIR);
- strcpy(file_path + strlen(MODULE_PARAM_DIR), name);
+ dir = open(path, O_RDONLY);
+ igt_assert(dir >= 0);
- igt_save_module_param(name, file_path);
+ igt_params_save(dir, path, name);
- fd = open(file_path, O_RDWR);
- igt_assert(write(fd, val, len) == len);
- igt_assert(close(fd) == 0);
+ igt_assert(igt_sysfs_set(dir, name, val));
+
+ igt_assert(close(dir) == 0);
}
+#define PARAM_VALUE_MAX_SZ 16
+
/**
* igt_set_module_param_int:
* @name: i915.ko parameter name
--
2.17.1
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply related [flat|nested] 35+ messages in thread* Re: [igt-dev] [PATCH i-g-t 3/8] lib/params: overhaul param saving
2020-04-21 16:17 ` [igt-dev] [PATCH i-g-t 3/8] lib/params: overhaul param saving Juha-Pekka Heikkila
@ 2020-04-28 12:26 ` Petri Latvala
2020-04-28 18:40 ` Juha-Pekka Heikkila
0 siblings, 1 reply; 35+ messages in thread
From: Petri Latvala @ 2020-04-28 12:26 UTC (permalink / raw)
To: Juha-Pekka Heikkila; +Cc: igt-dev, Jani Nikula
On Tue, Apr 21, 2020 at 07:17:20PM +0300, Juha-Pekka Heikkila wrote:
> From: Jani Nikula <jani.nikula@intel.com>
>
> More generic, use existing functions.
>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> Signed-off-by: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>
> ---
> lib/igt_params.c | 101 +++++++++++++++++++----------------------------
> 1 file changed, 41 insertions(+), 60 deletions(-)
>
> diff --git a/lib/igt_params.c b/lib/igt_params.c
> index f220e73b..b5ac1266 100644
> --- a/lib/igt_params.c
> +++ b/lib/igt_params.c
> @@ -34,92 +34,74 @@
> #include "igt_params.h"
> #include "igt_sysfs.h"
>
> -#define MODULE_PARAM_DIR "/sys/module/i915/parameters/"
> -#define PARAM_NAME_MAX_SZ 32
> -#define PARAM_VALUE_MAX_SZ 16
> -#define PARAM_FILE_PATH_MAX_SZ (strlen(MODULE_PARAM_DIR) + PARAM_NAME_MAX_SZ)
> -
> struct module_param_data {
> - char name[PARAM_NAME_MAX_SZ];
> - char original_value[PARAM_VALUE_MAX_SZ];
> + char *path;
> + char *name;
> + char *original_value;
>
> struct module_param_data *next;
> };
> struct module_param_data *module_params = NULL;
>
> -static void igt_module_param_exit_handler(int sig)
> +static void igt_params_exit_handler(int sig)
> {
> - const size_t dir_len = strlen(MODULE_PARAM_DIR);
> - char file_path[PARAM_FILE_PATH_MAX_SZ];
> struct module_param_data *data;
> - int fd;
> -
> - /* We don't need to assert string sizes on this function since they were
> - * already checked before being stored on the lists. Besides,
> - * igt_assert() is not AS-Safe. */
> - strcpy(file_path, MODULE_PARAM_DIR);
> + int dir;
>
> for (data = module_params; data != NULL; data = data->next) {
> - strcpy(file_path + dir_len, data->name);
> -
> - fd = open(file_path, O_RDWR);
> - if (fd >= 0) {
> - int size = strlen (data->original_value);
> -
> - if (size != write(fd, data->original_value, size)) {
> - const char msg[] = "WARNING: Module parameters "
> - "may not have been reset to their "
> - "original values\n";
> - assert(write(STDERR_FILENO, msg, sizeof(msg))
> - == sizeof(msg));
> - }
> -
> - close(fd);
> + dir = open(data->path, O_RDONLY);
> +
> + if (!igt_sysfs_set(dir, data->name, data->original_value)) {
> + const char msg[] = "WARNING: Module parameters "
> + "may not have been reset to their "
> + "original values\n";
> + assert(write(STDERR_FILENO, msg, sizeof(msg))
> + == sizeof(msg));
> }
> +
> + close(dir);
> }
> /* free() is not AS-Safe, so we can't call it here. */
> }
>
> /**
> - * igt_save_module_param:
> - * @name: name of the i915.ko module parameter
> - * @file_path: full sysfs file path for the parameter
> + * igt_params_save:
> + * @dir: file handle for path
> + * @path: full path to the sysfs directory
> + * @name: name of the sysfs attribute
> *
> - * Reads the current value of an i915.ko module parameter, saves it on an array,
> - * then installs an exit handler to restore it when the program exits.
> + * Reads the current value of a sysfs attribute, saves it on an array, then
> + * installs an exit handler to restore it when the program exits.
> *
> * It is safe to call this function multiple times for the same parameter.
> *
> * Notice that this function is called by igt_set_module_param(), so that one -
> * or one of its wrappers - is the only function the test programs need to call.
> */
> -static void igt_save_module_param(const char *name, const char *file_path)
> +static void igt_params_save(int dir, const char *path, const char *name)
> {
> struct module_param_data *data;
> - size_t n;
> - int fd;
>
> /* Check if this parameter is already saved. */
> for (data = module_params; data != NULL; data = data->next)
> - if (strncmp(data->name, name, PARAM_NAME_MAX_SZ) == 0)
> + if (strcmp(data->path, path) == 0 &&
> + strcmp(data->name, name) == 0)
> return;
>
> if (!module_params)
> - igt_install_exit_handler(igt_module_param_exit_handler);
> + igt_install_exit_handler(igt_params_exit_handler);
>
> data = calloc(1, sizeof (*data));
> igt_assert(data);
>
> - strncpy(data->name, name, PARAM_NAME_MAX_SZ - 1);
> -
> - fd = open(file_path, O_RDONLY);
> - igt_assert(fd >= 0);
> + data->path = strdup(path);
> + igt_assert(data->path);
>
> - n = read(fd, data->original_value, PARAM_VALUE_MAX_SZ);
> - igt_assert_f(n > 0 && n < PARAM_VALUE_MAX_SZ,
> - "Need to increase PARAM_VALUE_MAX_SZ\n");
> + data->name = strdup(name);
> + igt_assert(data->name);
>
> - igt_assert(close(fd) == 0);
> + data->original_value = igt_sysfs_get(dir, name);
> + igt_assert(data->original_value);
>
> data->next = module_params;
> module_params = data;
> @@ -205,22 +187,21 @@ bool igt_params_set(int device, const char *parameter, const char *fmt, ...)
> */
> void igt_set_module_param(const char *name, const char *val)
> {
> - char file_path[PARAM_FILE_PATH_MAX_SZ];
> - size_t len = strlen(val);
> - int fd;
> + const char *path = "/sys/module/i915/parameters";
> + int dir;
>
> - igt_assert_f(strlen(name) < PARAM_NAME_MAX_SZ,
> - "Need to increase PARAM_NAME_MAX_SZ\n");
> - strcpy(file_path, MODULE_PARAM_DIR);
> - strcpy(file_path + strlen(MODULE_PARAM_DIR), name);
> + dir = open(path, O_RDONLY);
> + igt_assert(dir >= 0);
>
> - igt_save_module_param(name, file_path);
> + igt_params_save(dir, path, name);
>
> - fd = open(file_path, O_RDWR);
> - igt_assert(write(fd, val, len) == len);
> - igt_assert(close(fd) == 0);
> + igt_assert(igt_sysfs_set(dir, name, val));
> +
> + igt_assert(close(dir) == 0);
> }
>
> +#define PARAM_VALUE_MAX_SZ 16
> +
Is this define still needed for something?
--
Petri Latvala
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply [flat|nested] 35+ messages in thread* Re: [igt-dev] [PATCH i-g-t 3/8] lib/params: overhaul param saving
2020-04-28 12:26 ` Petri Latvala
@ 2020-04-28 18:40 ` Juha-Pekka Heikkila
2020-04-28 19:29 ` Juha-Pekka Heikkila
0 siblings, 1 reply; 35+ messages in thread
From: Juha-Pekka Heikkila @ 2020-04-28 18:40 UTC (permalink / raw)
To: Petri Latvala; +Cc: igt-dev, Jani Nikula
On 28.4.2020 15.26, Petri Latvala wrote:
> On Tue, Apr 21, 2020 at 07:17:20PM +0300, Juha-Pekka Heikkila wrote:
>> From: Jani Nikula <jani.nikula@intel.com>
>>
>> More generic, use existing functions.
>>
>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>> Signed-off-by: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>
>> ---
>> lib/igt_params.c | 101 +++++++++++++++++++----------------------------
>> 1 file changed, 41 insertions(+), 60 deletions(-)
>>
>> diff --git a/lib/igt_params.c b/lib/igt_params.c
>> index f220e73b..b5ac1266 100644
>> --- a/lib/igt_params.c
>> +++ b/lib/igt_params.c
>> @@ -34,92 +34,74 @@
>> #include "igt_params.h"
>> #include "igt_sysfs.h"
>>
>> -#define MODULE_PARAM_DIR "/sys/module/i915/parameters/"
>> -#define PARAM_NAME_MAX_SZ 32
>> -#define PARAM_VALUE_MAX_SZ 16
>> -#define PARAM_FILE_PATH_MAX_SZ (strlen(MODULE_PARAM_DIR) + PARAM_NAME_MAX_SZ)
>> -
>> struct module_param_data {
>> - char name[PARAM_NAME_MAX_SZ];
>> - char original_value[PARAM_VALUE_MAX_SZ];
>> + char *path;
>> + char *name;
>> + char *original_value;
>>
>> struct module_param_data *next;
>> };
>> struct module_param_data *module_params = NULL;
>>
>> -static void igt_module_param_exit_handler(int sig)
>> +static void igt_params_exit_handler(int sig)
>> {
>> - const size_t dir_len = strlen(MODULE_PARAM_DIR);
>> - char file_path[PARAM_FILE_PATH_MAX_SZ];
>> struct module_param_data *data;
>> - int fd;
>> -
>> - /* We don't need to assert string sizes on this function since they were
>> - * already checked before being stored on the lists. Besides,
>> - * igt_assert() is not AS-Safe. */
>> - strcpy(file_path, MODULE_PARAM_DIR);
>> + int dir;
>>
>> for (data = module_params; data != NULL; data = data->next) {
>> - strcpy(file_path + dir_len, data->name);
>> -
>> - fd = open(file_path, O_RDWR);
>> - if (fd >= 0) {
>> - int size = strlen (data->original_value);
>> -
>> - if (size != write(fd, data->original_value, size)) {
>> - const char msg[] = "WARNING: Module parameters "
>> - "may not have been reset to their "
>> - "original values\n";
>> - assert(write(STDERR_FILENO, msg, sizeof(msg))
>> - == sizeof(msg));
>> - }
>> -
>> - close(fd);
>> + dir = open(data->path, O_RDONLY);
>> +
>> + if (!igt_sysfs_set(dir, data->name, data->original_value)) {
>> + const char msg[] = "WARNING: Module parameters "
>> + "may not have been reset to their "
>> + "original values\n";
>> + assert(write(STDERR_FILENO, msg, sizeof(msg))
>> + == sizeof(msg));
>> }
>> +
>> + close(dir);
>> }
>> /* free() is not AS-Safe, so we can't call it here. */
>> }
>>
>> /**
>> - * igt_save_module_param:
>> - * @name: name of the i915.ko module parameter
>> - * @file_path: full sysfs file path for the parameter
>> + * igt_params_save:
>> + * @dir: file handle for path
>> + * @path: full path to the sysfs directory
>> + * @name: name of the sysfs attribute
>> *
>> - * Reads the current value of an i915.ko module parameter, saves it on an array,
>> - * then installs an exit handler to restore it when the program exits.
>> + * Reads the current value of a sysfs attribute, saves it on an array, then
>> + * installs an exit handler to restore it when the program exits.
>> *
>> * It is safe to call this function multiple times for the same parameter.
>> *
>> * Notice that this function is called by igt_set_module_param(), so that one -
>> * or one of its wrappers - is the only function the test programs need to call.
>> */
>> -static void igt_save_module_param(const char *name, const char *file_path)
>> +static void igt_params_save(int dir, const char *path, const char *name)
>> {
>> struct module_param_data *data;
>> - size_t n;
>> - int fd;
>>
>> /* Check if this parameter is already saved. */
>> for (data = module_params; data != NULL; data = data->next)
>> - if (strncmp(data->name, name, PARAM_NAME_MAX_SZ) == 0)
>> + if (strcmp(data->path, path) == 0 &&
>> + strcmp(data->name, name) == 0)
>> return;
>>
>> if (!module_params)
>> - igt_install_exit_handler(igt_module_param_exit_handler);
>> + igt_install_exit_handler(igt_params_exit_handler);
>>
>> data = calloc(1, sizeof (*data));
>> igt_assert(data);
>>
>> - strncpy(data->name, name, PARAM_NAME_MAX_SZ - 1);
>> -
>> - fd = open(file_path, O_RDONLY);
>> - igt_assert(fd >= 0);
>> + data->path = strdup(path);
>> + igt_assert(data->path);
>>
>> - n = read(fd, data->original_value, PARAM_VALUE_MAX_SZ);
>> - igt_assert_f(n > 0 && n < PARAM_VALUE_MAX_SZ,
>> - "Need to increase PARAM_VALUE_MAX_SZ\n");
>> + data->name = strdup(name);
>> + igt_assert(data->name);
>>
>> - igt_assert(close(fd) == 0);
>> + data->original_value = igt_sysfs_get(dir, name);
>> + igt_assert(data->original_value);
>>
>> data->next = module_params;
>> module_params = data;
>> @@ -205,22 +187,21 @@ bool igt_params_set(int device, const char *parameter, const char *fmt, ...)
>> */
>> void igt_set_module_param(const char *name, const char *val)
>> {
>> - char file_path[PARAM_FILE_PATH_MAX_SZ];
>> - size_t len = strlen(val);
>> - int fd;
>> + const char *path = "/sys/module/i915/parameters";
>> + int dir;
>>
>> - igt_assert_f(strlen(name) < PARAM_NAME_MAX_SZ,
>> - "Need to increase PARAM_NAME_MAX_SZ\n");
>> - strcpy(file_path, MODULE_PARAM_DIR);
>> - strcpy(file_path + strlen(MODULE_PARAM_DIR), name);
>> + dir = open(path, O_RDONLY);
>> + igt_assert(dir >= 0);
>>
>> - igt_save_module_param(name, file_path);
>> + igt_params_save(dir, path, name);
>>
>> - fd = open(file_path, O_RDWR);
>> - igt_assert(write(fd, val, len) == len);
>> - igt_assert(close(fd) == 0);
>> + igt_assert(igt_sysfs_set(dir, name, val));
>> +
>> + igt_assert(close(dir) == 0);
>> }
>>
>> +#define PARAM_VALUE_MAX_SZ 16
>> +
>
> Is this define still needed for something?
Look like it has moved inside igt_aux.c so it's not needed here, I'll
take it away.
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply [flat|nested] 35+ messages in thread* Re: [igt-dev] [PATCH i-g-t 3/8] lib/params: overhaul param saving
2020-04-28 18:40 ` Juha-Pekka Heikkila
@ 2020-04-28 19:29 ` Juha-Pekka Heikkila
0 siblings, 0 replies; 35+ messages in thread
From: Juha-Pekka Heikkila @ 2020-04-28 19:29 UTC (permalink / raw)
To: Petri Latvala; +Cc: igt-dev, Jani Nikula
On 28.4.2020 21.40, Juha-Pekka Heikkila wrote:
> On 28.4.2020 15.26, Petri Latvala wrote:
>> On Tue, Apr 21, 2020 at 07:17:20PM +0300, Juha-Pekka Heikkila wrote:
>>> From: Jani Nikula <jani.nikula@intel.com>
>>>
>>> More generic, use existing functions.
>>>
>>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>>> Signed-off-by: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>
>>> ---
>>> lib/igt_params.c | 101 +++++++++++++++++++----------------------------
>>> 1 file changed, 41 insertions(+), 60 deletions(-)
>>>
>>> diff --git a/lib/igt_params.c b/lib/igt_params.c
>>> index f220e73b..b5ac1266 100644
>>> --- a/lib/igt_params.c
>>> +++ b/lib/igt_params.c
>>> @@ -34,92 +34,74 @@
>>> #include "igt_params.h"
>>> #include "igt_sysfs.h"
>>> -#define MODULE_PARAM_DIR "/sys/module/i915/parameters/"
>>> -#define PARAM_NAME_MAX_SZ 32
>>> -#define PARAM_VALUE_MAX_SZ 16
>>> -#define PARAM_FILE_PATH_MAX_SZ (strlen(MODULE_PARAM_DIR) +
>>> PARAM_NAME_MAX_SZ)
>>> -
>>> struct module_param_data {
>>> - char name[PARAM_NAME_MAX_SZ];
>>> - char original_value[PARAM_VALUE_MAX_SZ];
>>> + char *path;
>>> + char *name;
>>> + char *original_value;
>>> struct module_param_data *next;
>>> };
>>> struct module_param_data *module_params = NULL;
>>> -static void igt_module_param_exit_handler(int sig)
>>> +static void igt_params_exit_handler(int sig)
>>> {
>>> - const size_t dir_len = strlen(MODULE_PARAM_DIR);
>>> - char file_path[PARAM_FILE_PATH_MAX_SZ];
>>> struct module_param_data *data;
>>> - int fd;
>>> -
>>> - /* We don't need to assert string sizes on this function since
>>> they were
>>> - * already checked before being stored on the lists. Besides,
>>> - * igt_assert() is not AS-Safe. */
>>> - strcpy(file_path, MODULE_PARAM_DIR);
>>> + int dir;
>>> for (data = module_params; data != NULL; data = data->next) {
>>> - strcpy(file_path + dir_len, data->name);
>>> -
>>> - fd = open(file_path, O_RDWR);
>>> - if (fd >= 0) {
>>> - int size = strlen (data->original_value);
>>> -
>>> - if (size != write(fd, data->original_value, size)) {
>>> - const char msg[] = "WARNING: Module parameters "
>>> - "may not have been reset to their "
>>> - "original values\n";
>>> - assert(write(STDERR_FILENO, msg, sizeof(msg))
>>> - == sizeof(msg));
>>> - }
>>> -
>>> - close(fd);
>>> + dir = open(data->path, O_RDONLY);
>>> +
>>> + if (!igt_sysfs_set(dir, data->name, data->original_value)) {
>>> + const char msg[] = "WARNING: Module parameters "
>>> + "may not have been reset to their "
>>> + "original values\n";
>>> + assert(write(STDERR_FILENO, msg, sizeof(msg))
>>> + == sizeof(msg));
>>> }
>>> +
>>> + close(dir);
>>> }
>>> /* free() is not AS-Safe, so we can't call it here. */
>>> }
>>> /**
>>> - * igt_save_module_param:
>>> - * @name: name of the i915.ko module parameter
>>> - * @file_path: full sysfs file path for the parameter
>>> + * igt_params_save:
>>> + * @dir: file handle for path
>>> + * @path: full path to the sysfs directory
>>> + * @name: name of the sysfs attribute
>>> *
>>> - * Reads the current value of an i915.ko module parameter, saves it
>>> on an array,
>>> - * then installs an exit handler to restore it when the program exits.
>>> + * Reads the current value of a sysfs attribute, saves it on an
>>> array, then
>>> + * installs an exit handler to restore it when the program exits.
>>> *
>>> * It is safe to call this function multiple times for the same
>>> parameter.
>>> *
>>> * Notice that this function is called by igt_set_module_param(),
>>> so that one -
>>> * or one of its wrappers - is the only function the test programs
>>> need to call.
>>> */
>>> -static void igt_save_module_param(const char *name, const char
>>> *file_path)
>>> +static void igt_params_save(int dir, const char *path, const char
>>> *name)
>>> {
>>> struct module_param_data *data;
>>> - size_t n;
>>> - int fd;
>>> /* Check if this parameter is already saved. */
>>> for (data = module_params; data != NULL; data = data->next)
>>> - if (strncmp(data->name, name, PARAM_NAME_MAX_SZ) == 0)
>>> + if (strcmp(data->path, path) == 0 &&
>>> + strcmp(data->name, name) == 0)
>>> return;
>>> if (!module_params)
>>> - igt_install_exit_handler(igt_module_param_exit_handler);
>>> + igt_install_exit_handler(igt_params_exit_handler);
>>> data = calloc(1, sizeof (*data));
>>> igt_assert(data);
>>> - strncpy(data->name, name, PARAM_NAME_MAX_SZ - 1);
>>> -
>>> - fd = open(file_path, O_RDONLY);
>>> - igt_assert(fd >= 0);
>>> + data->path = strdup(path);
>>> + igt_assert(data->path);
>>> - n = read(fd, data->original_value, PARAM_VALUE_MAX_SZ);
>>> - igt_assert_f(n > 0 && n < PARAM_VALUE_MAX_SZ,
>>> - "Need to increase PARAM_VALUE_MAX_SZ\n");
>>> + data->name = strdup(name);
>>> + igt_assert(data->name);
>>> - igt_assert(close(fd) == 0);
>>> + data->original_value = igt_sysfs_get(dir, name);
>>> + igt_assert(data->original_value);
>>> data->next = module_params;
>>> module_params = data;
>>> @@ -205,22 +187,21 @@ bool igt_params_set(int device, const char
>>> *parameter, const char *fmt, ...)
>>> */
>>> void igt_set_module_param(const char *name, const char *val)
>>> {
>>> - char file_path[PARAM_FILE_PATH_MAX_SZ];
>>> - size_t len = strlen(val);
>>> - int fd;
>>> + const char *path = "/sys/module/i915/parameters";
>>> + int dir;
>>> - igt_assert_f(strlen(name) < PARAM_NAME_MAX_SZ,
>>> - "Need to increase PARAM_NAME_MAX_SZ\n");
>>> - strcpy(file_path, MODULE_PARAM_DIR);
>>> - strcpy(file_path + strlen(MODULE_PARAM_DIR), name);
>>> + dir = open(path, O_RDONLY);
>>> + igt_assert(dir >= 0);
>>> - igt_save_module_param(name, file_path);
>>> + igt_params_save(dir, path, name);
>>> - fd = open(file_path, O_RDWR);
>>> - igt_assert(write(fd, val, len) == len);
>>> - igt_assert(close(fd) == 0);
>>> + igt_assert(igt_sysfs_set(dir, name, val));
>>> +
>>> + igt_assert(close(dir) == 0);
>>> }
>>> +#define PARAM_VALUE_MAX_SZ 16
>>> +
>>
>> Is this define still needed for something?
>
> Look like it has moved inside igt_aux.c so it's not needed here, I'll
> take it away.
Actually, it was used by code below it. It basically is assumption
integer expands to string that will be PARAM_VALUE_MAX_SZ length at maximum.
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply [flat|nested] 35+ messages in thread
* [igt-dev] [PATCH i-g-t 4/8] params open with path return
2020-04-21 16:17 [igt-dev] [PATCH i-g-t 0/8] Use device dependant module parameters Juha-Pekka Heikkila
` (2 preceding siblings ...)
2020-04-21 16:17 ` [igt-dev] [PATCH i-g-t 3/8] lib/params: overhaul param saving Juha-Pekka Heikkila
@ 2020-04-21 16:17 ` Juha-Pekka Heikkila
2020-04-28 12:31 ` Petri Latvala
2020-04-21 16:17 ` [igt-dev] [PATCH i-g-t 5/8] igt/params: add generic saving module parameter set Juha-Pekka Heikkila
` (6 subsequent siblings)
10 siblings, 1 reply; 35+ messages in thread
From: Juha-Pekka Heikkila @ 2020-04-21 16:17 UTC (permalink / raw)
To: igt-dev; +Cc: Jani Nikula
From: Jani Nikula <jani.nikula@intel.com>
Signed-off-by: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>
---
lib/igt_params.c | 29 ++++++++++++++++++-----------
1 file changed, 18 insertions(+), 11 deletions(-)
diff --git a/lib/igt_params.c b/lib/igt_params.c
index b5ac1266..fe4b1df3 100644
--- a/lib/igt_params.c
+++ b/lib/igt_params.c
@@ -107,17 +107,7 @@ static void igt_params_save(int dir, const char *path, const char *name)
module_params = data;
}
-/**
- * igt_params_open:
- * @device: fd of the device
- *
- * This opens the module parameters directory (under sysfs) corresponding
- * to the device for use with igt_sysfs_set() and igt_sysfs_get().
- *
- * Returns:
- * The directory fd, or -1 on failure.
- */
-int igt_params_open(int device)
+static int __igt_params_open(int device, char **outpath)
{
int dir, params = -1;
@@ -141,11 +131,28 @@ int igt_params_open(int device)
sprintf(path, "/sys/module/%s/parameters", name);
params = open(path, O_RDONLY);
+ if (params >= 0 && outpath)
+ *outpath = strdup(path);
}
return params;
}
+/**
+ * igt_params_open:
+ * @device: fd of the device
+ *
+ * This opens the module parameters directory (under sysfs) corresponding
+ * to the device for use with igt_sysfs_set() and igt_sysfs_get().
+ *
+ * Returns:
+ * The directory fd, or -1 on failure.
+ */
+int igt_params_open(int device)
+{
+ return __igt_params_open(device, NULL);
+}
+
/**
* igt_params_set:
* @device: fd of the device
--
2.17.1
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply related [flat|nested] 35+ messages in thread* [igt-dev] [PATCH i-g-t 5/8] igt/params: add generic saving module parameter set
2020-04-21 16:17 [igt-dev] [PATCH i-g-t 0/8] Use device dependant module parameters Juha-Pekka Heikkila
` (3 preceding siblings ...)
2020-04-21 16:17 ` [igt-dev] [PATCH i-g-t 4/8] params open with path return Juha-Pekka Heikkila
@ 2020-04-21 16:17 ` Juha-Pekka Heikkila
2020-04-28 12:40 ` Petri Latvala
2020-04-21 16:17 ` [igt-dev] [PATCH i-g-t 6/8] igt/params: use igt_params_set_save for igt_set_module_param* Juha-Pekka Heikkila
` (5 subsequent siblings)
10 siblings, 1 reply; 35+ messages in thread
From: Juha-Pekka Heikkila @ 2020-04-21 16:17 UTC (permalink / raw)
To: igt-dev; +Cc: Jani Nikula
From: Jani Nikula <jani.nikula@intel.com>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
Signed-off-by: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>
---
lib/igt_params.c | 56 ++++++++++++++++++++++++++++++++++++++++--------
lib/igt_params.h | 3 +++
2 files changed, 50 insertions(+), 9 deletions(-)
diff --git a/lib/igt_params.c b/lib/igt_params.c
index fe4b1df3..8de47b02 100644
--- a/lib/igt_params.c
+++ b/lib/igt_params.c
@@ -153,6 +153,29 @@ int igt_params_open(int device)
return __igt_params_open(device, NULL);
}
+__attribute__((format(printf, 3, 0)))
+static bool __igt_params_set(int device, const char *parameter,
+ const char *fmt, va_list ap, bool save)
+{
+ char *path = NULL;
+ int dir;
+ int ret;
+
+ dir = __igt_params_open(device, save ? &path : NULL);
+ if (dir < 0)
+ return false;
+
+ if (save)
+ igt_params_save(dir, path, parameter);
+
+ ret = igt_sysfs_vprintf(dir, parameter, fmt, ap);
+
+ close(dir);
+ free(path);
+
+ return ret > 0;
+}
+
/**
* igt_params_set:
* @device: fd of the device
@@ -164,20 +187,35 @@ int igt_params_open(int device)
bool igt_params_set(int device, const char *parameter, const char *fmt, ...)
{
va_list ap;
- int dir;
- int ret;
-
- dir = igt_params_open(device);
- if (dir < 0)
- return false;
+ bool ret;
va_start(ap, fmt);
- ret = igt_sysfs_vprintf(dir, parameter, fmt, ap);
+ ret = __igt_params_set(device, parameter, fmt, ap, false);
va_end(ap);
- close(dir);
+ return ret;
+}
- return ret > 0;
+/**
+ * igt_params_set_save:
+ * @device: fd of the device (or -1 to default to Intel)
+ * @parameter: the name of the parameter to set
+ * @fmt: printf-esque format string
+ *
+ * Save the original value to be restored.
+ *
+ * Returns true on success
+ */
+bool igt_params_set_save(int device, const char *parameter, const char *fmt, ...)
+{
+ va_list ap;
+ bool ret;
+
+ va_start(ap, fmt);
+ ret = __igt_params_set(device, parameter, fmt, ap, true);
+ va_end(ap);
+
+ return ret;
}
/**
diff --git a/lib/igt_params.h b/lib/igt_params.h
index 52eed77f..1b750d23 100644
--- a/lib/igt_params.h
+++ b/lib/igt_params.h
@@ -31,6 +31,9 @@ int igt_params_open(int device);
__attribute__((format(printf, 3, 4)))
bool igt_params_set(int device, const char *parameter, const char *fmt, ...);
+__attribute__((format(printf, 3, 4)))
+bool igt_params_set_save(int device, const char *parameter, const char *fmt, ...);
+
void igt_set_module_param(const char *name, const char *val);
void igt_set_module_param_int(const char *name, int val);
--
2.17.1
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply related [flat|nested] 35+ messages in thread* Re: [igt-dev] [PATCH i-g-t 5/8] igt/params: add generic saving module parameter set
2020-04-21 16:17 ` [igt-dev] [PATCH i-g-t 5/8] igt/params: add generic saving module parameter set Juha-Pekka Heikkila
@ 2020-04-28 12:40 ` Petri Latvala
2020-04-28 18:43 ` Juha-Pekka Heikkila
0 siblings, 1 reply; 35+ messages in thread
From: Petri Latvala @ 2020-04-28 12:40 UTC (permalink / raw)
To: Juha-Pekka Heikkila; +Cc: igt-dev, Jani Nikula
On Tue, Apr 21, 2020 at 07:17:22PM +0300, Juha-Pekka Heikkila wrote:
> From: Jani Nikula <jani.nikula@intel.com>
>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> Signed-off-by: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>
> ---
> lib/igt_params.c | 56 ++++++++++++++++++++++++++++++++++++++++--------
> lib/igt_params.h | 3 +++
> 2 files changed, 50 insertions(+), 9 deletions(-)
>
> diff --git a/lib/igt_params.c b/lib/igt_params.c
> index fe4b1df3..8de47b02 100644
> --- a/lib/igt_params.c
> +++ b/lib/igt_params.c
> @@ -153,6 +153,29 @@ int igt_params_open(int device)
> return __igt_params_open(device, NULL);
> }
>
> +__attribute__((format(printf, 3, 0)))
> +static bool __igt_params_set(int device, const char *parameter,
> + const char *fmt, va_list ap, bool save)
> +{
> + char *path = NULL;
> + int dir;
> + int ret;
> +
> + dir = __igt_params_open(device, save ? &path : NULL);
> + if (dir < 0)
> + return false;
> +
> + if (save)
> + igt_params_save(dir, path, parameter);
> +
> + ret = igt_sysfs_vprintf(dir, parameter, fmt, ap);
> +
> + close(dir);
> + free(path);
> +
> + return ret > 0;
> +}
> +
> /**
> * igt_params_set:
> * @device: fd of the device
> @@ -164,20 +187,35 @@ int igt_params_open(int device)
> bool igt_params_set(int device, const char *parameter, const char *fmt, ...)
> {
> va_list ap;
> - int dir;
> - int ret;
> -
> - dir = igt_params_open(device);
> - if (dir < 0)
> - return false;
> + bool ret;
>
> va_start(ap, fmt);
> - ret = igt_sysfs_vprintf(dir, parameter, fmt, ap);
> + ret = __igt_params_set(device, parameter, fmt, ap, false);
> va_end(ap);
>
> - close(dir);
> + return ret;
> +}
>
> - return ret > 0;
> +/**
> + * igt_params_set_save:
> + * @device: fd of the device (or -1 to default to Intel)
Default to Intel? Which Intel? Where is this defaulting implemented?
--
Petri Latvala
> + * @parameter: the name of the parameter to set
> + * @fmt: printf-esque format string
> + *
> + * Save the original value to be restored.
> + *
> + * Returns true on success
> + */
> +bool igt_params_set_save(int device, const char *parameter, const char *fmt, ...)
> +{
> + va_list ap;
> + bool ret;
> +
> + va_start(ap, fmt);
> + ret = __igt_params_set(device, parameter, fmt, ap, true);
> + va_end(ap);
> +
> + return ret;
> }
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply [flat|nested] 35+ messages in thread* Re: [igt-dev] [PATCH i-g-t 5/8] igt/params: add generic saving module parameter set
2020-04-28 12:40 ` Petri Latvala
@ 2020-04-28 18:43 ` Juha-Pekka Heikkila
0 siblings, 0 replies; 35+ messages in thread
From: Juha-Pekka Heikkila @ 2020-04-28 18:43 UTC (permalink / raw)
To: Petri Latvala; +Cc: igt-dev, Jani Nikula
On 28.4.2020 15.40, Petri Latvala wrote:
> On Tue, Apr 21, 2020 at 07:17:22PM +0300, Juha-Pekka Heikkila wrote:
>> From: Jani Nikula <jani.nikula@intel.com>
>>
>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>> Signed-off-by: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>
>> ---
>> lib/igt_params.c | 56 ++++++++++++++++++++++++++++++++++++++++--------
>> lib/igt_params.h | 3 +++
>> 2 files changed, 50 insertions(+), 9 deletions(-)
>>
>> diff --git a/lib/igt_params.c b/lib/igt_params.c
>> index fe4b1df3..8de47b02 100644
>> --- a/lib/igt_params.c
>> +++ b/lib/igt_params.c
>> @@ -153,6 +153,29 @@ int igt_params_open(int device)
>> return __igt_params_open(device, NULL);
>> }
>>
>> +__attribute__((format(printf, 3, 0)))
>> +static bool __igt_params_set(int device, const char *parameter,
>> + const char *fmt, va_list ap, bool save)
>> +{
>> + char *path = NULL;
>> + int dir;
>> + int ret;
>> +
>> + dir = __igt_params_open(device, save ? &path : NULL);
>> + if (dir < 0)
>> + return false;
>> +
>> + if (save)
>> + igt_params_save(dir, path, parameter);
>> +
>> + ret = igt_sysfs_vprintf(dir, parameter, fmt, ap);
>> +
>> + close(dir);
>> + free(path);
>> +
>> + return ret > 0;
>> +}
>> +
>> /**
>> * igt_params_set:
>> * @device: fd of the device
>> @@ -164,20 +187,35 @@ int igt_params_open(int device)
>> bool igt_params_set(int device, const char *parameter, const char *fmt, ...)
>> {
>> va_list ap;
>> - int dir;
>> - int ret;
>> -
>> - dir = igt_params_open(device);
>> - if (dir < 0)
>> - return false;
>> + bool ret;
>>
>> va_start(ap, fmt);
>> - ret = igt_sysfs_vprintf(dir, parameter, fmt, ap);
>> + ret = __igt_params_set(device, parameter, fmt, ap, false);
>> va_end(ap);
>>
>> - close(dir);
>> + return ret;
>> +}
>>
>> - return ret > 0;
>> +/**
>> + * igt_params_set_save:
>> + * @device: fd of the device (or -1 to default to Intel)
>
>
> Default to Intel? Which Intel? Where is this defaulting implemented?
Yea, that comment will need some improvements. Wording smell bit like
it's coming from legacy times.
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply [flat|nested] 35+ messages in thread
* [igt-dev] [PATCH i-g-t 6/8] igt/params: use igt_params_set_save for igt_set_module_param*
2020-04-21 16:17 [igt-dev] [PATCH i-g-t 0/8] Use device dependant module parameters Juha-Pekka Heikkila
` (4 preceding siblings ...)
2020-04-21 16:17 ` [igt-dev] [PATCH i-g-t 5/8] igt/params: add generic saving module parameter set Juha-Pekka Heikkila
@ 2020-04-21 16:17 ` Juha-Pekka Heikkila
2020-04-28 12:54 ` Petri Latvala
2020-04-21 16:17 ` [igt-dev] [PATCH i-g-t 7/8] lib/debugfs: use regular module param functions for prefault_disable Juha-Pekka Heikkila
` (4 subsequent siblings)
10 siblings, 1 reply; 35+ messages in thread
From: Juha-Pekka Heikkila @ 2020-04-21 16:17 UTC (permalink / raw)
To: igt-dev; +Cc: Jani Nikula
Unify parameter access.
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
Signed-off-by: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>
---
lib/igt_params.c | 133 +++++++++++++++++++++++++++++++++++------------
1 file changed, 100 insertions(+), 33 deletions(-)
diff --git a/lib/igt_params.c b/lib/igt_params.c
index 8de47b02..369d224c 100644
--- a/lib/igt_params.c
+++ b/lib/igt_params.c
@@ -27,12 +27,14 @@
#include <stdbool.h>
#include <stdio.h>
#include <sys/ioctl.h>
+#include <sys/stat.h>
#include <i915_drm.h>
#include "igt_core.h"
#include "igt_params.h"
#include "igt_sysfs.h"
+#include "igt_debugfs.h"
struct module_param_data {
char *path;
@@ -107,15 +109,59 @@ static void igt_params_save(int dir, const char *path, const char *name)
module_params = data;
}
-static int __igt_params_open(int device, char **outpath)
+static int __igt_params_open(int device, char **outpath, const char *param)
{
int dir, params = -1;
+ struct stat buffer;
+ char searchname[64];
+ char searchpath[PATH_MAX];
+ char *foundname, *ctx;
- dir = igt_sysfs_open(device);
+ dir = igt_debugfs_dir(device);
if (dir >= 0) {
- params = openat(dir,
- "device/driver/module/parameters",
- O_RDONLY);
+ int devname;
+
+ devname = openat(dir, "name", O_RDONLY);
+ igt_assert(devname >= 0);
+
+ read(devname, searchname, sizeof(searchname));
+ close(devname);
+
+ foundname = strtok_r(searchname, " ", &ctx);
+ igt_assert(foundname);
+
+ snprintf(searchpath, PATH_MAX, "%s_params", foundname);
+ params = openat(dir, searchpath, O_RDONLY);
+
+ if (params >= 0) {
+ char *debugfspath = malloc(PATH_MAX);
+
+ igt_debugfs_path(device, debugfspath, PATH_MAX);
+ if (param != NULL) {
+ char filepath[PATH_MAX];
+
+ snprintf(filepath, PATH_MAX, "%s/%s",
+ debugfspath, param);
+
+ if (stat(filepath, &buffer) == 0) {
+ if (outpath != NULL)
+ *outpath = debugfspath;
+ else
+ free(debugfspath);
+ } else {
+ free(debugfspath);
+ close(params);
+ params = -1;
+ }
+ } else if (outpath != NULL) {
+ /*
+ * Caller is responsible to free this.
+ */
+ *outpath = debugfspath;
+ } else {
+ free(debugfspath);
+ }
+ }
close(dir);
}
@@ -124,11 +170,51 @@ static int __igt_params_open(int device, char **outpath)
char name[32] = "";
char path[PATH_MAX];
- memset(&version, 0, sizeof(version));
- version.name_len = sizeof(name);
- version.name = name;
- ioctl(device, DRM_IOCTL_VERSION, &version);
-
+ if (device == -1) {
+ /*
+ * find default device
+ */
+ int file, i;
+ const char *debugfs_root = igt_debugfs_mount();
+
+ igt_assert(debugfs_root);
+
+ for (i = 0; i < 63; i++) {
+ char testpath[PATH_MAX];
+
+ snprintf(searchpath, PATH_MAX,
+ "%s/dri/%d/name", debugfs_root, i);
+
+ file = open(searchpath, O_RDONLY);
+
+ if (file < 0)
+ continue;
+
+ read(file, searchname, sizeof(searchname));
+ close(file);
+
+ foundname = strtok_r(searchname, " ", &ctx);
+ if (!foundname)
+ continue;
+
+ snprintf(testpath, PATH_MAX,
+ "/sys/module/%s/parameters",
+ foundname);
+
+ if (stat(testpath, &buffer) == 0 &&
+ S_ISDIR(buffer.st_mode)) {
+ memcpy(name, foundname,
+ (sizeof(name) < strlen(foundname) ?
+ sizeof(name) : strlen(foundname)));
+ break;
+ }
+ }
+ } else {
+ memset(&version, 0, sizeof(version));
+ version.name_len = sizeof(name);
+ version.name = name;
+ ioctl(device, DRM_IOCTL_VERSION, &version);
+ }
sprintf(path, "/sys/module/%s/parameters", name);
params = open(path, O_RDONLY);
if (params >= 0 && outpath)
@@ -150,7 +236,7 @@ static int __igt_params_open(int device, char **outpath)
*/
int igt_params_open(int device)
{
- return __igt_params_open(device, NULL);
+ return __igt_params_open(device, NULL, NULL);
}
__attribute__((format(printf, 3, 0)))
@@ -161,7 +247,7 @@ static bool __igt_params_set(int device, const char *parameter,
int dir;
int ret;
- dir = __igt_params_open(device, save ? &path : NULL);
+ dir = __igt_params_open(device, save ? &path : NULL, parameter);
if (dir < 0)
return false;
@@ -232,21 +318,9 @@ bool igt_params_set_save(int device, const char *parameter, const char *fmt, ...
*/
void igt_set_module_param(const char *name, const char *val)
{
- const char *path = "/sys/module/i915/parameters";
- int dir;
-
- dir = open(path, O_RDONLY);
- igt_assert(dir >= 0);
-
- igt_params_save(dir, path, name);
-
- igt_assert(igt_sysfs_set(dir, name, val));
-
- igt_assert(close(dir) == 0);
+ igt_assert(igt_params_set_save(-1, name, "%s", val));
}
-#define PARAM_VALUE_MAX_SZ 16
-
/**
* igt_set_module_param_int:
* @name: i915.ko parameter name
@@ -257,12 +331,5 @@ void igt_set_module_param(const char *name, const char *val)
*/
void igt_set_module_param_int(const char *name, int val)
{
- char str[PARAM_VALUE_MAX_SZ];
- int n;
-
- n = snprintf(str, PARAM_VALUE_MAX_SZ, "%d\n", val);
- igt_assert_f(n < PARAM_VALUE_MAX_SZ,
- "Need to increase PARAM_VALUE_MAX_SZ\n");
-
- igt_set_module_param(name, str);
+ igt_assert(igt_params_set_save(-1, name, "%d", val));
}
--
2.17.1
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply related [flat|nested] 35+ messages in thread* Re: [igt-dev] [PATCH i-g-t 6/8] igt/params: use igt_params_set_save for igt_set_module_param*
2020-04-21 16:17 ` [igt-dev] [PATCH i-g-t 6/8] igt/params: use igt_params_set_save for igt_set_module_param* Juha-Pekka Heikkila
@ 2020-04-28 12:54 ` Petri Latvala
2020-04-28 19:04 ` Juha-Pekka Heikkila
0 siblings, 1 reply; 35+ messages in thread
From: Petri Latvala @ 2020-04-28 12:54 UTC (permalink / raw)
To: Juha-Pekka Heikkila; +Cc: igt-dev, Jani Nikula
On Tue, Apr 21, 2020 at 07:17:23PM +0300, Juha-Pekka Heikkila wrote:
> Unify parameter access.
>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> Signed-off-by: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>
> ---
> lib/igt_params.c | 133 +++++++++++++++++++++++++++++++++++------------
> 1 file changed, 100 insertions(+), 33 deletions(-)
>
> diff --git a/lib/igt_params.c b/lib/igt_params.c
> index 8de47b02..369d224c 100644
> --- a/lib/igt_params.c
> +++ b/lib/igt_params.c
> @@ -27,12 +27,14 @@
> #include <stdbool.h>
> #include <stdio.h>
> #include <sys/ioctl.h>
> +#include <sys/stat.h>
>
> #include <i915_drm.h>
>
> #include "igt_core.h"
> #include "igt_params.h"
> #include "igt_sysfs.h"
> +#include "igt_debugfs.h"
>
> struct module_param_data {
> char *path;
> @@ -107,15 +109,59 @@ static void igt_params_save(int dir, const char *path, const char *name)
> module_params = data;
> }
>
> -static int __igt_params_open(int device, char **outpath)
> +static int __igt_params_open(int device, char **outpath, const char *param)
> {
> int dir, params = -1;
> + struct stat buffer;
> + char searchname[64];
> + char searchpath[PATH_MAX];
> + char *foundname, *ctx;
>
> - dir = igt_sysfs_open(device);
> + dir = igt_debugfs_dir(device);
> if (dir >= 0) {
> - params = openat(dir,
> - "device/driver/module/parameters",
> - O_RDONLY);
> + int devname;
> +
> + devname = openat(dir, "name", O_RDONLY);
> + igt_assert(devname >= 0);
> +
> + read(devname, searchname, sizeof(searchname));
> + close(devname);
> +
> + foundname = strtok_r(searchname, " ", &ctx);
> + igt_assert(foundname);
> +
> + snprintf(searchpath, PATH_MAX, "%s_params", foundname);
> + params = openat(dir, searchpath, O_RDONLY);
> +
> + if (params >= 0) {
> + char *debugfspath = malloc(PATH_MAX);
> +
> + igt_debugfs_path(device, debugfspath, PATH_MAX);
> + if (param != NULL) {
> + char filepath[PATH_MAX];
> +
> + snprintf(filepath, PATH_MAX, "%s/%s",
> + debugfspath, param);
> +
> + if (stat(filepath, &buffer) == 0) {
> + if (outpath != NULL)
> + *outpath = debugfspath;
> + else
> + free(debugfspath);
> + } else {
> + free(debugfspath);
> + close(params);
> + params = -1;
> + }
> + } else if (outpath != NULL) {
> + /*
> + * Caller is responsible to free this.
> + */
> + *outpath = debugfspath;
> + } else {
> + free(debugfspath);
> + }
> + }
> close(dir);
> }
>
> @@ -124,11 +170,51 @@ static int __igt_params_open(int device, char **outpath)
> char name[32] = "";
> char path[PATH_MAX];
>
> - memset(&version, 0, sizeof(version));
> - version.name_len = sizeof(name);
> - version.name = name;
> - ioctl(device, DRM_IOCTL_VERSION, &version);
> -
> + if (device == -1) {
> + /*
> + * find default device
> + */
Oh, here's the defaulting.
Like stated in the replies for one of the other patches, I'm not
greatly fond of carrying around a concept of a "default" device. Are
there users of -1 today?
This is also considerably more code that one would expect from just
"use params_set_save" as the subject states.
> + int file, i;
> + const char *debugfs_root = igt_debugfs_mount();
> +
> + igt_assert(debugfs_root);
> +
> + for (i = 0; i < 63; i++) {
> + char testpath[PATH_MAX];
> +
> + snprintf(searchpath, PATH_MAX,
> + "%s/dri/%d/name", debugfs_root, i);
> +
> + file = open(searchpath, O_RDONLY);
> +
> + if (file < 0)
> + continue;
> +
> + read(file, searchname, sizeof(searchname));
> + close(file);
> +
> + foundname = strtok_r(searchname, " ", &ctx);
> + if (!foundname)
> + continue;
> +
> + snprintf(testpath, PATH_MAX,
> + "/sys/module/%s/parameters",
> + foundname);
> +
> + if (stat(testpath, &buffer) == 0 &&
> + S_ISDIR(buffer.st_mode)) {
> + memcpy(name, foundname,
> + (sizeof(name) < strlen(foundname) ?
> + sizeof(name) : strlen(foundname)));
If foundname is larger than the array, doesn't this leave 'name'
without a nul-termination?
--
Petri Latvala
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply [flat|nested] 35+ messages in thread* Re: [igt-dev] [PATCH i-g-t 6/8] igt/params: use igt_params_set_save for igt_set_module_param*
2020-04-28 12:54 ` Petri Latvala
@ 2020-04-28 19:04 ` Juha-Pekka Heikkila
0 siblings, 0 replies; 35+ messages in thread
From: Juha-Pekka Heikkila @ 2020-04-28 19:04 UTC (permalink / raw)
To: Petri Latvala; +Cc: igt-dev, Jani Nikula
On 28.4.2020 15.54, Petri Latvala wrote:
> On Tue, Apr 21, 2020 at 07:17:23PM +0300, Juha-Pekka Heikkila wrote:
>> Unify parameter access.
>>
>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>> Signed-off-by: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>
>> ---
>> lib/igt_params.c | 133 +++++++++++++++++++++++++++++++++++------------
>> 1 file changed, 100 insertions(+), 33 deletions(-)
>>
>> diff --git a/lib/igt_params.c b/lib/igt_params.c
>> index 8de47b02..369d224c 100644
>> --- a/lib/igt_params.c
>> +++ b/lib/igt_params.c
>> @@ -27,12 +27,14 @@
>> #include <stdbool.h>
>> #include <stdio.h>
>> #include <sys/ioctl.h>
>> +#include <sys/stat.h>
>>
>> #include <i915_drm.h>
>>
>> #include "igt_core.h"
>> #include "igt_params.h"
>> #include "igt_sysfs.h"
>> +#include "igt_debugfs.h"
>>
>> struct module_param_data {
>> char *path;
>> @@ -107,15 +109,59 @@ static void igt_params_save(int dir, const char *path, const char *name)
>> module_params = data;
>> }
>>
>> -static int __igt_params_open(int device, char **outpath)
>> +static int __igt_params_open(int device, char **outpath, const char *param)
>> {
>> int dir, params = -1;
>> + struct stat buffer;
>> + char searchname[64];
>> + char searchpath[PATH_MAX];
>> + char *foundname, *ctx;
>>
>> - dir = igt_sysfs_open(device);
>> + dir = igt_debugfs_dir(device);
>> if (dir >= 0) {
>> - params = openat(dir,
>> - "device/driver/module/parameters",
>> - O_RDONLY);
>> + int devname;
>> +
>> + devname = openat(dir, "name", O_RDONLY);
>> + igt_assert(devname >= 0);
>> +
>> + read(devname, searchname, sizeof(searchname));
>> + close(devname);
>> +
>> + foundname = strtok_r(searchname, " ", &ctx);
>> + igt_assert(foundname);
>> +
>> + snprintf(searchpath, PATH_MAX, "%s_params", foundname);
>> + params = openat(dir, searchpath, O_RDONLY);
>> +
>> + if (params >= 0) {
>> + char *debugfspath = malloc(PATH_MAX);
>> +
>> + igt_debugfs_path(device, debugfspath, PATH_MAX);
>> + if (param != NULL) {
>> + char filepath[PATH_MAX];
>> +
>> + snprintf(filepath, PATH_MAX, "%s/%s",
>> + debugfspath, param);
>> +
>> + if (stat(filepath, &buffer) == 0) {
>> + if (outpath != NULL)
>> + *outpath = debugfspath;
>> + else
>> + free(debugfspath);
>> + } else {
>> + free(debugfspath);
>> + close(params);
>> + params = -1;
>> + }
>> + } else if (outpath != NULL) {
>> + /*
>> + * Caller is responsible to free this.
>> + */
>> + *outpath = debugfspath;
>> + } else {
>> + free(debugfspath);
>> + }
>> + }
>> close(dir);
>> }
>>
>> @@ -124,11 +170,51 @@ static int __igt_params_open(int device, char **outpath)
>> char name[32] = "";
>> char path[PATH_MAX];
>>
>> - memset(&version, 0, sizeof(version));
>> - version.name_len = sizeof(name);
>> - version.name = name;
>> - ioctl(device, DRM_IOCTL_VERSION, &version);
>> -
>> + if (device == -1) {
>> + /*
>> + * find default device
>> + */
>
> Oh, here's the defaulting.
>
> Like stated in the replies for one of the other patches, I'm not
> greatly fond of carrying around a concept of a "default" device. Are
> there users of -1 today?
There was handful of those, kms_busy comes first to my mind since I was
looking at it today. Agreed this defaulting doesn't sound good idea but
I'd first put plumbing in place and once there's no users scrap it.
There's for example kms_force_connector_basic@force-load-detect test
which I cannot now go test how it go if change this behavior since it
says igt_require(connector->connector_type == DRM_MODE_CONNECTOR_VGA)
>
> This is also considerably more code that one would expect from just
> "use params_set_save" as the subject states.
>
>
>
>
>> + int file, i;
>> + const char *debugfs_root = igt_debugfs_mount();
>> +
>> + igt_assert(debugfs_root);
>> +
>> + for (i = 0; i < 63; i++) {
>> + char testpath[PATH_MAX];
>> +
>> + snprintf(searchpath, PATH_MAX,
>> + "%s/dri/%d/name", debugfs_root, i);
>> +
>> + file = open(searchpath, O_RDONLY);
>> +
>> + if (file < 0)
>> + continue;
>> +
>> + read(file, searchname, sizeof(searchname));
>> + close(file);
>> +
>> + foundname = strtok_r(searchname, " ", &ctx);
>> + if (!foundname)
>> + continue;
>> +
>> + snprintf(testpath, PATH_MAX,
>> + "/sys/module/%s/parameters",
>> + foundname);
>> +
>> + if (stat(testpath, &buffer) == 0 &&
>> + S_ISDIR(buffer.st_mode)) {
>> + memcpy(name, foundname,
>> + (sizeof(name) < strlen(foundname) ?
>> + sizeof(name) : strlen(foundname)));
>
> If foundname is larger than the array, doesn't this leave 'name'
> without a nul-termination?
ah, I got caught by the famous buffer overflow thingy. I'll fix.
Thanks for the comments.
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply [flat|nested] 35+ messages in thread
* [igt-dev] [PATCH i-g-t 7/8] lib/debugfs: use regular module param functions for prefault_disable
2020-04-21 16:17 [igt-dev] [PATCH i-g-t 0/8] Use device dependant module parameters Juha-Pekka Heikkila
` (5 preceding siblings ...)
2020-04-21 16:17 ` [igt-dev] [PATCH i-g-t 6/8] igt/params: use igt_params_set_save for igt_set_module_param* Juha-Pekka Heikkila
@ 2020-04-21 16:17 ` Juha-Pekka Heikkila
2020-04-21 18:10 ` Chris Wilson
2020-04-21 16:17 ` [igt-dev] [PATCH i-g-t 8/8] tests/gem_eio: switch to using igt_params_set() Juha-Pekka Heikkila
` (3 subsequent siblings)
10 siblings, 1 reply; 35+ messages in thread
From: Juha-Pekka Heikkila @ 2020-04-21 16:17 UTC (permalink / raw)
To: igt-dev; +Cc: Jani Nikula
From: Jani Nikula <jani.nikula@intel.com>
Functional change switches from igt_require to igt_assert on errors.
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
Signed-off-by: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>
---
benchmarks/gem_exec_reloc.c | 2 +-
lib/igt_params.c | 24 ++++++++++++++++++++++++
lib/igt_params.h | 7 +++++++
3 files changed, 32 insertions(+), 1 deletion(-)
diff --git a/benchmarks/gem_exec_reloc.c b/benchmarks/gem_exec_reloc.c
index 0a2454cb..d7841b58 100644
--- a/benchmarks/gem_exec_reloc.c
+++ b/benchmarks/gem_exec_reloc.c
@@ -38,7 +38,7 @@
#include "drm.h"
#include "intel_reg.h"
#include "ioctl_wrappers.h"
-#include "igt_debugfs.h"
+#include "igt_params.h"
#include "drmtest.h"
#include "i915/gem_mman.h"
diff --git a/lib/igt_params.c b/lib/igt_params.c
index 369d224c..2e0e727f 100644
--- a/lib/igt_params.c
+++ b/lib/igt_params.c
@@ -333,3 +333,27 @@ void igt_set_module_param_int(const char *name, int val)
{
igt_assert(igt_params_set_save(-1, name, "%d", val));
}
+
+/**
+ * igt_disable_prefault:
+ *
+ * Disable prefaulting in certain gem ioctls through the module param. As usual
+ * this installs an exit handler to clean up and re-enable prefaulting even when
+ * the test exited abnormally.
+ *
+ * igt_enable_prefault() will enable normale operation again.
+ */
+void igt_disable_prefault(void)
+{
+ igt_require(igt_params_set_save(-1, "prefault_disable", "Y"));
+}
+
+/**
+ * igt_enable_prefault:
+ *
+ * Enable prefault (again) through the module param.
+ */
+void igt_enable_prefault(void)
+{
+ igt_require(igt_params_set_save(-1, "prefault_disable", "N"));
+}
diff --git a/lib/igt_params.h b/lib/igt_params.h
index 1b750d23..f18ea436 100644
--- a/lib/igt_params.h
+++ b/lib/igt_params.h
@@ -37,4 +37,11 @@ bool igt_params_set_save(int device, const char *parameter, const char *fmt, ...
void igt_set_module_param(const char *name, const char *val);
void igt_set_module_param_int(const char *name, int val);
+/*
+ * Prefault control
+ */
+
+void igt_disable_prefault(void);
+void igt_enable_prefault(void);
+
#endif /* __IGT_PARAMS_H__ */
--
2.17.1
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply related [flat|nested] 35+ messages in thread* Re: [igt-dev] [PATCH i-g-t 7/8] lib/debugfs: use regular module param functions for prefault_disable
2020-04-21 16:17 ` [igt-dev] [PATCH i-g-t 7/8] lib/debugfs: use regular module param functions for prefault_disable Juha-Pekka Heikkila
@ 2020-04-21 18:10 ` Chris Wilson
2020-04-21 18:30 ` Juha-Pekka Heikkila
0 siblings, 1 reply; 35+ messages in thread
From: Chris Wilson @ 2020-04-21 18:10 UTC (permalink / raw)
To: Juha-Pekka Heikkila, igt-dev; +Cc: Jani Nikula
Quoting Juha-Pekka Heikkila (2020-04-21 17:17:24)
> From: Jani Nikula <jani.nikula@intel.com>
>
> Functional change switches from igt_require to igt_assert on errors.
Adding entirely unused functions for fun?
-Chris
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [igt-dev] [PATCH i-g-t 7/8] lib/debugfs: use regular module param functions for prefault_disable
2020-04-21 18:10 ` Chris Wilson
@ 2020-04-21 18:30 ` Juha-Pekka Heikkila
2020-04-21 18:36 ` Chris Wilson
0 siblings, 1 reply; 35+ messages in thread
From: Juha-Pekka Heikkila @ 2020-04-21 18:30 UTC (permalink / raw)
To: Chris Wilson, igt-dev; +Cc: Jani Nikula
On 21.4.2020 21.10, Chris Wilson wrote:
> Quoting Juha-Pekka Heikkila (2020-04-21 17:17:24)
>> From: Jani Nikula <jani.nikula@intel.com>
>>
>> Functional change switches from igt_require to igt_assert on errors.
>
> Adding entirely unused functions for fun?
That probably is from legacy, Jani's igt branch that I took was not very
new.
Chris, you have opinion about patch 6/8 on what's going on there?
There's in __igt_params_open(..) those two blocks of file checking which
on first block is checking if wanted parameter file is in selected
device directory and second block is just about 'guessing' default
device name. I'm somehow thinking there would be easier way for those
but I didn't find other way so I ended up reading and parsing files. It
seems to work but not feeling comfortable with it.
/Juha-Pekka
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [igt-dev] [PATCH i-g-t 7/8] lib/debugfs: use regular module param functions for prefault_disable
2020-04-21 18:30 ` Juha-Pekka Heikkila
@ 2020-04-21 18:36 ` Chris Wilson
2020-04-21 18:59 ` Juha-Pekka Heikkila
0 siblings, 1 reply; 35+ messages in thread
From: Chris Wilson @ 2020-04-21 18:36 UTC (permalink / raw)
To: Juha-Pekka Heikkila, igt-dev; +Cc: Jani Nikula
Quoting Juha-Pekka Heikkila (2020-04-21 19:30:00)
> On 21.4.2020 21.10, Chris Wilson wrote:
> > Quoting Juha-Pekka Heikkila (2020-04-21 17:17:24)
> >> From: Jani Nikula <jani.nikula@intel.com>
> >>
> >> Functional change switches from igt_require to igt_assert on errors.
> >
> > Adding entirely unused functions for fun?
>
> That probably is from legacy, Jani's igt branch that I took was not very
> new.
>
> Chris, you have opinion about patch 6/8 on what's going on there?
> There's in __igt_params_open(..) those two blocks of file checking which
> on first block is checking if wanted parameter file is in selected
> device directory and second block is just about 'guessing' default
> device name. I'm somehow thinking there would be easier way for those
> but I didn't find other way so I ended up reading and parsing files. It
> seems to work but not feeling comfortable with it.
Heh, not guilty of igt_set_module_param(). I would rip out any implicit
device lookup and make sure there's always a device fd to work with. If
that is not possible, then make the module name a parameter.
-Chris
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [igt-dev] [PATCH i-g-t 7/8] lib/debugfs: use regular module param functions for prefault_disable
2020-04-21 18:36 ` Chris Wilson
@ 2020-04-21 18:59 ` Juha-Pekka Heikkila
2020-04-22 6:13 ` Jani Nikula
0 siblings, 1 reply; 35+ messages in thread
From: Juha-Pekka Heikkila @ 2020-04-21 18:59 UTC (permalink / raw)
To: Chris Wilson, igt-dev; +Cc: Jani Nikula
On 21.4.2020 21.36, Chris Wilson wrote:
> Quoting Juha-Pekka Heikkila (2020-04-21 19:30:00)
>> On 21.4.2020 21.10, Chris Wilson wrote:
>>> Quoting Juha-Pekka Heikkila (2020-04-21 17:17:24)
>>>> From: Jani Nikula <jani.nikula@intel.com>
>>>>
>>>> Functional change switches from igt_require to igt_assert on errors.
>>>
>>> Adding entirely unused functions for fun?
>>
>> That probably is from legacy, Jani's igt branch that I took was not very
>> new.
>>
>> Chris, you have opinion about patch 6/8 on what's going on there?
>> There's in __igt_params_open(..) those two blocks of file checking which
>> on first block is checking if wanted parameter file is in selected
>> device directory and second block is just about 'guessing' default
>> device name. I'm somehow thinking there would be easier way for those
>> but I didn't find other way so I ended up reading and parsing files. It
>> seems to work but not feeling comfortable with it.
>
> Heh, not guilty of igt_set_module_param(). I would rip out any implicit
> device lookup and make sure there's always a device fd to work with. If
> that is not possible, then make the module name a parameter.
I'll need to give it some thought with making module name as parameter.
There's not many tests that would require change but those tests will
need to be added with some code that will recognize what driver are they on.
/Juha-Pekka
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [igt-dev] [PATCH i-g-t 7/8] lib/debugfs: use regular module param functions for prefault_disable
2020-04-21 18:59 ` Juha-Pekka Heikkila
@ 2020-04-22 6:13 ` Jani Nikula
0 siblings, 0 replies; 35+ messages in thread
From: Jani Nikula @ 2020-04-22 6:13 UTC (permalink / raw)
To: juhapekka.heikkila, Chris Wilson, igt-dev
On Tue, 21 Apr 2020, Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com> wrote:
> On 21.4.2020 21.36, Chris Wilson wrote:
>> Quoting Juha-Pekka Heikkila (2020-04-21 19:30:00)
>>> On 21.4.2020 21.10, Chris Wilson wrote:
>>>> Quoting Juha-Pekka Heikkila (2020-04-21 17:17:24)
>>>>> From: Jani Nikula <jani.nikula@intel.com>
>>>>>
>>>>> Functional change switches from igt_require to igt_assert on errors.
>>>>
>>>> Adding entirely unused functions for fun?
>>>
>>> That probably is from legacy, Jani's igt branch that I took was not very
>>> new.
>>>
>>> Chris, you have opinion about patch 6/8 on what's going on there?
>>> There's in __igt_params_open(..) those two blocks of file checking which
>>> on first block is checking if wanted parameter file is in selected
>>> device directory and second block is just about 'guessing' default
>>> device name. I'm somehow thinking there would be easier way for those
>>> but I didn't find other way so I ended up reading and parsing files. It
>>> seems to work but not feeling comfortable with it.
>>
>> Heh, not guilty of igt_set_module_param(). I would rip out any implicit
>> device lookup and make sure there's always a device fd to work with. If
>> that is not possible, then make the module name a parameter.
>
> I'll need to give it some thought with making module name as parameter.
> There's not many tests that would require change but those tests will
> need to be added with some code that will recognize what driver are they on.
I think way back when I initially wrote this stuff, the implicit device
lookup was more widespread in igt than it is now. Always passing the fd
should be the end goal IMO, and passing the module name should be a
transitional thing if switching everything to fd is too much work right
now.
BR,
Jani.
--
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply [flat|nested] 35+ messages in thread
* [igt-dev] [PATCH i-g-t 8/8] tests/gem_eio: switch to using igt_params_set()
2020-04-21 16:17 [igt-dev] [PATCH i-g-t 0/8] Use device dependant module parameters Juha-Pekka Heikkila
` (6 preceding siblings ...)
2020-04-21 16:17 ` [igt-dev] [PATCH i-g-t 7/8] lib/debugfs: use regular module param functions for prefault_disable Juha-Pekka Heikkila
@ 2020-04-21 16:17 ` Juha-Pekka Heikkila
2020-04-21 17:02 ` [igt-dev] ✗ Fi.CI.BAT: failure for Use device dependant module parameters (rev3) Patchwork
` (2 subsequent siblings)
10 siblings, 0 replies; 35+ messages in thread
From: Juha-Pekka Heikkila @ 2020-04-21 16:17 UTC (permalink / raw)
To: igt-dev; +Cc: Jani Nikula
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
Signed-off-by: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>
---
tests/i915/gem_eio.c | 57 +++++++++++++++++++-------------------------
1 file changed, 24 insertions(+), 33 deletions(-)
diff --git a/tests/i915/gem_eio.c b/tests/i915/gem_eio.c
index 1ec60941..ad59f83a 100644
--- a/tests/i915/gem_eio.c
+++ b/tests/i915/gem_eio.c
@@ -52,20 +52,11 @@
IGT_TEST_DESCRIPTION("Test that specific ioctls report a wedged GPU (EIO).");
-static bool i915_reset_control(bool enable)
+static bool i915_reset_control(int fd, bool enable)
{
- const char *path = "/sys/module/i915/parameters/reset";
- int fd, ret;
-
igt_debug("%s GPU reset\n", enable ? "Enabling" : "Disabling");
- fd = open(path, O_RDWR);
- igt_require(fd >= 0);
-
- ret = write(fd, &"01"[enable], 1) == 1;
- close(fd);
-
- return ret;
+ return igt_params_set(fd, "reset", "%d", enable);
}
static void trigger_reset(int fd)
@@ -106,9 +97,9 @@ static void wedge_gpu(int fd)
/* First idle the GPU then disable GPU resets before injecting a hang */
gem_quiescent_gpu(fd);
- igt_require(i915_reset_control(false));
+ igt_require(i915_reset_control(fd, false));
manual_hang(fd);
- igt_assert(i915_reset_control(true));
+ igt_assert(i915_reset_control(fd, true));
}
static int __gem_throttle(int fd)
@@ -346,7 +337,7 @@ static void __test_banned(int fd)
gem_write(fd, obj.handle, 0, &bbe, sizeof(bbe));
gem_quiescent_gpu(fd);
- igt_require(i915_reset_control(true));
+ igt_require(i915_reset_control(fd, true));
igt_until_timeout(5) {
igt_spin_t *hang;
@@ -404,9 +395,9 @@ static void test_wait(int fd, unsigned int flags, unsigned int wait)
*/
if (flags & TEST_WEDGE)
- igt_require(i915_reset_control(false));
+ igt_require(i915_reset_control(fd, false));
else
- igt_require(i915_reset_control(true));
+ igt_require(i915_reset_control(fd, true));
hang = spin_sync(fd, 0, I915_EXEC_DEFAULT);
@@ -415,7 +406,7 @@ static void test_wait(int fd, unsigned int flags, unsigned int wait)
igt_spin_free(fd, hang);
- igt_require(i915_reset_control(true));
+ igt_require(i915_reset_control(fd, true));
trigger_reset(fd);
close(fd);
@@ -430,12 +421,12 @@ static void test_suspend(int fd, int state)
igt_system_suspend_autoresume(state, SUSPEND_TEST_DEVICES);
/* Check we can suspend when the driver is already wedged */
- igt_require(i915_reset_control(false));
+ igt_require(i915_reset_control(fd, false));
manual_hang(fd);
igt_system_suspend_autoresume(state, SUSPEND_TEST_DEVICES);
- igt_require(i915_reset_control(true));
+ igt_require(i915_reset_control(fd, true));
trigger_reset(fd);
close(fd);
}
@@ -470,7 +461,7 @@ static void test_inflight(int fd, unsigned int wait)
gem_quiescent_gpu(fd);
igt_debug("Starting %s on engine '%s'\n", __func__, e->name);
- igt_require(i915_reset_control(false));
+ igt_require(i915_reset_control(fd, false));
hang = spin_sync(fd, 0, eb_ring(e));
obj[0].handle = hang->handle;
@@ -495,7 +486,7 @@ static void test_inflight(int fd, unsigned int wait)
}
igt_spin_free(fd, hang);
- igt_assert(i915_reset_control(true));
+ igt_assert(i915_reset_control(fd, true));
trigger_reset(fd);
gem_close(fd, obj[1].handle);
@@ -520,7 +511,7 @@ static void test_inflight_suspend(int fd)
fd = gem_reopen_driver(fd);
igt_require_gem(fd);
igt_require(gem_has_exec_fence(fd));
- igt_require(i915_reset_control(false));
+ igt_require(i915_reset_control(fd, false));
memset(obj, 0, sizeof(obj));
obj[0].flags = EXEC_OBJECT_WRITE;
@@ -553,7 +544,7 @@ static void test_inflight_suspend(int fd)
}
igt_spin_free(fd, hang);
- igt_assert(i915_reset_control(true));
+ igt_assert(i915_reset_control(fd, true));
trigger_reset(fd);
close(fd);
}
@@ -601,7 +592,7 @@ static void test_inflight_contexts(int fd, unsigned int wait)
gem_quiescent_gpu(fd);
igt_debug("Starting %s on engine '%s'\n", __func__, e->name);
- igt_require(i915_reset_control(false));
+ igt_require(i915_reset_control(fd, false));
memset(obj, 0, sizeof(obj));
obj[0].flags = EXEC_OBJECT_WRITE;
@@ -636,7 +627,7 @@ static void test_inflight_contexts(int fd, unsigned int wait)
igt_spin_free(fd, hang);
gem_close(fd, obj[1].handle);
- igt_assert(i915_reset_control(true));
+ igt_assert(i915_reset_control(fd, true));
trigger_reset(fd);
for (unsigned int n = 0; n < ARRAY_SIZE(ctx); n++)
@@ -663,7 +654,7 @@ static void test_inflight_external(int fd)
fence = igt_cork_plug(&cork, fd);
- igt_require(i915_reset_control(false));
+ igt_require(i915_reset_control(fd, false));
hang = __spin_poll(fd, 0, 0);
memset(&obj, 0, sizeof(obj));
@@ -695,7 +686,7 @@ static void test_inflight_external(int fd)
close(fence);
igt_spin_free(fd, hang);
- igt_assert(i915_reset_control(true));
+ igt_assert(i915_reset_control(fd, true));
trigger_reset(fd);
close(fd);
}
@@ -714,7 +705,7 @@ static void test_inflight_internal(int fd, unsigned int wait)
fd = gem_reopen_driver(fd);
igt_require_gem(fd);
- igt_require(i915_reset_control(false));
+ igt_require(i915_reset_control(fd, false));
hang = spin_sync(fd, 0, 0);
memset(obj, 0, sizeof(obj));
@@ -745,7 +736,7 @@ static void test_inflight_internal(int fd, unsigned int wait)
}
igt_spin_free(fd, hang);
- igt_assert(i915_reset_control(true));
+ igt_assert(i915_reset_control(fd, true));
trigger_reset(fd);
close(fd);
}
@@ -781,7 +772,7 @@ static void reset_stress(int fd, uint32_t ctx0,
gem_quiescent_gpu(fd);
- igt_require(i915_reset_control(flags & TEST_WEDGE ?
+ igt_require(i915_reset_control(fd, flags & TEST_WEDGE ?
false : true));
/*
@@ -803,7 +794,7 @@ static void reset_stress(int fd, uint32_t ctx0,
igt_assert_eq(sync_fence_status(hang->out_fence), -EIO);
/* Unwedge by forcing a reset. */
- igt_assert(i915_reset_control(true));
+ igt_assert(i915_reset_control(fd, true));
trigger_reset(fd);
gem_quiescent_gpu(fd);
@@ -918,7 +909,7 @@ static int fd = -1;
static void
exit_handler(int sig)
{
- i915_reset_control(true);
+ i915_reset_control(fd, true);
igt_force_gpu_reset(fd);
}
@@ -933,7 +924,7 @@ igt_main
igt_allow_hang(fd, 0, 0);
- igt_require(i915_reset_control(true));
+ igt_require(i915_reset_control(fd, true));
igt_force_gpu_reset(fd);
igt_install_exit_handler(exit_handler);
}
--
2.17.1
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply related [flat|nested] 35+ messages in thread* [igt-dev] ✗ Fi.CI.BAT: failure for Use device dependant module parameters (rev3)
2020-04-21 16:17 [igt-dev] [PATCH i-g-t 0/8] Use device dependant module parameters Juha-Pekka Heikkila
` (7 preceding siblings ...)
2020-04-21 16:17 ` [igt-dev] [PATCH i-g-t 8/8] tests/gem_eio: switch to using igt_params_set() Juha-Pekka Heikkila
@ 2020-04-21 17:02 ` Patchwork
2020-04-21 18:08 ` [igt-dev] ✓ Fi.CI.BAT: success for Use device dependant module parameters (rev4) Patchwork
2020-04-22 1:05 ` [igt-dev] ✓ Fi.CI.IGT: " Patchwork
10 siblings, 0 replies; 35+ messages in thread
From: Patchwork @ 2020-04-21 17:02 UTC (permalink / raw)
To: Juha-Pekka Heikkila; +Cc: igt-dev
== Series Details ==
Series: Use device dependant module parameters (rev3)
URL : https://patchwork.freedesktop.org/series/76163/
State : failure
== Summary ==
CI Bug Log - changes from CI_DRM_8345 -> IGTPW_4495
====================================================
Summary
-------
**FAILURE**
Serious unknown changes coming with IGTPW_4495 absolutely need to be
verified manually.
If you think the reported changes have nothing to do with the changes
introduced in IGTPW_4495, please notify your bug team to allow them
to document this new failure mode, which will reduce false positives in CI.
External URL: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4495/index.html
Possible new issues
-------------------
Here are the unknown changes that may have been introduced in IGTPW_4495:
### IGT changes ###
#### Possible regressions ####
* igt@i915_selftest@live@coherency:
- fi-byt-j1900: [PASS][1] -> [FAIL][2]
[1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8345/fi-byt-j1900/igt@i915_selftest@live@coherency.html
[2]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4495/fi-byt-j1900/igt@i915_selftest@live@coherency.html
Known issues
------------
Here are the changes found in IGTPW_4495 that come from known issues:
### IGT changes ###
#### Issues hit ####
* igt@i915_pm_rpm@module-reload:
- fi-kbl-guc: [PASS][3] -> [FAIL][4] ([i915#579])
[3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8345/fi-kbl-guc/igt@i915_pm_rpm@module-reload.html
[4]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4495/fi-kbl-guc/igt@i915_pm_rpm@module-reload.html
* igt@i915_selftest@live@gt_pm:
- fi-skl-guc: [PASS][5] -> [DMESG-FAIL][6] ([i915#1751])
[5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8345/fi-skl-guc/igt@i915_selftest@live@gt_pm.html
[6]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4495/fi-skl-guc/igt@i915_selftest@live@gt_pm.html
* igt@i915_selftest@live@reset:
- fi-icl-guc: [PASS][7] -> [INCOMPLETE][8] ([i915#1580])
[7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8345/fi-icl-guc/igt@i915_selftest@live@reset.html
[8]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4495/fi-icl-guc/igt@i915_selftest@live@reset.html
#### Possible fixes ####
* igt@i915_selftest@live@gt_heartbeat:
- fi-kbl-soraka: [DMESG-FAIL][9] ([i915#541]) -> [PASS][10]
[9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8345/fi-kbl-soraka/igt@i915_selftest@live@gt_heartbeat.html
[10]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4495/fi-kbl-soraka/igt@i915_selftest@live@gt_heartbeat.html
#### Warnings ####
* igt@i915_pm_rpm@basic-rte:
- fi-kbl-guc: [FAIL][11] ([i915#579]) -> [SKIP][12] ([fdo#109271])
[11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8345/fi-kbl-guc/igt@i915_pm_rpm@basic-rte.html
[12]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4495/fi-kbl-guc/igt@i915_pm_rpm@basic-rte.html
* igt@i915_pm_rpm@module-reload:
- fi-kbl-x1275: [FAIL][13] ([i915#62]) -> [SKIP][14] ([fdo#109271])
[13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8345/fi-kbl-x1275/igt@i915_pm_rpm@module-reload.html
[14]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4495/fi-kbl-x1275/igt@i915_pm_rpm@module-reload.html
[fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
[i915#1580]: https://gitlab.freedesktop.org/drm/intel/issues/1580
[i915#1751]: https://gitlab.freedesktop.org/drm/intel/issues/1751
[i915#541]: https://gitlab.freedesktop.org/drm/intel/issues/541
[i915#579]: https://gitlab.freedesktop.org/drm/intel/issues/579
[i915#62]: https://gitlab.freedesktop.org/drm/intel/issues/62
Participating hosts (50 -> 43)
------------------------------
Missing (7): fi-cml-u2 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-ctg-p8600 fi-byt-clapper fi-bdw-samus
Build changes
-------------
* CI: CI-20190529 -> None
* IGT: IGT_5602 -> IGTPW_4495
CI-20190529: 20190529
CI_DRM_8345: e1fa8774e58e663bec8257f678c2f8fd17088292 @ git://anongit.freedesktop.org/gfx-ci/linux
IGTPW_4495: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4495/index.html
IGT_5602: a8fcccd15dcc2dd409edd23785a2d6f6e85fb682 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4495/index.html
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply [flat|nested] 35+ messages in thread* [igt-dev] ✓ Fi.CI.BAT: success for Use device dependant module parameters (rev4)
2020-04-21 16:17 [igt-dev] [PATCH i-g-t 0/8] Use device dependant module parameters Juha-Pekka Heikkila
` (8 preceding siblings ...)
2020-04-21 17:02 ` [igt-dev] ✗ Fi.CI.BAT: failure for Use device dependant module parameters (rev3) Patchwork
@ 2020-04-21 18:08 ` Patchwork
2020-04-22 1:05 ` [igt-dev] ✓ Fi.CI.IGT: " Patchwork
10 siblings, 0 replies; 35+ messages in thread
From: Patchwork @ 2020-04-21 18:08 UTC (permalink / raw)
To: Juha-Pekka Heikkila; +Cc: igt-dev
== Series Details ==
Series: Use device dependant module parameters (rev4)
URL : https://patchwork.freedesktop.org/series/76163/
State : success
== Summary ==
CI Bug Log - changes from IGT_5604 -> IGTPW_4496
====================================================
Summary
-------
**SUCCESS**
No regressions found.
External URL: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4496/index.html
Known issues
------------
Here are the changes found in IGTPW_4496 that come from known issues:
### IGT changes ###
#### Issues hit ####
* igt@i915_selftest@live@active:
- fi-bwr-2160: [PASS][1] -> [INCOMPLETE][2] ([i915#489])
[1]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5604/fi-bwr-2160/igt@i915_selftest@live@active.html
[2]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4496/fi-bwr-2160/igt@i915_selftest@live@active.html
#### Possible fixes ####
* igt@i915_selftest@live@gt_heartbeat:
- fi-kbl-soraka: [DMESG-FAIL][3] ([i915#541]) -> [PASS][4]
[3]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5604/fi-kbl-soraka/igt@i915_selftest@live@gt_heartbeat.html
[4]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4496/fi-kbl-soraka/igt@i915_selftest@live@gt_heartbeat.html
* igt@i915_selftest@live@gt_mocs:
- fi-snb-2600: [FAIL][5] ([i915#1763]) -> [PASS][6]
[5]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5604/fi-snb-2600/igt@i915_selftest@live@gt_mocs.html
[6]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4496/fi-snb-2600/igt@i915_selftest@live@gt_mocs.html
#### Warnings ####
* igt@runner@aborted:
- fi-bwr-2160: [FAIL][7] -> [FAIL][8] ([i915#489])
[7]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5604/fi-bwr-2160/igt@runner@aborted.html
[8]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4496/fi-bwr-2160/igt@runner@aborted.html
[i915#1763]: https://gitlab.freedesktop.org/drm/intel/issues/1763
[i915#489]: https://gitlab.freedesktop.org/drm/intel/issues/489
[i915#541]: https://gitlab.freedesktop.org/drm/intel/issues/541
Participating hosts (49 -> 43)
------------------------------
Additional (1): fi-kbl-7560u
Missing (7): fi-cml-u2 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-ctg-p8600 fi-byt-clapper fi-bdw-samus
Build changes
-------------
* CI: CI-20190529 -> None
* IGT: IGT_5604 -> IGTPW_4496
CI-20190529: 20190529
CI_DRM_8345: e1fa8774e58e663bec8257f678c2f8fd17088292 @ git://anongit.freedesktop.org/gfx-ci/linux
IGTPW_4496: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4496/index.html
IGT_5604: 18cc19ece602ba552a8386222b49e7e82820f9aa @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4496/index.html
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply [flat|nested] 35+ messages in thread* [igt-dev] ✓ Fi.CI.IGT: success for Use device dependant module parameters (rev4)
2020-04-21 16:17 [igt-dev] [PATCH i-g-t 0/8] Use device dependant module parameters Juha-Pekka Heikkila
` (9 preceding siblings ...)
2020-04-21 18:08 ` [igt-dev] ✓ Fi.CI.BAT: success for Use device dependant module parameters (rev4) Patchwork
@ 2020-04-22 1:05 ` Patchwork
10 siblings, 0 replies; 35+ messages in thread
From: Patchwork @ 2020-04-22 1:05 UTC (permalink / raw)
To: Juha-Pekka Heikkila; +Cc: igt-dev
== Series Details ==
Series: Use device dependant module parameters (rev4)
URL : https://patchwork.freedesktop.org/series/76163/
State : success
== Summary ==
CI Bug Log - changes from IGT_5604_full -> IGTPW_4496_full
====================================================
Summary
-------
**SUCCESS**
No regressions found.
External URL: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4496/index.html
Known issues
------------
Here are the changes found in IGTPW_4496_full that come from known issues:
### IGT changes ###
#### Issues hit ####
* igt@gem_exec_balancer@hang:
- shard-tglb: [PASS][1] -> [FAIL][2] ([i915#1277])
[1]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5604/shard-tglb3/igt@gem_exec_balancer@hang.html
[2]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4496/shard-tglb5/igt@gem_exec_balancer@hang.html
* igt@kms_cursor_legacy@2x-long-flip-vs-cursor-legacy:
- shard-glk: [PASS][3] -> [FAIL][4] ([i915#72])
[3]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5604/shard-glk5/igt@kms_cursor_legacy@2x-long-flip-vs-cursor-legacy.html
[4]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4496/shard-glk1/igt@kms_cursor_legacy@2x-long-flip-vs-cursor-legacy.html
* igt@kms_draw_crc@draw-method-rgb565-mmap-wc-untiled:
- shard-glk: [PASS][5] -> [FAIL][6] ([i915#52] / [i915#54]) +2 similar issues
[5]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5604/shard-glk9/igt@kms_draw_crc@draw-method-rgb565-mmap-wc-untiled.html
[6]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4496/shard-glk1/igt@kms_draw_crc@draw-method-rgb565-mmap-wc-untiled.html
* igt@kms_draw_crc@draw-method-xrgb8888-mmap-gtt-untiled:
- shard-apl: [PASS][7] -> [FAIL][8] ([i915#52] / [i915#54] / [i915#95]) +1 similar issue
[7]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5604/shard-apl1/igt@kms_draw_crc@draw-method-xrgb8888-mmap-gtt-untiled.html
[8]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4496/shard-apl2/igt@kms_draw_crc@draw-method-xrgb8888-mmap-gtt-untiled.html
* igt@kms_draw_crc@fill-fb:
- shard-apl: [PASS][9] -> [FAIL][10] ([i915#95])
[9]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5604/shard-apl1/igt@kms_draw_crc@fill-fb.html
[10]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4496/shard-apl1/igt@kms_draw_crc@fill-fb.html
* igt@kms_hdr@bpc-switch-suspend:
- shard-apl: [PASS][11] -> [DMESG-WARN][12] ([i915#180])
[11]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5604/shard-apl3/igt@kms_hdr@bpc-switch-suspend.html
[12]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4496/shard-apl4/igt@kms_hdr@bpc-switch-suspend.html
* igt@kms_mmap_write_crc@main:
- shard-kbl: [PASS][13] -> [FAIL][14] ([i915#93] / [i915#95])
[13]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5604/shard-kbl4/igt@kms_mmap_write_crc@main.html
[14]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4496/shard-kbl6/igt@kms_mmap_write_crc@main.html
* igt@kms_plane@plane-position-covered-pipe-a-planes:
- shard-snb: [PASS][15] -> [SKIP][16] ([fdo#109271]) +4 similar issues
[15]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5604/shard-snb4/igt@kms_plane@plane-position-covered-pipe-a-planes.html
[16]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4496/shard-snb5/igt@kms_plane@plane-position-covered-pipe-a-planes.html
* igt@kms_plane_alpha_blend@pipe-a-constant-alpha-mid:
- shard-kbl: [PASS][17] -> [FAIL][18] ([fdo#108145] / [i915#265] / [i915#93] / [i915#95]) +1 similar issue
[17]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5604/shard-kbl4/igt@kms_plane_alpha_blend@pipe-a-constant-alpha-mid.html
[18]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4496/shard-kbl6/igt@kms_plane_alpha_blend@pipe-a-constant-alpha-mid.html
* igt@kms_plane_alpha_blend@pipe-a-coverage-vs-premult-vs-constant:
- shard-apl: [PASS][19] -> [FAIL][20] ([fdo#108145] / [i915#265] / [i915#95]) +1 similar issue
[19]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5604/shard-apl7/igt@kms_plane_alpha_blend@pipe-a-coverage-vs-premult-vs-constant.html
[20]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4496/shard-apl2/igt@kms_plane_alpha_blend@pipe-a-coverage-vs-premult-vs-constant.html
* igt@kms_psr@psr2_primary_mmap_cpu:
- shard-iclb: [PASS][21] -> [SKIP][22] ([fdo#109441]) +2 similar issues
[21]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5604/shard-iclb2/igt@kms_psr@psr2_primary_mmap_cpu.html
[22]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4496/shard-iclb3/igt@kms_psr@psr2_primary_mmap_cpu.html
* igt@kms_setmode@basic:
- shard-apl: [PASS][23] -> [FAIL][24] ([i915#31])
[23]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5604/shard-apl2/igt@kms_setmode@basic.html
[24]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4496/shard-apl6/igt@kms_setmode@basic.html
#### Possible fixes ####
* {igt@gem_exec_schedule@pi-distinct-iova@bcs0}:
- shard-glk: [FAIL][25] ([i915#859]) -> [PASS][26]
[25]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5604/shard-glk9/igt@gem_exec_schedule@pi-distinct-iova@bcs0.html
[26]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4496/shard-glk5/igt@gem_exec_schedule@pi-distinct-iova@bcs0.html
* igt@gem_exec_suspend@basic-s3:
- shard-kbl: [DMESG-WARN][27] ([i915#180] / [i915#93] / [i915#95]) -> [PASS][28]
[27]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5604/shard-kbl4/igt@gem_exec_suspend@basic-s3.html
[28]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4496/shard-kbl2/igt@gem_exec_suspend@basic-s3.html
* igt@i915_selftest@live@requests:
- shard-tglb: [INCOMPLETE][29] ([i915#1531] / [i915#1658]) -> [PASS][30]
[29]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5604/shard-tglb1/igt@i915_selftest@live@requests.html
[30]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4496/shard-tglb2/igt@i915_selftest@live@requests.html
* igt@kms_big_fb@x-tiled-16bpp-rotate-0:
- shard-glk: [FAIL][31] ([i915#1119]) -> [PASS][32]
[31]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5604/shard-glk6/igt@kms_big_fb@x-tiled-16bpp-rotate-0.html
[32]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4496/shard-glk4/igt@kms_big_fb@x-tiled-16bpp-rotate-0.html
* igt@kms_cursor_crc@pipe-a-cursor-256x85-offscreen:
- shard-kbl: [FAIL][33] ([i915#54] / [i915#93] / [i915#95]) -> [PASS][34] +2 similar issues
[33]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5604/shard-kbl1/igt@kms_cursor_crc@pipe-a-cursor-256x85-offscreen.html
[34]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4496/shard-kbl7/igt@kms_cursor_crc@pipe-a-cursor-256x85-offscreen.html
* igt@kms_draw_crc@draw-method-rgb565-pwrite-untiled:
- shard-glk: [FAIL][35] ([i915#177] / [i915#52] / [i915#54]) -> [PASS][36]
[35]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5604/shard-glk8/igt@kms_draw_crc@draw-method-rgb565-pwrite-untiled.html
[36]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4496/shard-glk9/igt@kms_draw_crc@draw-method-rgb565-pwrite-untiled.html
* igt@kms_draw_crc@draw-method-rgb565-render-ytiled:
- shard-glk: [FAIL][37] ([i915#52] / [i915#54]) -> [PASS][38] +6 similar issues
[37]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5604/shard-glk8/igt@kms_draw_crc@draw-method-rgb565-render-ytiled.html
[38]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4496/shard-glk6/igt@kms_draw_crc@draw-method-rgb565-render-ytiled.html
* {igt@kms_flip@2x-flip-vs-absolute-wf_vblank@ab-vga1-hdmi-a1}:
- shard-hsw: [INCOMPLETE][39] ([i915#61]) -> [PASS][40] +2 similar issues
[39]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5604/shard-hsw7/igt@kms_flip@2x-flip-vs-absolute-wf_vblank@ab-vga1-hdmi-a1.html
[40]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4496/shard-hsw2/igt@kms_flip@2x-flip-vs-absolute-wf_vblank@ab-vga1-hdmi-a1.html
* {igt@kms_flip@flip-vs-suspend-interruptible@c-dp1}:
- shard-apl: [DMESG-WARN][41] ([i915#180]) -> [PASS][42] +4 similar issues
[41]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5604/shard-apl8/igt@kms_flip@flip-vs-suspend-interruptible@c-dp1.html
[42]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4496/shard-apl4/igt@kms_flip@flip-vs-suspend-interruptible@c-dp1.html
* {igt@kms_flip@flip-vs-suspend@c-dp1}:
- shard-kbl: [DMESG-WARN][43] ([i915#180]) -> [PASS][44] +7 similar issues
[43]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5604/shard-kbl4/igt@kms_flip@flip-vs-suspend@c-dp1.html
[44]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4496/shard-kbl1/igt@kms_flip@flip-vs-suspend@c-dp1.html
* igt@kms_frontbuffer_tracking@fbc-suspend:
- shard-apl: [DMESG-WARN][45] ([i915#180] / [i915#95]) -> [PASS][46]
[45]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5604/shard-apl8/igt@kms_frontbuffer_tracking@fbc-suspend.html
[46]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4496/shard-apl7/igt@kms_frontbuffer_tracking@fbc-suspend.html
* igt@kms_plane@plane-panning-bottom-right-suspend-pipe-b-planes:
- shard-kbl: [INCOMPLETE][47] ([i915#155]) -> [PASS][48]
[47]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5604/shard-kbl1/igt@kms_plane@plane-panning-bottom-right-suspend-pipe-b-planes.html
[48]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4496/shard-kbl1/igt@kms_plane@plane-panning-bottom-right-suspend-pipe-b-planes.html
* igt@kms_plane_lowres@pipe-a-tiling-x:
- shard-glk: [FAIL][49] ([i915#899]) -> [PASS][50]
[49]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5604/shard-glk8/igt@kms_plane_lowres@pipe-a-tiling-x.html
[50]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4496/shard-glk7/igt@kms_plane_lowres@pipe-a-tiling-x.html
* igt@kms_psr@psr2_cursor_mmap_cpu:
- shard-iclb: [SKIP][51] ([fdo#109441]) -> [PASS][52] +5 similar issues
[51]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5604/shard-iclb8/igt@kms_psr@psr2_cursor_mmap_cpu.html
[52]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4496/shard-iclb2/igt@kms_psr@psr2_cursor_mmap_cpu.html
#### Warnings ####
* igt@i915_pm_dc@dc6-dpms:
- shard-kbl: [FAIL][53] ([i915#454]) -> [FAIL][54] ([i915#454] / [i915#93] / [i915#95])
[53]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5604/shard-kbl6/igt@i915_pm_dc@dc6-dpms.html
[54]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4496/shard-kbl2/igt@i915_pm_dc@dc6-dpms.html
* igt@i915_pm_dc@dc6-psr:
- shard-tglb: [SKIP][55] ([i915#468]) -> [FAIL][56] ([i915#454])
[55]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5604/shard-tglb2/igt@i915_pm_dc@dc6-psr.html
[56]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4496/shard-tglb7/igt@i915_pm_dc@dc6-psr.html
* igt@i915_pm_rpm@modeset-lpsp-stress-no-wait:
- shard-snb: [SKIP][57] ([fdo#109271]) -> [INCOMPLETE][58] ([i915#82])
[57]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5604/shard-snb5/igt@i915_pm_rpm@modeset-lpsp-stress-no-wait.html
[58]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4496/shard-snb6/igt@i915_pm_rpm@modeset-lpsp-stress-no-wait.html
* igt@kms_content_protection@uevent:
- shard-kbl: [FAIL][59] ([i915#357] / [i915#93] / [i915#95]) -> [FAIL][60] ([i915#357])
[59]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5604/shard-kbl2/igt@kms_content_protection@uevent.html
[60]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4496/shard-kbl7/igt@kms_content_protection@uevent.html
- shard-apl: [FAIL][61] ([i915#357] / [i915#95]) -> [FAIL][62] ([i915#357])
[61]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5604/shard-apl7/igt@kms_content_protection@uevent.html
[62]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4496/shard-apl7/igt@kms_content_protection@uevent.html
* igt@kms_fbcon_fbt@fbc:
- shard-kbl: [FAIL][63] ([i915#64]) -> [FAIL][64] ([i915#93] / [i915#95])
[63]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5604/shard-kbl6/igt@kms_fbcon_fbt@fbc.html
[64]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4496/shard-kbl1/igt@kms_fbcon_fbt@fbc.html
- shard-apl: [FAIL][65] ([i915#1525]) -> [FAIL][66] ([i915#95])
[65]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5604/shard-apl2/igt@kms_fbcon_fbt@fbc.html
[66]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4496/shard-apl6/igt@kms_fbcon_fbt@fbc.html
* igt@kms_plane_alpha_blend@pipe-a-alpha-basic:
- shard-apl: [FAIL][67] ([fdo#108145] / [i915#265]) -> [FAIL][68] ([fdo#108145] / [i915#265] / [i915#95])
[67]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5604/shard-apl1/igt@kms_plane_alpha_blend@pipe-a-alpha-basic.html
[68]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4496/shard-apl4/igt@kms_plane_alpha_blend@pipe-a-alpha-basic.html
- shard-kbl: [FAIL][69] ([fdo#108145] / [i915#265]) -> [FAIL][70] ([fdo#108145] / [i915#265] / [i915#93] / [i915#95])
[69]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5604/shard-kbl2/igt@kms_plane_alpha_blend@pipe-a-alpha-basic.html
[70]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4496/shard-kbl7/igt@kms_plane_alpha_blend@pipe-a-alpha-basic.html
* igt@kms_psr2_su@page_flip:
- shard-iclb: [FAIL][71] ([i915#608]) -> [SKIP][72] ([fdo#109642] / [fdo#111068])
[71]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5604/shard-iclb2/igt@kms_psr2_su@page_flip.html
[72]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4496/shard-iclb6/igt@kms_psr2_su@page_flip.html
* igt@kms_setmode@basic:
- shard-kbl: [FAIL][73] ([i915#31] / [i915#93] / [i915#95]) -> [FAIL][74] ([i915#31])
[73]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_5604/shard-kbl6/igt@kms_setmode@basic.html
[74]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4496/shard-kbl1/igt@kms_setmode@basic.html
{name}: This element is suppressed. This means it is ignored when computing
the status of the difference (SUCCESS, WARNING, or FAILURE).
[fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
[fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
[fdo#109441]: https://bugs.freedesktop.org/show_bug.cgi?id=109441
[fdo#109642]: https://bugs.freedesktop.org/show_bug.cgi?id=109642
[fdo#111068]: https://bugs.freedesktop.org/show_bug.cgi?id=111068
[i915#1119]: https://gitlab.freedesktop.org/drm/intel/issues/1119
[i915#1277]: https://gitlab.freedesktop.org/drm/intel/issues/1277
[i915#1525]: https://gitlab.freedesktop.org/drm/intel/issues/1525
[i915#1531]: https://gitlab.freedesktop.org/drm/intel/issues/1531
[i915#1542]: https://gitlab.freedesktop.org/drm/intel/issues/1542
[i915#155]: https://gitlab.freedesktop.org/drm/intel/issues/155
[i915#1658]: https://gitlab.freedesktop.org/drm/intel/issues/1658
[i915#177]: https://gitlab.freedesktop.org/drm/intel/issues/177
[i915#180]: https://gitlab.freedesktop.org/drm/intel/issues/180
[i915#265]: https://gitlab.freedesktop.org/drm/intel/issues/265
[i915#31]: https://gitlab.freedesktop.org/drm/intel/issues/31
[i915#357]: https://gitlab.freedesktop.org/drm/intel/issues/357
[i915#454]: https://gitlab.freedesktop.org/drm/intel/issues/454
[i915#468]: https://gitlab.freedesktop.org/drm/intel/issues/468
[i915#52]: https://gitlab.freedesktop.org/drm/intel/issues/52
[i915#54]: https://gitlab.freedesktop.org/drm/intel/issues/54
[i915#608]: https://gitlab.freedesktop.org/drm/intel/issues/608
[i915#61]: https://gitlab.freedesktop.org/drm/intel/issues/61
[i915#64]: https://gitlab.freedesktop.org/drm/intel/issues/64
[i915#72]: https://gitlab.freedesktop.org/drm/intel/issues/72
[i915#82]: https://gitlab.freedesktop.org/drm/intel/issues/82
[i915#859]: https://gitlab.freedesktop.org/drm/intel/issues/859
[i915#899]: https://gitlab.freedesktop.org/drm/intel/issues/899
[i915#93]: https://gitlab.freedesktop.org/drm/intel/issues/93
[i915#95]: https://gitlab.freedesktop.org/drm/intel/issues/95
Participating hosts (8 -> 8)
------------------------------
No changes in participating hosts
Build changes
-------------
* CI: CI-20190529 -> None
* IGT: IGT_5604 -> IGTPW_4496
CI-20190529: 20190529
CI_DRM_8345: e1fa8774e58e663bec8257f678c2f8fd17088292 @ git://anongit.freedesktop.org/gfx-ci/linux
IGTPW_4496: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4496/index.html
IGT_5604: 18cc19ece602ba552a8386222b49e7e82820f9aa @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_4496/index.html
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply [flat|nested] 35+ messages in thread
* [igt-dev] [PATCH i-g-t 1/8] lib/params: add igt_params.c for module parameter access
2020-04-20 12:17 [igt-dev] [PATCH i-g-t 0/8] Use device dependant module parameters Juha-Pekka Heikkila
@ 2020-04-20 12:17 ` Juha-Pekka Heikkila
0 siblings, 0 replies; 35+ messages in thread
From: Juha-Pekka Heikkila @ 2020-04-20 12:17 UTC (permalink / raw)
To: igt-dev; +Cc: Jani Nikula
From: Jani Nikula <jani.nikula@intel.com>
We have generic helpers for sysfs access in igt_sysfs.c, but we also
have a number of module parameter access specific helpers scattered here
and there. Start gathering the latter into a file of its own.
For i915, the long-term goal is to migrate from module parameters to
device specific debugfs parameters. With all igt module param access
centralized in one place, we can make the transition much easier.
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
lib/Makefile.sources | 2 +
lib/i915/gem_submission.c | 1 +
lib/igt.h | 1 +
lib/igt_aux.c | 143 +-----------------
lib/igt_aux.h | 3 -
lib/igt_gt.c | 1 +
lib/igt_params.c | 244 +++++++++++++++++++++++++++++++
lib/igt_params.h | 38 +++++
lib/igt_psr.c | 1 +
lib/igt_sysfs.c | 68 ---------
lib/igt_sysfs.h | 5 -
lib/meson.build | 1 +
tests/i915/gem_ctx_persistence.c | 1 +
13 files changed, 291 insertions(+), 218 deletions(-)
create mode 100644 lib/igt_params.c
create mode 100644 lib/igt_params.h
diff --git a/lib/Makefile.sources b/lib/Makefile.sources
index 1e2c88ae..2ead9d02 100644
--- a/lib/Makefile.sources
+++ b/lib/Makefile.sources
@@ -47,6 +47,8 @@ lib_source_list = \
igt_list.h \
igt_matrix.c \
igt_matrix.h \
+ igt_params.c \
+ igt_params.h \
igt_primes.c \
igt_primes.h \
igt_rand.c \
diff --git a/lib/i915/gem_submission.c b/lib/i915/gem_submission.c
index 72de0c22..bb58a207 100644
--- a/lib/i915/gem_submission.c
+++ b/lib/i915/gem_submission.c
@@ -32,6 +32,7 @@
#include "igt_core.h"
#include "igt_gt.h"
+#include "igt_params.h"
#include "igt_sysfs.h"
#include "intel_chipset.h"
#include "intel_reg.h"
diff --git a/lib/igt.h b/lib/igt.h
index a6c4e44d..0bf98aa5 100644
--- a/lib/igt.h
+++ b/lib/igt.h
@@ -37,6 +37,7 @@
#include "igt_frame.h"
#include "igt_gt.h"
#include "igt_kms.h"
+#include "igt_params.h"
#include "igt_pm.h"
#include "igt_stats.h"
#ifdef HAVE_CHAMELIUM
diff --git a/lib/igt_aux.c b/lib/igt_aux.c
index ecab5d99..c55b2916 100644
--- a/lib/igt_aux.c
+++ b/lib/igt_aux.c
@@ -60,6 +60,7 @@
#include "igt_aux.h"
#include "igt_debugfs.h"
#include "igt_gt.h"
+#include "igt_params.h"
#include "igt_rand.h"
#include "igt_sysfs.h"
#include "config.h"
@@ -1116,148 +1117,6 @@ void igt_unlock_mem(void)
locked_mem = NULL;
}
-
-#define MODULE_PARAM_DIR "/sys/module/i915/parameters/"
-#define PARAM_NAME_MAX_SZ 32
-#define PARAM_VALUE_MAX_SZ 16
-#define PARAM_FILE_PATH_MAX_SZ (strlen(MODULE_PARAM_DIR) + PARAM_NAME_MAX_SZ)
-
-struct module_param_data {
- char name[PARAM_NAME_MAX_SZ];
- char original_value[PARAM_VALUE_MAX_SZ];
-
- struct module_param_data *next;
-};
-struct module_param_data *module_params = NULL;
-
-static void igt_module_param_exit_handler(int sig)
-{
- const size_t dir_len = strlen(MODULE_PARAM_DIR);
- char file_path[PARAM_FILE_PATH_MAX_SZ];
- struct module_param_data *data;
- int fd;
-
- /* We don't need to assert string sizes on this function since they were
- * already checked before being stored on the lists. Besides,
- * igt_assert() is not AS-Safe. */
- strcpy(file_path, MODULE_PARAM_DIR);
-
- for (data = module_params; data != NULL; data = data->next) {
- strcpy(file_path + dir_len, data->name);
-
- fd = open(file_path, O_RDWR);
- if (fd >= 0) {
- int size = strlen (data->original_value);
-
- if (size != write(fd, data->original_value, size)) {
- const char msg[] = "WARNING: Module parameters "
- "may not have been reset to their "
- "original values\n";
- assert(write(STDERR_FILENO, msg, sizeof(msg))
- == sizeof(msg));
- }
-
- close(fd);
- }
- }
- /* free() is not AS-Safe, so we can't call it here. */
-}
-
-/**
- * igt_save_module_param:
- * @name: name of the i915.ko module parameter
- * @file_path: full sysfs file path for the parameter
- *
- * Reads the current value of an i915.ko module parameter, saves it on an array,
- * then installs an exit handler to restore it when the program exits.
- *
- * It is safe to call this function multiple times for the same parameter.
- *
- * Notice that this function is called by igt_set_module_param(), so that one -
- * or one of its wrappers - is the only function the test programs need to call.
- */
-static void igt_save_module_param(const char *name, const char *file_path)
-{
- struct module_param_data *data;
- size_t n;
- int fd;
-
- /* Check if this parameter is already saved. */
- for (data = module_params; data != NULL; data = data->next)
- if (strncmp(data->name, name, PARAM_NAME_MAX_SZ) == 0)
- return;
-
- if (!module_params)
- igt_install_exit_handler(igt_module_param_exit_handler);
-
- data = calloc(1, sizeof (*data));
- igt_assert(data);
-
- strncpy(data->name, name, PARAM_NAME_MAX_SZ - 1);
-
- fd = open(file_path, O_RDONLY);
- igt_assert(fd >= 0);
-
- n = read(fd, data->original_value, PARAM_VALUE_MAX_SZ);
- igt_assert_f(n > 0 && n < PARAM_VALUE_MAX_SZ,
- "Need to increase PARAM_VALUE_MAX_SZ\n");
-
- igt_assert(close(fd) == 0);
-
- data->next = module_params;
- module_params = data;
-}
-
-/**
- * igt_set_module_param:
- * @name: i915.ko parameter name
- * @val: i915.ko parameter value
- *
- * This function sets the desired value for the given i915.ko parameter. It also
- * takes care of saving and restoring the values that were already set before
- * the test was run.
- *
- * Please consider using igt_set_module_param_int() for the integer and bool
- * parameters.
- */
-void igt_set_module_param(const char *name, const char *val)
-{
- char file_path[PARAM_FILE_PATH_MAX_SZ];
- size_t len = strlen(val);
- int fd;
-
- igt_assert_f(strlen(name) < PARAM_NAME_MAX_SZ,
- "Need to increase PARAM_NAME_MAX_SZ\n");
- strcpy(file_path, MODULE_PARAM_DIR);
- strcpy(file_path + strlen(MODULE_PARAM_DIR), name);
-
- igt_save_module_param(name, file_path);
-
- fd = open(file_path, O_RDWR);
- igt_assert(write(fd, val, len) == len);
- igt_assert(close(fd) == 0);
-}
-
-/**
- * igt_set_module_param_int:
- * @name: i915.ko parameter name
- * @val: i915.ko parameter value
- *
- * This is a wrapper for igt_set_module_param() that takes an integer instead of
- * a string. Please see igt_set_module_param().
- */
-void igt_set_module_param_int(const char *name, int val)
-{
- char str[PARAM_VALUE_MAX_SZ];
- int n;
-
- n = snprintf(str, PARAM_VALUE_MAX_SZ, "%d\n", val);
- igt_assert_f(n < PARAM_VALUE_MAX_SZ,
- "Need to increase PARAM_VALUE_MAX_SZ\n");
-
- igt_set_module_param(name, str);
-}
-
/**
* igt_is_process_running:
* @comm: Name of process in the form found in /proc/pid/comm (limited to 15
diff --git a/lib/igt_aux.h b/lib/igt_aux.h
index 04d22904..bf57ccf5 100644
--- a/lib/igt_aux.h
+++ b/lib/igt_aux.h
@@ -285,9 +285,6 @@ double igt_stop_siglatency(struct igt_mean *result);
bool igt_allow_unlimited_files(void);
-void igt_set_module_param(const char *name, const char *val);
-void igt_set_module_param_int(const char *name, int val);
-
int igt_is_process_running(const char *comm);
int igt_terminate_process(int sig, const char *comm);
void igt_lsof(const char *dpath);
diff --git a/lib/igt_gt.c b/lib/igt_gt.c
index 256c7cbc..22340a3d 100644
--- a/lib/igt_gt.c
+++ b/lib/igt_gt.c
@@ -35,6 +35,7 @@
#include "igt_aux.h"
#include "igt_core.h"
#include "igt_gt.h"
+#include "igt_params.h"
#include "igt_sysfs.h"
#include "igt_debugfs.h"
#include "ioctl_wrappers.h"
diff --git a/lib/igt_params.c b/lib/igt_params.c
new file mode 100644
index 00000000..4bd2b1f2
--- /dev/null
+++ b/lib/igt_params.c
@@ -0,0 +1,244 @@
+/*
+ * Copyright © 2019 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 <assert.h>
+#include <fcntl.h>
+#include <limits.h>
+#include <stdbool.h>
+#include <stdio.h>
+#include <sys/ioctl.h>
+
+#include <i915_drm.h>
+
+#include "igt_core.h"
+#include "igt_params.h"
+#include "igt_sysfs.h"
+
+#define MODULE_PARAM_DIR "/sys/module/i915/parameters/"
+#define PARAM_NAME_MAX_SZ 32
+#define PARAM_VALUE_MAX_SZ 16
+#define PARAM_FILE_PATH_MAX_SZ (strlen(MODULE_PARAM_DIR) + PARAM_NAME_MAX_SZ)
+
+struct module_param_data {
+ char name[PARAM_NAME_MAX_SZ];
+ char original_value[PARAM_VALUE_MAX_SZ];
+
+ struct module_param_data *next;
+};
+struct module_param_data *module_params = NULL;
+
+static void igt_module_param_exit_handler(int sig)
+{
+ const size_t dir_len = strlen(MODULE_PARAM_DIR);
+ char file_path[PARAM_FILE_PATH_MAX_SZ];
+ struct module_param_data *data;
+ int fd;
+
+ /* We don't need to assert string sizes on this function since they were
+ * already checked before being stored on the lists. Besides,
+ * igt_assert() is not AS-Safe. */
+ strcpy(file_path, MODULE_PARAM_DIR);
+
+ for (data = module_params; data != NULL; data = data->next) {
+ strcpy(file_path + dir_len, data->name);
+
+ fd = open(file_path, O_RDWR);
+ if (fd >= 0) {
+ int size = strlen (data->original_value);
+
+ if (size != write(fd, data->original_value, size)) {
+ const char msg[] = "WARNING: Module parameters "
+ "may not have been reset to their "
+ "original values\n";
+ assert(write(STDERR_FILENO, msg, sizeof(msg))
+ == sizeof(msg));
+ }
+
+ close(fd);
+ }
+ }
+ /* free() is not AS-Safe, so we can't call it here. */
+}
+
+/**
+ * igt_save_module_param:
+ * @name: name of the i915.ko module parameter
+ * @file_path: full sysfs file path for the parameter
+ *
+ * Reads the current value of an i915.ko module parameter, saves it on an array,
+ * then installs an exit handler to restore it when the program exits.
+ *
+ * It is safe to call this function multiple times for the same parameter.
+ *
+ * Notice that this function is called by igt_set_module_param(), so that one -
+ * or one of its wrappers - is the only function the test programs need to call.
+ */
+static void igt_save_module_param(const char *name, const char *file_path)
+{
+ struct module_param_data *data;
+ size_t n;
+ int fd;
+
+ /* Check if this parameter is already saved. */
+ for (data = module_params; data != NULL; data = data->next)
+ if (strncmp(data->name, name, PARAM_NAME_MAX_SZ) == 0)
+ return;
+
+ if (!module_params)
+ igt_install_exit_handler(igt_module_param_exit_handler);
+
+ data = calloc(1, sizeof (*data));
+ igt_assert(data);
+
+ strncpy(data->name, name, PARAM_NAME_MAX_SZ - 1);
+
+ fd = open(file_path, O_RDONLY);
+ igt_assert(fd >= 0);
+
+ n = read(fd, data->original_value, PARAM_VALUE_MAX_SZ);
+ igt_assert_f(n > 0 && n < PARAM_VALUE_MAX_SZ,
+ "Need to increase PARAM_VALUE_MAX_SZ\n");
+
+ igt_assert(close(fd) == 0);
+
+ data->next = module_params;
+ module_params = data;
+}
+
+/**
+ * igt_sysfs_open_parameters:
+ * @device: fd of the device
+ *
+ * This opens the module parameters directory (under sysfs) corresponding
+ * to the device for use with igt_sysfs_set() and igt_sysfs_get().
+ *
+ * Returns:
+ * The directory fd, or -1 on failure.
+ */
+int igt_sysfs_open_parameters(int device)
+{
+ int dir, params = -1;
+
+ dir = igt_sysfs_open(device);
+ if (dir >= 0) {
+ params = openat(dir,
+ "device/driver/module/parameters",
+ O_RDONLY);
+ close(dir);
+ }
+
+ if (params < 0) { /* builtin? */
+ drm_version_t version;
+ char name[32] = "";
+ char path[PATH_MAX];
+
+ memset(&version, 0, sizeof(version));
+ version.name_len = sizeof(name);
+ version.name = name;
+ ioctl(device, DRM_IOCTL_VERSION, &version);
+
+ sprintf(path, "/sys/module/%s/parameters", name);
+ params = open(path, O_RDONLY);
+ }
+
+ return params;
+}
+
+/**
+ * igt_sysfs_set_parameters:
+ * @device: fd of the device
+ * @parameter: the name of the parameter to set
+ * @fmt: printf-esque format string
+ *
+ * Returns true on success
+ */
+bool igt_sysfs_set_parameter(int device,
+ const char *parameter,
+ const char *fmt, ...)
+{
+ va_list ap;
+ int dir;
+ int ret;
+
+ dir = igt_sysfs_open_parameters(device);
+ if (dir < 0)
+ return false;
+
+ va_start(ap, fmt);
+ ret = igt_sysfs_vprintf(dir, parameter, fmt, ap);
+ va_end(ap);
+
+ close(dir);
+
+ return ret > 0;
+}
+
+/**
+ * igt_set_module_param:
+ * @name: i915.ko parameter name
+ * @val: i915.ko parameter value
+ *
+ * This function sets the desired value for the given i915.ko parameter. It also
+ * takes care of saving and restoring the values that were already set before
+ * the test was run.
+ *
+ * Please consider using igt_set_module_param_int() for the integer and bool
+ * parameters.
+ */
+void igt_set_module_param(const char *name, const char *val)
+{
+ char file_path[PARAM_FILE_PATH_MAX_SZ];
+ size_t len = strlen(val);
+ int fd;
+
+ igt_assert_f(strlen(name) < PARAM_NAME_MAX_SZ,
+ "Need to increase PARAM_NAME_MAX_SZ\n");
+ strcpy(file_path, MODULE_PARAM_DIR);
+ strcpy(file_path + strlen(MODULE_PARAM_DIR), name);
+
+ igt_save_module_param(name, file_path);
+
+ fd = open(file_path, O_RDWR);
+ igt_assert(write(fd, val, len) == len);
+ igt_assert(close(fd) == 0);
+}
+
+/**
+ * igt_set_module_param_int:
+ * @name: i915.ko parameter name
+ * @val: i915.ko parameter value
+ *
+ * This is a wrapper for igt_set_module_param() that takes an integer instead of
+ * a string. Please see igt_set_module_param().
+ */
+void igt_set_module_param_int(const char *name, int val)
+{
+ char str[PARAM_VALUE_MAX_SZ];
+ int n;
+
+ n = snprintf(str, PARAM_VALUE_MAX_SZ, "%d\n", val);
+ igt_assert_f(n < PARAM_VALUE_MAX_SZ,
+ "Need to increase PARAM_VALUE_MAX_SZ\n");
+
+ igt_set_module_param(name, str);
+}
diff --git a/lib/igt_params.h b/lib/igt_params.h
new file mode 100644
index 00000000..cf0fd18f
--- /dev/null
+++ b/lib/igt_params.h
@@ -0,0 +1,38 @@
+/*
+ * Copyright © 2019 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_PARAMS_H__
+#define __IGT_PARAMS_H__
+
+#include <stdbool.h>
+
+int igt_sysfs_open_parameters(int device);
+
+__attribute__((format(printf, 3, 4)))
+bool igt_sysfs_set_parameter(int device, const char *parameter,
+ const char *fmt, ...);
+
+void igt_set_module_param(const char *name, const char *val);
+void igt_set_module_param_int(const char *name, int val);
+
+#endif /* __IGT_PARAMS_H__ */
diff --git a/lib/igt_psr.c b/lib/igt_psr.c
index 83ccacdd..2bcf14ea 100644
--- a/lib/igt_psr.c
+++ b/lib/igt_psr.c
@@ -21,6 +21,7 @@
* IN THE SOFTWARE.
*/
+#include "igt_params.h"
#include "igt_psr.h"
#include "igt_sysfs.h"
#include <errno.h>
diff --git a/lib/igt_sysfs.c b/lib/igt_sysfs.c
index 7528c3bd..6aafe534 100644
--- a/lib/igt_sysfs.c
+++ b/lib/igt_sysfs.c
@@ -135,74 +135,6 @@ int igt_sysfs_open(int device)
return open(path, O_RDONLY);
}
-/**
- * igt_sysfs_set_parameters:
- * @device: fd of the device
- * @parameter: the name of the parameter to set
- * @fmt: printf-esque format string
- *
- * Returns true on success
- */
-bool igt_sysfs_set_parameter(int device,
- const char *parameter,
- const char *fmt, ...)
-{
- va_list ap;
- int dir;
- int ret;
-
- dir = igt_sysfs_open_parameters(device);
- if (dir < 0)
- return false;
-
- va_start(ap, fmt);
- ret = igt_sysfs_vprintf(dir, parameter, fmt, ap);
- va_end(ap);
-
- close(dir);
-
- return ret > 0;
-}
-
-/**
- * igt_sysfs_open_parameters:
- * @device: fd of the device
- *
- * This opens the module parameters directory (under sysfs) corresponding
- * to the device for use with igt_sysfs_set() and igt_sysfs_get().
- *
- * Returns:
- * The directory fd, or -1 on failure.
- */
-int igt_sysfs_open_parameters(int device)
-{
- int dir, params = -1;
-
- dir = igt_sysfs_open(device);
- if (dir >= 0) {
- params = openat(dir,
- "device/driver/module/parameters",
- O_RDONLY);
- close(dir);
- }
-
- if (params < 0) { /* builtin? */
- drm_version_t version;
- char name[32] = "";
- char path[PATH_MAX];
-
- memset(&version, 0, sizeof(version));
- version.name_len = sizeof(name);
- version.name = name;
- ioctl(device, DRM_IOCTL_VERSION, &version);
-
- sprintf(path, "/sys/module/%s/parameters", name);
- params = open(path, O_RDONLY);
- }
-
- return params;
-}
-
/**
* igt_sysfs_write:
* @dir: directory for the device from igt_sysfs_open()
diff --git a/lib/igt_sysfs.h b/lib/igt_sysfs.h
index b646df30..64935a5c 100644
--- a/lib/igt_sysfs.h
+++ b/lib/igt_sysfs.h
@@ -30,11 +30,6 @@
char *igt_sysfs_path(int device, char *path, int pathlen);
int igt_sysfs_open(int device);
-int igt_sysfs_open_parameters(int device);
-bool igt_sysfs_set_parameter(int device,
- const char *parameter,
- const char *fmt, ...)
- __attribute__((format(printf,3,4)));
int igt_sysfs_read(int dir, const char *attr, void *data, int len);
int igt_sysfs_write(int dir, const char *attr, const void *data, int len);
diff --git a/lib/meson.build b/lib/meson.build
index e2060430..c9af0403 100644
--- a/lib/meson.build
+++ b/lib/meson.build
@@ -17,6 +17,7 @@ lib_sources = [
'igt_halffloat.c',
'igt_matrix.c',
'igt_perf.c',
+ 'igt_params.c',
'igt_primes.c',
'igt_rand.c',
'igt_rapl.c',
diff --git a/tests/i915/gem_ctx_persistence.c b/tests/i915/gem_ctx_persistence.c
index 3d52987d..965b788b 100644
--- a/tests/i915/gem_ctx_persistence.c
+++ b/tests/i915/gem_ctx_persistence.c
@@ -38,6 +38,7 @@
#include "igt_dummyload.h"
#include "igt_gt.h"
#include "igt_sysfs.h"
+#include "igt_params.h"
#include "ioctl_wrappers.h" /* gem_wait()! */
#include "sw_sync.h"
--
2.26.0
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [igt-dev] [PATCH i-g-t 1/8] lib/params: add igt_params.c for module parameter access
2020-04-19 15:17 [igt-dev] [PATCH i-g-t 0/8] Use device dependant module parameters Juha-Pekka Heikkila
@ 2020-04-19 15:17 ` Juha-Pekka Heikkila
0 siblings, 0 replies; 35+ messages in thread
From: Juha-Pekka Heikkila @ 2020-04-19 15:17 UTC (permalink / raw)
To: igt-dev; +Cc: Jani Nikula
From: Jani Nikula <jani.nikula@intel.com>
We have generic helpers for sysfs access in igt_sysfs.c, but we also
have a number of module parameter access specific helpers scattered here
and there. Start gathering the latter into a file of its own.
For i915, the long-term goal is to migrate from module parameters to
device specific debugfs parameters. With all igt module param access
centralized in one place, we can make the transition much easier.
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
lib/Makefile.sources | 2 +
lib/i915/gem_submission.c | 1 +
lib/igt.h | 1 +
lib/igt_aux.c | 143 +-----------------
lib/igt_aux.h | 3 -
lib/igt_gt.c | 1 +
lib/igt_params.c | 244 +++++++++++++++++++++++++++++++
lib/igt_params.h | 38 +++++
lib/igt_psr.c | 1 +
lib/igt_sysfs.c | 68 ---------
lib/igt_sysfs.h | 5 -
lib/meson.build | 1 +
tests/i915/gem_ctx_persistence.c | 1 +
13 files changed, 291 insertions(+), 218 deletions(-)
create mode 100644 lib/igt_params.c
create mode 100644 lib/igt_params.h
diff --git a/lib/Makefile.sources b/lib/Makefile.sources
index 1e2c88ae..2ead9d02 100644
--- a/lib/Makefile.sources
+++ b/lib/Makefile.sources
@@ -47,6 +47,8 @@ lib_source_list = \
igt_list.h \
igt_matrix.c \
igt_matrix.h \
+ igt_params.c \
+ igt_params.h \
igt_primes.c \
igt_primes.h \
igt_rand.c \
diff --git a/lib/i915/gem_submission.c b/lib/i915/gem_submission.c
index 72de0c22..bb58a207 100644
--- a/lib/i915/gem_submission.c
+++ b/lib/i915/gem_submission.c
@@ -32,6 +32,7 @@
#include "igt_core.h"
#include "igt_gt.h"
+#include "igt_params.h"
#include "igt_sysfs.h"
#include "intel_chipset.h"
#include "intel_reg.h"
diff --git a/lib/igt.h b/lib/igt.h
index a6c4e44d..0bf98aa5 100644
--- a/lib/igt.h
+++ b/lib/igt.h
@@ -37,6 +37,7 @@
#include "igt_frame.h"
#include "igt_gt.h"
#include "igt_kms.h"
+#include "igt_params.h"
#include "igt_pm.h"
#include "igt_stats.h"
#ifdef HAVE_CHAMELIUM
diff --git a/lib/igt_aux.c b/lib/igt_aux.c
index ecab5d99..c55b2916 100644
--- a/lib/igt_aux.c
+++ b/lib/igt_aux.c
@@ -60,6 +60,7 @@
#include "igt_aux.h"
#include "igt_debugfs.h"
#include "igt_gt.h"
+#include "igt_params.h"
#include "igt_rand.h"
#include "igt_sysfs.h"
#include "config.h"
@@ -1116,148 +1117,6 @@ void igt_unlock_mem(void)
locked_mem = NULL;
}
-
-#define MODULE_PARAM_DIR "/sys/module/i915/parameters/"
-#define PARAM_NAME_MAX_SZ 32
-#define PARAM_VALUE_MAX_SZ 16
-#define PARAM_FILE_PATH_MAX_SZ (strlen(MODULE_PARAM_DIR) + PARAM_NAME_MAX_SZ)
-
-struct module_param_data {
- char name[PARAM_NAME_MAX_SZ];
- char original_value[PARAM_VALUE_MAX_SZ];
-
- struct module_param_data *next;
-};
-struct module_param_data *module_params = NULL;
-
-static void igt_module_param_exit_handler(int sig)
-{
- const size_t dir_len = strlen(MODULE_PARAM_DIR);
- char file_path[PARAM_FILE_PATH_MAX_SZ];
- struct module_param_data *data;
- int fd;
-
- /* We don't need to assert string sizes on this function since they were
- * already checked before being stored on the lists. Besides,
- * igt_assert() is not AS-Safe. */
- strcpy(file_path, MODULE_PARAM_DIR);
-
- for (data = module_params; data != NULL; data = data->next) {
- strcpy(file_path + dir_len, data->name);
-
- fd = open(file_path, O_RDWR);
- if (fd >= 0) {
- int size = strlen (data->original_value);
-
- if (size != write(fd, data->original_value, size)) {
- const char msg[] = "WARNING: Module parameters "
- "may not have been reset to their "
- "original values\n";
- assert(write(STDERR_FILENO, msg, sizeof(msg))
- == sizeof(msg));
- }
-
- close(fd);
- }
- }
- /* free() is not AS-Safe, so we can't call it here. */
-}
-
-/**
- * igt_save_module_param:
- * @name: name of the i915.ko module parameter
- * @file_path: full sysfs file path for the parameter
- *
- * Reads the current value of an i915.ko module parameter, saves it on an array,
- * then installs an exit handler to restore it when the program exits.
- *
- * It is safe to call this function multiple times for the same parameter.
- *
- * Notice that this function is called by igt_set_module_param(), so that one -
- * or one of its wrappers - is the only function the test programs need to call.
- */
-static void igt_save_module_param(const char *name, const char *file_path)
-{
- struct module_param_data *data;
- size_t n;
- int fd;
-
- /* Check if this parameter is already saved. */
- for (data = module_params; data != NULL; data = data->next)
- if (strncmp(data->name, name, PARAM_NAME_MAX_SZ) == 0)
- return;
-
- if (!module_params)
- igt_install_exit_handler(igt_module_param_exit_handler);
-
- data = calloc(1, sizeof (*data));
- igt_assert(data);
-
- strncpy(data->name, name, PARAM_NAME_MAX_SZ - 1);
-
- fd = open(file_path, O_RDONLY);
- igt_assert(fd >= 0);
-
- n = read(fd, data->original_value, PARAM_VALUE_MAX_SZ);
- igt_assert_f(n > 0 && n < PARAM_VALUE_MAX_SZ,
- "Need to increase PARAM_VALUE_MAX_SZ\n");
-
- igt_assert(close(fd) == 0);
-
- data->next = module_params;
- module_params = data;
-}
-
-/**
- * igt_set_module_param:
- * @name: i915.ko parameter name
- * @val: i915.ko parameter value
- *
- * This function sets the desired value for the given i915.ko parameter. It also
- * takes care of saving and restoring the values that were already set before
- * the test was run.
- *
- * Please consider using igt_set_module_param_int() for the integer and bool
- * parameters.
- */
-void igt_set_module_param(const char *name, const char *val)
-{
- char file_path[PARAM_FILE_PATH_MAX_SZ];
- size_t len = strlen(val);
- int fd;
-
- igt_assert_f(strlen(name) < PARAM_NAME_MAX_SZ,
- "Need to increase PARAM_NAME_MAX_SZ\n");
- strcpy(file_path, MODULE_PARAM_DIR);
- strcpy(file_path + strlen(MODULE_PARAM_DIR), name);
-
- igt_save_module_param(name, file_path);
-
- fd = open(file_path, O_RDWR);
- igt_assert(write(fd, val, len) == len);
- igt_assert(close(fd) == 0);
-}
-
-/**
- * igt_set_module_param_int:
- * @name: i915.ko parameter name
- * @val: i915.ko parameter value
- *
- * This is a wrapper for igt_set_module_param() that takes an integer instead of
- * a string. Please see igt_set_module_param().
- */
-void igt_set_module_param_int(const char *name, int val)
-{
- char str[PARAM_VALUE_MAX_SZ];
- int n;
-
- n = snprintf(str, PARAM_VALUE_MAX_SZ, "%d\n", val);
- igt_assert_f(n < PARAM_VALUE_MAX_SZ,
- "Need to increase PARAM_VALUE_MAX_SZ\n");
-
- igt_set_module_param(name, str);
-}
-
/**
* igt_is_process_running:
* @comm: Name of process in the form found in /proc/pid/comm (limited to 15
diff --git a/lib/igt_aux.h b/lib/igt_aux.h
index 04d22904..bf57ccf5 100644
--- a/lib/igt_aux.h
+++ b/lib/igt_aux.h
@@ -285,9 +285,6 @@ double igt_stop_siglatency(struct igt_mean *result);
bool igt_allow_unlimited_files(void);
-void igt_set_module_param(const char *name, const char *val);
-void igt_set_module_param_int(const char *name, int val);
-
int igt_is_process_running(const char *comm);
int igt_terminate_process(int sig, const char *comm);
void igt_lsof(const char *dpath);
diff --git a/lib/igt_gt.c b/lib/igt_gt.c
index 256c7cbc..22340a3d 100644
--- a/lib/igt_gt.c
+++ b/lib/igt_gt.c
@@ -35,6 +35,7 @@
#include "igt_aux.h"
#include "igt_core.h"
#include "igt_gt.h"
+#include "igt_params.h"
#include "igt_sysfs.h"
#include "igt_debugfs.h"
#include "ioctl_wrappers.h"
diff --git a/lib/igt_params.c b/lib/igt_params.c
new file mode 100644
index 00000000..4bd2b1f2
--- /dev/null
+++ b/lib/igt_params.c
@@ -0,0 +1,244 @@
+/*
+ * Copyright © 2019 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 <assert.h>
+#include <fcntl.h>
+#include <limits.h>
+#include <stdbool.h>
+#include <stdio.h>
+#include <sys/ioctl.h>
+
+#include <i915_drm.h>
+
+#include "igt_core.h"
+#include "igt_params.h"
+#include "igt_sysfs.h"
+
+#define MODULE_PARAM_DIR "/sys/module/i915/parameters/"
+#define PARAM_NAME_MAX_SZ 32
+#define PARAM_VALUE_MAX_SZ 16
+#define PARAM_FILE_PATH_MAX_SZ (strlen(MODULE_PARAM_DIR) + PARAM_NAME_MAX_SZ)
+
+struct module_param_data {
+ char name[PARAM_NAME_MAX_SZ];
+ char original_value[PARAM_VALUE_MAX_SZ];
+
+ struct module_param_data *next;
+};
+struct module_param_data *module_params = NULL;
+
+static void igt_module_param_exit_handler(int sig)
+{
+ const size_t dir_len = strlen(MODULE_PARAM_DIR);
+ char file_path[PARAM_FILE_PATH_MAX_SZ];
+ struct module_param_data *data;
+ int fd;
+
+ /* We don't need to assert string sizes on this function since they were
+ * already checked before being stored on the lists. Besides,
+ * igt_assert() is not AS-Safe. */
+ strcpy(file_path, MODULE_PARAM_DIR);
+
+ for (data = module_params; data != NULL; data = data->next) {
+ strcpy(file_path + dir_len, data->name);
+
+ fd = open(file_path, O_RDWR);
+ if (fd >= 0) {
+ int size = strlen (data->original_value);
+
+ if (size != write(fd, data->original_value, size)) {
+ const char msg[] = "WARNING: Module parameters "
+ "may not have been reset to their "
+ "original values\n";
+ assert(write(STDERR_FILENO, msg, sizeof(msg))
+ == sizeof(msg));
+ }
+
+ close(fd);
+ }
+ }
+ /* free() is not AS-Safe, so we can't call it here. */
+}
+
+/**
+ * igt_save_module_param:
+ * @name: name of the i915.ko module parameter
+ * @file_path: full sysfs file path for the parameter
+ *
+ * Reads the current value of an i915.ko module parameter, saves it on an array,
+ * then installs an exit handler to restore it when the program exits.
+ *
+ * It is safe to call this function multiple times for the same parameter.
+ *
+ * Notice that this function is called by igt_set_module_param(), so that one -
+ * or one of its wrappers - is the only function the test programs need to call.
+ */
+static void igt_save_module_param(const char *name, const char *file_path)
+{
+ struct module_param_data *data;
+ size_t n;
+ int fd;
+
+ /* Check if this parameter is already saved. */
+ for (data = module_params; data != NULL; data = data->next)
+ if (strncmp(data->name, name, PARAM_NAME_MAX_SZ) == 0)
+ return;
+
+ if (!module_params)
+ igt_install_exit_handler(igt_module_param_exit_handler);
+
+ data = calloc(1, sizeof (*data));
+ igt_assert(data);
+
+ strncpy(data->name, name, PARAM_NAME_MAX_SZ - 1);
+
+ fd = open(file_path, O_RDONLY);
+ igt_assert(fd >= 0);
+
+ n = read(fd, data->original_value, PARAM_VALUE_MAX_SZ);
+ igt_assert_f(n > 0 && n < PARAM_VALUE_MAX_SZ,
+ "Need to increase PARAM_VALUE_MAX_SZ\n");
+
+ igt_assert(close(fd) == 0);
+
+ data->next = module_params;
+ module_params = data;
+}
+
+/**
+ * igt_sysfs_open_parameters:
+ * @device: fd of the device
+ *
+ * This opens the module parameters directory (under sysfs) corresponding
+ * to the device for use with igt_sysfs_set() and igt_sysfs_get().
+ *
+ * Returns:
+ * The directory fd, or -1 on failure.
+ */
+int igt_sysfs_open_parameters(int device)
+{
+ int dir, params = -1;
+
+ dir = igt_sysfs_open(device);
+ if (dir >= 0) {
+ params = openat(dir,
+ "device/driver/module/parameters",
+ O_RDONLY);
+ close(dir);
+ }
+
+ if (params < 0) { /* builtin? */
+ drm_version_t version;
+ char name[32] = "";
+ char path[PATH_MAX];
+
+ memset(&version, 0, sizeof(version));
+ version.name_len = sizeof(name);
+ version.name = name;
+ ioctl(device, DRM_IOCTL_VERSION, &version);
+
+ sprintf(path, "/sys/module/%s/parameters", name);
+ params = open(path, O_RDONLY);
+ }
+
+ return params;
+}
+
+/**
+ * igt_sysfs_set_parameters:
+ * @device: fd of the device
+ * @parameter: the name of the parameter to set
+ * @fmt: printf-esque format string
+ *
+ * Returns true on success
+ */
+bool igt_sysfs_set_parameter(int device,
+ const char *parameter,
+ const char *fmt, ...)
+{
+ va_list ap;
+ int dir;
+ int ret;
+
+ dir = igt_sysfs_open_parameters(device);
+ if (dir < 0)
+ return false;
+
+ va_start(ap, fmt);
+ ret = igt_sysfs_vprintf(dir, parameter, fmt, ap);
+ va_end(ap);
+
+ close(dir);
+
+ return ret > 0;
+}
+
+/**
+ * igt_set_module_param:
+ * @name: i915.ko parameter name
+ * @val: i915.ko parameter value
+ *
+ * This function sets the desired value for the given i915.ko parameter. It also
+ * takes care of saving and restoring the values that were already set before
+ * the test was run.
+ *
+ * Please consider using igt_set_module_param_int() for the integer and bool
+ * parameters.
+ */
+void igt_set_module_param(const char *name, const char *val)
+{
+ char file_path[PARAM_FILE_PATH_MAX_SZ];
+ size_t len = strlen(val);
+ int fd;
+
+ igt_assert_f(strlen(name) < PARAM_NAME_MAX_SZ,
+ "Need to increase PARAM_NAME_MAX_SZ\n");
+ strcpy(file_path, MODULE_PARAM_DIR);
+ strcpy(file_path + strlen(MODULE_PARAM_DIR), name);
+
+ igt_save_module_param(name, file_path);
+
+ fd = open(file_path, O_RDWR);
+ igt_assert(write(fd, val, len) == len);
+ igt_assert(close(fd) == 0);
+}
+
+/**
+ * igt_set_module_param_int:
+ * @name: i915.ko parameter name
+ * @val: i915.ko parameter value
+ *
+ * This is a wrapper for igt_set_module_param() that takes an integer instead of
+ * a string. Please see igt_set_module_param().
+ */
+void igt_set_module_param_int(const char *name, int val)
+{
+ char str[PARAM_VALUE_MAX_SZ];
+ int n;
+
+ n = snprintf(str, PARAM_VALUE_MAX_SZ, "%d\n", val);
+ igt_assert_f(n < PARAM_VALUE_MAX_SZ,
+ "Need to increase PARAM_VALUE_MAX_SZ\n");
+
+ igt_set_module_param(name, str);
+}
diff --git a/lib/igt_params.h b/lib/igt_params.h
new file mode 100644
index 00000000..cf0fd18f
--- /dev/null
+++ b/lib/igt_params.h
@@ -0,0 +1,38 @@
+/*
+ * Copyright © 2019 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_PARAMS_H__
+#define __IGT_PARAMS_H__
+
+#include <stdbool.h>
+
+int igt_sysfs_open_parameters(int device);
+
+__attribute__((format(printf, 3, 4)))
+bool igt_sysfs_set_parameter(int device, const char *parameter,
+ const char *fmt, ...);
+
+void igt_set_module_param(const char *name, const char *val);
+void igt_set_module_param_int(const char *name, int val);
+
+#endif /* __IGT_PARAMS_H__ */
diff --git a/lib/igt_psr.c b/lib/igt_psr.c
index 83ccacdd..2bcf14ea 100644
--- a/lib/igt_psr.c
+++ b/lib/igt_psr.c
@@ -21,6 +21,7 @@
* IN THE SOFTWARE.
*/
+#include "igt_params.h"
#include "igt_psr.h"
#include "igt_sysfs.h"
#include <errno.h>
diff --git a/lib/igt_sysfs.c b/lib/igt_sysfs.c
index 7528c3bd..6aafe534 100644
--- a/lib/igt_sysfs.c
+++ b/lib/igt_sysfs.c
@@ -135,74 +135,6 @@ int igt_sysfs_open(int device)
return open(path, O_RDONLY);
}
-/**
- * igt_sysfs_set_parameters:
- * @device: fd of the device
- * @parameter: the name of the parameter to set
- * @fmt: printf-esque format string
- *
- * Returns true on success
- */
-bool igt_sysfs_set_parameter(int device,
- const char *parameter,
- const char *fmt, ...)
-{
- va_list ap;
- int dir;
- int ret;
-
- dir = igt_sysfs_open_parameters(device);
- if (dir < 0)
- return false;
-
- va_start(ap, fmt);
- ret = igt_sysfs_vprintf(dir, parameter, fmt, ap);
- va_end(ap);
-
- close(dir);
-
- return ret > 0;
-}
-
-/**
- * igt_sysfs_open_parameters:
- * @device: fd of the device
- *
- * This opens the module parameters directory (under sysfs) corresponding
- * to the device for use with igt_sysfs_set() and igt_sysfs_get().
- *
- * Returns:
- * The directory fd, or -1 on failure.
- */
-int igt_sysfs_open_parameters(int device)
-{
- int dir, params = -1;
-
- dir = igt_sysfs_open(device);
- if (dir >= 0) {
- params = openat(dir,
- "device/driver/module/parameters",
- O_RDONLY);
- close(dir);
- }
-
- if (params < 0) { /* builtin? */
- drm_version_t version;
- char name[32] = "";
- char path[PATH_MAX];
-
- memset(&version, 0, sizeof(version));
- version.name_len = sizeof(name);
- version.name = name;
- ioctl(device, DRM_IOCTL_VERSION, &version);
-
- sprintf(path, "/sys/module/%s/parameters", name);
- params = open(path, O_RDONLY);
- }
-
- return params;
-}
-
/**
* igt_sysfs_write:
* @dir: directory for the device from igt_sysfs_open()
diff --git a/lib/igt_sysfs.h b/lib/igt_sysfs.h
index b646df30..64935a5c 100644
--- a/lib/igt_sysfs.h
+++ b/lib/igt_sysfs.h
@@ -30,11 +30,6 @@
char *igt_sysfs_path(int device, char *path, int pathlen);
int igt_sysfs_open(int device);
-int igt_sysfs_open_parameters(int device);
-bool igt_sysfs_set_parameter(int device,
- const char *parameter,
- const char *fmt, ...)
- __attribute__((format(printf,3,4)));
int igt_sysfs_read(int dir, const char *attr, void *data, int len);
int igt_sysfs_write(int dir, const char *attr, const void *data, int len);
diff --git a/lib/meson.build b/lib/meson.build
index e2060430..c9af0403 100644
--- a/lib/meson.build
+++ b/lib/meson.build
@@ -17,6 +17,7 @@ lib_sources = [
'igt_halffloat.c',
'igt_matrix.c',
'igt_perf.c',
+ 'igt_params.c',
'igt_primes.c',
'igt_rand.c',
'igt_rapl.c',
diff --git a/tests/i915/gem_ctx_persistence.c b/tests/i915/gem_ctx_persistence.c
index 3d52987d..965b788b 100644
--- a/tests/i915/gem_ctx_persistence.c
+++ b/tests/i915/gem_ctx_persistence.c
@@ -38,6 +38,7 @@
#include "igt_dummyload.h"
#include "igt_gt.h"
#include "igt_sysfs.h"
+#include "igt_params.h"
#include "ioctl_wrappers.h" /* gem_wait()! */
#include "sw_sync.h"
--
2.17.1
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev
^ permalink raw reply related [flat|nested] 35+ messages in thread