[PATCH i-g-t] tests/intel/xe_pm: s3 vs s2idle: Validate only the platform default
Lucas De Marchi
lucas.demarchi at intel.com
Mon May 20 19:56:11 UTC 2024
On Mon, May 20, 2024 at 03:08:57PM GMT, Rodrigo Vivi wrote:
>It is very common to reach issues from other drivers when testing
>the full non-default cycle.
>
>Get back to i915 approach and validate only the default mem_sleep
>level of the mem sleep state.
>
>However, keep the option in place where the developer can force
>the validation of the state using an extra test argument.
so this will read /sys/power/mem_sleep and execute only that level
why not "up to that level" so we still catch and differentiate issues
about failing to freeze vs failing to suspend?
what current issue is this actually fixing?
>
>v2: Fix Subject (Kamil)
>
>Cc: Kamil Konieczny <kamil.konieczny at linux.intel.com>
>Cc: Francois Dugast <francois.dugast at intel.com>
>Signed-off-by: Rodrigo Vivi <rodrigo.vivi at intel.com>
>---
> lib/igt_aux.c | 31 +++++++++++++++++++++-----
> lib/igt_aux.h | 2 ++
> tests/intel/xe_pm.c | 54 ++++++++++++++++++++++++++++++++++++++++++++-
> 3 files changed, 80 insertions(+), 7 deletions(-)
>
>diff --git a/lib/igt_aux.c b/lib/igt_aux.c
>index 3407cc4f2..7770a05fc 100644
>--- a/lib/igt_aux.c
>+++ b/lib/igt_aux.c
>@@ -852,7 +852,15 @@ static bool is_state_supported(int power_dir, enum igt_suspend_state state)
> return str;
> }
>
>-static int get_mem_sleep(void)
>+/**
>+ * igt_system_get_mem_sleep:
>+ *
>+ * Used to query the current configured mem_sleep. In special to stash the
>+ * default of the system before any test operation.
>+ *
>+ * Return: Current mem_sleep configured.
>+ */
>+enum igt_mem_sleep igt_system_get_mem_sleep(void)
> {
> char *mem_sleep_states;
> char *mem_sleep_state;
>@@ -891,14 +899,25 @@ static int get_mem_sleep(void)
> return mem_sleep;
> }
>
>-static void set_mem_sleep(int power_dir, enum igt_mem_sleep sleep)
>+/**
>+ * igt_system_set_mem_sleep:
>+ *
>+ * Used to configure the mem_sleep. Useful to return the platform to its default.
>+ */
>+void igt_system_set_mem_sleep(enum igt_mem_sleep sleep)
> {
>+ int power_dir;
>+
> igt_assert(sleep < MEM_SLEEP_NUM);
>
>+ igt_require((power_dir = open("/sys/power", O_RDONLY)) >= 0);
>+
> igt_assert_eq(faccessat(power_dir, "mem_sleep", W_OK, 0), 0);
>
> igt_assert(igt_sysfs_set(power_dir, "mem_sleep",
> mem_sleep_name[sleep]));
>+
>+ close(power_dir);
> }
>
> static bool is_mem_sleep_state_supported(int power_dir, enum igt_mem_sleep state)
>@@ -1018,11 +1037,11 @@ void igt_system_suspend_autoresume(enum igt_suspend_state state,
> igt_aux_enable_pm_suspend_dbg(power_dir);
>
> if (state == SUSPEND_STATE_S3) {
>- orig_mem_sleep = get_mem_sleep();
>+ orig_mem_sleep = igt_system_get_mem_sleep();
> igt_skip_on_f(!is_mem_sleep_state_supported(power_dir, MEM_SLEEP_DEEP),
> "S3 not supported in this system.\n");
>- set_mem_sleep(power_dir, MEM_SLEEP_DEEP);
>- igt_skip_on_f(get_mem_sleep() != MEM_SLEEP_DEEP,
>+ igt_system_set_mem_sleep(MEM_SLEEP_DEEP);
>+ igt_skip_on_f(igt_system_get_mem_sleep() != MEM_SLEEP_DEEP,
> "S3 not possible in this system.\n");
> }
>
>@@ -1034,7 +1053,7 @@ void igt_system_suspend_autoresume(enum igt_suspend_state state,
> suspend_via_sysfs(power_dir, state);
>
> if (orig_mem_sleep)
>- set_mem_sleep(power_dir, orig_mem_sleep);
>+ igt_system_set_mem_sleep(orig_mem_sleep);
>
> set_pm_test(power_dir, orig_test);
> close(power_dir);
>diff --git a/lib/igt_aux.h b/lib/igt_aux.h
>index 4664559dc..8fd21c37b 100644
>--- a/lib/igt_aux.h
>+++ b/lib/igt_aux.h
>@@ -214,6 +214,8 @@ enum igt_mem_sleep {
> MEM_SLEEP_NUM,
> };
>
>+enum igt_mem_sleep igt_system_get_mem_sleep(void);
>+void igt_system_set_mem_sleep(enum igt_mem_sleep sleep);
> void igt_system_suspend_autoresume(enum igt_suspend_state state,
> enum igt_suspend_test test);
> void igt_set_autoresume_delay(int delay_secs);
>diff --git a/tests/intel/xe_pm.c b/tests/intel/xe_pm.c
>index b4a8c4d15..29d576522 100644
>--- a/tests/intel/xe_pm.c
>+++ b/tests/intel/xe_pm.c
>@@ -44,11 +44,18 @@ typedef struct {
> struct pci_device *pci_root;
> char pci_slot_name[NAME_MAX];
> drmModeResPtr res;
>+ enum igt_mem_sleep default_mem_sleep;
> } device_t;
>
> uint64_t orig_threshold;
> int fw_handle = -1;
>
>+static struct param {
>+ bool ignore_default_mem_sleep;
>+} params = {
>+ .ignore_default_mem_sleep = false,
>+};
>+
> static void dpms_on_off(device_t device, int mode)
> {
> int i;
>@@ -221,6 +228,21 @@ static void close_fw_handle(int sig)
> close(fw_handle);
> }
>
>+static void assert_default_mem_sleep(device_t device, enum igt_suspend_state s_state)
>+{
>+ if (s_state == NO_SUSPEND)
>+ return;
>+
>+ if (params.ignore_default_mem_sleep)
>+ return;
>+
>+ if (device.default_mem_sleep == MEM_SLEEP_S2IDLE)
>+ igt_require(s_state == SUSPEND_STATE_FREEZE);
>+
>+ if (device.default_mem_sleep == MEM_SLEEP_DEEP)
else if ...
>+ igt_require(s_state == SUSPEND_STATE_S3);
isn't this actually killing the S4 tests (which right now are up to
device suspend only...)?
Lucas De Marchi
>+}
>+
> #define MAX_VMAS 2
> /**
> * SUBTEST: %s-basic
>@@ -327,6 +349,8 @@ test_exec(device_t device, struct drm_xe_engine_class_instance *eci,
> bool check_rpm = (d_state == IGT_ACPI_D3Hot ||
> d_state == IGT_ACPI_D3Cold);
>
>+ assert_default_mem_sleep(device, s_state);
>+
> igt_assert(n_exec_queues <= MAX_N_EXEC_QUEUES);
> igt_assert(n_execs > 0);
>
>@@ -636,6 +660,8 @@ static void test_mocs_suspend_resume(device_t device, int s_state)
> {
> int gt;
>
>+ assert_default_mem_sleep(device, s_state);
>+
> xe_for_each_gt(device.fd_xe, gt) {
> char path[256];
>
>@@ -669,7 +695,28 @@ static void test_mocs_suspend_resume(device_t device, int s_state)
> }
> }
>
>-igt_main
>+static int opt_handler(int opt, int opt_index, void *data)
>+{
>+ switch(opt) {
>+ case 'm':
>+ params.ignore_default_mem_sleep = true;
>+ igt_debug("Ignoring default mem_state!\n");
>+ break;
>+ default:
>+ return IGT_OPT_HANDLER_ERROR;
>+ }
>+ return IGT_OPT_HANDLER_SUCCESS;
>+}
>+
>+struct option long_options[] = {
>+ {"ignore-default-mem-sleep", no_argument, NULL, 'm'},
>+ {0, 0, 0, 0}
>+};
>+
>+const char *help_str =
>+ " --ignore-default-mem-sleep\tUsed to enable s3 tests on systems where s2idle is the default and vice-versa\n";
>+
>+igt_main_args("", long_options, help_str, opt_handler, NULL)
> {
> struct drm_xe_engine_class_instance *hwe;
> device_t device;
>@@ -705,6 +752,7 @@ igt_main
>
> igt_fixture {
> memset(&device, 0, sizeof(device));
>+ device.default_mem_sleep = igt_system_get_mem_sleep();
> device.fd_xe = drm_open_driver(DRIVER_XE);
> device.pci_xe = igt_device_get_pci_device(device.fd_xe);
> device.pci_root = igt_device_get_pci_root_port(device.fd_xe);
>@@ -725,6 +773,7 @@ igt_main
> enum igt_suspend_test test = s->state == SUSPEND_STATE_DISK ?
> SUSPEND_TEST_DEVICES : SUSPEND_TEST_NONE;
>
>+ assert_default_mem_sleep(device, s->state);
> igt_system_suspend_autoresume(s->state, test);
> }
>
>@@ -738,6 +787,7 @@ igt_main
> enum igt_suspend_test test = s->state == SUSPEND_STATE_DISK ?
> SUSPEND_TEST_DEVICES : SUSPEND_TEST_NONE;
>
>+ assert_default_mem_sleep(device, s->state);
> igt_system_suspend_autoresume(s->state, test);
> xe_for_each_engine(device.fd_xe, hwe)
> test_exec(device, hwe, 1, 2, NO_SUSPEND,
>@@ -848,5 +898,7 @@ igt_main
> igt_restore_runtime_pm();
> drmModeFreeResources(device.res);
> drm_close_driver(device.fd_xe);
>+ igt_system_set_mem_sleep(device.default_mem_sleep);
> }
>+
> }
>--
>2.44.0
>
More information about the igt-dev
mailing list