Re: [igt-dev] [PATCH] pm_rpm: Skip PC8 tests when set limit doesn't allow entry

[Paulo Zanoni <paulo.r.zanoni@intel.com>]

Em Sex, 2018-05-11 às 14:31 +0300, Arkadiusz Hiler escreveu:
> On some devices BIOS limits possible Package C-states via setting one
> of
> the MSRs. The test now skips if the limit is set to a shallower PC-
> state
> than PC8.
> 
> The timeout for PC8+ residency change is lowered to 30s. During
> testing
> the entry always happened in ~10s,

That's related to our autosuspend_delay_ms value, and related to the
random chance of having non-graphics activity in the machine. 120s is
definitely way too conservative.

I didn't check the specific MSR register bits, so I can't give a R-B:

Acked-by: Paulo Zanoni <paulo.r.zanoni@intel.com>

(if you split this as Martin suggested, feel free to add the Acked-by
tag to both patches)


>  so thrice that should be a safe bet.
> (active USB keyboard, network and screen, no powertop --auto-tune)
> 
> Cc: Paulo Zanoni <paulo.r.zanoni@intel.com>
> Cc: Martin Peres <martin.peres@intel.com>
> Signed-off-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
> ---
>  tests/pm_rpm.c | 19 ++++++++++++++++---
>  1 file changed, 16 insertions(+), 3 deletions(-)
> 
> diff --git a/tests/pm_rpm.c b/tests/pm_rpm.c
> index d2b837fe..51d8f366 100644
> --- a/tests/pm_rpm.c
> +++ b/tests/pm_rpm.c
> @@ -50,6 +50,11 @@
>  #include "igt_sysfs.h"
>  #include "igt_debugfs.h"
>  
> +#define MSR_PKG_CST_CONFIG_CONTROL	0xE2
> +/* HSW/BDW: */
> +#define  PKG_CST_LIMIT_MASK		0xF
> +#define  PKG_CST_LIMIT_C8		0x6
> +
>  #define MSR_PC8_RES	0x630
>  #define MSR_PC9_RES	0x631
>  #define MSR_PC10_RES	0x632
> @@ -139,6 +144,14 @@ static bool supports_pc8_plus_residencies(void)
>  	if (rc != sizeof(val))
>  		return false;
>  
> +	rc = pread(msr_fd, &val, sizeof(uint64_t),
> MSR_PKG_CST_CONFIG_CONTROL);
> +	if (rc != sizeof(val))
> +		return false;
> +	if ((val & PKG_CST_LIMIT_MASK) < PKG_CST_LIMIT_C8) {
> +		igt_info("PKG C-states limited below PC8 by the
> BIOS\n");
> +		return false;
> +	}
> +
>  	return true;
>  }
>  
> @@ -728,7 +741,7 @@ static void pc8_residency_subtest(void)
>  
>  	/* Make sure PC8+ residencies move! */
>  	disable_all_screens(&ms_data);
> -	igt_assert_f(pc8_plus_residency_changed(120),
> +	igt_assert_f(pc8_plus_residency_changed(30),
>  		     "Machine is not reaching PC8+ states, please
> check its "
>  		     "configuration.\n");
>  
> @@ -757,7 +770,7 @@ static void modeset_subtest(enum screen_type
> type, int rounds, int wait_flags)
>  		if (wait_flags & WAIT_STATUS)
>  			igt_assert(wait_for_suspended());
>  		if (wait_flags & WAIT_PC8_RES)
> -			igt_assert(pc8_plus_residency_changed(120));
> +			igt_assert(pc8_plus_residency_changed(30));
>  		if (wait_flags & WAIT_EXTRA)
>  			sleep(5);
>  
> @@ -1262,7 +1275,7 @@ static void gem_execbuf_stress_subtest(int
> rounds, int wait_flags)
>  		if (wait_flags & WAIT_STATUS)
>  			igt_assert(wait_for_suspended());
>  		if (wait_flags & WAIT_PC8_RES)
> -			igt_assert(pc8_plus_residency_changed(120));
> +			igt_assert(pc8_plus_residency_changed(30));
>  		if (wait_flags & WAIT_EXTRA)
>  			sleep(5);
>  	}
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev