On 4/3/2023 8:36 AM, Dixit, Ashutosh wrote:
On Mon, 03 Apr 2023 08:23:45 -0700, Belgaumkar, Vinay wrote:

On 3/31/2023 4:56 PM, Dixit, Ashutosh wrote:
On Mon, 27 Mar 2023 19:00:28 -0700, Vinay Belgaumkar wrote:
Hi Vinay,

+/*
+ * Too many intermediate components and steps before freq is adjusted
+ * Specially if workload is under execution, so let's wait 100 ms.
+ */
+#define ACT_FREQ_LATENCY_US 100000
+
+static uint32_t get_freq(int dirfd, uint8_t id)
+{
+       uint32_t val;
+
+       igt_require(igt_sysfs_rps_scanf(dirfd, id, "%u", &val) == 1);
igt_assert?
ok.
+static void test_freq_basic_api(int dirfd, int gt)
+{
+       uint32_t rpn, rp0, rpe;
+
+       /* Save frequencies */
+       rpn = get_freq(dirfd, RPS_RPn_FREQ_MHZ);
+       rp0 = get_freq(dirfd, RPS_RP0_FREQ_MHZ);
+       rpe = get_freq(dirfd, RPS_RP1_FREQ_MHZ);
+       igt_info("System min freq: %dMHz; max freq: %dMHz\n", rpn, rp0);
+
+       /*
+        * Negative bound tests
+        * RPn is the floor
+        * RP0 is the ceiling
+        */
+       igt_assert(set_freq(dirfd, RPS_MIN_FREQ_MHZ, rpn - 1) < 0);
+       igt_assert(set_freq(dirfd, RPS_MIN_FREQ_MHZ, rp0 + 1) < 0);
+       igt_assert(set_freq(dirfd, RPS_MIN_FREQ_MHZ, rpn - 1) < 0);
Is this supposed to be RPS_MAX_FREQ_MHZ?
We could do this check for max as well. But this is trying to see if min
can be set to below rpn.
In that case this statement is the same as the first one (2 lines
above). Is that needed?

ah, yes. Need more coffee. That should be RPS_MAX_FREQ_MHZ.

Thanks,

Vinay.




+       igt_assert(set_freq(dirfd, RPS_MAX_FREQ_MHZ, rp0 + 1) < 0);
+
After addressing the above, this is:

Reviewed-by: Ashutosh Dixit <ashutosh.di...@intel.com>

Also, before merging it would be good to see the results of the new
tests. So could you add a HAX patch adding the new tests to
fast-feedback.testlist and resend the series?
Sure, will do. Thanks for the review.

Vinay.

Thanks.
--
Ashutosh

Reply via email to