[igt-dev] [PATCH i-g-t] tests/i915_pm_freq_api: Add a suspend subtest

[Dixit, Ashutosh]

On Wed, 07 Jun 2023 15:31:33 -0700, Belgaumkar, Vinay wrote:
>
> On 6/7/2023 2:12 PM, Dixit, Ashutosh wrote:
> > On Tue, 06 Jun 2023 13:35:35 -0700, Vinay Belgaumkar wrote:
> > Hi Vinay,
> >
> >> Verify that SLPC API works as expected after a suspend.
> >>
> >> Signed-off-by: Vinay Belgaumkar <vinay.belgaumkar at intel.com>
> >> ---
> >>   tests/i915/i915_pm_freq_api.c | 30 ++++++++++++++++++++++++++++++
> >>   1 file changed, 30 insertions(+)
> >>
> >> diff --git a/tests/i915/i915_pm_freq_api.c b/tests/i915/i915_pm_freq_api.c
> >> index 9005cd220..f35f1f8e0 100644
> >> --- a/tests/i915/i915_pm_freq_api.c
> >> +++ b/tests/i915/i915_pm_freq_api.c
> >> @@ -18,6 +18,9 @@
> >>    *
> >>    * SUBTEST: freq-reset
> >>    * Description: Test basic freq API works after a reset
> >> + *
> >> + * SUBTEST: freq-suspend
> >> + * Description: Test basic freq API works after a runtime suspend
> >>    */
> >>
> >>   IGT_TEST_DESCRIPTION("Test SLPC freq API");
> >> @@ -99,6 +102,24 @@ static void test_reset(int i915, int dirfd, int gt)
> >>	igt_assert(get_freq(dirfd, RPS_MAX_FREQ_MHZ) == rpn);
> >>   }
> >>
> >> +static void test_suspend(int i915, int dirfd, int gt)
> >> +{
> >> +	uint32_t rpn = get_freq(dirfd, RPS_RPn_FREQ_MHZ);
> >> +
> >> +	igt_assert(set_freq(dirfd, RPS_MIN_FREQ_MHZ, rpn) > 0);
> >> +	igt_assert(set_freq(dirfd, RPS_MAX_FREQ_MHZ, rpn) > 0);
> >> +	usleep(ACT_FREQ_LATENCY_US);
> >> +	igt_assert(get_freq(dirfd, RPS_MIN_FREQ_MHZ) == rpn);
> >> +	igt_assert(get_freq(dirfd, RPS_MAX_FREQ_MHZ) == rpn);
> >> +
> >> +	/* Manually trigger a suspend */
> >> +	igt_system_suspend_autoresume(SUSPEND_STATE_S3,
> >> +				      SUSPEND_TEST_NONE);
> >> +
> >> +	igt_assert(get_freq(dirfd, RPS_MIN_FREQ_MHZ) == rpn);
> >> +	igt_assert(get_freq(dirfd, RPS_MAX_FREQ_MHZ) == rpn);
> > I am wondering what the purpose/value of this test (and also "freq-reset")
> > is?  How can the "set" min/max set freq (which are just input settings)
> > change whether or not there is a suspend/resume or a reset? Especially when
> > we just return cached min/max values from i915?
>
> It is mainly checking that we don't smother the softlimit during a reset or
> suspend flow.

How can softlimit which is a ordinary variable in memory get clobbered by
suspend resume?

> In addition, it also tests the read/write interface works as expected
> after those events.

There's no write. Sorry, but I'm not convinced. There should be some more
meat to the test.

Maybe we can write a test which will check /all/ sysfs values are the same
after a suspend resume cycle? Why do only these specific ones have to be
checked?

Thanks.
--
Ashutosh


>
> Thanks,
>
> Vinay.
>
> >
> > Thanks.
> > --
> > Ashutosh
> >
> >
> >> +}
> >> +
> >>   igt_main
> >>   {
> >>	int i915 = -1;
> >> @@ -143,6 +164,15 @@ igt_main
> >>				test_reset(i915, dirfd, gt);
> >>	}
> >>
> >> +	igt_describe("Test basic freq API works after suspend");
> >> +	igt_subtest_with_dynamic_f("freq-suspend") {
> >> +		int dirfd, gt;
> >> +
> >> +		for_each_sysfs_gt_dirfd(i915, dirfd, gt)
> >> +			igt_dynamic_f("gt%u", gt)
> >> +				test_suspend(i915, dirfd, gt);
> >> +	}
> >> +
> >>	igt_fixture {
> >>		int dirfd, gt;
> >>		/* Restore frequencies */
> >> --
> >> 2.38.1
> >>