[igt-dev] [PATCH i-g-t v5] tests/i915/gem_huc_copy: Enable a HuC copy test
Argenziano, Antonio
antonio.argenziano at intel.com
Fri May 8 16:31:02 UTC 2020
On 4/14/2020 2:24 PM, Robert M. Fosha wrote:
> From: Sally Qi <feng.qi at intel.com>
>
> This test case loads the HuC copy firmware to copy the content of
> the source buffer to the destination buffer.
>
> v2: (Tony Ye)
> * Restructured some functions and files.
> * Defined the copy buffer size as 4K explicitly as the HuC Copy kernel
> always copy 4K bytes from src buffer to dst buffer.
>
> v3: (Feng Qi, Antonio Argenziano, Tony Ye)
> * Restructured some functions as igt requested, exclude libdrm function call.
> * Remove huc function wrappers
> * Random initialize source input buffer
>
> v4: (Robert Fosha)
> * Fix autotools build failure.
>
> v5: (Feng Qi, Tony Ye)
> * Released all bo buffer after huc copying.
> * Restructured huc_copy() function.
>
> v6: (Feng Qi)
> * Fixed the function of huc enabling and status check
> * Added huc_copy to fast feedback testlist
>
> v7: (Tony Ye, Feng Qi, Robert Fosha, Chris Wilson, Michal Wajdeczko)
> * Check error with HUC_STATUS ioctl instead of debugfs
>
> Signed-off-by: Feng Qi <feng.qi at intel.com>
> Signed-off-by: Tony Ye <tony.ye at intel.com>
> Cc: Michal Wajdeczko <michal.wajdeczko at intel.com>
> Cc: Chris Wilson <chris at chris-wilson.co.uk>
> Acked-by: Antonio Argenziano <antonio.argenziano at intel.com>
> ---
> lib/Makefile.sources | 2 +
> lib/huc_copy.c | 107 +++++++++++++++++++++
> lib/huc_copy.h | 50 ++++++++++
> lib/intel_batchbuffer.c | 20 ++++
> lib/intel_batchbuffer.h | 20 ++++
> lib/meson.build | 1 +
> tests/Makefile.sources | 3 +
> tests/i915/gem_huc_copy.c | 130 ++++++++++++++++++++++++++
> tests/intel-ci/fast-feedback.testlist | 1 +
> tests/meson.build | 1 +
> 10 files changed, 335 insertions(+)
> create mode 100644 lib/huc_copy.c
> create mode 100644 lib/huc_copy.h
> create mode 100644 tests/i915/gem_huc_copy.c
>
> diff --git a/lib/Makefile.sources b/lib/Makefile.sources
> index 1e2c88ae..a882b0e7 100644
> --- a/lib/Makefile.sources
> +++ b/lib/Makefile.sources
> @@ -84,6 +84,8 @@ lib_source_list = \
> ioctl_wrappers.h \
> media_fill.c \
> media_fill.h \
> + huc_copy.c \
> + huc_copy.h \
> media_spin.h \
> media_spin.c \
> gpgpu_fill.h \
> diff --git a/lib/huc_copy.c b/lib/huc_copy.c
> new file mode 100644
> index 00000000..c3dece9f
> --- /dev/null
> +++ b/lib/huc_copy.c
> @@ -0,0 +1,107 @@
> +/*
> + * Copyright © 2019 Intel Corporation
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the next
> + * paragraph) shall be included in all copies or substantial portions of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
> + * IN THE SOFTWARE.
> + *
> + */
> +
> +#include <i915_drm.h>
> +#include "huc_copy.h"
> +
> +static void
> +gen9_emit_huc_virtual_addr_state(struct drm_i915_gem_exec_object2 *src,
> + struct drm_i915_gem_exec_object2 *dst,
> + struct drm_i915_gem_relocation_entry *reloc_src,
> + struct drm_i915_gem_relocation_entry *reloc_dst,
> + uint32_t *buf,
> + int *i)
> +{
> + buf[(*i)++] = HUC_VIRTUAL_ADDR_STATE;
> +
> + for (int j = 0; j < HUC_VIRTUAL_ADDR_REGION_NUM; j++) {
> + if (j == HUC_VIRTUAL_ADDR_REGION_SRC) {
> + buf[(*i)++] = src->offset;
> +
> + reloc_src->target_handle = src->handle;
> + reloc_src->delta = 0;
> + reloc_src->offset = (*i - 1) * sizeof(buf[0]);
> + reloc_src->read_domains = 0;
> + reloc_src->write_domain = 0;
> + } else if (j == HUC_VIRTUAL_ADDR_REGION_DST) {
> + buf[(*i)++] = dst->offset;
> +
> + reloc_dst->target_handle = dst->handle;
> + reloc_dst->delta = 0;
> + reloc_dst->offset = (*i - 1) * sizeof(buf[0]);
> + reloc_dst->read_domains = 0;
> + reloc_dst->write_domain = I915_GEM_DOMAIN_RENDER;
> + } else {
> + buf[(*i)++] = 0;
> + }
> + buf[(*i)++] = 0;
> + buf[(*i)++] = 0;
> + }
> +}
> +
> +void
> +gen9_huc_copyfunc(int fd,
> + struct drm_i915_gem_exec_object2 *obj,
> + int n_bo)
Not sure you really need 'n_bo', it is always assumed to be 3 no matter
what. If you really want to keep it I think you should check it == 3.
> +{
> + struct drm_i915_gem_relocation_entry reloc[2];
> + struct drm_i915_gem_execbuffer2 execbuf;
> + int i = 0;
> + uint32_t buf[63];
> +
> + /* load huc kernel */
> + buf[i++] = HUC_IMEM_STATE;
> + buf[i++] = 0;
> + buf[i++] = 0;
> + buf[i++] = 0;
> + buf[i++] = 0x3;
> +
> + buf[i++] = MFX_WAIT;
> + buf[i++] = MFX_WAIT;
> +
> + buf[i++] = HUC_PIPE_MODE_SELECT;
> + buf[i++] = 0;
> + buf[i++] = 0;
> +
> + buf[i++] = MFX_WAIT;
> +
> + memset(reloc, 0, sizeof(reloc));
> + gen9_emit_huc_virtual_addr_state(&obj[0], &obj[1], &reloc[0], &reloc[1], buf, &i);
> +
> + buf[i++] = HUC_START;
> + buf[i++] = 1;
> +
> + buf[i++] = MI_BATCH_BUFFER_END;
> +
> + gem_write(fd, obj[2].handle, 0, buf, sizeof(buf));
> + obj[2].relocation_count = 2;
> + obj[2].relocs_ptr = to_user_pointer(reloc);
> +
> + memset(&execbuf, 0, sizeof(execbuf));
> + execbuf.buffers_ptr = to_user_pointer(obj);
> + execbuf.buffer_count = n_bo;
> + execbuf.flags = I915_EXEC_BSD;
> +
> + gem_execbuf(fd, &execbuf);
> +}
> diff --git a/lib/huc_copy.h b/lib/huc_copy.h
> new file mode 100644
> index 00000000..088f7954
> --- /dev/null
> +++ b/lib/huc_copy.h
> @@ -0,0 +1,50 @@
> +/*
> + * Copyright © 2019 Intel Corporation
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the next
> + * paragraph) shall be included in all copies or substantial portions of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
> + * IN THE SOFTWARE.
> + *
> + */
> +
> +#ifndef HUC_COPY_H
> +#define HUC_COPY_H
> +
> +#include <stdint.h>
> +#include <string.h>
> +#include "ioctl_wrappers.h"
> +#include "intel_reg.h"
> +
> +#define PARALLEL_VIDEO_PIPE (0x3<<29)
> +#define MFX_WAIT (PARALLEL_VIDEO_PIPE|(0x1<<27)|(0x1<<8))
> +
> +#define HUC_IMEM_STATE (PARALLEL_VIDEO_PIPE|(0x2<<27)|(0xb<<23)|(0x1<<16)|0x3)
> +#define HUC_PIPE_MODE_SELECT (PARALLEL_VIDEO_PIPE|(0x2<<27)|(0xb<<23)|0x1)
> +#define HUC_START (PARALLEL_VIDEO_PIPE|(0x2<<27)|(0xb<<23)|(0x21<<16))
> +#define HUC_VIRTUAL_ADDR_STATE (PARALLEL_VIDEO_PIPE|(0x2<<27)|(0xb<<23)|(0x4<<16)|0x2f)
> +
> +#define HUC_VIRTUAL_ADDR_REGION_NUM 16
> +#define HUC_VIRTUAL_ADDR_REGION_SRC 0
> +#define HUC_VIRTUAL_ADDR_REGION_DST 14
> +
> +void
> +gen9_huc_copyfunc(int fd,
> + struct drm_i915_gem_exec_object2 *obj,
> + int n_bo);
> +
> +#endif /* HUC_COPY_H */
> diff --git a/lib/intel_batchbuffer.c b/lib/intel_batchbuffer.c
> index f1a45b47..93ac4546 100644
> --- a/lib/intel_batchbuffer.c
> +++ b/lib/intel_batchbuffer.c
> @@ -45,6 +45,7 @@
> #include "gpgpu_fill.h"
> #include "igt_aux.h"
> #include "i830_reg.h"
> +#include "huc_copy.h"
>
> #include <i915_drm.h>
>
> @@ -1171,3 +1172,22 @@ igt_media_spinfunc_t igt_get_media_spinfunc(int devid)
>
> return spin;
> }
> +
> +/**
> + * igt_get_huc_copyfunc:
> + * @devid: pci device id
> + *
> + * Returns:
> + *
> + * The platform-specific huc copy function pointer for the device specified
> + * with @devid. Will return NULL when no media spin function is implemented.
> + */
> +igt_huc_copyfunc_t igt_get_huc_copyfunc(int devid)
> +{
> + igt_huc_copyfunc_t copy = NULL;
> +
> + if (IS_GEN12(devid) || IS_GEN11(devid) || IS_GEN9(devid))
> + copy = gen9_huc_copyfunc;
> +
> + return copy;
> +}
> diff --git a/lib/intel_batchbuffer.h b/lib/intel_batchbuffer.h
> index 442f3a18..a3d4ec36 100644
> --- a/lib/intel_batchbuffer.h
> +++ b/lib/intel_batchbuffer.h
> @@ -422,4 +422,24 @@ typedef void (*igt_media_spinfunc_t)(struct intel_batchbuffer *batch,
>
> igt_media_spinfunc_t igt_get_media_spinfunc(int devid);
>
> +/**
> + * igt_huc_copyfunc_t:
> + * @fd: drm fd
> + * @obj: drm_i915_gem_exec_object2 buffer array
> + * obj[0] is source buffer
> + * obj[1] is destination buffer
> + * obj[2] is execution buffer
> + * @n_bo: the size of @obj array
> + *
> + * This is the type of the per-platform huc copy functions.
> + *
> + * The huc copy function emits a batchbuffer to the VDBOX engine to
> + * invoke the HuC Copy kernel to copy 4K bytes from the source buffer
> + * to the destination buffer.
> + */
> +typedef void (*igt_huc_copyfunc_t)(int fd,
> + struct drm_i915_gem_exec_object2 *obj,
> + int n_bo);
> +
> +igt_huc_copyfunc_t igt_get_huc_copyfunc(int devid);
> #endif
> diff --git a/lib/meson.build b/lib/meson.build
> index e2060430..3f8f42f4 100644
> --- a/lib/meson.build
> +++ b/lib/meson.build
> @@ -1,5 +1,6 @@
> lib_sources = [
> 'drmtest.c',
> + 'huc_copy.c',
> 'i915/gem_context.c',
> 'i915/gem_engine_topology.c',
> 'i915/gem_scheduler.c',
> diff --git a/tests/Makefile.sources b/tests/Makefile.sources
> index 4e44c98c..ec94d25b 100644
> --- a/tests/Makefile.sources
> +++ b/tests/Makefile.sources
> @@ -314,6 +314,9 @@ gem_media_fill_SOURCES = i915/gem_media_fill.c
> TESTS_progs += gem_media_vme
> gem_media_vme_SOURCES = i915/gem_media_vme.c
>
> +TESTS_progs += gem_huc_copy
> +gem_huc_copy_SOURCES = i915/gem_huc_copy.c
> +
> TESTS_progs += gem_mmap
> gem_mmap_SOURCES = i915/gem_mmap.c
>
> diff --git a/tests/i915/gem_huc_copy.c b/tests/i915/gem_huc_copy.c
> new file mode 100644
> index 00000000..06a9129b
> --- /dev/null
> +++ b/tests/i915/gem_huc_copy.c
> @@ -0,0 +1,130 @@
> +/*
> + * Copyright © 2019 Intel Corporation
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the next
> + * paragraph) shall be included in all copies or substantial portions of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
> + * IN THE SOFTWARE.
> + */
> +#include "igt.h"
> +#include <stdbool.h>
> +#include <unistd.h>
> +#include <stdlib.h>
> +#include <sys/ioctl.h>
> +#include <stdio.h>
> +#include <string.h>
> +#include <fcntl.h>
> +#include <inttypes.h>
> +#include <errno.h>
> +#include <sys/stat.h>
> +#include <sys/time.h>
> +#include "drm.h"
> +#include "igt_sysfs.c"
> +
> +IGT_TEST_DESCRIPTION("A very simple workload for the HuC.");
> +
> +#define HUC_COPY_DATA_BUF_SIZE 4096
> +
> +static void
> +compare_huc_copy_result(int drm_fd, uint32_t src_handle, uint32_t dst_handle)
> +{
> + char src_output[HUC_COPY_DATA_BUF_SIZE];
> + char dst_output[HUC_COPY_DATA_BUF_SIZE];
> +
> + gem_read(drm_fd, src_handle, 0, src_output, HUC_COPY_DATA_BUF_SIZE);
> + gem_read(drm_fd, dst_handle, 0, dst_output, HUC_COPY_DATA_BUF_SIZE);
> +
> + for (int i = 0; i < HUC_COPY_DATA_BUF_SIZE; i++)
> + igt_assert_f(src_output[i] == dst_output[i],
> + "Exepected %c, found %c at %4d.\n",
> + src_output[i], dst_output[i], i);
> +}
> +
> +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 drm_fd = -1;
> + uint32_t devid;
> + igt_huc_copyfunc_t huc_copy;
> +
> + igt_fixture {
> + int status;
> +
> + drm_fd = drm_open_driver(DRIVER_INTEL);
> + igt_require_gem(drm_fd);
> + devid = intel_get_drm_devid(drm_fd);
> + huc_copy = igt_get_huc_copyfunc(devid);
> +
> + igt_require_f(huc_copy, "no huc_copy function\n");
> + status = get_huc_status(drm_fd);
> + igt_skip_on_f(status == -ENODEV,
> + "HuC is not present on this platform!\n");
> + igt_skip_on_f(status == -EOPNOTSUPP,
> + "HuC firmware is disabled!\n");
> + igt_fail_on_f(status < 0, "HuC firmware loading error: %i, %s\n",
> + -status, strerror(-status));
> + igt_fail_on_f(status == 0, "HuC firmware is not running!\n");
Couple of things here:
1. Not convinced that failing in the fixture is ever a good idea, it
will cause a lot of failures but possibly be linked to one single failure.
2. Is the value of status an error condition? Why is that? I would
expect a HuC loading test to tell me in what conditions it expects the
firmware to be loaded and running and when it expect it not to be loaded.
Antonio
> + }
> +
> + igt_describe("Make sure that Huc firmware works"
> + "by copying a char array using Huc"
> + "and verifying the copied result");
> +
> + igt_subtest("huc-copy") {
> + char inputs[HUC_COPY_DATA_BUF_SIZE];
> + struct drm_i915_gem_exec_object2 obj[3];
> +
> + /* Initialize src buffer randomly */
> + srand(time(NULL));
> + for (int i = 0; i < HUC_COPY_DATA_BUF_SIZE; i++)
> + inputs[i] = (char) (rand() % 256);
> +
> + memset(obj, 0, sizeof(obj));
> + /* source buffer object for storing input */
> + obj[0].handle = gem_create(drm_fd, HUC_COPY_DATA_BUF_SIZE);
> + /* destination buffer object to receive input */
> + obj[1].handle = gem_create(drm_fd, HUC_COPY_DATA_BUF_SIZE);
> + /* execution buffer object */
> + obj[2].handle = gem_create(drm_fd, 4096);
> +
> + gem_write(drm_fd, obj[0].handle, 0, inputs, HUC_COPY_DATA_BUF_SIZE);
> +
> + huc_copy(drm_fd, obj, 3);
> + compare_huc_copy_result(drm_fd, obj[0].handle, obj[1].handle);
> +
> + gem_close(drm_fd, obj[0].handle);
> + gem_close(drm_fd, obj[1].handle);
> + gem_close(drm_fd, obj[2].handle);
> + }
> +
> + igt_fixture
> + close(drm_fd);
> +}
> diff --git a/tests/intel-ci/fast-feedback.testlist b/tests/intel-ci/fast-feedback.testlist
> index 2ccad438..f332d8ec 100644
> --- a/tests/intel-ci/fast-feedback.testlist
> +++ b/tests/intel-ci/fast-feedback.testlist
> @@ -30,6 +30,7 @@ igt at gem_flink_basic@bad-open
> igt at gem_flink_basic@basic
> igt at gem_flink_basic@double-flink
> igt at gem_flink_basic@flink-lifetime
> +igt at gem_huc_copy@huc_copy
> igt at gem_linear_blits@basic
> igt at gem_mmap@basic
> igt at gem_mmap_gtt@basic
> diff --git a/tests/meson.build b/tests/meson.build
> index e882f4dc..a4934a85 100644
> --- a/tests/meson.build
> +++ b/tests/meson.build
> @@ -163,6 +163,7 @@ i915_progs = [
> 'gem_gtt_cpu_tlb',
> 'gem_gtt_hog',
> 'gem_gtt_speed',
> + 'gem_huc_copy',
> 'gem_linear_blits',
> 'gem_lut_handle',
> 'gem_madvise',
More information about the igt-dev
mailing list