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

[Lis, Tomasz]

On 2019-02-22 15:17, Prathap Kumar Valsan wrote:
> From: "Kumar Valsan, Prathap" <prathap.kumar.valsan at 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 at intel.com>
> ---
> Changes in v2:
> - Cleaned up the code based on review comments from Lucas and Chris
>   tests/i915/gem_mocs_settings.c | 96 ++++++++++++++++++++++++++--------
>   1 file changed, 73 insertions(+), 23 deletions(-)
>
> diff --git a/tests/i915/gem_mocs_settings.c b/tests/i915/gem_mocs_settings.c
> index 5b3b6bc1..6d111076 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,
> @@ -72,36 +75,66 @@ struct mocs_table {
>   	const struct mocs_entry	*table;
>   };
>   
> +static const struct mocs_entry icelake_mocs_pte = {0x00000004, 0x0030};
> +static const struct mocs_entry mocs_pte = {0x00000038, 0x0030};
> +
>   /* 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 ... GEN11_NUM_MOCS_ENTRIES - 1] = icelake_mocs_pte,
> +	[0]  = { 0x00000005, 0x0010},
> +	[1]  = { 0x00000004, 0x0030},
> +	[2]  = { 0x00000037, 0x0030},
> +	[3]  = { 0x00000005, 0x0010},
> +	[4]  = { 0x00000005, 0x0030},
> +	[5]  = { 0x00000037, 0x0010},
> +	[6]  = { 0x00000017, 0x0010},
> +	[7]  = { 0x00000017, 0x0030},
> +	[8]  = { 0x00000027, 0x0010},
> +	[9]  = { 0x00000027, 0x0030},
> +	[10] = { 0x00000077, 0x0010},
> +	[11] = { 0x00000077, 0x0030},
> +	[12] = { 0x00000057, 0x0010},
> +	[13] = { 0x00000057, 0x0030},
> +	[14] = { 0x00000067, 0x0010},
> +	[15] = { 0x00000067, 0x0030},
> +	[18] = { 0x00060037, 0x0030},
> +	[19] = { 0x00000737, 0x0030},
> +	[20] = { 0x00000337, 0x0030},
> +	[21] = { 0x00000137, 0x0030},
> +	[22] = { 0x000003b7, 0x0030},
> +	[23] = { 0x000007b7, 0x0030},
> +	[62] = { 0x00000037, 0x0010},
> +	[63] = { 0x00000037, 0x0010},
> +};
> +
> +static const struct mocs_entry dirty_icelake_mocs_table[GEN11_NUM_MOCS_ENTRIES] = {
> +	[0 ... GEN11_NUM_MOCS_ENTRIES - 1] = { 0x0007FFFF, 0x003F },
>   };
>   
> -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 ... GEN9_NUM_MOCS_ENTRIES - 1] = mocs_pte,
> +	[0] = { 0x00000009, 0x0010},
> +	[1] = { 0x00000038, 0x0030},
> +	[2] = { 0x0000003b, 0x0030},
>   };
>   
> -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 },
>   };
>   
> -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 ... GEN9_NUM_MOCS_ENTRIES - 1] = mocs_pte,
> +	[0] = { 0x00000009, 0x0010},
> +	[1] = { 0x00000038, 0x0030},
> +	[2] = { 0x00000039, 0x0030},
>   };
>   
> -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 },
> +};
> +
> +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 +160,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;
> @@ -374,13 +416,21 @@ static void check_mocs_values(int fd, unsigned engine, uint32_t ctx_id, bool dir
>   
>   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);
>   }
>   
The "-dirty" subcase of the test doesn't seem suitable to current hardware..

The table is now global. So writing it from one context and checking if 
another context is unaffected is bound to fail.

Shouldn't the test for gen11+ be restricted to reading the values and 
checking whether they match pre-defined array?
(we may still do a test if everything is fine after reset/suspend, that 
won't hurt)
And if we are writing dirty values, shouldn't there be a restoration to 
original values afterwards?

-Tomasz