Re: [Intel-gfx] [igt-dev] [PATCH v2 i-g-t] i915_pm_freq_api: Add some debug to tests

["Dixit, Ashutosh" <ashutosh.dixit@intel.com>]

On Mon, 17 Jul 2023 11:42:13 -0700, Vinay Belgaumkar wrote:
>
> Some subtests seem to be failing in CI, use igt_assert_(lt/eq) which
> print the values being compared and some additional debug as well.
>
> v2: Print GT as well (Ashutosh)
>
> Signed-off-by: Vinay Belgaumkar <vinay.belgaumkar@intel.com>
> ---
>  tests/i915/i915_pm_freq_api.c | 18 ++++++++----------
>  1 file changed, 8 insertions(+), 10 deletions(-)
>
> diff --git a/tests/i915/i915_pm_freq_api.c b/tests/i915/i915_pm_freq_api.c
> index 522abee35..a7bbd4896 100644
> --- a/tests/i915/i915_pm_freq_api.c
> +++ b/tests/i915/i915_pm_freq_api.c
> @@ -55,6 +55,7 @@ static void test_freq_basic_api(int dirfd, int gt)
>	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_debug("GT: %d, RPn: %d, RPe: %d, RP0: %d", gt, rpn, rpe, rp0);
>
>	/*
>	 * Negative bound tests
> @@ -90,21 +91,18 @@ static void test_reset(int i915, int dirfd, int gt, int count)
>	int fd;
>
>	for (int i = 0; i < count; i++) {
> -		igt_assert_f(set_freq(dirfd, RPS_MIN_FREQ_MHZ, rpn) > 0,
> -			     "Failed after %d good cycles\n", i);
> -		igt_assert_f(set_freq(dirfd, RPS_MAX_FREQ_MHZ, rpn) > 0,
> -			     "Failed after %d good cycles\n", i);
> +		igt_debug("Running cycle: %d", i);
> +		igt_assert(set_freq(dirfd, RPS_MIN_FREQ_MHZ, rpn) > 0);
> +		igt_assert(set_freq(dirfd, RPS_MAX_FREQ_MHZ, rpn) > 0);

I am R-b'ing this but stuff like this should be using igt_assert_lt()
according to the commit message?

This _lt stuff has to be fixed all over the file, not just this patch, if
it brings any value (again according to the commit message).

Let me know if you want to fix this now or in a later patch. I'll wait
before merging.

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

>		usleep(ACT_FREQ_LATENCY_US);
> -		igt_assert_f(get_freq(dirfd, RPS_CUR_FREQ_MHZ) == rpn,
> -			     "Failed after %d good cycles\n", i);
> +		igt_assert_eq(get_freq(dirfd, RPS_CUR_FREQ_MHZ), rpn);
>
>		/* Manually trigger a GT reset */
>		fd = igt_debugfs_gt_open(i915, gt, "reset", O_WRONLY);
>		igt_require(fd >= 0);
>		igt_ignore_warn(write(fd, "1\n", 2));
>
> -		igt_assert_f(get_freq(dirfd, RPS_CUR_FREQ_MHZ) == rpn,
> -			     "Failed after %d good cycles\n", i);
> +		igt_assert_eq(get_freq(dirfd, RPS_CUR_FREQ_MHZ), rpn);
>	}
>	close(fd);
>  }
> @@ -116,13 +114,13 @@ static void test_suspend(int i915, int dirfd, int gt)
>	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_CUR_FREQ_MHZ) == rpn);
> +	igt_assert_eq(get_freq(dirfd, RPS_CUR_FREQ_MHZ), rpn);
>
>	/* Manually trigger a suspend */
>	igt_system_suspend_autoresume(SUSPEND_STATE_S3,
>				      SUSPEND_TEST_NONE);
>
> -	igt_assert(get_freq(dirfd, RPS_CUR_FREQ_MHZ) == rpn);
> +	igt_assert_eq(get_freq(dirfd, RPS_CUR_FREQ_MHZ), rpn);
>  }
>
>  int i915 = -1;
> --
> 2.38.1
>