Re: [igt-dev] [PATCH i-g-t v3] tests/xe: Verify actual frequency on the basis of GT state

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

On Tue, 18 Jul 2023 04:17:36 -0700, Badal Nilawar wrote:

Hi Badal,

> diff --git a/lib/igt_pm.h b/lib/igt_pm.h
> index 71ec2f239..2fc7b98a1 100644
> --- a/lib/igt_pm.h
> +++ b/lib/igt_pm.h
> @@ -89,5 +89,6 @@ bool i915_is_slpc_enabled(int drm_fd);
>  int igt_pm_get_runtime_suspended_time(struct pci_device *pci_dev);
>  int igt_pm_get_runtime_usage(struct pci_device *pci_dev);
>  void igt_pm_ignore_slpc_efficient_freq(int i915, int gtfd, bool val);
> +bool xe_is_gt_in_c6(int fd, int gt);
>
>  #endif /* IGT_PM_H */
> diff --git a/tests/xe/xe_guc_pc.c b/tests/xe/xe_guc_pc.c
> index c34df8d60..a04b700dd 100644
> --- a/tests/xe/xe_guc_pc.c
> +++ b/tests/xe/xe_guc_pc.c
> @@ -218,7 +218,7 @@ static void test_freq_basic_api(int fd, int gt_id)
>   * Run type: FULL
>   */
>
> -static void test_freq_fixed(int fd, int gt_id)
> +static void test_freq_fixed(int fd, int gt_id, bool gt_idle)
>  {
>	uint32_t rpn = get_freq(fd, gt_id, "rpn");
>	uint32_t rpe = get_freq(fd, gt_id, "rpe");
> @@ -226,6 +226,9 @@ static void test_freq_fixed(int fd, int gt_id)
>
>	igt_debug("Starting testing fixed request\n");
>
> +	if (gt_idle)
> +		igt_require(igt_wait(xe_is_gt_in_c6(fd, gt_id), 1000, 1));
> +
>	/*
>	 * For Fixed freq we need to set both min and max to the desired value
>	 * Then we check if hardware is actually operating at the desired freq
> @@ -235,13 +238,30 @@ static void test_freq_fixed(int fd, int gt_id)
>	igt_assert(set_freq(fd, gt_id, "max", rpn) > 0);
>	usleep(ACT_FREQ_LATENCY_US);
>	igt_assert(get_freq(fd, gt_id, "cur") == rpn);
> -	igt_assert(get_freq(fd, gt_id, "act") == rpn);
> +
> +	if (gt_idle) {
> +		/*
> +		 * Before checking act freq usleep is added here and other places to
> +		 * ensure GT is idle as previous get_freq call to read cur freq
> +		 * forcewake the GT.
> +		 */
> +		usleep(ACT_FREQ_LATENCY_US);

Sorry I missed this yesterday. Basically why do we have the
igt_wait(xe_is_gt_in_c6()) above at the top when we are waking up the gt
when reading cur freq later?

Maybe instead of this usleep, we can this do here:

		igt_assert(igt_wait(xe_is_gt_in_c6(fd, gt_id), 1000, 1));

And get rid of the igt_require(igt_wait()) at the top?

Thanks.
--
Ashutosh