[PATCH i-g-t] tests/xe_pm: s3 vs s2idle: Validate only the platform default

Kamil Konieczny kamil.konieczny at linux.intel.com
Fri May 17 17:57:55 UTC 2024


Hi Rodrigo,
On 2024-05-16 at 14:07:32 -0400, Rodrigo Vivi wrote:

please change subject, was
[PATCH i-g-t] tests/xe_pm: s3 vs s2idle: Validate only the platform default
------------------ ^

imho should be:
[PATCH i-g-t] tests/intel/xe_pm: s3 vs s2idle: Validate only the platform default


Regards,
Kamil

> 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.
> 
> 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 9cb00c7f1..ccb15134a 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)
> +		igt_require(s_state == SUSPEND_STATE_S3);
> +}
> +
>  #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,
> @@ -845,5 +895,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