Re: [PATCH i-g-t v2] tests/intel/gem_exec_endless: Use the per-gt interface to set frequencies

[Janusz Krzysztofik <janusz.krzysztofik@linux.intel.com>]

Hi Krzysztof,

On Tue, 2026-04-07 at 12:52 +0200, Krzysztof Niemiec wrote:
> The test alters the environment, specifically the gt frequencies, for
> its duration. Using the legacy interface however makes it impossible for
> the pre-test state to be restored due to its limitations in multi-gt
> systems. Use the per-gt interface instead to properly clean up.

While I think your solution may be correct, your description is not clear 
enough for me.  How is it possible for the legacy interface to work as 
expected when altering the environment, but not ant longer when restoring 
defaults?

Thanks,
Janusz


> 
> Signed-off-by: Krzysztof Niemiec <krzysztof.niemiec@intel.com>
> ---
> 
> v2:
>   - Remove the redundant fd open from the i915_for_each_gt loop, as one
>     is already created as part of the loop (Sebastian)
>   - Remove extra newline (Krzysztof K)
> 
>  tests/intel/gem_exec_endless.c | 33 +++++++++++++++++----------------
>  1 file changed, 17 insertions(+), 16 deletions(-)
> 
> diff --git a/tests/intel/gem_exec_endless.c b/tests/intel/gem_exec_endless.c
> index 7f35e985c..f3ed8556c 100644
> --- a/tests/intel/gem_exec_endless.c
> +++ b/tests/intel/gem_exec_endless.c
> @@ -316,28 +316,28 @@ static void endless_dispatch(int i915, const struct intel_execution_engine2 *e)
>  		for_each_if(gem_class_can_store_dword(i915, (e)->class)) \
>  			igt_dynamic_f("%s", (e)->name)
>  
> -static void pin_rps(int sysfs)
> +static void pin_rps(int sysfs_gt)
>  {
>  	unsigned int max;
>  
> -	if (igt_sysfs_scanf(sysfs, "gt_RP0_freq_mhz", "%u", &max) != 1)
> +	if (igt_sysfs_scanf(sysfs_gt, "rps_RP0_freq_mhz", "%u", &max) != 1)
>  		return;
>  
> -	igt_sysfs_printf(sysfs, "gt_min_freq_mhz", "%u", max);
> -	igt_sysfs_printf(sysfs, "gt_max_freq_mhz", "%u", max);
> -	igt_sysfs_printf(sysfs, "gt_boost_freq_mhz", "%u", max);
> +	igt_sysfs_printf(sysfs_gt, "rps_min_freq_mhz", "%u", max);
> +	igt_sysfs_printf(sysfs_gt, "rps_max_freq_mhz", "%u", max);
> +	igt_sysfs_printf(sysfs_gt, "rps_boost_freq_mhz", "%u", max);
>  }
>  
> -static void unpin_rps(int sysfs)
> +static void unpin_rps(int sysfs_gt)
>  {
>  	unsigned int v;
>  
> -	if (igt_sysfs_scanf(sysfs, "gt_RPn_freq_mhz", "%u", &v) == 1)
> -		igt_sysfs_printf(sysfs, "gt_min_freq_mhz", "%u", v);
> +	if (igt_sysfs_scanf(sysfs_gt, "rps_RPn_freq_mhz", "%u", &v) == 1)
> +		igt_sysfs_printf(sysfs_gt, "rps_min_freq_mhz", "%u", v);
>  
> -	if (igt_sysfs_scanf(sysfs, "gt_RP0_freq_mhz", "%u", &v) == 1) {
> -		igt_sysfs_printf(sysfs, "gt_max_freq_mhz", "%u", v);
> -		igt_sysfs_printf(sysfs, "gt_boost_freq_mhz", "%u", v);
> +	if (igt_sysfs_scanf(sysfs_gt, "rps_RP0_freq_mhz", "%u", &v) == 1) {
> +		igt_sysfs_printf(sysfs_gt, "rps_max_freq_mhz", "%u", v);
> +		igt_sysfs_printf(sysfs_gt, "rps_boost_freq_mhz", "%u", v);
>  	}
>  }
>  
> @@ -355,7 +355,7 @@ int igt_main()
>  
>  	igt_subtest_group() {
>  		struct intel_mmio_data mmio;
> -		int sysfs;
> +		int sysfs_gt, gt;
>  
>  		igt_fixture() {
>  			igt_require(gem_scheduler_enabled(i915));
> @@ -365,16 +365,17 @@ int igt_main()
>  						   igt_device_get_pci_device(i915),
>  						   false);
>  
> -			sysfs = igt_sysfs_open(i915);
> -			pin_rps(sysfs);
> +			i915_for_each_gt(i915, sysfs_gt, gt)
> +				pin_rps(sysfs_gt);
>  		}
>  
>  		test_each_engine("dispatch", i915, e)
>  				endless_dispatch(i915, e);
>  
>  		igt_fixture() {
> -			unpin_rps(sysfs);
> -			close(sysfs);
> +			i915_for_each_gt(i915, sysfs_gt, gt)
> +				unpin_rps(sysfs_gt);
> +
>  			intel_register_access_fini(&mmio);
>  		}
>  	}