[Intel-gfx] [PATCH v3] test/gem_mocs_settings: Testing MOCS register settings
Peter Antoine
peter.antoine at intel.com
Mon Apr 11 15:02:17 UTC 2016
On Mon, 11 Apr 2016, Chris Wilson wrote:
> On Mon, Apr 11, 2016 at 01:51:25PM +0100, Peter Antoine wrote:
>> The MOCS registers were added in Gen9 and define the caching policy.
>> The registers are split into two sets. The first set controls the
>> EDRAM policy and have a set for each engine, the second set controls
>> the L3 policy. The two sets use the same index.
>>
>> The RCS registers and the L3CC registers are stored in the RCS context.
>>
>> The test checks that the registers are correct by checking the values by
>> directly reading them via MMIO, then again it tests them by reading them
>> from within a batch buffer. RCS engine is tested last as it programs the
>> registers via a batch buffer and this will invalidate the test for
>> workloads that don't use the render ring or don't run a render batch
>> first.
>>
>> v2: Reorganised the structure.
>> Added more tests. (Chris Wilson)
>> v3: Fixed a few bugs. (Chris Wilson)
>>
>> Signed-off-by: Peter Antoine <peter.antoine at intel.com>
>> ---
>> lib/ioctl_wrappers.c | 13 ++
>> lib/ioctl_wrappers.h | 1 +
>> tests/Makefile.sources | 1 +
>> tests/gem_mocs_settings.c | 559 ++++++++++++++++++++++++++++++++++++++++++++++
>> 4 files changed, 574 insertions(+)
>> create mode 100644 tests/gem_mocs_settings.c
>>
>> diff --git a/lib/ioctl_wrappers.c b/lib/ioctl_wrappers.c
>> index 076bce8..8d74128 100644
>> --- a/lib/ioctl_wrappers.c
>> +++ b/lib/ioctl_wrappers.c
>> @@ -1382,6 +1382,19 @@ void gem_require_ring(int fd, unsigned ring)
>> igt_require(gem_has_ring(fd, ring));
>> }
>>
>> +/**
>> + * gem_has_mocs:
>> + * @fd: open i915 drm file descriptor
>> + *
>> + * Feature test macro to query whether the device has MOCS registers.
>> + * These exist gen 9+.
>> + */
>> +void gem_has_mocs(int fd)
>
> gem_has_mocs() returns bool
> gem_require_mocs() calls igt_require() and doesn't return if not
> available.
>
Done.
>> +{
>> + const int gen = intel_gen(intel_get_drm_devid(fd));
>> + igt_require(gen >= 9);
>> +}
>> +
>> /* prime */
>>
>> /**
>> diff --git a/lib/ioctl_wrappers.h b/lib/ioctl_wrappers.h
>> index d986f61..ebedf1e 100644
>> --- a/lib/ioctl_wrappers.h
>> +++ b/lib/ioctl_wrappers.h
>> @@ -154,6 +154,7 @@ bool gem_has_softpin(int fd);
>> void gem_require_caching(int fd);
>> bool gem_has_ring(int fd, unsigned ring);
>> void gem_require_ring(int fd, unsigned ring);
>> +void gem_has_mocs(int fd);
>>
>> /* prime */
>> struct local_dma_buf_sync {
>> diff --git a/tests/Makefile.sources b/tests/Makefile.sources
>> index 43f232f..d483c9e 100644
>> --- a/tests/Makefile.sources
>> +++ b/tests/Makefile.sources
>> @@ -148,6 +148,7 @@ TESTS_progs = \
>> gem_lut_handle \
>> gem_mmap_offset_exhaustion \
>> gem_media_fill \
>> + gem_mocs_settings \
>> gem_gpgpu_fill \
>> gem_pin \
>> gem_reg_read \
>> diff --git a/tests/gem_mocs_settings.c b/tests/gem_mocs_settings.c
>> new file mode 100644
>> index 0000000..03c5089
>> --- /dev/null
>> +++ b/tests/gem_mocs_settings.c
>> @@ -0,0 +1,559 @@
>> +/*
>> + * Copyright © 2016 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.
>> + *
>> + */
>> +
>> +/** @file gem_mocs_settings.c
>> + *
>> + * Check that the MOCs cache settings are valid.
>> + */
>> +
>> +#include "igt.h"
>> +#include "igt_gt.h"
>> +
>> +#define MAX_NUMBER_MOCS_REGISTERS (64)
>> +
>> +enum {
>> + NONE,
>> + RESET,
>> + SUSPEND,
>> + HIBERNATE
>> +};
>> +
>> +#define GEN9_LNCFCMOCS0 (0xB020) /* L3 Cache Control base */
>> +#define GEN9_GFX_MOCS_0 (0xc800) /* Graphics MOCS base register*/
>> +#define GEN9_MFX0_MOCS_0 (0xc900) /* Media 0 MOCS base register*/
>> +#define GEN9_MFX1_MOCS_0 (0xcA00) /* Media 1 MOCS base register*/
>> +#define GEN9_VEBOX_MOCS_0 (0xcB00) /* Video MOCS base register*/
>> +#define GEN9_BLT_MOCS_0 (0xcc00) /* Blitter MOCS base register*/
>> +
>> +#define ENGINE_GFX (I915_EXEC_RENDER)
>> +#define ENGINE_MFX0 (I915_EXEC_BSD)
>> +#define ENGINE_MFX0_ALT (I915_EXEC_BSD | 1 << 13)
>> +#define ENGINE_MFX1 (I915_EXEC_BSD | 2 << 13)
>> +#define ENGINE_BLT (I915_EXEC_BLT)
>> +#define ENGINE_VEBOX (I915_EXEC_VEBOX)
>> +
>> +struct mocs_entry {
>> + uint32_t control_value;
>> + uint16_t l3cc_value;
>> +};
>> +
>> +struct mocs_table {
>> + uint32_t size;
>> + const struct mocs_entry *table;
>> +};
>> +
>
> /* The first entries in the MOCS tables are defined by uABI */
>
Added.
>> +static const struct mocs_entry skylake_mocs_table[] = {
>> + { 0x00000009, 0x0010 },
>> + { 0x00000038, 0x0030 },
>> + { 0x0000003b, 0x0030 },
>> +};
>> +
>> +static const struct mocs_entry broxton_mocs_table[] = {
>> + { 0x00000009, 0x0010 },
>> + { 0x00000038, 0x0030 },
>> + { 0x0000003b, 0x0030 },
>> +};
>> +
>> +static const struct mocs_entry dirty_table_mocs_table[] = {
>> + { 0x00007FFF, 0x003F },
>> + { 0x00007FFF, 0x003F },
>> + { 0x00007FFF, 0x003F },
>> +};
>> +
>> +static const uint32_t write_values[] = {
>> + 0xFFFFFFFF,
>> + 0xFFFFFFFF,
>> + 0xFFFFFFFF,
>> + 0xFFFFFFFF
>> +};
>
>> +static void test_mocs_values(int fd)
>> +{
>> + const struct intel_execution_engine *e;
>> +
>> + for (e = intel_execution_engines; e->name; e++)
>> + if (e->exec_id != I915_EXEC_DEFAULT &&
>> + gem_has_ring(fd, e->exec_id | e->flags))
>
> You could use
>
> unsigned engine;
>
> for_each_engine(fd, engine)
> if (engine != 0) test_mocs_control_values(fd, engine);
>
> Just to hide the gem_has_ring().
Done.
>
>> +
>> +
>> + test_mocs_l3cc_values(fd);
>> +}
>> +
>> +#define MI_STORE_REGISTER_MEM_64_BIT_ADDR ((0x24 << 23) | 2)
>> +
>> +static int create_read_batch(struct drm_i915_gem_relocation_entry *reloc,
>> + uint32_t *batch,
>> + uint32_t dst_handle,
>> + uint32_t size,
>> + uint32_t reg_base)
>> +{
>> + unsigned int index;
>> + unsigned int offset = 0;
>> +
>> + for (index = 0, offset = 0; index < size; index++, offset += 4)
>> + {
>> + batch[offset] = MI_STORE_REGISTER_MEM_64_BIT_ADDR;
>> + batch[offset+1] = reg_base + (index * sizeof(uint32_t));
>> + batch[offset+2] = index * sizeof(uint32_t); /* reloc */
>> + batch[offset+3] = 0;
>> +
>> + reloc[index].offset = (offset + 2) * sizeof(uint32_t);
>> + reloc[index].delta = index * sizeof(uint32_t);
>> + reloc[index].target_handle = dst_handle;
>> + reloc[index].write_domain = I915_GEM_DOMAIN_RENDER;
>
> Will cause an -EINVAL as you are specifing a write_domain but not the
> corresponding read_domain.
Removed.
(Did not cause an error when tested)
>
>> +static void do_read_registers(int fd,
>> + uint32_t ctx_id,
>> + uint32_t dst_handle,
>> + uint32_t reg_base,
>> + uint32_t size,
>> + uint32_t engine_id)
>> +{
>> + gem_execbuf(fd, &execbuf);
>> + gem_wait(fd, dst_handle, &timeout_ns);
>
> As mentioned earlier, tried to avoid manual wait/sync so that we stress
> the kernel harder to do the right thing. Sometimes the sync is required
> as part of the test, but only rarely and should be well documented.
replaced with set_domain.
>
>> + gem_close(fd, handle);
>> +}
>
>> +static void check_control_registers(int fd,
>> + const struct intel_execution_engine *engine,
>> + uint32_t ctx_id,
>> + bool dirty)
>> +{
>> + uint32_t dst_handle = gem_create(fd, 4096);
>> + uint32_t reg_base = get_engine_base(engine->exec_id | engine->flags);
>> + uint32_t *read_regs;
>> + struct mocs_table table;
>> +
>> + igt_assert(get_mocs_settings(intel_get_drm_devid(fd), &table, dirty));
>> +
>> + do_read_registers(fd,
>> + ctx_id,
>> + dst_handle,
>> + reg_base,
>> + table.size,
>> + engine->exec_id | engine->flags);
>> +
>> + read_regs = gem_mmap__cpu(fd, dst_handle, 0, 4096, PROT_READ);
>
> Here you need the corresponding
> gem_set_comdin(fd, dst_handle, DOMAIN_CPU, 0);
> and in check_l3cc_registers.
Added into do_read_registers() so should handle both.
>
>> +
>> + for (int index = 0; index < table.size; index++)
>> + igt_assert_eq(read_regs[index],
>> + table.table[index].control_value);
>> +
>> + munmap(read_regs, 4096);
>> + gem_close(fd, dst_handle);
>> +}
>
> Otherwise, looking good. (Could use snooping or gem_read to minimise the
> clflush on bxt, but I don't think anyone will notice over 4k).
> -Chris
>
>
Ok.
Will run it on something not a BXT and then push again.
So the other patched are good as well?
Thanks for the review.
Peter.
--
Peter Antoine (Android Graphics Driver Software Engineer)
---------------------------------------------------------------------
Intel Corporation (UK) Limited
Registered No. 1134945 (England)
Registered Office: Pipers Way, Swindon SN3 1RJ
VAT No: 860 2173 47
More information about the Intel-gfx
mailing list