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

[Chris Wilson]

Quoting Michal Wajdeczko (2019-05-19 21:16:34)
> Add simple test to check that HuC firmware is available.
> Use existing I915_GETPARAM and debugfs entry.
> 
> v2: make it even simpler
> v3: dump with IGT_LOG_INFO, decode status
> 
> Signed-off-by: Michal Wajdeczko <michal.wajdeczko at intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen at linux.intel.com>
> Cc: Martin Peres <martin.peres at linux.intel.com>
> Cc: Chris Wilson <chris at chris-wilson.co.uk>
> Cc: Tony Ye <tony.ye at intel.com>
> Reviewed-by: Chris Wilson <chris at chris-wilson.co.uk> #v2
> ---
>  tests/Makefile.sources |  3 +++
>  tests/i915/i915_huc.c  | 53 ++++++++++++++++++++++++++++++++++++++++++
>  tests/meson.build      |  1 +
>  3 files changed, 57 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..2b4bc0d9
> --- /dev/null
> +++ b/tests/i915/i915_huc.c
> @@ -0,0 +1,53 @@
> +/* 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("Test HuC firmware");
> +
> +static int get_huc_status(int fd)
> +{
> +       int status = 0;
> +       drm_i915_getparam_t gp = {
> +               .param = I915_PARAM_HUC_STATUS,
> +               .value = &status,
> +       };
> +
> +       if (igt_ioctl(fd, DRM_IOCTL_I915_GETPARAM, &gp))
> +               return -errno;
> +
> +       errno = 0;
> +       return status;
> +}
> +
> +igt_main
> +{
> +       int fd;
> +
> +       igt_fixture {
> +               int status;
> +
> +               fd = drm_open_driver(DRIVER_INTEL);
> +               igt_require_intel(fd);
> +               __igt_debugfs_dump(fd, "i915_huc_load_status", IGT_LOG_INFO);
> +
> +               status = get_huc_status(fd);
> +               igt_skip_on_f(status == -ENODEV,
> +                             "HuC is not available on this platform!\n");
> +               igt_skip_on_f(status < 0, "HuC firmware error: %i, %s\n",
> +                             -status, strerror(-status));
> +               igt_skip_on_f(status == 0, "HuC firmware is not loaded!\n");

Yup, that I think is for the best; intelligible error messages even I
can digest.

(I was thinking a switch, e.g.
static bool has_addfb2_iface(int fd)
{
        struct local_drm_mode_fb_cmd2 f = {};
        int err;

        err = 0;
        if (drmIoctl(fd, DRM_IOCTL_MODE_ADDFB2, &f))
                err = -errno;
        switch (err) {
        case -ENOTTY: /* ioctl unrecognised (kernel too old) */
        case -ENOTSUP: /* driver doesn't support KMS */
                return false;

                /*
                 * The only other valid response is -EINVAL, but we leave
                 * that for the actual tests themselves to discover for
                 * more accurate reporting.
                 */
        default:
                return true;
        }
}
igt_require(has_adddb2_iface(fd))

But with sensible error messages, you win ;)

Just need to hold off until we agree on the best error codes.
-Chris