[igt-dev] [PATCH i-g-t] xe/xe_huc_copy: use IS_TIGERLAKE macro

[Lucas De Marchi]

On Tue, Apr 04, 2023 at 09:52:00AM +0200, Mauro Carvalho Chehab wrote:
>On Mon, 3 Apr 2023 23:27:01 -0700
>Lucas De Marchi <lucas.demarchi at intel.com> wrote:
>
>> On Tue, Mar 28, 2023 at 10:39:08AM +0200, Mauro Carvalho Chehab wrote:
>> >From: Mauro Carvalho Chehab <mchehab at kernel.org>
>> >
>> >Instead of hardcoding the PCI IDs at the test, use a macro
>> >to check if the platform is compatible with the test.
>> >
>> >Signed-off-by: Mauro Carvalho Chehab <mchehab at kernel.org>
>>
>> neither this or the previous solution scale well. Can we get this info
>> from huc_info in debugfs?
>>
>> We are already loading HuC in platforms other than TGL
>
>Changing IGT to not use IS_platform macros anymore is not an easy task,
>as this is used on lots of place, for both i915 and Xe drivers.

I'm not talking generically everywhere in igt. I'm talking specifically
about huc. For i915 there is a query, for xe we don't have it. But the
same info can be obtained from the huc_info file in debugfs.

>If debugfs can be used for it on both drivers, I guess such macros
>could be changed to use huc_info from debugfs. Still, not sure if
>this is worth, specially since I don't think HUC is mandatory at

huc is not mandatory on any platform, but if we want to test
xe-huc-copy we better check we have huc rather than checking for
platform. Otherwise we will be forever out of sync with kernel.
It is already for the short lifespan of this test and will continue
to be as we add support for huc to more platforms. We support it
on ADL-S already, ADL-P is probably ok to enable and was just hold back
since it was increasing the error rate of unrelated things.
We will soon have RKL. Then there are the newer platforms.

>the i915 driver, and such macros are meant to work with both
>drivers.

i915 already uses something else. This is a xe-specific test that moved
to filter by pciid because we don't have the query... It should rather
moved to debugfs directly.

+Anusha / +Matt Atwood. Maybe they already have something in the works
for igt.

Lucas De Marchi

>
>Regards,
>Mauro
>
>>
>> Lucas De Marchi
>>
>> >---
>> > tests/xe/xe_huc_copy.c | 38 +-------------------------------------
>> > 1 file changed, 1 insertion(+), 37 deletions(-)
>> >
>> >diff --git a/tests/xe/xe_huc_copy.c b/tests/xe/xe_huc_copy.c
>> >index ee3896cef8b9..cd68dbb5ac50 100644
>> >--- a/tests/xe/xe_huc_copy.c
>> >+++ b/tests/xe/xe_huc_copy.c
>> >@@ -152,42 +152,6 @@ test_huc_copy(int fd)
>> > 	xe_vm_destroy(fd, vm);
>> > }
>> >
>> >-static bool
>> >-is_device_supported(int fd)
>> >-{
>> >-	struct drm_xe_query_config *config;
>> >-	struct drm_xe_device_query query = {
>> >-		.extensions = 0,
>> >-		.query = DRM_XE_DEVICE_QUERY_CONFIG,
>> >-		.size = 0,
>> >-		.data = 0,
>> >-	};
>> >-	uint16_t devid;
>> >-
>> >-	igt_assert_eq(igt_ioctl(fd, DRM_IOCTL_XE_DEVICE_QUERY, &query), 0);
>> >-
>> >-	config = malloc(query.size);
>> >-	igt_assert(config);
>> >-
>> >-	query.data = to_user_pointer(config);
>> >-	igt_assert_eq(igt_ioctl(fd, DRM_IOCTL_XE_DEVICE_QUERY, &query), 0);
>> >-
>> >-	devid = config->info[XE_QUERY_CONFIG_REV_AND_DEVICE_ID] & 0xffff;
>> >-	return (
>> >-			devid == 0x9A60 ||
>> >-			devid == 0x9A68 ||
>> >-			devid == 0x9A70 ||
>> >-			devid == 0x9A40 ||
>> >-			devid == 0x9A49 ||
>> >-			devid == 0x9A59 ||
>> >-			devid == 0x9A78 ||
>> >-			devid == 0x9AC0 ||
>> >-			devid == 0x9AC9 ||
>> >-			devid == 0x9AD9 ||
>> >-			devid == 0x9AF8
>> >-		);
>> >-}
>> >-
>> > igt_main
>> > {
>> > 	int xe;
>> >@@ -198,7 +162,7 @@ igt_main
>> > 	}
>> >
>> > 	igt_subtest("huc_copy") {
>> >-		igt_skip_on(!is_device_supported(xe));
>> >+		igt_skip_on(!IS_TIGERLAKE(intel_get_drm_devid(xe)));
>> > 		test_huc_copy(xe);
>> > 	}
>> >
>> >--
>> >2.39.2
>> >