NAK: [PATCH] s3: check slp_s0_residency_usec for s2idle

[Alex Hung]

On 2020-07-29 6:54 p.m., Alex Hung wrote:
> If slp_s0_residency_usec == 0, CPU doesn't reach deepest power saving
> state.


This was done according to
https://bugs.launchpad.net/plainbox-provider-checkbox/+bug/1838409.

However, Intel's website,
https://01.org/blogs/qwang59/2018/how-achieve-s0ix-states-linux, points
out it should be increasing with successful s2idle, so I will modify the
patch to compare with the previous value in V2.


> 
> Signed-off-by: Alex Hung <alex.hung at canonical.com>
> ---
>  src/acpi/s3/s3.c       | 21 ++++++++++++++++++++-
>  src/lib/src/fwts_cpu.c |  3 +--
>  2 files changed, 21 insertions(+), 3 deletions(-)
> 
> diff --git a/src/acpi/s3/s3.c b/src/acpi/s3/s3.c
> index 45a1e6e2..8b017ed3 100644
> --- a/src/acpi/s3/s3.c
> +++ b/src/acpi/s3/s3.c
> @@ -33,6 +33,7 @@
>  #define PM_SUSPEND_PMUTILS		"pm-suspend"
>  #define PM_SUSPEND_HYBRID_PMUTILS	"pm-suspend-hybrid"
>  #define PM_SUSPEND_PATH			"/sys/power/mem_sleep"
> +#define PM_S2IDLE_SLP_S0		"/sys/kernel/debug/pmc_core/slp_s0_residency_usec"
>  
>  static char sleep_type[7];
>  
> @@ -209,6 +210,7 @@ static int s3_do_suspend_resume(fwts_framework *fw,
>  	int *hw_errors,
>  	int *pm_errors,
>  	int *hook_errors,
> +	int *s2idle_errors,
>  	int delay,
>  	int percent)
>  {
> @@ -332,6 +334,17 @@ static int s3_do_suspend_resume(fwts_framework *fw,
>  		(*hook_errors)++;
>  	}
>  
> +	if (!strncmp(sleep_type, "s2idle", strlen("s2idle"))) {
> +		int residency = atoi(fwts_get(PM_S2IDLE_SLP_S0));
> +		bool intel;
> +
> +		if (fwts_cpu_is_Intel(&intel) == FWTS_OK && intel && !residency) {
> +			(*s2idle_errors)++;
> +			fwts_failed(fw, LOG_LEVEL_HIGH, "S2idleNotDeepest",
> +				"Expected %s to be non-zero.", PM_S2IDLE_SLP_S0);
> +		}
> +	}
> +
>  	if (duration < delay) {
>  		(*pm_errors)++;
>  		fwts_failed(fw, LOG_LEVEL_MEDIUM, "ShortSuspend",
> @@ -525,6 +538,7 @@ static int s3_test_multiple(fwts_framework *fw)
>  	int hook_errors = 0;
>  	int klog_oopses = 0;
>  	int klog_warn_ons = 0;
> +	int s2idle_errors = 0;
>  	int suspend_too_long = 0;
>  	int resume_too_long = 0;
>  	int awake_delay = s3_min_delay * 1000;
> @@ -554,7 +568,7 @@ static int s3_test_multiple(fwts_framework *fw)
>  			fwts_log_error(fw, "Cannot read kernel log.");
>  
>  		ret = s3_do_suspend_resume(fw, &hw_errors, &pm_errors,
> -					   &hook_errors, s3_sleep_delay,
> +					   &hook_errors, &s2idle_errors, s3_sleep_delay,
>  					   percent);
>  		if (ret == FWTS_OUT_OF_MEMORY) {
>  			fwts_log_error(fw, "%s cycle %d failed - out of memory error.", sleep_type, i+1);
> @@ -631,6 +645,11 @@ static int s3_test_multiple(fwts_framework *fw)
>  	else
>  		fwts_passed(fw, "No kernel WARN_ON warnings detected.");
>  
> +	if (s2idle_errors > 0)
> +		fwts_log_info(fw, "Found %d s2idle errors.", s2idle_errors);
> +	else
> +		fwts_passed(fw, "No s2idle errors detected.");
> +
>  	if ((klog_errors + pm_errors + hw_errors + klog_oopses) > 0) {
>  		fwts_log_info(fw, "Found %d errors and %d oopses doing %d suspend/resume cycle(s).",
>  			klog_errors + pm_errors + hw_errors, klog_oopses, s3_multiple);
> diff --git a/src/lib/src/fwts_cpu.c b/src/lib/src/fwts_cpu.c
> index dc603468..0fde1cde 100644
> --- a/src/lib/src/fwts_cpu.c
> +++ b/src/lib/src/fwts_cpu.c
> @@ -197,8 +197,7 @@ static int fwts_cpu_matches_vendor_id(const char *vendor_id, bool *matches)
>  		return FWTS_ERROR;
>  	}
>  
> -        *matches = (strstr(cpu->vendor_id, vendor_id) != NULL);
> -
> +	*matches = (strstr(cpu->vendor_id, vendor_id) != NULL);
>  	fwts_cpu_free_info(cpu);
>  
>  	return FWTS_OK;
> 


-- 
Cheers,
Alex Hung