Quoting Daniele Ceraolo Spurio (2018-03-08 23:46:28)
> Check that the entries are in reverse gen order and that the first entry
> and all the following entries with gen > 0 have an mmio base set.
>
> Suggested-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> Cc: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@xxxxxxxxx>
> ---
> drivers/gpu/drm/i915/intel_engine_cs.c | 1 +
> .../gpu/drm/i915/selftests/i915_mock_selftests.h | 1 +
> drivers/gpu/drm/i915/selftests/intel_engine_cs.c | 48 ++++++++++++++++++++++
> 3 files changed, 50 insertions(+)
> create mode 100644 drivers/gpu/drm/i915/selftests/intel_engine_cs.c
>
> diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c
> index 08711665061c..a33171d82aee 100644
> --- a/drivers/gpu/drm/i915/intel_engine_cs.c
> +++ b/drivers/gpu/drm/i915/intel_engine_cs.c
> @@ -2131,4 +2131,5 @@ void intel_disable_engine_stats(struct intel_engine_cs *engine)
>
> #if IS_ENABLED(CONFIG_DRM_I915_SELFTEST)
> #include "selftests/mock_engine.c"
> +#include "selftests/intel_engine_cs.c"
> #endif
> diff --git a/drivers/gpu/drm/i915/selftests/i915_mock_selftests.h b/drivers/gpu/drm/i915/selftests/i915_mock_selftests.h
> index 9a48aa441743..2842f93ca29e 100644
> --- a/drivers/gpu/drm/i915/selftests/i915_mock_selftests.h
> +++ b/drivers/gpu/drm/i915/selftests/i915_mock_selftests.h
> @@ -23,3 +23,4 @@ selftest(vma, i915_vma_mock_selftests)
> selftest(evict, i915_gem_evict_mock_selftests)
> selftest(gtt, i915_gem_gtt_mock_selftests)
> selftest(hugepages, i915_gem_huge_page_mock_selftests)
> +selftest(engine, intel_engine_cs_mock_selftests)
Plonk this after uncore. It's a lowlevel sanity check that should come
before we start looking at features.
> diff --git a/drivers/gpu/drm/i915/selftests/intel_engine_cs.c b/drivers/gpu/drm/i915/selftests/intel_engine_cs.c
> new file mode 100644
> index 000000000000..8ef453905520
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/selftests/intel_engine_cs.c
> @@ -0,0 +1,48 @@
> +/*
> + * SPDX-License-Identifier: GPL-2.0
> + *
> + * Copyright © 2018 Intel Corporation
> + */
> +
> +#include "../i915_selftest.h"
> +
> +static int intel_mmio_bases_check(void)
> +{
> + const struct engine_info *info;
> + int i, j;
> + u32 gen;
> + s32 prev;
> +
> + for (i = 0; i < ARRAY_SIZE(intel_engines); i++) {
> + info = &intel_engines[i];
> +
> + for (prev = -1, j = MAX_MMIO_BASES -1; j >= 0; j--) {
> + gen = info->mmio_bases[j].gen;
> +
> + if (prev >= (s32)gen) {
> + pr_err("%s: engine[%d]: mmio base for gen %x "
> + "is before the one for gen %x\n",
> + __func__, i, gen, prev);
> + return -EINVAL;
> + }
> +
> + if ((j == 0 || gen > 0) && !info->mmio_bases[j].base) {
Ok, setting gen=0 upset us. Make that gen=1 in the previous patch.
Looping backwards here definitely seems to make it harder than it needs
to be. We only need to validate the array as seen by the algorithm so,
u8 prev = U8_MAX;
for (j = 0; j < MAX_MMIO_BASES; j++) {
u8 gen = info->mmio_bases[j].gen;
if (gen >= prev) {
...
return -EINVAL;
}
if (gen == 0)
break;
if (!info->mmio.bases[j].base) {
...
return -EINVAL;
}
prev = gen;
}
pr_info("%s: min gen supported for %s = %d\n", __func__, magic_engine_name(i), prev);
I'm not sure how we could automate that check (not without hardcoding
the same information twice), so just print it.
> +
> +int intel_engine_cs_mock_selftests(void)
> +{
> + return intel_mmio_bases_check();
I'd stick this in a
static const struct i915_subtest tests[] = {
SUBTEST(mmio_bases_check),
};
return i915_subtests(tests, NULL);
just for the convenience of adding more.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
