[Intel-xe] [PATCH] drm/xe: Add mocs kunit
Matt Roper
matthew.d.roper at intel.com
Wed Nov 1 22:38:44 UTC 2023
On Tue, Oct 31, 2023 at 12:10:30AM +0530, Ruthuvikas Ravikumar wrote:
> This kunit verifies the hardware values of mocs and
> l3cc registers with the KMD programmed values.
>
> v7: correct checkpath
> v6: Change ssize_t type.
> Change forcewake domain to XE_FW_GT.
> Update change of MOCS registers are multicast on Xe_HP and beyond
> patch.
>
> v5: Release forcewake.
> Remove single statement braces.
> Fix debug statements.
>
> v4: Drop stratch and vaddr.
> Fix debug statements.
> Fix indentation.
>
> v3: Fix checkpath.
>
> v2: Fix checkpath.
For future reference, you might want to pass the "-v" flag to git
format-patch so that it's easier to keep the different versions of your
patches separate in the inbox. I.e., including a "-v8" flag when
running format-patch on your next version will ensure it gets generated
as [PATCH v8] and be easy to distinguish from v7, v9, etc. in the email
clients.
>
> Cc: Aravind Iddamsetty <aravind.iddamsetty at intel.com>
> Cc: Mathew D Roper <matthew.d.roper at intel.com>
> Signed-off-by: Ruthuvikas Ravikumar <ruthuvikas.ravikumar at intel.com>
> ---
> drivers/gpu/drm/xe/tests/Makefile | 1 +
> drivers/gpu/drm/xe/tests/xe_mocs.c | 165 ++++++++++++++++++++++++
> drivers/gpu/drm/xe/tests/xe_mocs_test.c | 24 ++++
> drivers/gpu/drm/xe/tests/xe_mocs_test.h | 13 ++
> drivers/gpu/drm/xe/xe_mocs.c | 4 +
> 5 files changed, 207 insertions(+)
> create mode 100644 drivers/gpu/drm/xe/tests/xe_mocs.c
> create mode 100644 drivers/gpu/drm/xe/tests/xe_mocs_test.c
> create mode 100644 drivers/gpu/drm/xe/tests/xe_mocs_test.h
>
> diff --git a/drivers/gpu/drm/xe/tests/Makefile b/drivers/gpu/drm/xe/tests/Makefile
> index 51f1a7f017d4..39d8a0892274 100644
> --- a/drivers/gpu/drm/xe/tests/Makefile
> +++ b/drivers/gpu/drm/xe/tests/Makefile
> @@ -4,6 +4,7 @@ obj-$(CONFIG_DRM_XE_KUNIT_TEST) += \
> xe_bo_test.o \
> xe_dma_buf_test.o \
> xe_migrate_test.o \
> + xe_mocs_test.o \
> xe_pci_test.o \
> xe_rtp_test.o \
> xe_wa_test.o
> diff --git a/drivers/gpu/drm/xe/tests/xe_mocs.c b/drivers/gpu/drm/xe/tests/xe_mocs.c
> new file mode 100644
> index 000000000000..b2f2a4a86989
> --- /dev/null
> +++ b/drivers/gpu/drm/xe/tests/xe_mocs.c
> @@ -0,0 +1,165 @@
> +// SPDX-License-Identifier: GPL-2.0 AND MIT
> +/*
> + * Copyright © 2022 Intel Corporation
> + */
> +
> +#include <kunit/test.h>
> +#include <kunit/visibility.h>
> +
> +#include "tests/xe_mocs_test.h"
> +#include "tests/xe_pci_test.h"
> +#include "tests/xe_test.h"
> +
> +#include "xe_pci.h"
> +#include "xe_pm.h"
> +#include "xe_gt.h"
> +#include "xe_mocs.h"
> +#include "xe_bo.h"
> +#include "xe_device.h"
Do xe_bo.h and xe_pm.h get used for anything in this file? I didn't
spot anything obvious, but I didn't actually compile it, so I may have
overlooked something.
> +
> +struct live_mocs {
> + struct xe_mocs_info table;
> + struct xe_mocs_info *mocs;
> + struct xe_mocs_info *l3cc;
Why do we need all three of these? 'table' holds both the global mocs
and the lncf mocs information, so both of these just wind up just being
pointers back to the same table. You simply pass &mocs.table in the
places you're using these other two pointers today.
> +};
> +
> +static int live_mocs_init(struct live_mocs *arg, struct xe_gt *gt)
> +{
> + unsigned int flags;
> + struct kunit *test = xe_cur_kunit();
> +
> + memset(arg, 0, sizeof(*arg));
> +
> + flags = get_mocs_settings(gt_to_xe(gt), &arg->table);
> +
> + kunit_info(test, "table size %d", arg->table.size);
> + kunit_info(test, "table uc_index %d", arg->table.uc_index);
> + kunit_info(test, "table n_entries %d", arg->table.n_entries);
> +
> + if (flags & HAS_GLOBAL_MOCS)
> + arg->mocs = &arg->table;
> +
> + arg->l3cc = &arg->table;
> +
> + return flags;
> +}
> +
> +static void read_l3cc_table(struct xe_gt *gt,
> + const struct xe_mocs_info *info)
> +{
> + unsigned int i;
> + u32 l3cc;
> + u32 reg_val;
> + u32 ret;
> +
> + struct kunit *test = xe_cur_kunit();
> +
> + xe_device_mem_access_get(gt_to_xe(gt));
> + ret = xe_force_wake_get(gt_to_fw(gt), XE_FW_GT);
> + if (ret)
> + goto out;
> + mocs_dbg(>_to_xe(gt)->drm, "L3CC entries:%d\n", info->n_entries);
> + for (i = 0;
> + i < (info->n_entries + 1) / 2 ?
> + (l3cc = l3cc_combine(get_entry_l3cc(info, 2 * i),
> + get_entry_l3cc(info, 2 * i + 1))), 1 : 0;
> + i++) {
> + if (GRAPHICS_VERx100(gt_to_xe(gt)) >= 1250) {
> + reg_val = xe_gt_mcr_unicast_read_any(gt, XEHP_LNCFCMOCS(i));
> + struct xe_reg_mcr reg_mcr = XEHP_LNCFCMOCS(i);
> +
> + mocs_dbg(>_to_xe(gt)->drm, "%d 0x%x 0x%x 0x%x\n", i, reg_mcr.__reg.addr,
> + reg_val, l3cc);
> + if (reg_val != l3cc)
> + KUNIT_FAIL(test, "l3cc reg 0x%x has incorrect val.\n",
> + reg_mcr.__reg.addr);
> + } else {
> + reg_val = xe_mmio_read32(gt, XELP_LNCFCMOCS(i));
> + struct xe_reg reg = XELP_LNCFCMOCS(i);
> +
> + mocs_dbg(>_to_xe(gt)->drm, "%d 0x%x 0x%x 0x%x\n", i, reg.addr,
> + reg_val, l3cc);
> + if (reg_val != l3cc)
> + KUNIT_FAIL(test, "l3cc reg 0x%x has incorrect val.\n",
> + reg.addr);
> + }
You only need to use the 'if' guard on the actual register reads. The
debugging and error messages can be pulled outside since the actual
register offset is the same for both the XELP and XEHP variants and you
don't really need to worry about using a specific form.
if (GRAPHICS_VERx100(gt_to_xe(gt)) >= 1250)
reg_val = xe_gt_mcr_unicast_read_any(gt, XEHP_LNCFCMOCS(i));
else
reg_val = xe_mmio_read32(gt, XELP_LNCFCMOCS(i));
mocs_dbg(>_to_xe(gt)->drm, "%d 0x%x 0x%x 0x%x\n", i,
XELP_LNCFCMOCS(i).addr)
if (reg_val != l3cc)
KUNIT_FAIL(test, "l3cc reg 0x%x has incorrect val.\n",
XELP_LNCFCMOCS(i).addr);
The same can also be done with the global mocs handling in
read_mocs_table() as well.
> +
> + }
> + xe_force_wake_put(gt_to_fw(gt), XE_FW_GT);
> +out:
> + xe_device_mem_access_put(gt_to_xe(gt));
> +}
> +
> +static void read_mocs_table(struct xe_gt *gt,
> + const struct xe_mocs_info *info)
> +{
> + struct xe_device *xe = gt_to_xe(gt);
> +
> + unsigned int i;
> + u32 mocs;
> + u32 reg_val;
> + u32 ret;
> +
> + struct kunit *test = xe_cur_kunit();
> +
> + xe_device_mem_access_get(gt_to_xe(gt));
> + ret = xe_force_wake_get(gt_to_fw(gt), XE_FW_GT);
> + if (ret)
> + goto out;
Should we just be doing a
KUNIT_ASSERT_EQ(test, ret, 0);
here so that it will be apparent if this fails? Otherwise if we fail to
grab forcewake for some reason, we'll just silently skip this test and
it will seem like everything was tested successfully.
> +
> + mocs_dbg(>_to_xe(gt)->drm, "Global MOCS entries:%d\n", info->n_entries);
> + drm_WARN_ONCE(&xe->drm, !info->unused_entries_index,
> + "Unused entries index should have been defined\n");
> + for (i = 0;
> + i < info->n_entries ? (mocs = get_entry_control(info, i)), 1 : 0;
> + i++) {
> +
> + if (GRAPHICS_VERx100(gt_to_xe(gt)) > 1250) {
> + reg_val = xe_gt_mcr_unicast_read_any(gt, XEHP_GLOBAL_MOCS(i));
> + struct xe_reg_mcr reg_mcr = XEHP_GLOBAL_MOCS(i);
> +
> + mocs_dbg(>_to_xe(gt)->drm, "%d 0x%x 0x%x 0x%x\n", i,
> + reg_mcr.__reg.addr, reg_val, mocs);
The indent of this line doesn't look right.
> + if (reg_val != mocs)
> + KUNIT_FAIL(test, "mocs reg 0x%x has incorrect val:\n",
> + reg_mcr.__reg.addr);
> + } else {
> + reg_val = xe_mmio_read32(gt, XELP_GLOBAL_MOCS(i));
> + struct xe_reg reg = XELP_GLOBAL_MOCS(i);
> +
> + mocs_dbg(>_to_xe(gt)->drm, "%d 0x%x 0x%x 0x%x\n", i,
> + reg.addr, reg_val, mocs);
Same here.
> + if (reg_val != mocs)
> + KUNIT_FAIL(test, "mocs reg 0x%x has incorrect val:\n",
> + reg.addr);
> + }
> + }
> + xe_force_wake_put(gt_to_fw(gt), XE_FW_GT);
> +out:
> + xe_device_mem_access_put(gt_to_xe(gt));
> +}
> +
> +static int mocs_kernel_test_run_device(struct xe_device *xe)
> +{
> + /* Basic check the system is configured with the expected mocs table */
> +
> + struct live_mocs mocs;
> + struct xe_gt *gt;
> +
> + unsigned int flags;
> + int id;
> +
> + for_each_gt(gt, xe, id) {
> + flags = live_mocs_init(&mocs, gt);
> + if (flags & HAS_GLOBAL_MOCS)
> + read_mocs_table(gt, mocs.mocs);
> + read_l3cc_table(gt, mocs.l3cc);
Since Xe2 platforms don't have LNCFCMOCS registers, we're probably going
to want to add a check on this one as well, just in case any of those
register offsets get recycled for other registers on Xe2 or later
platforms. I.e., add a check for HAS_LNCF_MOCS (which just got added to
drm-xe-next) before calling this function
Matt
> + }
> + return 0;
> +}
> +
> +void xe_live_mocs_kernel_kunit(struct kunit *test)
> +{
> + xe_call_for_each_device(mocs_kernel_test_run_device);
> +}
> +EXPORT_SYMBOL_IF_KUNIT(xe_live_mocs_kernel_kunit);
> diff --git a/drivers/gpu/drm/xe/tests/xe_mocs_test.c b/drivers/gpu/drm/xe/tests/xe_mocs_test.c
> new file mode 100644
> index 000000000000..ef56bd517b28
> --- /dev/null
> +++ b/drivers/gpu/drm/xe/tests/xe_mocs_test.c
> @@ -0,0 +1,24 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * Copyright © 2022 Intel Corporation
> + */
> +
> +#include "xe_mocs_test.h"
> +
> +#include <kunit/test.h>
> +
> +static struct kunit_case xe_mocs_tests[] = {
> + KUNIT_CASE(xe_live_mocs_kernel_kunit),
> + {}
> +};
> +
> +static struct kunit_suite xe_mocs_test_suite = {
> + .name = "xe_mocs",
> + .test_cases = xe_mocs_tests,
> +};
> +
> +kunit_test_suite(xe_mocs_test_suite);
> +
> +MODULE_AUTHOR("Intel Corporation");
> +MODULE_LICENSE("GPL");
> +MODULE_IMPORT_NS(EXPORTED_FOR_KUNIT_TESTING);
> diff --git a/drivers/gpu/drm/xe/tests/xe_mocs_test.h b/drivers/gpu/drm/xe/tests/xe_mocs_test.h
> new file mode 100644
> index 000000000000..7faa3575e6c3
> --- /dev/null
> +++ b/drivers/gpu/drm/xe/tests/xe_mocs_test.h
> @@ -0,0 +1,13 @@
> +/* SPDX-License-Identifier: GPL-2.0 AND MIT */
> +/*
> + * Copyright © 2023 Intel Corporation
> + */
> +
> +#ifndef _XE_MOCS_TEST_H_
> +#define _XE_MOCS_TEST_H_
> +
> +struct kunit;
> +
> +void xe_live_mocs_kernel_kunit(struct kunit *test);
> +
> +#endif
> diff --git a/drivers/gpu/drm/xe/xe_mocs.c b/drivers/gpu/drm/xe/xe_mocs.c
> index 19a8146ded9a..a73419ea0711 100644
> --- a/drivers/gpu/drm/xe/xe_mocs.c
> +++ b/drivers/gpu/drm/xe/xe_mocs.c
> @@ -579,3 +579,7 @@ void xe_mocs_init(struct xe_gt *gt)
> if (table.table)
> init_l3cc_table(gt, &table);
> }
> +
> +#if IS_ENABLED(CONFIG_DRM_XE_KUNIT_TEST)
> +#include "tests/xe_mocs.c"
> +#endif
> --
> 2.25.1
>
--
Matt Roper
Graphics Software Engineer
Linux GPU Platform Enablement
Intel Corporation
More information about the Intel-xe
mailing list