[igt-dev] [PATCH i-g-t 1/7] lib/params: add igt_params.c for module parameter access
Juha-Pekka Heikkila
juhapekka.heikkila at gmail.com
Tue Apr 28 20:22:49 UTC 2020
From: Jani Nikula <jani.nikula at 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 at intel.com>
Signed-off-by: Juha-Pekka Heikkila <juhapekka.heikkila at gmail.com>
Reviewed-by: Petri Latvala <petri.latvala at 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..9ffb533c 100644
--- a/lib/meson.build
+++ b/lib/meson.build
@@ -16,6 +16,7 @@ lib_sources = [
'igt_gt.c',
'igt_halffloat.c',
'igt_matrix.c',
+ 'igt_params.c',
'igt_perf.c',
'igt_primes.c',
'igt_rand.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
More information about the igt-dev
mailing list