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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 6/7/2023 4:11 PM, Belgaumkar, Vinay wrote:

On 6/7/2023 3:56 PM, Dixit, Ashutosh wrote:
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?

It shouldn't, but funnier things have happened. Anyways, I can add a check for cur_freq and ensure that is at min. That will prove we applied the soft limit after suspend.

Thanks,

Vinay.


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.
There are writes in the IGT fixture after the test completes.

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?

This test is specific to the freq api, hence just min/max entries.

Thanks,

Vinay.


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




[Index of Archives]     [AMD Graphics]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux