Re: [igt-dev] [PATCH i-g-t v4] i915/gem_mocs_settings: Add mocs table for icelake

Kumar Valsan, Prathap
2019-03-04T20:23:48Z

On Mon, Mar 04, 2019 at 03:27:25PM -0500, Prathap Kumar Valsan wrote:
> From: "Kumar Valsan, Prathap" <prathap.kumar.valsan@intel.com>
>
> This patch adds mocs table for icelake with expected L3 and eDRAM
> control values.
>
> Signed-off-by: Kumar Valsan, Prathap <prathap.kumar.valsan@intel.com>
> ---
> Changes in v4:
> - L3 control registers are global in icelake.
> Test validates that dirty writes from userspace
> to L3 control registers are being dropped.
> Changes in v3:
> - There are holes in the mocs table(Lucas Pointed out).
> In icelake index 16 and 17 are reserved.
> So test shouldn't be checking them.
> Changes in v2:
> - Cleaned up the code based on review
> comments from Lucas and Chris
>
> tests/i915/gem_mocs_settings.c | 126 +++++++++++++++++++++++++--------
> 1 file changed, 96 insertions(+), 30 deletions(-)
>
> diff --git a/tests/i915/gem_mocs_settings.c b/tests/i915/gem_mocs_settings.c
> index 5b3b6bc1..66615007 100644
> --- a/tests/i915/gem_mocs_settings.c
> +++ b/tests/i915/gem_mocs_settings.c
> @@ -33,6 +33,9 @@
> #include "igt_sysfs.h"
>
> #define MAX_NUMBER_MOCS_REGISTERS (64)
> +#define GEN9_NUM_MOCS_ENTRIES 62 /* 62 out of 64 - 63 & 64 are reserved. */
> +#define GEN11_NUM_MOCS_ENTRIES 64 /* 63-64 are reserved, but configured. */
> +
> enum {
> NONE,
> RESET,
> @@ -61,10 +64,13 @@ static const char * const test_modes[] = {
> #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 ICELAKE_MOCS_PTE {0x00000004, 0x0030, 0x1}
> +#define MOCS_PTE {0x00000038, 0x0030, 0x1}
>
> struct mocs_entry {
> uint32_t control_value;
> uint16_t l3cc_value;
> + uint8_t used;
> };
>
> struct mocs_table {
> @@ -73,35 +79,63 @@ struct mocs_table {
> };
>
> /* The first entries in the MOCS tables are defined by uABI */
> -static const struct mocs_entry skylake_mocs_table[] = {
> - { 0x00000009, 0x0010 },
> - { 0x00000038, 0x0030 },
> - { 0x0000003b, 0x0030 },
> +static const struct mocs_entry icelake_mocs_table[GEN11_NUM_MOCS_ENTRIES] = {
> + [0] = { 0x00000005, 0x0010, 0x1},
> + [1] = ICELAKE_MOCS_PTE,
> + [2] = { 0x00000037, 0x0030, 0x1},
> + [3] = { 0x00000005, 0x0010, 0x1},
> + [4] = { 0x00000005, 0x0030, 0x1},
> + [5] = { 0x00000037, 0x0010, 0x1},
> + [6] = { 0x00000017, 0x0010, 0x1},
> + [7] = { 0x00000017, 0x0030, 0x1},
> + [8] = { 0x00000027, 0x0010, 0x1},
> + [9] = { 0x00000027, 0x0030, 0x1},
> + [10] = { 0x00000077, 0x0010, 0x1},
> + [11] = { 0x00000077, 0x0030, 0x1},
> + [12] = { 0x00000057, 0x0010, 0x1},
> + [13] = { 0x00000057, 0x0030, 0x1},
> + [14] = { 0x00000067, 0x0010, 0x1},
> + [15] = { 0x00000067, 0x0030, 0x1},
> + [18] = { 0x00060037, 0x0030, 0x1},
> + [19] = { 0x00000737, 0x0030, 0x1},
> + [20] = { 0x00000337, 0x0030, 0x1},
> + [21] = { 0x00000137, 0x0030, 0x1},
> + [22] = { 0x000003b7, 0x0030, 0x1},
> + [23] = { 0x000007b7, 0x0030, 0x1},
> + [24 ... 61] = ICELAKE_MOCS_PTE,
> + [62] = { 0x00000037, 0x0010, 0x1},
> + [63] = { 0x00000037, 0x0010, 0x1},
> +};
> +
> +static const struct mocs_entry dirty_icelake_mocs_table[GEN11_NUM_MOCS_ENTRIES] = {
> + [0 ... 15] = { 0x0007FFFF, 0x003F, 0x1 },
> + [18 ... GEN11_NUM_MOCS_ENTRIES - 1] = { 0x0007FFFF, 0x003F, 0x1 },
> };
>
> -static const struct mocs_entry dirty_skylake_mocs_table[] = {
> - { 0x00003FFF, 0x003F }, /* no snoop bit */
> - { 0x00003FFF, 0x003F },
> - { 0x00003FFF, 0x003F },
> +static const struct mocs_entry skylake_mocs_table[GEN9_NUM_MOCS_ENTRIES] = {
> + [0] = { 0x00000009, 0x0010, 0x1},
> + [1] = MOCS_PTE,
> + [2] = { 0x0000003b, 0x0030, 0x1},
> + [3 ... GEN9_NUM_MOCS_ENTRIES - 1] = MOCS_PTE,
> };
>
> -static const struct mocs_entry broxton_mocs_table[] = {
> - { 0x00000009, 0x0010 },
> - { 0x00000038, 0x0030 },
> - { 0x00000039, 0x0030 },
> +static const struct mocs_entry dirty_skylake_mocs_table[GEN9_NUM_MOCS_ENTRIES] = {
> + [0 ... GEN9_NUM_MOCS_ENTRIES - 1] = { 0x00003FFF, 0x003F, 0x1 },
> };
>
> -static const struct mocs_entry dirty_broxton_mocs_table[] = {
> - { 0x00007FFF, 0x003F },
> - { 0x00007FFF, 0x003F },
> - { 0x00007FFF, 0x003F },
> +static const struct mocs_entry broxton_mocs_table[GEN9_NUM_MOCS_ENTRIES] = {
> + [0] = { 0x00000009, 0x0010, 0x1},
> + [1] = MOCS_PTE,
> + [2] = { 0x00000039, 0x0030, 0x1},
> + [3 ... GEN9_NUM_MOCS_ENTRIES - 1] = MOCS_PTE,
> };
>
> -static const uint32_t write_values[] = {
> - 0xFFFFFFFF,
> - 0xFFFFFFFF,
> - 0xFFFFFFFF,
> - 0xFFFFFFFF
> +static const struct mocs_entry dirty_broxton_mocs_table[GEN9_NUM_MOCS_ENTRIES] = {
> + [0 ... GEN9_NUM_MOCS_ENTRIES - 1] = { 0x00007FFF, 0x003F, 0x1 },
> +};
> +
> +static const uint32_t write_values[MAX_NUMBER_MOCS_REGISTERS] = {
> + [0 ... MAX_NUMBER_MOCS_REGISTERS - 1] = 0xFFFFFFFF,
> };
>
> static bool get_mocs_settings(int fd, struct mocs_table *table, bool dirty)
> @@ -127,6 +161,15 @@ static bool get_mocs_settings(int fd, struct mocs_table *table, bool dirty)
> table->table = broxton_mocs_table;
> }
> result = true;
> + } else if (IS_ICELAKE(devid)) {
> + if (dirty) {
> + table->size = ARRAY_SIZE(dirty_icelake_mocs_table);
> + table->table = dirty_icelake_mocs_table;
> + } else {
> + table->size = ARRAY_SIZE(icelake_mocs_table);
> + table->table = icelake_mocs_table;
> + }
> + result = true;
> }
>
> return result;
> @@ -283,9 +326,12 @@ static void check_control_registers(int fd,
> read_regs = gem_mmap__cpu(fd, dst_handle, 0, 4096, PROT_READ);
>
> gem_set_domain(fd, dst_handle, I915_GEM_DOMAIN_CPU, 0);
> - for (int index = 0; index < table.size; index++)
> + for (int index = 0; index < table.size; index++) {
> + if (!table.table[index].used)
> + continue;
> igt_assert_eq_u32(read_regs[index],
> table.table[index].control_value);
> + }
>
> munmap(read_regs, 4096);
> gem_close(fd, dst_handle);
> @@ -315,10 +361,14 @@ static void check_l3cc_registers(int fd,
> gem_set_domain(fd, dst_handle, I915_GEM_DOMAIN_CPU, 0);
>
> for (index = 0; index < table.size / 2; index++) {
> - igt_assert_eq_u32(read_regs[index] & 0xffff,
> - table.table[index * 2].l3cc_value);
> - igt_assert_eq_u32(read_regs[index] >> 16,
> - table.table[index * 2 + 1].l3cc_value);
> + if (table.table[index * 2].used) {
> + igt_assert_eq_u32(read_regs[index] & 0xffff,
> + table.table[index * 2].l3cc_value);
> + }
> + if (table.table[index * 2 + 1].used) {
> + igt_assert_eq_u32(read_regs[index] >> 16,
> + table.table[index * 2 + 1].l3cc_value);
> + }
> }
>
> if (table.size & 1)
> @@ -368,19 +418,35 @@ static void check_mocs_values(int fd, unsigned engine, uint32_t ctx_id, bool dir
> {
> check_control_registers(fd, engine, ctx_id, dirty);
>
> - if (engine == I915_EXEC_RENDER)
> - check_l3cc_registers(fd, engine, ctx_id, dirty);
> + if (engine == I915_EXEC_RENDER) {
> + uint32_t devid = intel_get_drm_devid(fd);
> + /* L3 control registers are global in icl.
> + * dirty write from userspace will be ignored/
> + */
> + if (dirty && IS_ICELAKE(devid))
> + check_l3cc_registers(fd, engine, ctx_id, !dirty);
> + else
> + check_l3cc_registers(fd, engine, ctx_id, dirty);
> + }
> }
After testing on icl platform, it is observed that LLC control registers
can be context saved and restored between contexts, however write to
L3 control registers are being dropped even though the writes is through
a priviliged batch buffer(I915_EXEC_SECURE). Need to confirm if this is
the expected behaviour.

With this patch test including "-dirty" subtests are passing on icl.

>
> static void write_dirty_mocs(int fd, unsigned engine, uint32_t ctx_id)
> {
> + uint32_t devid = intel_get_drm_devid(fd);
> + int num_of_mocs_entries;
> +
> + if (IS_ICELAKE(devid))
> + num_of_mocs_entries = GEN11_NUM_MOCS_ENTRIES;
> + else
> + num_of_mocs_entries = GEN9_NUM_MOCS_ENTRIES;
> +
> write_registers(fd, ctx_id, get_engine_base(engine),
> - write_values, ARRAY_SIZE(write_values),
> + write_values, num_of_mocs_entries,
> engine);
>
> if (engine == I915_EXEC_RENDER)
> write_registers(fd, ctx_id, GEN9_LNCFCMOCS0,
> - write_values, ARRAY_SIZE(write_values),
> + write_values, num_of_mocs_entries/2,
> engine);
> }
>
> --
> 2.20.1
>