[PATCH i-g-t v3] Do not require QUERY_OA_UNITS for all Xe tests

[Kamil Konieczny]

Hi Peter,
On 2024-07-29 at 07:25:39 +0200, Peter Senna Tschudin wrote:

in v3 you again dropped prefix from subject. Second nit is about
description, rather then describing word-for-word your C code
changes, describe why and what you do. Some hints on how to write
good description you can find in CONTRIBUTE.md

imho you could start that on current Fedora 40 with kernel
command line (cite here so one can reproduce it) all current Xe
tests are failing, and to enable almost all of them you changed
how xe lib is handling missing XE...OA and also added a skip to
xe_oa test in such scenario.

What you changed is already in a patch itself (in diff --git).

Regards,
Kamil

> Changes xe_query_oa_units_new() to return NULL instead of aborting
> due to assertion failure when querying for DRM_XE_DEVICE_QUERY_OA_UNITS.
> 
> It adds a skip to xe_oa.c to make sure it will not try to run the tests if the
> query has failed before.
> 
> The motivation is as follows:
>  - The only test that requires oa_units is tests/intel/xe_oa.c, however current
>    code will prevent all Xe tests from running if OA_UNITS is not supported
>  - Before this patch, trying to run IGT on the Xe module from Fedora 40
>    (6.9.9-200.fc40.x86_64) fails
>  - After this patch, we can run some tests, such as xe_exec_basic
>    successfully
> 
> Here is an example of the impact of this patch:
>  1. Use Fedora 40 with the following kernel command line arguments:
>     "xe.force_probe=* i915.force_probe=!* modprobe.blacklist=i915,ast,snd_hda_intel,xe"
> 
>  2. Running IGT without this patch will fail:
>      $ sudo ./build/tests/xe_exec_basic
>      $ echo $?
>      98
> 
>  3. Running after fix applied will succeed:
>     sudo ./build/tests/xe_exec_basic
>      $ sudo ./build/tests/xe_exec_basic
>      $ echo $?
>      0
> 
> V3:
>  - Update comment syntax and shorten comments
>  - Update comment content to describe from kernel version perspective only
>  - Update test to make '!= 0' implicit
> 
> V2:
>  - Cleanup headers from message body
>  - Add 'lib/xe/xe_query:' to subject line
> 
> Signed-off-by: Peter Senna Tschudin <peter.senna at intel.com>
> ---
>  lib/xe/xe_query.c   | 4 +++-
>  tests/intel/xe_oa.c | 6 +++++-
>  2 files changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/lib/xe/xe_query.c b/lib/xe/xe_query.c
> index 84eaaac96..8694fa3f9 100644
> --- a/lib/xe/xe_query.c
> +++ b/lib/xe/xe_query.c
> @@ -124,7 +124,9 @@ static struct drm_xe_query_oa_units *xe_query_oa_units_new(int fd)
>  		.data = 0,
>  	};
>  
> -	igt_assert_eq(igt_ioctl(fd, DRM_IOCTL_XE_DEVICE_QUERY, &query), 0);
> +	/* Support older kernels where this uapi is not yet available */
> +	if (igt_ioctl(fd, DRM_IOCTL_XE_DEVICE_QUERY, &query))
> +		return NULL;
>  
>  	oa_units = malloc(query.size);
>  	igt_assert(oa_units);
> diff --git a/tests/intel/xe_oa.c b/tests/intel/xe_oa.c
> index ff2218300..1f82dce06 100644
> --- a/tests/intel/xe_oa.c
> +++ b/tests/intel/xe_oa.c
> @@ -4503,6 +4503,7 @@ static const char *xe_engine_class_name(uint32_t engine_class)
>  igt_main
>  {
>  	struct drm_xe_engine_class_instance *hwe = NULL;
> +	struct xe_device *xe_dev;
>  
>  	igt_fixture {
>  		struct stat sb;
> @@ -4527,7 +4528,10 @@ igt_main
>  		igt_assert_eq(drm_fd, -1);
>  
>  		drm_fd = drm_open_driver(DRIVER_XE);
> -		xe_device_get(drm_fd);
> +		xe_dev = xe_device_get(drm_fd);
> +
> +		/* See xe_query_oa_units_new() */
> +		igt_require(xe_dev->oa_units);
>  
>  		devid = intel_get_drm_devid(drm_fd);
>  		sysfs = igt_sysfs_open(drm_fd);
> -- 
> 2.34.1
>