Re: [igt-dev] [Intel-gfx] [PATCH i-g-t] i915/perf_pmu: Check that while parked, we report min freq or below

[Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>]

On 29/11/2019 11:45, Chris Wilson wrote:
> While the HW is parked, the GPU should be turned off and clocks stop
> (i.e. running at 0Hz). We should report either the last frequency we
> program (which should be the minimum legal value) or a more truthful 0.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> ---
>   tests/perf_pmu.c | 40 ++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 40 insertions(+)
> 
> diff --git a/tests/perf_pmu.c b/tests/perf_pmu.c
> index 54842784c..a1520d2c4 100644
> --- a/tests/perf_pmu.c
> +++ b/tests/perf_pmu.c
> @@ -1477,6 +1477,44 @@ test_frequency(int gem_fd)
>   	__assert_within_epsilon(max[0], max_freq, tolerance, 0.15f);
>   }
>   
> +static void
> +test_frequency_idle(int gem_fd)
> +{
> +	uint32_t min_freq;
> +	uint64_t val[2], start[2], slept;
> +	double idle[2];
> +	int fd, sysfs;
> +
> +	sysfs = igt_sysfs_open(gem_fd);
> +	igt_require(sysfs >= 0);
> +
> +	min_freq = igt_sysfs_get_u32(sysfs, "gt_RPn_freq_mhz");
> +	close(sysfs);
> +
> +	/* While parked, our convention is to report the GPU at 0Hz */
> +
> +	fd = open_group(I915_PMU_REQUESTED_FREQUENCY, -1);
> +	open_group(I915_PMU_ACTUAL_FREQUENCY, fd);
> +
> +	gem_quiescent_gpu(gem_fd); /* Be idle! */
> +	measured_usleep(2000); /* Wait for timers to cease */
> +
> +	slept = pmu_read_multi(fd, 2, start);
> +	measured_usleep(batch_duration_ns / 1000);
> +	slept = pmu_read_multi(fd, 2, val) - slept;
> +
> +	idle[0] = 1e9*(val[0] - start[0]) / slept;
> +	idle[1] = 1e9*(val[1] - start[1]) / slept;
> +
> +	igt_info("Idle frequency: requested %.1f, actual %.1f; HW min %u\n",
> +		 idle[0], idle[1], min_freq);
> +
> +	igt_assert_f(idle[0] <= min_freq,
> +		     "Request frequency should be 0 while parked!\n");
> +	igt_assert_f(idle[1] <= min_freq,
> +		     "Actual frequency should be 0 while parked!\n");
> +}
> +
>   static bool wait_for_rc6(int fd)
>   {
>   	struct timespec tv = {};
> @@ -1967,6 +2005,8 @@ igt_main
>   	 */
>   	igt_subtest("frequency")
>   		test_frequency(fd);
> +	igt_subtest("frequency-idle")
> +		test_frequency_idle(fd);
>   
>   	/**
>   	 * Test interrupt count reporting.
> 

Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

So we are going with it and leaving sysfs as is?

Regards,

Tvrtko
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev