Re: [PATCH i-g-t v4 3/3] tests/intel/xe_pmu: Refine engine activity accuracy test

[Riana Tauro <riana.tauro@intel.com>]

On 1/6/2026 10:18 PM, Soham Purkait wrote:
> Creating a new spinner during each pass was introducing unnecessary
> overhead, which could distort timing-sensitive measurements
> and increase error rates. The cumulative cost of repeated spinner
> initialization was impacting test accuracy and efficiency.
> 
> To address this, a single spinner instance is now reused and properly
> ended and reset between iterations. This approach reduces the overhead
> associated with repeated spinner creation, saving time across multiple
> passes by eliminating the cumulative time spent on spinner
> initialization (creation time × number of passes). As the spinner's active
> duration more accurately reflects actual engine busyness, this leads to
> more precise measurements of the percentage of engine busyness in terms
> of spinner running time.
> 
> With this change, the engine-activity-accuracy test results clearly show
> an improvement in error reduction ranging from 98.97% to 99.53%.
> 
> v1:
>   - Add how this patch improves the engine activity accuracy test. (Lucas)
>   - Show the numbers with the said improvement. (Lucas)
> 
> v2:
>   - Add an overview of the issue. (Riana)
>   - Add the links in Closes. (Riana)
>   - Add improvement in percentage. (Kamil)
> 
> v3:
>   - Add spin_sync_wait() with excess time adjusted
>     accordingly. (Riana)
> 
> Fixes: 477154cbad2c ("tests/intel/xe_pmu: Add tests to validate engine activity accuracy")
> Closes: https://gitlab.freedesktop.org/drm/xe/kernel/-/issues/6251
> Signed-off-by: Soham Purkait <soham.purkait@intel.com>


Reviewed-by: Riana Tauro <riana.tauro@intel.com>

> ---
>   tests/intel/xe_pmu.c | 17 +++++++++++++----
>   1 file changed, 13 insertions(+), 4 deletions(-)
> 
> diff --git a/tests/intel/xe_pmu.c b/tests/intel/xe_pmu.c
> index c8b3cebf0..33852270a 100644
> --- a/tests/intel/xe_pmu.c
> +++ b/tests/intel/xe_pmu.c
> @@ -534,6 +534,10 @@ static void accuracy(int fd, struct drm_xe_engine_class_instance *eci,
>   		intel_allocator_init();
>   		ahnd = intel_allocator_open(fd, 0, INTEL_ALLOCATOR_RELOC);
>   
> +		spin = igt_spin_new(fd, .ahnd = ahnd, .vm = vm, .hwe = eci);
> +		xe_spin_end(spin->xe_spin);
> +		xe_spin_sync_wait(fd, spin);
> +
>   		for (int pass = 0; pass < ARRAY_SIZE(timeout); pass++) {
>   			unsigned int target_idle_us = idle_us;
>   			struct timespec start = { };
> @@ -546,21 +550,25 @@ static void accuracy(int fd, struct drm_xe_engine_class_instance *eci,
>   
>   			while (pass_ns < timeout[pass]) {
>   				unsigned long loop_ns, loop_active_ns, loop_idle_ns, now;
> +				unsigned long after_sync;
>   				double err, prev_avg, cur_val;
>   
>   				/* idle sleep */
>   				igt_measured_usleep(target_idle_us);
>   
>   				/* start spinner */
> -				spin = igt_spin_new(fd, .ahnd = ahnd, .vm = vm, .hwe = eci);
> +				xe_spin_reset(fd, spin);
>   				loop_idle_ns = igt_nsec_elapsed(&start);
>   				igt_measured_usleep(active_us);
> -				igt_spin_free(fd, spin);
> +				xe_spin_end(spin->xe_spin);
>   
>   				now = igt_nsec_elapsed(&start);
> +				xe_spin_sync_wait(fd, spin);
> +				after_sync = igt_nsec_elapsed(&start);
> +
>   				loop_active_ns = now - loop_idle_ns;
>   				loop_ns = now - pass_ns;
> -				pass_ns = now;
> +				pass_ns = after_sync;
>   
>   				pass_active_ns += loop_active_ns;
>   				total_active_ns += loop_active_ns;
> @@ -590,6 +598,7 @@ static void accuracy(int fd, struct drm_xe_engine_class_instance *eci,
>   				      sizeof(expected));
>   		}
>   
> +		igt_spin_free(fd, spin);
>   		xe_vm_destroy(fd, vm);
>   		put_ahnd(ahnd);
>   	}
> @@ -622,7 +631,7 @@ static void accuracy(int fd, struct drm_xe_engine_class_instance *eci,
>   	igt_info("error=%.2f%% (%.2f%% vs %.2f%%)\n",
>   		 (engine_activity - expected) * 100, 100 * engine_activity, 100 * expected);
>   
> -	assert_within(100.0 * engine_activity, 100.0 * expected, 3);
> +	assert_within(100.0 * engine_activity, 100.0 * expected, 2);
>   }
>   
>   static void engine_activity_all_fn(int fd, struct drm_xe_engine_class_instance *eci, int num_fns)