Re: [igt-dev] [PATCH i-g-t v2] tests/i915: Add simple test for HuC

Chris Wilson
2019-05-17T20:45:27Z

Quoting Michal Wajdeczko (2019-05-17 21:21:54)
> Add simple test to check that HuC firmware is available.
> Use existing I915_GETPARAM and debugfs entry.
>
> v2: make it even simpler
>
> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Cc: Martin Peres <martin.peres@linux.intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Tony Ye <tony.ye@intel.com>
> ---
> tests/Makefile.sources | 3 +++
> tests/i915/i915_huc.c | 59 ++++++++++++++++++++++++++++++++++++++++++
> tests/meson.build | 1 +
> 3 files changed, 63 insertions(+)
> create mode 100644 tests/i915/i915_huc.c
>
> diff --git a/tests/Makefile.sources b/tests/Makefile.sources
> index 7f921f6c..dfa3fcd3 100644
> --- a/tests/Makefile.sources
> +++ b/tests/Makefile.sources
> @@ -475,6 +475,9 @@ i915_getparams_basic_SOURCES = i915/i915_getparams_basic.c
> TESTS_progs += i915_hangman
> i915_hangman_SOURCES = i915/i915_hangman.c
>
> +TESTS_progs += i915_huc
> +i915_huc_SOURCES = i915/i915_huc.c
> +
> TESTS_progs += i915_module_load
> i915_module_load_SOURCES = i915/i915_module_load.c
>
> diff --git a/tests/i915/i915_huc.c b/tests/i915/i915_huc.c
> new file mode 100644
> index 00000000..e4e6c532
> --- /dev/null
> +++ b/tests/i915/i915_huc.c
> @@ -0,0 +1,59 @@
> +/* SPDX-License-Identifier: MIT */
> +/*
> + * Copyright (c) 2019 Intel Corporation
> + */
> +
> +#include "igt.h"
> +#include <fcntl.h>
> +#include <i915_drm.h>
> +#include <sys/ioctl.h>
> +
> +IGT_TEST_DESCRIPTION("Check that HuC firmware is available.");
> +
> +static int huc_status(int fd)
> +{
> + int loaded = 0;
> + drm_i915_getparam_t gp = {
> + .param = I915_PARAM_HUC_STATUS,
> + .value = &loaded,
> + };
> +
> + if (ioctl(fd, DRM_IOCTL_I915_GETPARAM, &gp))
> + return -errno;

(mutters about errno upsetting igt_assert and making a mess)

> +
> + return loaded;
> +}
> +
> +igt_main
> +{
> + int fd;
> +
> + igt_fixture {
> + fd = drm_open_driver(DRIVER_INTEL);
> + igt_require_intel(fd);
> + igt_require(has_huc(fd));

This is now redundant...

> + igt_debugfs_dump(fd, "i915_huc_load_status");
> + igt_require(huc_status(fd) > 0);

As we get the debugfs and HUC_STATUS.

> + }
> +
> + igt_subtest("basic")
> + igt_success();
> +
> + igt_fixture {
> + close(fd);

Worksforme as a starting point,
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
(preferably without the extra igt_require(has_huc))

I trust that anyone who contemplates demanding that HuC be enabled by
default follows up with an actual test to prove the HuC is functional
and we don't accidentally break it.
-Chris