Re: [igt-dev] [PATCH i-g-t 1/4] tests/gem_ctx_freq: Add support for multi-gts

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

On Mon, 05 Jun 2023 18:21:47 -0700, Ashutosh Dixit wrote:
>
> From: Vinay Belgaumkar <vinay.belgaumkar@intel.com>
>
> Run subtest sysfs-range on available sysfs interfaces.
>
> Signed-off-by: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com>
> Signed-off-by: Vinay Belgaumkar <vinay.belgaumkar@intel.com>
> ---
>  tests/i915/gem_ctx_freq.c | 99 ++++++++++++++++++++++++++-------------
>  1 file changed, 66 insertions(+), 33 deletions(-)
>
> diff --git a/tests/i915/gem_ctx_freq.c b/tests/i915/gem_ctx_freq.c
> index 2c30934daa23..b2f6c2af55e1 100644
> --- a/tests/i915/gem_ctx_freq.c
> +++ b/tests/i915/gem_ctx_freq.c
> @@ -24,12 +24,15 @@
>
>  #include <errno.h>
>  #include <fcntl.h>
> +#include <limits.h>
>  #include <sched.h>
>  #include <stdlib.h>
>  #include <stdint.h>
>  #include <unistd.h>
>
>  #include "i915/gem.h"
> +#include "i915/gem_engine_topology.h"
> +#include "i915_drm.h"
>  #include "igt.h"
>  #include "igt_perf.h"
>  #include "igt_sysfs.h"
> @@ -49,6 +52,7 @@
>  #define SAMPLE_PERIOD (USEC_PER_SEC / 10)
>  #define PMU_TOLERANCE 100
>
> +static int i915 = -1;
>  static int sysfs = -1;
>
>  static void kick_rps_worker(void)
> @@ -90,7 +94,7 @@ static void pmu_assert(double actual, double target)
>		     actual, target, PMU_TOLERANCE);
>  }
>
> -static void busy_wait_until_idle(int i915, igt_spin_t *spin)
> +static void busy_wait_until_idle(igt_spin_t *spin)
>  {
>	igt_spin_end(spin);
>	do {
> @@ -98,9 +102,9 @@ static void busy_wait_until_idle(int i915, igt_spin_t *spin)
>	} while (gem_bo_busy(i915, spin->handle));
>  }
>
> -static void __igt_spin_free_idle(int i915, igt_spin_t *spin)
> +static void __igt_spin_free_idle(igt_spin_t *spin)
>  {
> -	busy_wait_until_idle(i915, spin);
> +	busy_wait_until_idle(spin);
>
>	igt_spin_free(i915, spin);
>  }
> @@ -115,23 +119,25 @@ static void triangle_fill(uint32_t *t, unsigned int nstep,
>	}
>  }
>
> -static void set_sysfs_freq(uint32_t min, uint32_t max)
> +static void set_sysfs_freq(int dirfd, uint32_t min, uint32_t max)
>  {
> -	igt_sysfs_printf(sysfs, "gt_min_freq_mhz", "%u", min);
> -	igt_sysfs_printf(sysfs, "gt_max_freq_mhz", "%u", max);
> +	igt_sysfs_rps_printf(dirfd, RPS_MIN_FREQ_MHZ, "%u", min);
> +	igt_sysfs_rps_printf(dirfd, RPS_MAX_FREQ_MHZ, "%u", max);
>  }
>
> -static bool get_sysfs_freq(uint32_t *min, uint32_t *max)
> +static bool get_sysfs_freq(int dirfd, uint32_t *min, uint32_t *max)
>  {
> -	return (igt_sysfs_scanf(sysfs, "gt_min_freq_mhz", "%u", min) == 1 &&
> -		igt_sysfs_scanf(sysfs, "gt_max_freq_mhz", "%u", max) == 1);
> +	return (igt_sysfs_rps_scanf(dirfd, RPS_MIN_FREQ_MHZ, "%u", min) == 1 &&
> +		igt_sysfs_rps_scanf(dirfd, RPS_MAX_FREQ_MHZ, "%u", max) == 1);
>  }
>
> -static void sysfs_range(int i915)
> +static void sysfs_range(int dirfd, int gt)
>  {
>  #define N_STEPS 10
>	uint32_t frequencies[TRIANGLE_SIZE(N_STEPS)];
> -	uint32_t sys_min, sys_max;
> +	struct i915_engine_class_instance *ci;
> +	uint32_t sys_min, sys_max, ctx;
> +	unsigned int count;
>	igt_spin_t *spin;
>	double measured;
>	int pmu;
> @@ -144,12 +150,19 @@ static void sysfs_range(int i915)
>	 * constriained sysfs range.
>	 */
>
> -	igt_require(get_sysfs_freq(&sys_min, &sys_max));
> +	igt_require(get_sysfs_freq(dirfd, &sys_min, &sys_max));
>	igt_info("System min freq: %dMHz; max freq: %dMHz\n", sys_min, sys_max);
>
>	triangle_fill(frequencies, N_STEPS, sys_min, sys_max);
>
> -	pmu = perf_i915_open(i915, I915_PMU_REQUESTED_FREQUENCY);
> +	ci = gem_list_engines(i915, 1 << gt, ~0U, &count);
> +	igt_require(ci);
> +	ctx = gem_context_create_for_engine(i915,
> +					    ci[0].engine_class,
> +					    ci[0].engine_instance);
> +	free(ci);
> +
> +	pmu = perf_i915_open(i915, __I915_PMU_REQUESTED_FREQUENCY(gt));
>	igt_require(pmu >= 0);
>
>	for (int outer = 0; outer <= 2*N_STEPS; outer++) {
> @@ -157,17 +170,17 @@ static void sysfs_range(int i915)
>		uint32_t cur, discard;
>
>		gem_quiescent_gpu(i915);
> -		spin = igt_spin_new(i915, .ahnd = ahnd);
> +		spin = igt_spin_new(i915, .ahnd = ahnd, .ctx_id = ctx);

Maybe we should add an explicit '.engine = 0' here (to make clear the spin
is being scheduled on "ci[0]" above, but that is probably clear
already. Other LGTM so this is:

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