[PATCH i-g-t, v3 2/2] drm-uapi/xe: Add test to query HuC firmware version
John Harrison
john.c.harrison at intel.com
Tue Feb 20 18:53:37 UTC 2024
On 2/19/2024 06:35, Francois Dugast wrote:
> This aligns with kernel commit ("drm/xe: Extend uAPI to query HuC
> micro-controler firmware version").
>
> v2:
> - Fix printing branch (Francois Dugast)
> - Make ENODEV the only accepted error (José Roberto de Souza)
>
> v3:
> - Share code between guc and huc tests (John Harrison)
>
> Cc: John Harrison <John.C.Harrison at Intel.com>
> Cc: José Roberto de Souza <jose.souza at intel.com>
> Cc: Lucas De Marchi <lucas.demarchi at intel.com>
> Signed-off-by: Francois Dugast <francois.dugast at intel.com>
> Reviewed-by: José Roberto de Souza <jose.souza at intel.com>
> ---
> include/drm-uapi/xe_drm.h | 1 +
> tests/intel/xe_query.c | 89 ++++++++++++++++++++++++++++++---------
> 2 files changed, 71 insertions(+), 19 deletions(-)
>
> diff --git a/include/drm-uapi/xe_drm.h b/include/drm-uapi/xe_drm.h
> index 3bd795f27..ce4acf9d4 100644
> --- a/include/drm-uapi/xe_drm.h
> +++ b/include/drm-uapi/xe_drm.h
> @@ -583,6 +583,7 @@ struct drm_xe_query_engine_cycles {
> struct drm_xe_query_uc_fw_version {
> /** @uc: The micro-controller type to query firmware version */
> #define XE_QUERY_UC_TYPE_GUC_SUBMISSION 0
> +#define XE_QUERY_UC_TYPE_HUC 1
> __u16 uc_type;
>
> /** @pad: MBZ */
> diff --git a/tests/intel/xe_query.c b/tests/intel/xe_query.c
> index bbe5955eb..b21c3820a 100644
> --- a/tests/intel/xe_query.c
> +++ b/tests/intel/xe_query.c
> @@ -730,18 +730,8 @@ static void test_engine_cycles_invalid(int fd)
> query_engine_cycles(fd, &ts);
> }
>
> -/**
> - * SUBTEST: query-uc-fw-version-guc
> - * Test category: functionality test
> - * Description: Display the GuC firmware submission version
> - *
> - * SUBTEST: multigpu-query-uc-fw-version-guc
> - * Test category: functionality test
> - * Sub-category: MultiGPU
> - * Description: Display GuC firmware submission version for all Xe devices.
> - */
> static void
> -test_query_uc_fw_version_guc(int fd)
> +test_query_uc_fw_version(int fd, uint32_t uc_type)
> {
> struct drm_xe_query_uc_fw_version *uc_fw_version;
> struct drm_xe_device_query query = {
> @@ -750,6 +740,7 @@ test_query_uc_fw_version_guc(int fd)
> .size = 0,
> .data = 0,
> };
> + int ret;
>
> igt_assert_eq(igt_ioctl(fd, DRM_IOCTL_XE_DEVICE_QUERY, &query), 0);
>
> @@ -757,20 +748,63 @@ test_query_uc_fw_version_guc(int fd)
> igt_assert(uc_fw_version);
>
> memset(uc_fw_version, 0, sizeof(*uc_fw_version));
> - uc_fw_version->uc_type = XE_QUERY_UC_TYPE_GUC_SUBMISSION;
> + uc_fw_version->uc_type = uc_type;
> query.data = to_user_pointer(uc_fw_version);
>
> - igt_assert_eq(igt_ioctl(fd, DRM_IOCTL_XE_DEVICE_QUERY, &query), 0);
> - igt_info("XE_QUERY_UC_TYPE_GUC_SUBMISSION %u.%u.%u.%u\n",
> - uc_fw_version->branch_ver,
> - uc_fw_version->major_ver,
> - uc_fw_version->minor_ver,
> - uc_fw_version->patch_ver);
> - igt_assert(uc_fw_version->major_ver > 0 || uc_fw_version->minor_ver > 0);
> + ret = igt_ioctl(fd, DRM_IOCTL_XE_DEVICE_QUERY, &query);
> +
> + switch (uc_type) {
> + case XE_QUERY_UC_TYPE_GUC_SUBMISSION:
> + igt_assert(ret == 0);
igt_assert_eq(ret, 0) otherwise it won't tell you what the bad value was.
> + igt_assert(uc_fw_version->major_ver > 0 ||
> + uc_fw_version->minor_ver > 0);
> + igt_info("XE_QUERY_UC_TYPE_GUC_SUBMISSION %u.%u.%u.%u\n",
> + uc_fw_version->branch_ver,
> + uc_fw_version->major_ver,
> + uc_fw_version->minor_ver,
> + uc_fw_version->patch_ver);
Do the print before the assert? That way it is more clear as to what
went wrong in the case of an error.
> + break;
> + case XE_QUERY_UC_TYPE_HUC:
> + if (ret == 0) {
> + igt_assert(uc_fw_version->major_ver > 0);
> + igt_info("XE_QUERY_UC_TYPE_HUC %u.%u.%u.%u\n",
> + uc_fw_version->branch_ver,
> + uc_fw_version->major_ver,
> + uc_fw_version->minor_ver,
> + uc_fw_version->patch_ver);
As above, print before assert?
> + } else if (errno == ENODEV) {
> + /*
> + * No HuC was found, either because it is not running
> + * yet or there is no media IP.
> + */
> + igt_info("XE_QUERY_UC_TYPE_HUC No HuC is running\n");
> + } else {
> + igt_assert(false);
Assert(false) gives no useful output to understand why the test failed.
Maybe have 'igt_assert_eq(errno, ENODEV)' inside a single else clause
that then prints the 'no HuC' message? That way it will print the actual
errno value in the case of failure.
> + }
> + break;
> + default:
> + igt_assert(false);
> + }
>
> free(uc_fw_version);
> }
>
> +/**
> + * SUBTEST: query-uc-fw-version-guc
> + * Test category: functionality test
> + * Description: Display the GuC firmware submission version
> + *
> + * SUBTEST: multigpu-query-uc-fw-version-guc
> + * Test category: functionality test
> + * Sub-category: MultiGPU
> + * Description: Display GuC firmware submission version for all Xe devices.
> + */
> +static void
> +test_query_uc_fw_version_guc(int fd)
> +{
> + test_query_uc_fw_version(fd, XE_QUERY_UC_TYPE_GUC_SUBMISSION);
> +}
> +
> /**
> * SUBTEST: query-invalid-uc-fw-version-mbz
> * Test category: functionality test
> @@ -820,6 +854,22 @@ test_query_uc_fw_version_invalid_mbz(int fd)
> free(uc_fw_version);
> }
>
> +/**
> + * SUBTEST: query-uc-fw-version-huc
> + * Test category: functionality test
> + * Description: Display the HuC firmware version
> + *
> + * SUBTEST: multigpu-query-uc-fw-version-huc
> + * Test category: functionality test
> + * Sub-category: MultiGPU
> + * Description: Display HuC firmware version for all Xe devices.
> + */
> +static void
> +test_query_uc_fw_version_huc(int fd)
> +{
> + test_query_uc_fw_version(fd, XE_QUERY_UC_TYPE_HUC);
> +}
> +
> igt_main
> {
> const struct {
> @@ -834,6 +884,7 @@ igt_main
> { "query-topology", test_query_gt_topology },
> { "query-cs-cycles", test_query_engine_cycles },
> { "query-uc-fw-version-guc", test_query_uc_fw_version_guc },
> + { "query-uc-fw-version-huc", test_query_uc_fw_version_huc },
> { "query-invalid-cs-cycles", test_engine_cycles_invalid },
> { "query-invalid-query", test_query_invalid_query },
> { "query-invalid-size", test_query_invalid_size },
More information about the igt-dev
mailing list