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

["Kalamarz, Lukasz" <lukasz.kalamarz@intel.com>]

On Tue, 2019-03-12 at 11:14 -0400, 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>
> ---

Chris and Tomek suggested creating negative scenario, that will verify
setting dirty mocs values instead of skipping for gen11+. But if they
will agree that this is not a blocker, then 
Signed-off-by: Lukasz Kalamarz <lukasz.kalamarz@intel.com>

> Changes in v5:
> - As mocs table is global, test need not write
>   mocs table. Below jira has the discussion. 
>   https://jira.devtools.intel.com/browse/VLK-1567
> 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 | 103 ++++++++++++++++++++++++-------
> --
>  1 file changed, 75 insertions(+), 28 deletions(-)
> 
> diff --git a/tests/i915/gem_mocs_settings.c
> b/tests/i915/gem_mocs_settings.c
> index 5b3b6bc1..901cef38 100644
> --- a/tests/i915/gem_mocs_settings.c
> +++ b/tests/i915/gem_mocs_settings.c
> @@ -32,7 +32,9 @@
>  #include "igt_perf.h"
>  #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 +63,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 +78,59 @@ 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
> 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 dirty_skylake_mocs_table[] = {
> -	{ 0x00003FFF, 0x003F }, /* no snoop bit */
> -	{ 0x00003FFF, 0x003F },
> -	{ 0x00003FFF, 0x003F },
> +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 broxton_mocs_table[] = {
> -	{ 0x00000009, 0x0010 },
> -	{ 0x00000038, 0x0030 },
> -	{ 0x00000039, 0x0030 },
> +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 struct mocs_entry dirty_broxton_mocs_table[] = {
> -	{ 0x00007FFF, 0x003F },
> -	{ 0x00007FFF, 0x003F },
> -	{ 0x00007FFF, 0x003F },
> +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[] = {
> -	0xFFFFFFFF,
> -	0xFFFFFFFF,
> -	0xFFFFFFFF,
> -	0xFFFFFFFF
> +static const uint32_t write_values[GEN9_NUM_MOCS_ENTRIES] = {
> +	[0 ... GEN9_NUM_MOCS_ENTRIES - 1] = 0xFFFFFFFF,
>  };
>  
>  static bool get_mocs_settings(int fd, struct mocs_table *table, bool
> dirty)
> @@ -127,6 +156,10 @@ 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)) {
> +		table->size  = ARRAY_SIZE(icelake_mocs_table);
> +		table->table = icelake_mocs_table;
> +		result = true;
>  	}
>  
>  	return result;
> @@ -283,9 +316,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 +351,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)
> @@ -380,7 +420,7 @@ static void write_dirty_mocs(int fd, unsigned
> engine, uint32_t ctx_id)
>  
>  	if (engine == I915_EXEC_RENDER)
>  		write_registers(fd, ctx_id, GEN9_LNCFCMOCS0,
> -				write_values, ARRAY_SIZE(write_values),
> +				write_values,
> ARRAY_SIZE(write_values)/2,
>  				engine);
>  }
>  
> @@ -390,6 +430,13 @@ static void run_test(int fd, unsigned engine,
> unsigned flags, unsigned mode)
>  	uint32_t ctx_clean_id;
>  	uint32_t ctx_dirty_id;
>  
> +	/* As mocs is global for GEN11+, trying to dirtying the mocs
> +	 * and testing context save and restore of mocs between
> contexts
> +	 * is bound to fail.
> +	 */
> +	if (flags & MOCS_DIRTY_VALUES)
> +		igt_skip_on(intel_gen(intel_get_drm_devid(fd)) >= 11);
> +
>  	gem_require_ring(fd, engine);
>  
>  	/* Skip if we don't know where the registers are for this
> engine */
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev