Re: [igt-dev] [PATCH i-g-t] tests/perf_pmu: Compare against requested freq in frequency subtest

Dixit, Ashutosh
2022-11-09T01:49:06Z

On Tue, 08 Nov 2022 01:24:14 -0800, Tvrtko Ursulin wrote:
>
> On 08/11/2022 01:31, Dixit, Ashutosh wrote:
> > On Mon, 07 Nov 2022 16:57:24 -0800, Belgaumkar, Vinay wrote:
> >> On 11/7/2022 4:22 PM, Dixit, Ashutosh wrote:
> >>> On Mon, 07 Nov 2022 16:18:31 -0800, Dixit, Ashutosh wrote:
> >>> Hi Vinay,
> >>>
> >>> A question for you below.
> >>>
> >>>> So I submitted this patch to repro the issue and to print out the requested
> >>>> freq from sysfs:
> >>>>
> >>>> https://patchwork.freedesktop.org/series/110630/
> >>>>
> >>>> And we can see the output here:
> >>>>
> >>>> https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_8061/bat-dg2-11/igt@perf_pmu@frequency.html
> >>>>
> >>>> ```
> >>>> IGT-Version: 1.26-g1bef4d081 (x86_64) (Linux: 6.1.0-rc4-CI_DRM_12352-gc55ac6a74bd1+ x86_64)
> >>>> Starting subtest: frequency
> >>>> Frequency: min=300, max=2050, boost=2050 MHz
> >>>> Min frequency: requested 349.7, actual 349.7
> >>>> Max frequency: requested 2048.0, actual 2048.0
> >>>> Sysfs requested: min 350, max 2050
> >>>> Stack trace:
> >>>> #0 ../../../usr/src/igt-gpu-tools/lib/igt_core.c:1908 __igt_fail_assert()
> >>>> #1 ../../../usr/src/igt-gpu-tools/tests/i915/perf_pmu.c:1656 __igt_unique____real_main2147()
> >>>> #2 ../../../usr/src/igt-gpu-tools/tests/i915/perf_pmu.c:2147 main()
> >>>> #3 [__libc_start_main+0xf3]
> >>>> #4 [_start+0x2e]
> >>>> Subtest frequency: FAIL (2.212s)
> >>>> ```
> >>>>
> >>>> So we clearly see the requested freq from sysfs is indeed 350 MHz so
> >>>> SLPC/PCODE is not honoring the set min == max == boost freq (and PMU is
> >>>> measuring what sysfs is showing). In general PCODE is the final arbiter in
> >>>> such cases and we do occasionally see instances where set freq limits are
> >>>> not honored.
> >>>>
> >>>> I would say if igt@perf_pmu@frequency is testing freq measured by PMU then
> >>>> the patch below is correct. Whether SLPC/PCODE is honoring the set freq
> >>>> limits should be tested in a SLPC test (which we also have).
> >>>
> >>> igt@perf_pmu@frequency sets 'min == max == boost == 300 MHz' but we still
> >>> see the requested freq to be 350 MHz. Do we have a SLPC test covering this
> >>> scenario or should we add one? This is failing on one of the DG2's.
> >>
> >> Does adding a delay help (around 20 ms for the h2g to go through
> >> typically)?
> >
> > There is no delay but the test calls gem_quiescent_gpu() after setting 'min
> > == max == boost == 300 MHz' and then launches a spinner. We are checking
> > the requested freq 500 ms after the spinner is started (so plenty of time
> > for the h2g) and still the requested freq is 350 MHz.
> >
> >> Also, is there a workload running when we change the min=max=boost to
> >> 300?
> >
> > No, the workload is started after setting the freq and calling
> > gem_quiescent_gpu().
>
> Implication here seems to be that gem_quiescent_gpu() would need to cover
> H2G communication - does it?

Don't think gem_quiescent_gpu should cover h2g delays. I have added a
separate 100 ms delay (which I've used elsewhere too) in v2. I am not sure
what is the delay caused by gem_quiescent_gpu and if that is sufficient we
don't need this additional 100 ms delay. Anyway the delay will make PMU
measurements a little more accurate but does not affect the overall result.

> >> We already check these things in our SLPC selftests.
>
> Is it then expected to respect the 300MHz max in this case? Or if it can't,
> should it be reflected in the sysfs readback?

The max is set to 300 MHz but is not respected. So the sysfs readback shows
max as 300 MHz but actual requested freq is higher. The flow is that i915
sets min/max using h2g. GuC FW validates the parameters in the top-half and
returns success (which results in max freq being set in i915 to 300 MHz)
but the actual min/max freq change in PCODE is done later in the
bottom-half because of which the delay is needed.

Though Vinay we never read the max freq back from FW, that is we call
intel_rps_get_max_frequency and not intel_guc_slpc_get_max_freq when
displaying in sysfs. That is we display i915 cached values rather than
interrogate FW, assuming that FW min/max values are same as the cached
values. I can add a couple more prints and see what actual FW values are.

Thanks.
--
Ashutosh