Re: [PATCH i-g-t 1/1] lib/xe/xe_query: Add EU stall info to xe_device

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

On Tue, 15 Jul 2025 23:25:58 -0700, Harish Chegondi wrote:
>
> Add EU stall data information obtained through the EU stall
> query IOCTL to xe_device.
>
> Cc: Ashutosh Dixit <ashutosh.dixit@intel.com>
> Signed-off-by: Harish Chegondi <harish.chegondi@intel.com>

LGTM,

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


> ---
>  lib/xe/xe_query.c         | 28 ++++++++++++++++++++++++++++
>  lib/xe/xe_query.h         |  3 +++
>  tests/intel/xe_eu_stall.c | 26 ++++++--------------------
>  3 files changed, 37 insertions(+), 20 deletions(-)
>
> diff --git a/lib/xe/xe_query.c b/lib/xe/xe_query.c
> index c0dba0182..3b8a682f8 100644
> --- a/lib/xe/xe_query.c
> +++ b/lib/xe/xe_query.c
> @@ -160,6 +160,32 @@ static struct drm_xe_query_mem_regions *xe_query_mem_regions_new(int fd)
>	return mem_regions;
>  }
>
> +static struct drm_xe_query_eu_stall *xe_query_eu_stall_new(int fd)
> +{
> +	struct drm_xe_query_eu_stall *query_eu_stall;
> +	struct drm_xe_device_query query = {
> +		.extensions = 0,
> +		.query = DRM_XE_DEVICE_QUERY_EU_STALL,
> +		.size = 0,
> +		.data = 0,
> +	};
> +
> +	/* Support older kernels where this uapi is not yet available */
> +	if (igt_ioctl(fd, DRM_IOCTL_XE_DEVICE_QUERY, &query))
> +		return NULL;
> +	igt_assert_neq(query.size, 0);
> +
> +	query_eu_stall = malloc(query.size);
> +	igt_assert(query_eu_stall);
> +
> +	query.data = to_user_pointer(query_eu_stall);
> +	igt_assert_eq(igt_ioctl(fd, DRM_IOCTL_XE_DEVICE_QUERY, &query), 0);
> +
> +	VG(VALGRIND_MAKE_MEM_DEFINED(query_eu_stall, query.size));
> +
> +	return query_eu_stall;
> +}
> +
>  static struct drm_xe_query_oa_units *xe_query_oa_units_new(int fd)
>  {
>	struct drm_xe_query_oa_units *oa_units;
> @@ -317,6 +343,7 @@ static void xe_device_free(struct xe_device *xe_dev)
>	free(xe_dev->engines);
>	free(xe_dev->mem_regions);
>	free(xe_dev->vram_size);
> +	free(xe_dev->eu_stall);
>	free(xe_dev);
>  }
>
> @@ -355,6 +382,7 @@ struct xe_device *xe_device_get(int fd)
>	xe_dev->memory_regions = __memory_regions(xe_dev->gt_list);
>	xe_dev->engines = xe_query_engines(fd);
>	xe_dev->mem_regions = xe_query_mem_regions_new(fd);
> +	xe_dev->eu_stall = xe_query_eu_stall_new(fd);
>	xe_dev->oa_units = xe_query_oa_units_new(fd);
>
>	/*
> diff --git a/lib/xe/xe_query.h b/lib/xe/xe_query.h
> index ca0b3235e..cc54ec956 100644
> --- a/lib/xe/xe_query.h
> +++ b/lib/xe/xe_query.h
> @@ -45,6 +45,9 @@ struct xe_device {
>	/** @engines: hardware engines */
>	struct drm_xe_query_engines *engines;
>
> +	/** @eu_stall: information about EU stall data */
> +	struct drm_xe_query_eu_stall *eu_stall;
> +
>	/** @mem_regions: regions memory information and usage */
>	struct drm_xe_query_mem_regions *mem_regions;
>
> diff --git a/tests/intel/xe_eu_stall.c b/tests/intel/xe_eu_stall.c
> index 88202488c..f09e525eb 100644
> --- a/tests/intel/xe_eu_stall.c
> +++ b/tests/intel/xe_eu_stall.c
> @@ -46,6 +46,7 @@
>  #include "igt_core.h"
>  #include "xe_drm.h"
>  #include "xe/xe_ioctl.h"
> +#include "xe/xe_query.h"
>
>  #define OBSERVATION_PARANOID	"/proc/sys/dev/xe/observation_paranoid"
>
> @@ -646,15 +647,10 @@ static struct option long_options[] = {
>  igt_main_args("e:g:o:r:u:w:", long_options, help_str, opt_handler, NULL)
>  {
>	bool blocking_read = true;
> -	int drm_fd, ret, idx;
> +	struct xe_device *xe_dev;
> +	int drm_fd, idx;
>	uint32_t devid;
>	struct stat sb;
> -	struct drm_xe_device_query query = {
> -		.extensions = 0,
> -		.query = DRM_XE_DEVICE_QUERY_EU_STALL,
> -		.size = 0,
> -		.data = 0,
> -	};
>
>	igt_fixture {
>		drm_fd = drm_open_driver(DRIVER_XE);
> @@ -663,20 +659,10 @@ igt_main_args("e:g:o:r:u:w:", long_options, help_str, opt_handler, NULL)
>
>		igt_require_f(igt_get_gpgpu_fillfunc(devid), "no gpgpu-fill function\n");
>		igt_require_f(!stat(OBSERVATION_PARANOID, &sb), "no observation_paranoid file\n");
> +		xe_dev = xe_device_get(drm_fd);
> +		igt_require_f(xe_dev->eu_stall, "EU stall monitoring is not available/supported\n");
>
> -		ret = igt_ioctl(drm_fd, DRM_IOCTL_XE_DEVICE_QUERY, &query);
> -		igt_skip_on_f(ret == -1 && errno == ENODEV,
> -			      "EU stall monitoring is not available on this platform\n");
> -		igt_skip_on_f(ret == -1 && errno == EINVAL,
> -			      "EU stall monitoring is not supported in the driver\n");
> -		igt_assert_neq(query.size, 0);
> -
> -		query_eu_stall_data = malloc(query.size);
> -		igt_assert(query_eu_stall_data);
> -
> -		query.data = to_user_pointer(query_eu_stall_data);
> -		igt_assert_eq(igt_ioctl(drm_fd, DRM_IOCTL_XE_DEVICE_QUERY, &query), 0);
> -
> +		query_eu_stall_data = xe_dev->eu_stall;
>		igt_assert(query_eu_stall_data->num_sampling_rates > 0);
>		/* If the user doesn't pass a sampling rate, use a mid sampling rate */
>		if (p_rate == 0) {
> --
> 2.48.1
>