Re: [PATCH i-g-t v1] tests/intel/xe_gt_freq: Change basic throttle API test criteria

[Rodrigo Vivi <rodrigo.vivi@intel.com>]

On Tue, Nov 05, 2024 at 12:02:28PM +0530, Raag Jadav wrote:
> Basic throttle API test is implemented in a way that expects multiple
> sysfs reads to provide atomic results. Since atomicity is never guaranteed
> between multiple sysfs reads, this is an incorrect expection from the test.
> Change the test criteria to check for boolean value from throttle status
> and throttle reasons sysfs entries.
> 
> Closes: https://gitlab.freedesktop.org/drm/xe/kernel/-/issues/3254
> Closes: https://gitlab.freedesktop.org/drm/xe/kernel/-/issues/3147
> Closes: https://gitlab.freedesktop.org/drm/xe/kernel/-/issues/2810

There's clear something wrong with the 'status'....

look at this tool: https://github.com/ulissesf/qmassa
look at the recorded example in the readme... status is poping up so much.
that is not normal. I noticed this here as well. status is flipping to 1
so many times while we have absolutely no one of the real reasons flipping
to 1 as well. This is the problem that needs to be solved.

> Signed-off-by: Raag Jadav <raag.jadav@intel.com>
> ---
>  tests/intel/xe_gt_freq.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/tests/intel/xe_gt_freq.c b/tests/intel/xe_gt_freq.c
> index de4d111ea..632415d33 100644
> --- a/tests/intel/xe_gt_freq.c
> +++ b/tests/intel/xe_gt_freq.c
> @@ -121,10 +121,8 @@ static void test_throttle_basic_api(int fd, int gt_id)
>  	reasons |= get_throttle(fd, gt_id, "reason_vr_tdc");
>  	reasons |= get_throttle(fd, gt_id, "reason_vr_thermalert");
>  
> -	if (status)
> -		igt_assert(reasons);
> -	else
> -		igt_assert(!reasons);
> +	igt_assert(status == true || status == false);
> +	igt_assert(reasons == true || reasons == false);

This would be the same as deleting the test case...

>  }
>  
>  /**
> -- 
> 2.34.1
>