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@xxxxxxxxx>
> >> ---
> >> 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
> >>
